nbf validation issue

[bolner]

Hi, in the current version:

php-jwt/src/ValidatesJWT.php

Line 89 in 926ef39

['nbf', $this->maxAge - $this->leeway, static::ERROR_TOKEN_NOT_NOW, 'Not now'],

That line should be:

['nbf', -$this->leeway, static::ERROR_TOKEN_NOT_NOW, 'Not now'],

When checking the nbf ("not before") time, then the "max age" value is not relevant.

Let's see an example:

• Current time = 2020-10-08 12:00:00
• Max age = 3 hours = 10800 sec
• nbf = 2020-10-08 11:00:00 (will be valid after this time)
• leeway = 5 sec (allow max 5 sec misalignment of server clocks)

Then the current code would say:

• The token fails the nbf check because:
  • reference value = 2020-10-08 11:00:00 + 3 hours - 5 sec = 2020-10-08 13:59:55
  • And the current time still hasn't reached this value yet.

But it should only subtract the leeway, and leave the irrelevant "max age" out of this:

• The nbf check is successful, because:
  • reference value = 2020-10-08 11:00:00 - 5 sec = 2020-10-08 10:59:55
  • And the current time is greater than this value.

[adhocore]

or maybe if nbf is used, we can adjust exp by adding maxage while generating jwt?

[adhocore]

and thank you for reporting issue 👍

[bolner]

or maybe if nbf is used, we can adjust exp by adding maxage while generating jwt?

I think of nbf and exp as two end points of an interval, like: start = nbf - leeway, end = exp + leeway. And for validation the question is whether the current time is inside or outside of this interval, which is fixed.

iat is a bit redundant with the previous two, and it's not always clear how to use it for validation, but one can say for example: start = iat - leeway, end = iat + max_age + leeway, so if the max_age changes on the server side, then that would immediately affect all tokens in use. But I think iat should not be validated without the developer "asking" for it through a parameter. (And they should know how it is used for validation.)

[adhocore]

i meant to say in addition to though.
the fix for nbf looks good, could you pls do a quick PR?

[bolner]

oh yes, a sec

[bolner]

in the "main" branch?

[bolner]

Oh I just saw that for the iat check, the sign of the leeway needs to be inverted, from minus to plus. Because of what I wrote above. It's a check for the upper value of the interval, and the leeway should always make the interval bigger. So it increases the upper values and decreases the lower ones. I have to go to sleep now though :) will check back tomorrow

[adhocore]

ok then 😁
leeway is to be used in a way it is advantageous to the jwt token client (and yes nbf should be plus)

edit: however because of the check equation

$fail = $timestamp <= $nbf - $leeway;
// or
$fail = $nbf >= $timestamp + $leeway;

we are good

[bolner]

oh i meant the iat line:

php-jwt/src/ValidatesJWT.php

Line 88 in 424a1d6

['iat', $this->maxAge - $this->leeway, static::ERROR_TOKEN_EXPIRED, 'Expired'],

That is for an upper interval check, but the leeway is subtracted.

[adhocore]

iat is, like you said, confusing. some implementations use it the same way as nbf.

the rfc is at: https://tools.ietf.org/html/rfc7519#section-4.1.6

i will compare with other impls and see what we can do here (in a few days)