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

Add support for JWK thumbprints #363

Closed
wants to merge 7 commits into from

Conversation

andrewbaxter
Copy link

@andrewbaxter andrewbaxter commented Jan 14, 2024

https://datatracker.ietf.org/doc/html/rfc7638

I republished the ring digests so that the feature could be used without matching ring versions.

The thumbprint json is hardcoded templates because I'm not sure if serde_json guarantees json string format (ordering, spacing, etc). The spec is written in such a way that quotation marks and backslashes won't appear in the serialized values so there's no risk of escape issues.

I think the readme's getting a bit crowded, but I didn't want to introduce unrelated changes here so I didn't touch it.

Aside from the unit test, I was testing with letsencrypt/pebble with EC keys and it seemed to be okay (although I switched from serde_json to hardcoded string templates after that).

src/lib.rs Outdated
@@ -22,3 +22,7 @@ pub use decoding::{decode, decode_header, DecodingKey, TokenData};
pub use encoding::{encode, EncodingKey};
pub use header::Header;
pub use validation::{get_current_timestamp, Validation};

pub use ring::digest::SHA256 as DIGEST_SHA256;
Copy link
Owner

Choose a reason for hiding this comment

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

Let's not expose ring at all. Make an enum with hidden internals and let users use that.

Copy link
Author

Choose a reason for hiding this comment

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

Done!

@andrewbaxter
Copy link
Author

Targetted at the ACME/JWS branch here andrewbaxter#1

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 this pull request may close these issues.

2 participants