Feat: Implement ES256K support for secp256k1 signed JWTs #461
Conversation
ThisIsMissEm
force-pushed
the
feat/implement-secp256k1-ES256K
branch
from
4c71cca to
bcb6d41
Compare
Keats
left a comment
Keats
left a comment
There was a problem hiding this comment.
I think we shouldn't expose it when using the aws-ls-crypto feature since we can't actually use it there
| ES384, | ||
|
|
||
| /// ECDSA using secp256k1 | ||
| ES256K, |
There was a problem hiding this comment.
should this be gated by rust_crypto feature since it's only available there? As well as all JWK operations?
There was a problem hiding this comment.
I'm not sure how to make this rust_crypto gated (that's a little beyond my current knowledge of rust)
would it be something like wrapping in: };
#[cfg(feature = "rust_crypto")]?
There was a problem hiding this comment.
I don't see a good way to achieve this gating without duplicating a tonne of implementation code, e.g., from_encoding_key and impl FromStr for KeyAlgorithm would both need a separate implementations for rust_crypto vs aws_lc_rs, since it doesn't look like you can do a cfg gate in a match call
There was a problem hiding this comment.
Maybe it'd be better to leave the ES256K option in the enum, but just do an implementation for aws-lc-rs that just returns return Err(new_error(ErrorKind::InvalidKeyFormat)); always?
There was a problem hiding this comment.
This is the cleanest way I can think of to make it compile under aws_lc_rs: 4639024
| assert!(res.is_ok()); | ||
| } | ||
|
|
||
| #[cfg(all(feature = "use_pem", feature = "rust_crypto"))] |
There was a problem hiding this comment.
I've gated these tests here on rust_crypto, but we could also assert that under aws_lc_rs these return an error?
2 participants
This implements #391, and I've tested it for compatibility with the JWTs that the AT Protocol codebase produces (it's a little complex on the node.js/javascript side, so I haven't included that but I can if need be).
The error I mentioned in that ticket was because I was accidentally doing
Validation::new(Algorithm::ES256),instead of&Validation::new(Algorithm::ES256K),when decoding the JWT, which, super silly mistake, but I caught it today!This would unblock: mike-engel/jwt-cli#402