Claim Peeking in JARM

[maennchen]

Why are you validating peeked claims instead of DecryptedResponse?

Originally posted by @maennchen in #321 (comment)

DecryptedResponse is still a binary. We validate against PeekClaims (rather than getting the claims after calling oidcc_jwt_util:verify_signature/3) because the spec says that the client must validate some of the claims before validating the signature: https://openid.net/specs/oauth-v2-jarm-final.html#name-processing-rules

Originally posted by @paulswartz in #321 (comment)

@paulswartz I'm not sure I understand your reasoning.

Currently the code is:

• decrypting
• peeking & validating peeked claims
• verifying & unpacking the decrypted token
• returning the verified claims

Why are we not:

• decypting
• verifying token & extracting claims
• check the verified claims

Do you read the spec as having a defined order to the steps? I doesn't specifically say to do so as far as I can see. (Might be wrong on this)

I also don't see any hints that peeking is expected nor do I see a good reason to do so.

Originally posted by @maennchen in #321 (comment)

Here's the text from https://openid.net/specs/oauth-v2-jarm-final.html#section-2.4-2

The client MUST process the JWT secured response as follows:

1. (OPTIONAL) The client decrypts the JWT using the default key for the respective issuer or, if applicable, determined by the kid JWT header parameter. The key might be a private key, where the corresponding public key is registered with the expected issuer of the response ("use":"enc" via the client's metadata jwks or jwks_uri) or a key derived from its client secret (see Section 2.2).
2. The client obtains the iss element from the JWT and checks whether its value is well known and identifies the expected issuer of the authorization process in examination. If the check fails, the client MUST abort processing and refuse the response.
3. The client obtains the aud element from the JWT and checks whether it matches the client id the client used to identify itself in the corresponding authorization request. If the check fails, the client MUST abort processing and refuse the response.
4. The client checks the JWT's exp element to determine if the JWT is still valid. If the check fails, the client MUST abort processing and refuse the response.
5. The client MUST check the signature of the JWT according to [RFC7515] and the algorithm none ("alg":"none") MUST NOT be accepted. If the check fails, the client MUST abort processing and refuse the response.

My read of this is:

1. decrypt if necessary
2. check the "iss" claim
3. check the "aud" claim
4. check the "exp" claim
5. check the signature and verify that "alg": "none" was not used

Since steps 2, 3, and 4 require checking the claims before the signature has been checked in step 5, we need to peek into the JWT to do that.

Originally posted by @paulswartz in #321 (comment)

[maennchen]

Extracted into a new issue from #321.

I'll check with a colleague to see how they read that part of the spec. It seems strange to me that the spec would require validating unverified claims and I'm not sure if the spec just specifies the steps or if it also specifies the order.

[paulswartz]

When I have some time, I'll try to look for some other implementations to see what they're doing.

[paulswartz]

Node's openid-client seems to do it this way as well (for all JWTs): https://github.com/panva/node-openid-client/blob/9d3cfb8bffc03d3b987a574f20ebbd209fec2cf1/lib/client.js#L896-L1086

Unfortunately, that's the only open-source client I can see that supports JARM.

[maennchen]

@paulswartz I’ve discussed the issue with some colleagues that implement an OpenID provider. The consensus is that the order of the steps in the specification is not relevant and that the claims should not be read before validating the signature. The exception would be the iss claim if the origin of the token is not known beforehand.