Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why validate that 'iat' is not in the future? #190

Closed
gobengo opened this issue Nov 17, 2015 · 16 comments · Fixed by #252
Closed

Why validate that 'iat' is not in the future? #190

gobengo opened this issue Nov 17, 2015 · 16 comments · Fixed by #252
Labels

Comments

@gobengo
Copy link

gobengo commented Nov 17, 2015

In

raise InvalidIssuedAtError('Issued At claim (iat) cannot be in'
:

if iat > (now + leeway):
    raise InvalidIssuedAtError('Issued At claim (iat) cannot be in'
                               ' the future.')

I just debugged an issue in prod where jwt.decode() failed because of this. Mostly because the other party's jwt lib added 'iat' a few seconds or minutes ahead of our clock time ('clock skew' as mentioned in JWT specs).

I can't find any place in the specs that says that a JWT should be invalid if 'iat' is in the future. It seems like it's just there to be informative. I can use 'nbf' if I want to specify a "time before which the token MUST NOT be accepted for processing"

I consulted

So either

  1. I'm wrong and there is a JWT spec that says this is important to check. I want to know this, because if it's out there, I shouldn't just catch these errors from PyJWT and pass. Regardless of whether @jpadilla wants to remove that raise in his lib.
  2. PyJWT is checking that unnecessarily, and we should remove it to be more compliant
@soldnermike
Copy link

+1

@gobengo gobengo mentioned this issue Nov 17, 2015
@mark-adams
Copy link
Contributor

RFC 7519 says:

The "iat" (issued at) claim identifies the time at which the JWT was issued.

In all circumstances except clock skew, it doesn't seem like it makes any sense for a JWT to be valid if, by its own claim, it hasn't been created yet. That's why we included this validation. JWT doesn't say you have to have this validation to be compliant, but it seems logical and enforces integrity in the data.

I'm against removing this validation, especially when you can already easily disable it by using jwt.decode(token, secret, options={'verify_iat': False}) or account for the clock skew by using the leeway parameter: jwt.decode(token, secret, leeway=10)

@gobengo
Copy link
Author

gobengo commented Nov 18, 2015

In all circumstances except clock skew

Clock skew is mentioned twice in that RFC because it's unavoidable in real-world and real-universe federated distributed systems. It's not like that's not an important circumstance.

So should I fork pyjwt and make another version that is only spec-compliant and doesn't include any extrastandard assertions that seem like they make sense but aren't in the RFC?

If you think the standard should enforce that check, then RFC an ammendment to the spec. Until then, it seems prudent for PyJWT to strive to implement the spec.

I did an (admittedly cursory) search of the source code of jwt.io-recommended libraries for Java and JavaScript, and neither appears to perform the assertion in question

So are those two wrong? Or is no one wrong? I think it's a desirable property of the JWT community that a JWT is valid or invalid regardless of which library/language you're using.

Unless either of the following are disproven that

  1. This assertion has no roots in the IETF JWT specs, and
  2. The majority of other JWT verify implementations don't do it

Then we will have to agree to disagree on

I'm against removing this validation

Because as it stands now PyJWT is (ostensibly) rejecting valid JWTs and I want PyJWT to allow valid JWTs because when it doesn't our customers get frustrated when their valid JWTs don't work. Using leeway does more than just make it so I can check that valid JWTs are valid.

What do others think?

I'll fork and pull request tomorrow.

@mark-adams
Copy link
Contributor

Thanks for the PR! Just a reminder, instead of having to fork and package your own version of PyJWT, which seems a bit like overkill, you can totally bypass the check (exactly what your PR does), if you simply pass options={'verify_iat': False} to your jwt.decode() call with the existing code.

I don't like the idea of removing this check completely. Plenty of libraries have reasonable functionality that is not necessarily part of any particular spec. The strongest argument would be that we should set verify_iat to False by default and leave this as a helpful feature that developers can turn on if they want the additional functionality. That's a PR I could get behind!

@jpadilla
Copy link
Owner

I'm with @mark-adams here. Setting options={'verify_iat': False} achieves the same as #191. I'm not sure about setting it to False by default though.

@pengale
Copy link

pengale commented Nov 25, 2015

Hi there. Since the iat check introduces a situation where the a default install of PyJWT will break in typical production circumstances (or, if you're lucky, in testing, which is what happened to me), I'd like to second the request to set verify_iat to False by default. I don't think that it's reasonable to expect any two servers to have a shared definition of "now", or to make strict checks based on time between two machines.

Regardless, thank you for all the work putting this library together -- it has been really helpful and useful. :-)

@mark-adams
Copy link
Contributor

Thanks petevg! Glad you enjoy it! :-)

