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

Support RS256/JWKS for signing/verifying OAUTH JWTs #15912

Closed
techknowlogick opened this issue May 17, 2021 · 4 comments · Fixed by #16010
Closed

Support RS256/JWKS for signing/verifying OAUTH JWTs #15912

techknowlogick opened this issue May 17, 2021 · 4 comments · Fixed by #16010
Labels
topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! type/feature Completely new functionality. Can only be merged if feature freeze is not active.

Comments

@techknowlogick
Copy link
Member

techknowlogick commented May 17, 2021

Background information on RS256 here: https://auth0.com/blog/navigating-rs256-and-jwks/

Utilizing RS256 (as an option) to sign JWTs means that a shared secret won't need to be shared with applications to verify the validity of the token (likely currently applications assume tokens are valid without checking signature).

Two applications that I tested using our OIDC well-known endpoint, which are Sourcegraph and Smallstep CA, fail due to them needing to verify tokens they receive.

I'm willing to payout a bounty of $100USD on this (minus whatever bogus fees paypal requires), and pay that directly to contributor who completes this ticket. This is instead of using bounty source as they takes slightly more off top than paypal directly (I'm going this way to incentivise completion even slightly more).

cc: @jonasfranz

Edit: For this ticket please also create a jwks_uri and add it to the wellknown oidc endpoint.

@techknowlogick techknowlogick added type/feature Completely new functionality. Can only be merged if feature freeze is not active. topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! issue/bounty This issue has a bounty associated. Whoever opens a PR and gets it merged can claim the bounty. labels May 17, 2021
@KN4CK3R
Copy link
Member

KN4CK3R commented May 26, 2021

I have implemented this and tested it with Sourcegraph:
grafik
grafik

Currently I create a random RSA key on startup. How should we make this configurable in the app.ini?
As files?

JWT_PUB_KEY_FILE = custom/jwt/key.pub
JWT_PRIV_KEY_FILE = custom/jwt/key.priv

Should both signing methods be available (HS256, RS256)? What should be the default?

@techknowlogick
Copy link
Member Author

@KN4CK3R ❤️ yeah, I think the same approach that the builtin SSH server has for generating certs is acceptable. I think that RS256 may be a better default, as section 10.2 of the OIDC spec says "symmetric encryption MUST NOT be used by public (non-confidential) Clients because of their inability to keep secrets.", so it'd allow more flexible use cases.

@techknowlogick
Copy link
Member Author

To close the loop on this, I've paid out the bounty prior to the merge of this PR. Thanks again to @KN4CK3R for their work on this :)

@techknowlogick techknowlogick removed the issue/bounty This issue has a bounty associated. Whoever opens a PR and gets it merged can claim the bounty. label Jun 17, 2021
@viceice
Copy link
Contributor

viceice commented Aug 23, 2021

Will this change invalidate all my api keys? Or can i simply restart my drone ci and it works as before?

Setting JWT_SIGNING_ALGORITHM: HS256 for now, so i don't have this breaking change.

@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants