JWTClaimsSet siltently ignores ParseException

[Former user]

Some of the JWTClaimsSet parsing methods silently ignore ParseException and instead return null, e.g.:

public Date getExpirationTime() {
    try {
        return getDateClaim(EXPIRATION_TIME_CLAIM);
    } catch (ParseException e) {
        return null;
    }
}

Since this is a library with security focus, such behavior is pretty problematic. For example when DefaultJWTClaimsVerifier is used with exp (expiration time) as required claim, validation would pass when an exp claim is present but getExpirationTime() fails parsing it, therefore effectively rendering the expiration time useless in that case.

A workaround could be to adjust the checks for DefaultJWTClaimsVerifier though this would make the code more difficult to maintain and would not help for user-written validation code.

I would therefore recommend adjusting all JWTClaimsSet methods which currently silently ignore ParseException.

[Vladimir Dzhuvinov]

Deprecated the default DefaultJWTClaimsVerifier constructor which was intended for internal use only, edited the JavaDocs to clarify exp and nbf checking: 2c896977

[Vladimir Dzhuvinov]

If exp is a mandatory claim in your app add it to the set of required claims when constructing DefaultJWTClaimsVerifier.

The JWTClaimsSet is intended to be a simple wrapper that returns valid exp values only, or null in all other cases, including illegal values. The security checks happen at the higher level. If the API is changed to throw a ParseException the client can still bypass the exp check by simply not setting it at all. So make sure all required claims explicitly listed.

‌

[Marcono1234]

Hello,

thank your for deprecating the insecure constructor!

However, the main point of this issue was that the JWTClaimsSet methods can cause malformed required claims to pass validation. The complete security relies on JWTClaimsSet#parse(java.util.Map) performing some validation (I overlooked that back when I created the issue, therefore its description might have been a little bit irritating). This decouples the logic from the other parsing methods (e.g. getExpirationTime()), which is somewhat risky. If in the future the class is refactored, or a user is able to create a JWTClaimsSet without calling parse(...), then token validation will pass successfully despite it containing malformed claims. Below is a rather contrived example showing that, there the exp claim has the value true. The point here is not to show that the user can (by accident) skip calling parse(...), but instead that the fact that the JWTClaimsSet methods silently ignore parsing exceptions can compromise security to some extent. When running the code it might help setting a break point here and then stepping over the calls to see that the required exp claim is detected, but not validated since getExpirationTime() silently ignores the parse exception.

import com.nimbusds.jose.JOSEObject;
import com.nimbusds.jose.JWSAlgorithm;
import com.nimbusds.jose.JWSHeader;
import com.nimbusds.jose.JWSSigner;
import com.nimbusds.jose.crypto.MACSigner;
import com.nimbusds.jose.jwk.source.ImmutableSecret;
import com.nimbusds.jose.jwk.source.JWKSource;
import com.nimbusds.jose.proc.JWSKeySelector;
import com.nimbusds.jose.proc.JWSVerificationKeySelector;
import com.nimbusds.jose.proc.SecurityContext;
import com.nimbusds.jose.util.Base64URL;
import com.nimbusds.jwt.JWTClaimsSet;
import com.nimbusds.jwt.SignedJWT;
import com.nimbusds.jwt.proc.DefaultJWTClaimsVerifier;
import com.nimbusds.jwt.proc.DefaultJWTProcessor;

import java.text.ParseException;
import java.util.Collections;
import java.util.Date;

/*
 * Contrived example showing that claim validation security relies in parts on
 * JWTClaimsSet#parse(java.util.Map) being called, because some JWTClaimsSet methods
 * such as getExpirationTime() silently ignore parse exceptions.
 *
 * If the call to JWTClaimsSet#parse(java.util.Map) is skipped somehow (or the
 * implementation changes in the future), malformed claims pass verification successfully,
 * as demonstrated by the testBadToken() method of this class.
 */

public class JWTClaimsSetTest {
    static class CustomSignedJWT extends SignedJWT {
        public CustomSignedJWT(Base64URL firstPart, Base64URL secondPart, Base64URL thirdPart) throws ParseException {
            super(firstPart, secondPart, thirdPart);
        }

        @Override
        public JWTClaimsSet getJWTClaimsSet() {
            JWTClaimsSet.Builder claimsSetBuilder = new JWTClaimsSet.Builder();
            getPayload().toJSONObject().forEach(claimsSetBuilder::claim);
            return claimsSetBuilder.build();
        }
    }

    public static void main(String[] args) throws Exception {
        testGoodToken();
        testBadToken();
    }

    private static void testGoodToken() throws Exception {
        byte[] secret = new byte[32];
        JWSSigner signer = new MACSigner(secret);

        JWTClaimsSet claimsSet = new JWTClaimsSet.Builder()
            .expirationTime(new Date(new Date().getTime() + 60 * 1000))
            .build();

        SignedJWT signedJWT = new SignedJWT(new JWSHeader(JWSAlgorithm.HS256), claimsSet);
        signedJWT.sign(signer);
        String token = signedJWT.serialize();
        verify(token, secret);
    }

    private static void testBadToken() throws Exception {
        byte[] secret = new byte[32];
        JWSSigner signer = new MACSigner(secret);

        JWTClaimsSet claimsSet = new JWTClaimsSet.Builder()
            // Malformed claim
            .claim("exp", true)
            .build();

        SignedJWT signedJWT = new SignedJWT(new JWSHeader(JWSAlgorithm.HS256), claimsSet);
        signedJWT.sign(signer);
        String token = signedJWT.serialize();
        verify(token, secret);
    }

    // Verify token; partially based on https://connect2id.com/products/nimbus-jose-jwt/examples/validating-jwt-access-tokens
    private static void verify(String token, byte[] secret) throws Exception {
        Base64URL[] parts = JOSEObject.split(token);
        SignedJWT jwt = new CustomSignedJWT(parts[0], parts[1], parts[2]);

        DefaultJWTProcessor<SecurityContext> jwtProcessor = new DefaultJWTProcessor<>();

        JWKSource<SecurityContext> keySource = new ImmutableSecret<>(secret);

        JWSAlgorithm expectedJWSAlg = JWSAlgorithm.HS256;
        JWSKeySelector<SecurityContext> keySelector =
            new JWSVerificationKeySelector<>(expectedJWSAlg, keySource);
        jwtProcessor.setJWSKeySelector(keySelector);

        jwtProcessor.setJWTClaimsSetVerifier(new DefaultJWTClaimsVerifier<>(
            new JWTClaimsSet.Builder().build(),
            Collections.singleton("exp")));

        SecurityContext ctx = null;
        JWTClaimsSet claimsSet = jwtProcessor.process(jwt, ctx);

        System.out.println(claimsSet.toJSONObject());
    }
}

‌

[Marcono1234]

@Vladimir Dzhuvinov could you please have another look at this?