Skip to content
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

Oauth2ExpirationIssuedAtValidationRule "issuedAt" vs "now" should have 1-5s leeway #3692

Closed
richardtreier opened this issue Dec 6, 2023 · 3 comments · Fixed by #3728
Closed
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@richardtreier
Copy link
Contributor

richardtreier commented Dec 6, 2023

Bug Report

Describe the Bug

OAuth2 token validation fails due to the assertion !now.isBefore(issuedAt) where issuedAt rounded to have no milliseconds, but now has millseconds and nanoseconds.

There have been multiple cases for us where clock skew caused failures aswell.

This time we experienced a situation where synchronizing the clocks did not work and the only solution was to forward the local connector's clock artificially. This, however, is not a solution for production, as all our connectors run in AKS nodes.

Expected Behavior

  • The token validation should have 1-5s leeway to account for clock skew.
  • The JWT Spec explicitly states "some leeway" for the expiration date, for example: https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.4
  • The token validation should be able to handle dates without millisecond values.

Observed Behavior

  • Comparison of dates with and without Milliseconds causes the hard comparison to fail.

Steps to Reproduce

  • This cannot be reproduced on a single machine.

Context Information

  • Eclipse EDC v0.2.1
  • The DAPS is a Keycloak having its system time managed by Azure Managed Kubernetes Services (AKS)
  • AKS nodes have time without milliseconds and nanoseconds.
  • We have observed 1-2s clock skew between AKS nodes.
  • The Connector is running in Docker on a freshly booted Windows PC with Time Syncs before and after the boot

Detailed Description

  • Here's a screenshot of the token failure, debugging: Oauth2ExpirationIssuedAtValidationRule

grafik

  • The difference clocks 250ms.

grafik

Possible Implementation

  • Add a fixed amount of leeway to all comparisons in Oauth2ExpirationIssuedAtValidationRule to account for clock skew between the DAPS and connectors
@yylian
Copy link

yylian commented Dec 7, 2023

This has already resulted in a case where one of two connector running on the same Azure AKS cluster, but on different nodes with a minimal time difference of about ~200-300 ms, failed to retrieve the catalog of the other connector. The rejection of the token comes from the MIW, but it's (very probably) the exact same issue.

@richardtreier
Copy link
Contributor Author

We would also like to offer contributing code for this. We have been looking into this process, and we'd love to move forward to being able to contribute to the Eclipse EDC in meaningful ways, where it would make sense. If that would be fine with you.

@jimmarino
Copy link
Contributor

We would also like to offer contributing code for this. We have been looking into this process, and we'd love to move forward to being able to contribute to the Eclipse EDC in meaningful ways, where it would make sense. If that would be fine with you.

Contributions are always welcome. Please post a solution proposal that includes a configurable not-before leeway or a targetted PR that does this.

@paullatzelsperger paullatzelsperger added feature_request New feature request, awaiting triage triage all new issues awaiting classification labels Dec 13, 2023
@paullatzelsperger paullatzelsperger added good first issue Good for newcomers enhancement New feature or request and removed feature_request New feature request, awaiting triage triage all new issues awaiting classification labels Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
4 participants