Relax restrictions on "jti" claim verification #113
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The RFC has the following to say about the "jti" claim: > The "jti" (JWT ID) claim provides a unique identifier for the JWT. > The identifier value MUST be assigned in a manner that ensures that > there is a negligible probability that the same value will be > accidentally assigned to a different data object; if the application > uses multiple issuers, collisions MUST be prevented among values > produced by different issuers as well. The "jti" claim can be used > to prevent the JWT from being replayed. The "jti" value is a case- > sensitive string. Use of this claim is OPTIONAL. > -> https://tools.ietf.org/html/rfc7519#section-4.1.7 The current implementation expects that the `jti` is built as: MD5( secret + ":" + iat ) anything else is not concidered a valid `jti`. There are several issues with the current implementation: 1. `secret + ":" + iat` is not exactly collision safe :) using random UUIDs is safer I'd say. 2. It was only verified when it was present in the payload, so it was skipped when it was missing from the payload 2. In the RFC there are no rules defined on how the "jti" claim should be validated This pull request simply relaxes the verification of the "jti" claim to be present. This is in line with other implementations like the one for Java or Python. (FYI, there are other implementations that perform additional checks, the one seen most common is to verify against a value supplied in the `verify` call) It is my opinion that the library should just provide basic validation and make no additional assumption about the "jti" claim. Any additional verification / validation (e.g. replay checks, value checks) must be checked by the application and should be out of the scope for the library.
|
|
aj-michael
added a commit
that referenced
this pull request
Oct 30, 2015
Relax restrictions on "jti" claim verification
|
Thanks @lwe, this was a blatant oversight on our part. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The RFC has the following to say about the "jti" claim:
The current implementation expects that the
jtiis built as:anything else is not considered a valid
jti. There are several issues with the current implementation:secret + ":" + iatis not exactly collision safe :) using random UUIDs is safer I'd say.This pull request simply relaxes the verification of the "jti" claim to be present. This is in line with other implementations like the one for Java or Python. (FYI, there are other implementations that perform additional checks, the one seen most common is to verify against a fixed value supplied in the
verifycall).It is my opinion that the library should just provide basic validation and make no additional assumption about the
jti. Any additional verification / validation (e.g. replay checks, value checks) must be checked by the application and should be out of the scope for the library.PS: Yes, one could argue about having the option to supply an expected value for the "jti" claim, but this is IMO not really reasonable, because it (should) change for every JWT generated. So it's not something quasi-static like e.g. the issuer or audience.