permit at+jwt typ header value in jwt access tokens

Author: richard-dennehy

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/jwt/JwtAuthenticator.java

@@ -136,7 +136,7 @@ private static List<JwtFieldValidator> configureFieldValidatorsForIdToken(RealmC
         }
 
         return List.of(
-            JwtTypeValidator.INSTANCE,
+            JwtTypeValidator.ID_TOKEN_INSTANCE,
             new JwtStringClaimValidator("iss", true, List.of(realmConfig.getSetting(JwtRealmSettings.ALLOWED_ISSUER)), List.of()),
             subjectClaimValidator,
             new JwtStringClaimValidator("aud", false, realmConfig.getSetting(JwtRealmSettings.ALLOWED_AUDIENCES), List.of()),
@@ -157,7 +157,7 @@ private static List<JwtFieldValidator> configureFieldValidatorsForAccessToken(
         final Clock clock = Clock.systemUTC();
 
         return List.of(
-            JwtTypeValidator.INSTANCE,
+            JwtTypeValidator.ACCESS_TOKEN_INSTANCE,
             new JwtStringClaimValidator("iss", true, List.of(realmConfig.getSetting(JwtRealmSettings.ALLOWED_ISSUER)), List.of()),
             getSubjectClaimValidator(realmConfig, fallbackClaimLookup),
             new JwtStringClaimValidator(

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/jwt/JwtTypeValidator.java

@@ -17,14 +17,20 @@
 
 public class JwtTypeValidator implements JwtFieldValidator {
 
-    private static final JOSEObjectTypeVerifier<SecurityContext> JWT_HEADER_TYPE_VERIFIER = new DefaultJOSEObjectTypeVerifier<>(
+    private final JOSEObjectTypeVerifier<SecurityContext> JWT_HEADER_TYPE_VERIFIER;
+
+    public static final JwtTypeValidator ID_TOKEN_INSTANCE = new JwtTypeValidator(JOSEObjectType.JWT, null);
+
+    // strictly speaking, this should only permit `at+jwt`, but removing the other two options is a breaking change
+    public static final JwtTypeValidator ACCESS_TOKEN_INSTANCE = new JwtTypeValidator(
         JOSEObjectType.JWT,
+        new JOSEObjectType("at+jwt"),
         null
     );
 
-    public static final JwtTypeValidator INSTANCE = new JwtTypeValidator();
-
-    private JwtTypeValidator() {}
+    private JwtTypeValidator(JOSEObjectType... allowedTypes) {
+        JWT_HEADER_TYPE_VERIFIER = new DefaultJOSEObjectTypeVerifier<>(allowedTypes);
+    }
 
     public void validate(JWSHeader jwsHeader, JWTClaimsSet jwtClaimsSet) {
         final JOSEObjectType jwtHeaderType = jwsHeader.getType();

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/jwt/JwtAuthenticatorIdTokenTypeTests.java

@@ -7,11 +7,21 @@
 
 package org.elasticsearch.xpack.security.authc.jwt;
 
+import com.nimbusds.jose.JWSHeader;
+import com.nimbusds.jose.util.Base64URL;
+import com.nimbusds.jwt.JWTClaimsSet;
+import com.nimbusds.jwt.SignedJWT;
+
+import org.elasticsearch.action.support.PlainActionFuture;
 import org.elasticsearch.xpack.core.security.authc.jwt.JwtRealmSettings;
 
 import java.text.ParseException;
+import java.util.Map;
 
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class JwtAuthenticatorIdTokenTypeTests extends JwtAuthenticatorTests {
 
@@ -28,4 +38,23 @@ public void testSubjectIsRequired() throws ParseException {
     public void testInvalidIssuerIsCheckedBeforeAlgorithm() throws ParseException {
         doTestInvalidIssuerIsCheckedBeforeAlgorithm(buildJwtAuthenticator());
     }
+
+    public void testAccessTokenHeaderTypeIsRejected() throws ParseException {
+        final JWTClaimsSet claimsSet = JWTClaimsSet.parse(Map.of());
+        final SignedJWT signedJWT = new SignedJWT(
+            JWSHeader.parse(Map.of("alg", allowedAlgorithm, "typ", "at+jwt")).toBase64URL(),
+            claimsSet.toPayload().toBase64URL(),
+            Base64URL.encode("signature")
+        );
+
+        final JwtAuthenticationToken jwtAuthenticationToken = mock(JwtAuthenticationToken.class);
+        when(jwtAuthenticationToken.getSignedJWT()).thenReturn(signedJWT);
+        when(jwtAuthenticationToken.getJWTClaimsSet()).thenReturn(signedJWT.getJWTClaimsSet());
+
+        final PlainActionFuture<JWTClaimsSet> future = new PlainActionFuture<>();
+        final JwtAuthenticator jwtAuthenticator = buildJwtAuthenticator();
+        jwtAuthenticator.authenticate(jwtAuthenticationToken, future);
+        final Exception e = expectThrows(IllegalArgumentException.class, future::actionGet);
+        assertThat(e.getMessage(), equalTo("invalid jwt typ header"));
+    }
 }

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/jwt/JwtRealmAuthenticateAccessTokenTypeTests.java

@@ -7,7 +7,6 @@
 
 package org.elasticsearch.xpack.security.authc.jwt;
 
-import com.nimbusds.jose.JOSEObjectType;
 import com.nimbusds.jose.jwk.JWK;
 import com.nimbusds.jwt.SignedJWT;
 import com.nimbusds.openid.connect.sdk.Nonce;
@@ -134,7 +133,7 @@ protected SecureString randomJwt(JwtIssuerAndRealm jwtIssuerAndRealm, User user)
 
         final Instant now = Instant.now().truncatedTo(ChronoUnit.SECONDS);
         unsignedJwt = JwtTestCase.buildUnsignedJwt(
-            randomBoolean() ? null : JOSEObjectType.JWT.toString(), // kty
+            randomFrom("at+jwt", "JWT", null), // typ
             randomBoolean() ? null : jwk.getKeyID(), // kid
             algJwkPair.alg(), // alg
             randomAlphaOfLengthBetween(10, 20), // jwtID

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/jwt/JwtTypeValidatorTests.java

@@ -19,30 +19,53 @@
 
 public class JwtTypeValidatorTests extends ESTestCase {
 
-    public void testValidType() throws ParseException {
+    public void testValidIdTokenType() throws ParseException {
         final String algorithm = randomAlphaOfLengthBetween(3, 8);
 
-        // typ is allowed to be missing
         final JWSHeader jwsHeader = JWSHeader.parse(
-            randomFrom(Map.of("alg", randomAlphaOfLengthBetween(3, 8)), Map.of("typ", "JWT", "alg", randomAlphaOfLengthBetween(3, 8)))
+            randomFrom(
+                // typ is allowed to be missing
+                Map.of("alg", algorithm),
+                Map.of("typ", "JWT", "alg", algorithm)
+            )
         );
 
         try {
-            JwtTypeValidator.INSTANCE.validate(jwsHeader, JWTClaimsSet.parse(Map.of()));
+            JwtTypeValidator.ID_TOKEN_INSTANCE.validate(jwsHeader, JWTClaimsSet.parse(Map.of()));
+        } catch (Exception e) {
+            throw new AssertionError("validation should have passed without exception", e);
+        }
+    }
+
+    public void testValidAccessTokenType() throws ParseException {
+        final String algorithm = randomAlphaOfLengthBetween(3, 8);
+
+        final JWSHeader jwsHeader = JWSHeader.parse(
+            randomFrom(
+                // typ is allowed to be missing
+                Map.of("alg", algorithm),
+                Map.of("typ", "JWT", "alg", algorithm),
+                Map.of("typ", "at+jwt", "alg", algorithm)
+            )
+        );
+
+        try {
+            JwtTypeValidator.ACCESS_TOKEN_INSTANCE.validate(jwsHeader, JWTClaimsSet.parse(Map.of()));
         } catch (Exception e) {
             throw new AssertionError("validation should have passed without exception", e);
         }
     }
 
     public void testInvalidType() throws ParseException {
+        final JwtTypeValidator validator = randomFrom(JwtTypeValidator.ID_TOKEN_INSTANCE, JwtTypeValidator.ACCESS_TOKEN_INSTANCE);
 
         final JWSHeader jwsHeader = JWSHeader.parse(
             Map.of("typ", randomAlphaOfLengthBetween(4, 8), "alg", randomAlphaOfLengthBetween(3, 8))
         );
 
         final IllegalArgumentException e = expectThrows(
             IllegalArgumentException.class,
-            () -> JwtTypeValidator.INSTANCE.validate(jwsHeader, JWTClaimsSet.parse(Map.of()))
+            () -> validator.validate(jwsHeader, JWTClaimsSet.parse(Map.of()))
         );
         assertThat(e.getMessage(), containsString("invalid jwt typ header"));
     }