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

Have a set of keys instead of just one #45

Closed
Tracked by #48
jakipatryk opened this issue Jun 7, 2023 · 2 comments · Fixed by #111
Closed
Tracked by #48

Have a set of keys instead of just one #45

jakipatryk opened this issue Jun 7, 2023 · 2 comments · Fixed by #111
Assignees
Labels
enhancement New feature or request

Comments

@jakipatryk
Copy link
Collaborator

jakipatryk commented Jun 7, 2023

Background

To allow more UX-friendly keys rotation, having a set of keys for signing JWTs is needed. Once a set of keys is used, the "oldest" one can stop signing keys for time equal to expiration of JWT set in config. After that time, it can be replaced with new one.

Example (more smooth rotation):

  1. Current keys: [A].
  2. New key B is generated (for example by rotation of A). Current keys [B, A].
  3. For some time (let's say 30 min) B is not used yet to sign JWT to allow backends based on LS to fetch this new set of keys.
  4. After that time, B starts signing JWTs. A becomes "idle" for some time.
  5. A gets removed. Current keys: [B].

A set of keys is also useful if we for some reason will have to revoke some JWTs, as we would only have to revoke one key from a set, leaving more users unaffected.

Feature

Have a set of keys for signing JWTs, each key having its own unique ID, and add this ID to header of JWT.

@jakipatryk jakipatryk added the enhancement New feature or request label Jun 7, 2023
@benedeki
Copy link
Collaborator

Why point 3? Why the B cannot be be used after generation?
Imho it's "enough" if A is accepted for some time even after it's life oeriod has passed, and it's not being sent out anymore.

@jakipatryk
Copy link
Collaborator Author

jakipatryk commented Dec 24, 2024

Why point 3? Why the B cannot be be used after generation? Imho it's "enough" if A is accepted for some time even after it's life oeriod has passed, and it's not being sent out anymore.

Point 3 gives backends ("resource servers") time to fetch this new public key B, so that they are aware that this is a valid key. As it is not used yet to sign JWTs, the window for backends to perform the fetch is exactly that time. Without point 3, LS would be signing JWTs with new key, which backends will not be aware of until they refetch keys.

Though backends can do always refetch keys from LS when verification of JWT fails (particularly if key ID kid in the JWT is unknown to the backend). With that, the "waiting period" is not needed. But this assumes backends do that. It might be safer to not assume that.

This ticket is just a proposal for now, nothing that is decided yet, we'll need to check what the industry standard is.

@TheLydonKing TheLydonKing self-assigned this Dec 30, 2024
@TheLydonKing TheLydonKing linked a pull request Jan 8, 2025 that will close this issue
dk1844 added a commit that referenced this issue Jan 9, 2025
dk1844 added a commit that referenced this issue Jan 10, 2025
#45 a suggestion to add sso to be able to use `aws sso login` to dev-launch LS
TheLydonKing added a commit that referenced this issue Jan 16, 2025
* Added Version Stage search to AwsSecretsUtils

* Implement keyConfigs to fetch Previous Keys if available

* Implement keyConfigs to fetch Previous Keys if available

* Implement JWTService

* Implement Tests

* #45 a suggestion to add sso to be able to use `aws sso login` to dev-launch LS

* Use new PublicKeySet

* Setup key phase out process

* Implement testing for phasing out keys

* Implement changes

* Simplify AwsSecretsUtils call for create date to one call

---------

Co-authored-by: Daniel K <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants