feat: add Jwk::from_decoding_key #475
Conversation
|
That will need to wait a bit, we need to merge #452 first that has a lot of change that would overlap with some of your changes. |
eyakubovich
force-pushed
the
ey/jwk-from-decoding-key
branch
from
5260d7c to
4016f48
Compare
|
@Keats @arckoor PTAL. I removed As an aside, why are function pointers used instead of traits? |
There was a problem hiding this comment.
As an aside, why are function pointers used instead of traits?
It was what happened to work when I implemented it, I didn't explicitly choose not to use traits, there wasn't any deep logic behind it.
I didn't necessarily plan for more stuff being added in the future
| pub(crate) fn ec_components_from_public_key( | ||
| pub_bytes: &[u8], | ||
| ) -> errors::Result<(EllipticCurve, Vec<u8>, Vec<u8>)> { | ||
| let (curve, pub_elem_bytes) = match pub_bytes.len() { | ||
| 65 => (EllipticCurve::P256, 32), | ||
| 97 => (EllipticCurve::P384, 48), | ||
| _ => return Err(ErrorKind::InvalidEcdsaKey.into()), | ||
| }; | ||
|
|
||
| if pub_bytes[0] != 4 { | ||
| return Err(ErrorKind::InvalidEcdsaKey.into()); | ||
| } | ||
|
|
||
| let (x, y) = pub_bytes[1..].split_at(pub_elem_bytes); | ||
| Ok((curve, x.to_vec(), y.to_vec())) | ||
| } | ||
|
|
There was a problem hiding this comment.
This doesn't depend on the crypto backend, and shouldn't be part of the public api. Remove that from EcPublicComponents, which then has only one attribute, so should be combined with RsaPublicComponents into something like PublicComponents or similar.
There was a problem hiding this comment.
I put it into the crypto backend for symmetry but also because the fact that it doesn't depend on the crypto backend is an implementation detail. From the user POV, it's a crypto related operation. Technically it's an operation on a data/file format, not a crypto algorithm but neither are the rest of the extraction functions.
If I pull it out, what module should I put into?
There was a problem hiding this comment.
The end user doesn't really care about this, third party providers do. If it were to remain, every other provider would need to reference it in their CryptoProvider, for seemingly no reason at all (also it would need to be public).
I guess you could pull it out to jwk.rs, but for consistency it's probably not too bad if it stays with the CryptoProvider
I'm not really set on that though
src/crypto/mod.rs
Outdated
| } | ||
|
|
||
| impl RsaPublicComponents { | ||
| /// Creates a instance filled with dummy functions that always panic |
src/crypto/mod.rs
Outdated
| /// Initialises all values to dummies. | ||
| /// Will lead to a panic when JWKs are required, so only use it if you don't want to support JWKs. | ||
| impl EcPublicComponents { | ||
| /// Creates a instance filled with dummy functions that always panic |
src/crypto/rust_crypto/mod.rs
Outdated
| pkcs1::{ | ||
| DecodeRsaPrivateKey, DecodeRsaPublicKey | ||
| }, |
src/crypto/mod.rs
Outdated
| pub rsa_public_components: RsaPublicComponents, | ||
| /// Functions to extract EC public key components from private and public keys | ||
| pub ec_public_components: EcPublicComponents, | ||
|
|
There was a problem hiding this comment.
why the newlines everywhere?
eyakubovich
force-pushed
the
ey/jwk-from-decoding-key
branch
2 times, most recently
from
4e8bd5f to
7a64e5d
Compare
eyakubovich
force-pushed
the
ey/jwk-from-decoding-key
branch
from
7a64e5d to
89c40ef
Compare
Allow for constructing a Jwk from a decoding key. This allows it to be created from a DER encoded file, for example. This patch flattens JwkUtils but adds separate structs for RSA and EC component extraction.
eyakubovich
force-pushed
the
ey/jwk-from-decoding-key
branch
from
89c40ef to
4cf5422
Compare
3 participants
Allow for constructing a Jwk from a decoding key.
This allows it to be created from a DER encoded file, for example.
This patch refactors some Jwk internals to reduce code duplication.