Feat: Added support for JWK decoding Ed25519 keys#506
Conversation
| let curve_type: EllipticCurve = match key.inner().len() { | ||
| 48 => Ok(EllipticCurve::Ed25519), | ||
| 73 => Ok(EllipticCurve::Ed448), | ||
| _ => Err(Error::from(ErrorKind::InvalidEddsaKey)), |
There was a problem hiding this comment.
I don't understand the point of classifying Ed448 curves if the provider just says "I don't support this" anyways. I.e. why have the extra enum value and detection logic if this could be a simple
match key.inner().len() {
48 => Ok(...),
_ => Err(...),
}I haven't worked with 448 before, why is this one checking for length of 73 when from_ed25519_encoding_key is checking for 57?
There was a problem hiding this comment.
The mismatch was actually a mistake, 57 bytes is the length of the key itself, 73 bytes is the length of the key including the ANS.1 header.
I have however, improved the checking logic to rather use OIDs instead of length checks which is more reliable. The logic was largely lifted from the pem/decoder#classify_pem method where it uses ans1 headers to determine algorithm types.
There was a problem hiding this comment.
As for why I would prefer to include Ed448 in the check is they are valid Ed algorithms, so to provide a clear error code (ErrorKind::UnsupportedAlgorithm) we need to first identify it as a valid Ed algorithm, then down the line we can state it is unsupported, rather than the catch all InvalidAlgorithm/InvalidEddsaKey.
| /// Type of cryptographic curve used by a key. This is defined in | ||
| /// [RFC 7518 #7.6](https://tools.ietf.org/html/rfc7518#section-7.6) | ||
| #[derive(Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize, Hash)] | ||
| #[non_exhaustive] |
There was a problem hiding this comment.
Following fa89018 the EllipticCurve's should also be non-exhaustive
|
Thanks for the review @arckoor! I have implemented the requested changes if I could please get a re-review. |
Summary: Added decoding JWK support for Ed25519 keys
Changes:
unimplementedJwk::from_encoding_keywhenAlgorithm::EdDSAxparameter (https://datatracker.ietf.org/doc/html/rfc8037#section-2)EllipticCurvetypeEd448for Ed448 curvesSolves: #244