Issued At RFC type mismatch causing false postives on verify_at

[ghost]

Hello there,

We had some troubles with ruby-jwt in production recently. The reason was that we put the unix timestamp as seconds since epoch in iat, but inside verify_iat ruby-jwt casts it using .to_f.

When comparing iat against Time.now.to_f you compare an int (rounded to .0) to a full decimal version, making iat.to_f > Time.now.to_f trigger a false positive.

The specification in https://tools.ietf.org/html/rfc7519#section-4.1.6 states that the iat should be a NumericDate as defined in RFC7519:

NumericDate
A JSON numeric value representing the number of seconds from
1970-01-01T00:00:00Z UTC until the specified UTC date/time,
ignoring leap seconds. This is equivalent to the IEEE Std 1003.1,
2013 Edition [POSIX.1] definition "Seconds Since the Epoch", in
which each day is accounted for by exactly 86400 seconds, other
than that non-integer values can be represented. See RFC 3339
[RFC3339] for details regarding date/times in general and UTC in
particular.

The fix for this should be trivial (replacing to_f with to_i) but before submitting a pull request I'd like to check with you if that fix makes sense.

[excpt]

@elyhaka I give you a green light for a PR. :)

[jakeprime]

I think there is a potential issue here because to_i always rounds down. I do not know the exact process that creates the iat, but if it rounds both ways we could have false positives. Consider this example:

• Token created at exactly 123.6
• iat value is rounded up to 124
• Token is verified at exactly 123.9
• to_i rounds this to 123
• 124 > 123 # => JWT::InvalidIatError

timestamps are deliberately shortened from realistic values for readability

Is there a case for allowing up to 1s delta, without treating it as a leeway for the purposes of the RFC as disussed in #273? I think that is reasonable given the values we are dealing are rounded and can be up to 1s off.

[anakinj]

I have the same concerns as @jakeprime. A delta would help, but is that allowed?

[anakinj]

Been playing around with this today and I think the problem is not that the values are handled as a floats.

Do you @elyhaka have some example values that causes this problem. Think that if we start converting to_i there will be even more problems with tokens created on the exact same second, like the example @jakeprime pointed out.

Something that could help is to allow giving a validation specific leeway.

JWT.decode(token, secret, true, { verify_iat: { leeway: 1 }, algorithm: 'HS256' })

[danleyden]

In the example above where the token is created at 123.6, the correct value for iat should be 123 (not 124) because the number of (completed) seconds since epoch is only 123... so the result of disallowing that would actually be correct.

A slightly different example where this may be a problem is with clock drift between the issuer and and checker. If the issuer is running (marginally) faster than checker, it is possible that the issuer issues the token at 124.001 per its local clock (and thus set to 124) whereas the checker's clock which is running slower perceives the time as 123.999. Depending on what hardware is in use and what synchronization is in place, drift of a few seconds isn't unheard of (unsynchronised desktop computer hardware commonly drifts 1-20s per day depending on the quality... hence modern operating systems tend to sync with NTP servers fairly regularly)

[anakinj]

The current check we are discussing is actually something like raise if iat.to_f > Time.now.to_f so using .to_f actually make the same second case pass the check.

Switching to_f -> to_i without changing the comparison to >= will increase the chance of failure.

[anakinj]

Would like to experiment with the following to improve the iat validation:

• Continue to be exact using to_f on the values
• Introduce a way to allow the client to define how big drift is allowed. (default to 0 to not change the behaviour)

Think how validation of iat should be done is not something that is defined by any standard or specification. So it's up to the validator to choose.

[anakinj]

Wrote a few tests to verify that the current behaviour is as good as it gets without any leeway in the verification #423

Also it seems that taking the leeway into consideration for this verification was removed in #257, reasoning was that there is no mentioning of leeway in the spec.

I think we have two options:

• Remove the iat verification because there is nothing in the RFC saying if and how it should be verified
• Allow consumers to define how the iat verification works by supporting a custom iat leeway when verifying

Personally i think supporting verifications with some custom interpretations is the way to go, would not break any of the current assumptions.

Any thoughts @excpt @ab320012

[anakinj]

Actually the only verification we can do according to the RFC is that the value is a NumericDate if it's given.

[vergenzt]

FYI I've submitted an errata request to RFC 7519 to update the spec to explicitly prohibit rejecting tokens with iat from "the future".

Feel free to follow along here: Discussion: JWT tokens containing iat values in the future should not be rejected