A couple of things in response:

How can it be unreasonable to assume two computers have a similar definition of when "now" is? We make these sort of assumptions all the time in all sorts of systems (public-key certificates used for HTTPS, EXP claims on JWT, TOTP-based two-factor authentication, etc.). I think it is completely reasonable, in particular when working with security, to assume two computers have a similar understanding of when "now" is. I do believe it IS unreasonable however to assume they have the EXACT same definition of now. That's why we include the ability to add the leeway parameter as almost everything on the Internet that relies on time does to compensate for the fact that exact time synchronization is hard.

Setting verify_iat to default to False is an option but we'd probably want to bump the version number and make it clear if we make that sort of change (because some people may be relying on that behavior). In the mean time, setting options={'verify_iat': False} seems like its still a reasonable workaround.

If there's a way we can make this behavior clearer in the documentation, I'm 100% sure we'd be glad to do that. Any ideas?

Thanks again for participating in the discussion 👍

@pengale
Copy link

pengale commented Nov 25, 2015

@mark-adams: Thanks for the quick response!

I think that the lightest weight change might simply to be to adjust the leeway to be something other than zero by default, and add it as an explicit param to PyJWT.decode, so that it's easy to find it with introspection tools or in the docstring for that function. As it is, I think that it's going to be pretty common for people to trip themselves up with the error if they don't know the external docs by heart.

When choosing between passing a value to leeway or disabling the check, I chose to disable the check. That might be because I'm coming more from a distributed database monkey's perspective, where I use timestamps to keep a record of things and help servers do things internally, but try to use other methods for syncing state across servers. I may be missing the security implications in skipping the check.

In any case, I think that I'd be happy with either solution -- just as long as the solution isn't to assume that all server clocks are perfectly in sync by default. It definitely makes sense to include it in a later version so that existing code doesn't break :-)

@gobengo
Copy link
Author

gobengo commented Nov 25, 2015

I updated my PR. See explanation: #191 (comment)

@ghost
Copy link

ghost commented Jan 11, 2017

Hi, here's a real word case of this issue causing problems.

Our infrastructure involves generating tokens on a server in Amazon's cloud and checking them on another one in Google's cloud using jwt.decode().

At midnight between 2016 December 31 and 2017 January 1, a leap second was introduced into UTC. Both clouds 'smeared' this second linearly over a period of time centered around that midnight:
https://aws.amazon.com/blogs/aws/look-before-you-leap-december-31-2016-leap-second-on-aws/
https://developers.google.com/time/smear

However, Amazon used 24 hour period and Google used 20 hour. This led to Amazon's clock being ahead of Google's on December 31 between 12:00:00 and 23:59:59, and our service was unavailable during most of that period due to jwt.decode() failing.

@nicktimko
Copy link

nicktimko commented Feb 3, 2017

So is the recommended fix to just add the options kwarg on all decode calls? Is there some package config I can tweak or something I can monkeypatch to disable it globally?

Still seems annoying to be bitten by enforcing something not mentioned in the RFC. All it says is that (emph. added)

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.

A negative age is startling I suppose, but nothing else seems to say that's forbidden.

@tld
Copy link

tld commented Feb 8, 2017

Sorry if it's been mentioned, I didn't catch it when I read through.

First off, +1 for keeping the check, especially since it can be disabled easily.

