adhocore / php-jwt

Ultra lightweight, dependency free and standalone JSON web token (JWT) library for PHP5.6 to PHP8.2. This library makes JWT a cheese. It is a minimal JWT integration for PHP.
https://github.com/adhocore/php-jwt
MIT License
290 stars 21 forks source link

nbf validation issue #20

Open bolner opened 3 years ago

bolner commented 3 years ago

Hi, in the current version: https://github.com/adhocore/php-jwt/blob/926ef39300872fd71bed27c235fae2ff8b1b89b0/src/ValidatesJWT.php#L89

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:

Then the current code would say:

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

adhocore commented 3 years ago

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

adhocore commented 3 years ago

and thank you for reporting issue šŸ‘

bolner commented 3 years ago

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 commented 3 years ago

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

bolner commented 3 years ago

oh yes, a sec

bolner commented 3 years ago

in the "main" branch?

bolner commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

oh i meant the iat line: https://github.com/adhocore/php-jwt/blob/424a1d66b729a316dd074e6382167765b810cd3d/src/ValidatesJWT.php#L88

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

adhocore commented 3 years ago

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)