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

Raw public keys support for JWT authentication #6680

Merged
merged 3 commits into from
Jan 3, 2025

Conversation

maxtropets
Copy link
Collaborator

@maxtropets maxtropets commented Dec 3, 2024

Closes #6601

Key points

  • JsonWebKeyData created to avoid further changes in JsonWebKey, which is used for UVM endorsements parsing, as well as a base class for a plethora of key classes, which in turn are used in various key conversion APIs. Didn’t have enough courage to change that here.
  • Crypto hierarchy issues ( Fix RSAPublicKey hierarchy and hidden virtual functions #6673 ) are not addressed here, but using RSA keys instead of misused EC keys has been fixed.
  • Following the approach of JWT issuer validation #6175, the new key metadata table has been introduced to work with the new tables, previous metadata becomes “legacy” alongside old public_signing_key and public_signing_key_issuer tables. These are to be pruned in Cleanup old JWT tables in 5.0.1+ #6222, the ticket has been updated to mention the current change.
  • nbf claim is no longer mandatory, as it's Entra-specific, details here.

@maxtropets maxtropets self-assigned this Dec 4, 2024
@maxtropets maxtropets added the run-long-test Run Long Test job label Dec 4, 2024
@maxtropets maxtropets changed the title WIP raw key JWT auth . Raw public keys in JWT authentication Dec 5, 2024
@maxtropets maxtropets changed the title Raw public keys in JWT authentication Raw public keys (n+e) in JWT authentication Dec 5, 2024
@maxtropets maxtropets changed the title Raw public keys (n+e) in JWT authentication Raw public keys support for JWT authentication Dec 5, 2024
@maxtropets
Copy link
Collaborator Author

Pending manual test, but ready for review

@maxtropets maxtropets marked this pull request as ready for review December 5, 2024 01:21
@maxtropets maxtropets requested a review from a team as a code owner December 5, 2024 01:21
Copy link
Member

@PallabPaul PallabPaul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for these changes! They look good to me overall. I made some comments which were mainly questions to get a better understanding of some parts of the code/logic that was implemented. Definitely want to dive deeper into the code outside of this PR as well to get a better understanding of the full flow but these changes look good as far as my knowledge of the jwt validation goes, thanks again!

include/ccf/service/tables/jwt.h Show resolved Hide resolved
include/ccf/service/tables/jwt.h Show resolved Hide resolved
src/crypto/openssl/rsa_public_key.cpp Show resolved Hide resolved
src/endpoints/authentication/jwt_auth.cpp Show resolved Hide resolved
src/http/http_jwt.h Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
include/ccf/crypto/jwk.h Outdated Show resolved Hide resolved
@maxtropets maxtropets force-pushed the f/6601-jwt-raw-pubkeys-2 branch 2 times, most recently from 6cd8590 to 0c3ef0e Compare December 26, 2024 10:56
@maxtropets maxtropets force-pushed the f/6601-jwt-raw-pubkeys-2 branch 3 times, most recently from f4349d6 to dfb5e6c Compare December 26, 2024 14:13
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
tests/infra/jwt_issuer.py Outdated Show resolved Hide resolved
@maxtropets maxtropets force-pushed the f/6601-jwt-raw-pubkeys-2 branch from 05c8e95 to 178fb54 Compare January 3, 2025 16:09
@maxtropets maxtropets force-pushed the f/6601-jwt-raw-pubkeys-2 branch from 178fb54 to 51ac2f4 Compare January 3, 2025 16:10
@achamayou achamayou enabled auto-merge January 3, 2025 16:26
@achamayou achamayou added this pull request to the merge queue Jan 3, 2025
Merged via the queue into microsoft:main with commit f0d567d Jan 3, 2025
13 checks passed
@achamayou achamayou deleted the f/6601-jwt-raw-pubkeys-2 branch January 3, 2025 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-long-test Run Long Test job
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JWT: support both certs and raw public keys
3 participants