Stop rejecting tokens with future :iat values.

[metametadata]

Partially addresses #39.

[delitescere]

I don't agree that ignoring iat is a good idea. If you want to add some sort of leeway argument (defaulting to 0) to the options, then that is a backwards-compatible change that also doesn't weaken existing consumer code.

[metametadata]

Hi @delitescere! Yes, my main motivation for PR was to get rid of occasional errors on iat validation. And this problem could probably be solved by adding the leeway instead.

But I have a better argument for this PR to be merged: validation of iat is not in RFC 7519 (JSON Web Signature (JWS)) which states that nbf is to be used for declining tokens "from the future":

4.1.5. "nbf" (Not Before) Claim

The "nbf" (not before) claim identifies the time before which the JWT
MUST NOT be accepted for processing. The processing of the "nbf"
claim requires that the current date/time MUST be after or equal to
the not-before date/time listed in the "nbf" claim. Implementers MAY
provide for some small leeway, usually no more than a few minutes, to
account for clock skew. Its value MUST be a number containing a
NumericDate value. Use of this claim is OPTIONAL.

4.1.6. "iat" (Issued At) Claim

The "iat" (issued at) claim identifies the time at which the JWT was
issued. This claim can be used to determine the age of the JWT. Its
value MUST be a number containing a NumericDate value. Use of this
claim is OPTIONAL.

There's also no such validation suggested in OAuth 2.0 or OpenID Connect 1.0 specs. After all, we should trust spec authors who already thought through all the security concerns and mitigation strategies and leave implementing "home-grown" validations up to consumers.

Note that I didn't touch max-age validation which is based on iat claim because it's at least mentioned in OIDC spec:

The iat Claim can be used to reject tokens that were issued too far away from the current time, limiting the amount of time that nonces need to be stored to prevent attacks. The acceptable range is Client specific.

I agree that this PR may be considered a breaking change for some consumers and this can be addressed by bumping the major number in semantic version of the lib.

[delitescere]

There's also nothing in my assumption that the Issued At and Not Before values are anywhere near to close. I may issue a token now that isn't to be used before 10 minutes from now. Even so, if the recipient receives the token with an Issued At after the Not Before, let alone now, it is invalid.

Again, I understand the compulsion. I don't agree with the impact.

[delitescere]

After all, we should trust spec authors who already thought through all the security concerns and mitigation strategies

Funny

[metametadata]

It looks like you have some concerns with OAuth spec, me too. But I guess we have to work with what we have.

if the recipient receives the token with an Issued At after the Not Before, let alone now, it is invalid

Yes, logically, it looks odd. But what is a security risk of approving such token in the client?

[delitescere]

But what is a security risk of approving such token in the client?

That's the point: like so many security-weakening decisions, we are relying on hindsight to define correct behaviour. "Ooops, I didn't realise they'd do that and open a vulnerability".

Let's say the consumer of the token is running on a host where the clock is being manipulated externally by a bad actor. The bad actor doesn't have direct access to the consumer host, nor any part of the system to do with the issuer, nor is able to generate or manipulate the token, but is able to replay arbitrary previous tokens. Perhaps the bad actor has hijacked the NTP responses the consumer's host is receiving and pushing its clock backwards. Some other component on the consumer is relying on the receipt of a valid token and then performing another action (e.g. removing all the cash in your bank account) based on the time. Or, a PRNG might be involved and because the clock is being manipulated, non-determinism in the "random" data has been removed. Because neither the issuer's clock nor the token can be manipulated, the window of this attack is either totally closed, or only as small as the leeway the consumer has applied to account for clock skew, if the iat is validated. If it's ignored, the attack window is literally "every token ever sent to this consumer".

However the vulnerability is exploited doesn't actually matter. What matters is that something which has a singular role in providing a security function should not be weakening that function simply because someone can't imagine how it might be weakening it, and our future selves aren't in a situation saying "Why, for bacon's sake, did we do that?".

[metametadata]

OK, maybe discussing the particular attack is not productive because we disagree on the higher level. Personally, I'd like the library to adhere to next statements:

1. Library validations must not be "stronger" than what is recommended by specs. Otherwise, people who build systems based on the specs cannot use the library because there's no way to turn off validations offending the spec. In particular, this means that library shouldn't try to add validations which look like a common sense (e.g. "iat < now", "iat < nbf", "nbf < exp", etc.) and/or mitigate a particular attack which spec authors didn't account for (as in your example I guess?).

2. If non-spec validations were previously added to the library they must be removed.

3. Library authors must inform users that the implemented validation logic has changed in new release by accordingly updating the semantic version.

4. Library user is responsible for deciding if the built-in validations are enough for his application. He has to implement additional validations manually if what library proposes is not enough for his use cases.

[delitescere]

@metametadata On reflection, I totally agree about point about the library validations, and thus the rest of your message. The JWT spec is weak, not the library.

May I add, though, that if the change to remove iat validation proceeds, a mechanism for additional arbitrary JWT validations be provided. Given it's a lisp, let's say the user of the library may pass a function that, if present, is invoked after the in-built (spec compliant) validations pass, is given the same context as the in-built validations, and the result if which is the result of the overall validation.

[danielcompton]

I wonder if it makes sense for this library to have a "sensible defaults" namespace, which adds enhanced validation that probably makes sense for the average project. This saves people from reinventing the wheel (possibly dangerously), while still allowing users to use the weaker spec compliant validations if they want.

[delitescere]

@danielcompton I thought about a declarative mechanism for rules like the ones @metametadata mentioned ("iat < now", "iat < nbf", "nbf < exp", etc.), "clock skew leeway = X seconds", but we are dealing with a lisp library and composable functions is the way to go. There could be built-in functions the library provides that do those checks (to not dangerously reinvent the wheel) but it would be up to the library user to provide an "additional validations" function that uses each of them.

[delitescere]

ps @funcool this is a really good library, thanks for the great work!

[niwinz]

I have readed twice this discussion and this one: jpadilla/pyjwt#190 and finally I have decided that i'm going to merge this. RFC is clear about this, if you want this functionality nbf should be used (its the purpose of it). I prefer leave this decision to validate this claim to the application and being compliant with the RFC.