However, would it not be worthwhile to consider adding a margin to the check? Even allowing the token to be signed merely two seconds into the future, would go a long way towards solving the real-world issues this causes. If you add up issue with leap second and a small amount of slew, you could easily end with 1.2 - 1.3 seconds of difference on systems that are both relatively synced to a timesource, and two seconds would be enough to cover those cases.

Also keep in mind that clocks sometimes are off by design. Some systems will jump clocks to deal with leap, others will do it "the right way", and some will gradually slew the clock. Again however, having a 2 second margin would be enough to cover it.

I'm not saying 2 seconds is an ideal value, just arguing that even a tiny amount would go a long way.

@nicktimko
Copy link

The issue is more that unless you explicitly disable the check every time or add in leeway for clock skew, this can cause random errors. The RFC talks about minutes of skew, which seems crazy, but even milliseconds of skew can cause a validation error:

  1. Say you have two servers, an Issuer perfectly at UTC and Validator at time = Issuer - 10 milliseconds.
  2. Time is 20000.005 (seconds past epoch)
  3. Issuer creates a token with an 'iat': 20000
  4. Validator reads token, time for it is 19999.996. It rejects the token.

Little demo:

successes = fails = 0
skew = datetime.timedelta(microseconds=10000)
for n in range(10000):
    token = jwt.encode({'iat': datetime.datetime.utcnow() + skew}, 'magic')
    try:
        jwt.decode(token, 'magic')
    except jwt.exceptions.InvalidTokenError:
        fails += 1
    else:
        successes += 1
print(successes, fails) # => 9851 149

@tld
Copy link

tld commented Feb 12, 2017

nicktimko, your example would be handled just fine with leeway though?

mark-adams added a commit that referenced this issue Apr 17, 2017
RFC 7519 does not specify or even suggest this type of validation on the
'iat' claim and it has caused issues for several consumers of PyJWT.

This change removes the validation on future 'iat' values and leaves
such things up to the application developer to implement.

Fixes #190.
mark-adams added a commit that referenced this issue Apr 17, 2017
RFC 7519 does not specify or even suggest this type of validation on the
'iat' claim and it has caused issues for several consumers of PyJWT.

This change removes the validation on future 'iat' values and leaves
such things up to the application developer to implement.

Fixes #190.
mark-adams added a commit that referenced this issue Apr 17, 2017
RFC 7519 does not specify or even suggest this type of validation on the
'iat' claim and it has caused issues for several consumers of PyJWT.

This change removes the validation on future 'iat' values and leaves
such things up to the application developer to implement.

Fixes #190.
seanh added a commit to hypothesis/h that referenced this issue Nov 7, 2017
Remove two tests that JWT's whose `iat` value claims that they were
issued in the future fail validation.

These two tests fail on newer versions of PyJWT:

#4672

This is because PyJWT no longer raises an exception for future `iat`
times:

jpadilla/pyjwt#190

PyJWT removed this validation because:

- Clock skew can cause one party to generate `iat` times a few seconds
or minutes ahead of another's current time

- The JWT spec (RFC 7519) doesn't say that a JWT with a future `iat`
should be considered invalid, these JWTs are valid

- Other JWT libraries don't do this check
seanh added a commit to hypothesis/h that referenced this issue Nov 7, 2017
Remove two tests that JWT's whose `iat` value claims that they were
issued in the future fail validation.

These two tests fail on newer versions of PyJWT:

#4672

This is because PyJWT no longer raises an exception for future `iat`
times:

jpadilla/pyjwt#190

PyJWT removed this validation because:

- Clock skew can cause one party to generate `iat` times a few seconds
or minutes ahead of another's current time

- The JWT spec (RFC 7519) doesn't say that a JWT with a future `iat`
should be considered invalid, these JWTs are valid

- Other JWT libraries don't do this check
@gobengo
Copy link
Author

gobengo commented Aug 10, 2023

#252

@vergenzt
Copy link

vergenzt commented Dec 7, 2023

Update for anybody finding this discussion later:

Two other related issues:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants