search

jpadilla / pyjwt

JSON Web Token implementation in Python
https://pyjwt.readthedocs.io
MIT License
4.99k stars 676 forks source link

Please stop validating that `iat <= now` by default #939

Open vergenzt opened 5 months ago

vergenzt commented 5 months ago

Opening a new issue since my previous rant about this was on a closed issue.

Request

Could we set the default value of verify_iat to False and publish this breaking change as version 3.0?

Clients who understand the risks and want to engage in this extra-spec behavior should opt in by setting verify_iat to True, and the need to do this should be announced in the changelog for this new major version. (Or maybe there could be a global variable in pyjwt to control the verify_iat default?)

Timeline of this behavior

Other related issues

Status of erratum report on official spec

I've filed an erratum report on the upstream JWT RFC here: https://www.rfc-editor.org/errata/eid7720. Discussion is ongoing, but the only controversy seems to be whether and/or how to publish the advice not do this, not whether or not this validation is appropriate. (Most seem to agree that this validation is not appropriate to perform.)

The discussion mailing list unfortunately appears to be private, but I've asked for either a public archive link (if one exists) and/or consent for me to share folks' responses if indeed the list is private; I'll update here if I receive either.

Justification

To copy/paste my previous comment on the closed issue:

This was litigated previously at https://github.com/jpadilla/pyjwt/issues/190. The JWT spec does NOT say to reject tokens with iat ("issued at") in the future, so this behavior goes beyond the spec and is inconsistent with many other JWT libraries.

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.

If token issuers want clients to specify that a token should not be accepted before a certain timestamp (which puts additional constraints upon clients by implying that clients' clocks should keep relatively in sync with a central clock source and/or need to check it with leeway) then the issuer is supposed to set nbf ("not before"):

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.

I am currently getting bit by this issue a lot in a large enterprise microservices environment (where token issuer, token user, and token-accepting server which validates offline using RSA are three different machines) where clock drift is coming into play. Myself and 10+ other people have now wasted multiple days of person-time trying to figure out why tokens were being rejected, talking about how best to address it, and managing the workloads & deliverables of the people investigating & talking about this.

Thoughts? Can we settle this once and for all?

vergenzt commented 5 months ago

cc @CollinEMac, @4dhyperplane, @ddhecker, @sb-keane, @mikkelsvartveit, @fdemmer, @gobengo, @nicktimko, @pengale -- tagging folks who 👍'd on my previous rant and/or have litigated this in the past 😅

mikkelsvartveit commented 5 months ago

I strongly agree with this, the current behavior is a terrible default for a JWT library

jpadilla commented 5 months ago

Thanks for the thoughtful and thorough issue. I'm down with rectifying this on a v3. We'll need to add a deprecation warning on the next v2 release.