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

jwk: Add keep pairs option for keys delete #1476

Closed
sawadashota opened this issue Jun 19, 2019 · 7 comments
Closed

jwk: Add keep pairs option for keys delete #1476

sawadashota opened this issue Jun 19, 2019 · 7 comments

Comments

@sawadashota
Copy link
Contributor

Is your feature request related to a problem? Please describe.

When rotating JWKs, I delete old keys at first and it takes time and effort like this.

#!/usr/bin/env sh

hydra keys get hydra.openid.id-token | jq -r '.Payload.keys[].kid' \
| awk 'NR>2 {print}' | sed -e 's/^"//' -e 's/"$//' \
| xargs -I{} hydra keys delete hydra.openid.id-token/{}

hydra keys get hydra.jwt.access-token | jq -r '.Payload.keys[].kid' \
| awk 'NR>2 {print}' | sed -e 's/^"//' -e 's/"$//' \
| xargs -I{} hydra keys delete hydra.jwt.access-token/{}

hydra keys get hydra.https-tls | jq -r '.Payload.keys[].kid' \
| awk 'NR>2 {print}' | sed -e 's/^"//' -e 's/"$//' \
| xargs -I{} hydra keys delete hydra.https-tls/{}

This complicated script means delete keys except latest key pairs each JWK Set.
I execute this before rotating system secret because hydra cannot serve if keys associated deleted system secret exists.

Describe the solution you'd like

Add keep-pairs option to delete a JSON Web Key Set endpoint and command.

For example:

  • API: /keys/{set}?keep-pairs=1
  • Command: hydra keys delete {set} --keep-pairs 1

This is same as following

hydra keys get {set} | jq -r '.Payload.keys[].kid' \
| awk 'NR>2 {print}' | sed -e 's/^"//' -e 's/"$//' \
| xargs -I{} hydra keys delete {set}/{}

Additional context

My system rotation step is here.
This proposal helps first step.

  1. Delete old JWKs (hydra.openid.id-token/hydra.jwt.access-token/hydra.https-tls)
  2. Delete a old system secret
  3. Add a new system secret
  4. Create JWKs (hydra.openid.id-token/hydra.jwt.access-token/hydra.https-tls)
  5. Restart hydra server
@aeneasr
Copy link
Member

aeneasr commented Jun 24, 2019

You don't need to delete old keys. In fact, it's not a good idea to delete old keys as you will instantaneously invalidate all keys so far. ORY Hydra always uses the most recent key for signature issuance. Therefore, if you rotate the key, the new one will automatically be used for signing.

@sawadashota
Copy link
Contributor Author

It was wrong! My proposal is for encryption key rotation.
https://openid.net/specs/openid-connect-core-1_0.html#RotateEncKeys

In this case, it's signing key rotation.
https://openid.net/specs/openid-connect-core-1_0.html#RotateSigKeys

Signing key rotation process is difference from encryption key rotation.
But I cannot found "DO NOT DELETE".
If key rotation cycle is a few months or a year, jwks_uri's response body will increase continuously due to expired keys.

Key deletion is infrequent and dangerous but I think it's need.
So that I proposed option for more safety key deletion.

@aeneasr
Copy link
Member

aeneasr commented Jun 28, 2019

If key rotation cycle is a few months or a year, jwks_uri's response body will increase continuously due to expired keys.

That's a good point, they should be cleaned up once in a while. Maybe we should add a created_at key and have hydra keys delete --older-than=2018-12-12? I think that would make it much clearer!

@sawadashota
Copy link
Contributor Author

Sounds good! It seems more safety.

@aeneasr
Copy link
Member

aeneasr commented Jul 11, 2019

Would you be up for a PR? :)

@sawadashota
Copy link
Contributor Author

Sure! Please wait for me

@aeneasr
Copy link
Member

aeneasr commented Sep 5, 2021

I am closing this issue as it has not received any engagement from the community or maintainers in a long time. That does not imply that the issue has no merit. If you feel strongly about this issue

  • open a PR referencing and resolving the issue;
  • leave a comment on it and discuss ideas how you could contribute towards resolving it;
  • open a new issue with updated details and a plan on resolving the issue.

We are cleaning up issues every now and then, primarily to keep the 4000+ issues in our backlog in check and to prevent maintainer burnout. Burnout in open source maintainership is a widespread and serious issue. It can lead to severe personal and health issues as well as enabling catastrophic attack vectors.

Thank you to anyone who participated in the issue! 🙏✌️

@aeneasr aeneasr closed this as completed Sep 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants