Keats / jsonwebtoken

JWT lib in rust
MIT License
1.62k stars 252 forks source link

Expiration validated incorrectly when system time is not UTC #300

Closed litcc closed 8 months ago

litcc commented 1 year ago

Related questions: https://github.com/Keats/jsonwebtoken/issues/253#issue-1253005342

https://github.com/Keats/jsonwebtoken/blob/8fba79b25459eacc33a80e1ee37ff8eba64079ca/src/validation.rs#L12-L38

https://github.com/Keats/jsonwebtoken/blob/8fba79b25459eacc33a80e1ee37ff8eba64079ca/src/validation.rs#L214-L231

https://github.com/Keats/jsonwebtoken/blob/8fba79b25459eacc33a80e1ee37ff8eba64079ca/src/validation.rs#L138-L144

The comment of Validation states that all validation is based on UTC timestamps, but in the actual code, the time obtained is the current time in the current system time zone, so there may be ambiguities, and when exp is passed in UTC timestamps, the time validation will not take effect.

Keats commented 1 year ago

Hmm I guess everyone runs their server with UTC. We could pull chrono (or any other lib giving the local offset) as a feature to determine the offset and then convert it to UTC timestamp

litcc commented 1 year ago

then the documentation can be amended to state that it does not necessarily complete all time checks via UTC, depending on the current system time zone.

It is also possible to add some configuration to Validationt, such as providing a user-configurable source for the current time, setting fn get_current_timestamp() -> u64 as the default source, which would avoid relying directly on third-party time libraries in the library and let the user choose which one to use

qsantos commented 9 months ago

Should this bug be closed? According to @nickguletskii in #311, this does not actually happen. If this is a real bug, it's critical, since this directly affect security and can be overlooked extremely easily.

I did encounter an issue with expiration validation, which led me here, but I just did not know about Validation::leeway. When taking it into account, I do not have any issue with expiration validation even though my system is currently UTC+0200.

litcc commented 8 months ago

Previously occurred out of time zone problems, unified adjustment of the server to use the UTC time zone after the repair, currently tested by the inability to reproduce the problem, so closed

qsantos commented 8 months ago

Thanks!