-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Allowed clock skew resolving #815
Comments
@laurids thank you for the issue, especially such a well-written one! I must admit, I was a little surprised by the variance represented across various servers. A production Authorization Server (AS) - responsible for real security implications of clients, applications and users, has up to a full 10 minute clock skew? The JWT RFC time-based assertions are expected to be reasonably close to the 'real' time in the world - i.e. synchronized close enough to atomic clocks around the world - such that a minute or two (or three?) should be more than sufficient. I would find it difficult to undertake this feature request given my bigger concern for an actual security apparatus (the AS) that is that far out of date, when so many security implications derive from an AS. In other words, that level of difference is a real security risk, and I feel it'd be better to get that AS on a reasonable system time than invest design, implementation and test time in JJWT for a feature that, at least IMO, helps circumvent the 'real' risk of a misconfigured AS. Also, for what it's worth this is the first such request in nearly 9 years we've had for a per-issuer clock skew, so at least on the surface, feels a little to me like JJWT modifications might not be the right place to address the issue. To be clear however, I'm not saying we shouldn't undertake the work, just expressing my doubts. If others requested such a feature, I'd be much more open to doing the work. At the very least, if we would undertake this kind of request, it'll have to wait until after probably the |
Hi @lhazlewood, thanks for the response! PS: I made a mistake when describing our possible workaround. We cannot just catch and ignore those exceptions, since the whole parsing method would obviously fail then. |
I was thinking about this more today, and I'm pretty sure this will be resolved via #474. You would be able to specify a custom |
Thanks for the update! Without having seen the code, this indeed sounds like it could solve our issue. |
Is your feature request related to a problem? Please describe.
At the time of parsing the JWT, we don't know which authorization server (AS) issued the JWT.
We want to enable configuring "allowed clock skew" per AS. E.g. for one AS, we allow 10 minutes deviation, for another we only allow 30 seconds.
Regarding the signature verification, we can accommodate modelling multiple AS's with the
setSigningKeyResolver
on theJwtParser
, since theresolveSigningKey
gets passed theClaims
andJwsHeader
containing both the Key Identifier and the Issuer, which we can use to identify the AS and lookup the key.However, for the allowed clock skew, there is only the
setAllowedClockSkewSeconds(long seconds)
, which we would have to set before parsing, when we don't know the specific AS.We cannot (mis)use the
setClock
either, becauseclock.now()
is only called once and is used for validating both expiration and "not before".Describe the solution you'd like
We would like a resolver that sets both the signing key and the allowed clock skew, or a separate resolver that just sets the allowed clock skew.
Describe alternatives you've considered
Alternatively, we could catch and ignore the
ExpiredJwtException
andPrematureJwtException
when parsing, and then verify expiration and "not before" explicitly afterwards. However, in that case, it would be nice to have the time validation as a separate public method so we could call that part of the code again afterwards.Additional context
N/A
Thank you for creating a good library whether or not you deem this a worthy feature request.
The text was updated successfully, but these errors were encountered: