-
Notifications
You must be signed in to change notification settings - Fork 16
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
Rename KeyType -> KeyAlgorithm #34
Conversation
/// Enum defining all supported cryptographic key types. | ||
pub enum KeyType { | ||
/// Enum defining all supported cryptographic algorithms for a [`Key`]. | ||
pub enum KeyAlgorithm { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The values for this enum appear to me to be Elliptic Curves (as defined in https://www.rfc-editor.org/rfc/rfc7518.html#section-6.2.1.1 and who's possible values are in the registry located on https://www.iana.org/assignments/jose/jose.xhtml#web-key-elliptic-curve). How about we call this Curve
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
supporting RSA 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious what thoughts are here, specially from @mistermoe and @decentralgabe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is not semantically correct. We need to align with industry used terms, like key type (kty
in JOSE land). https://datatracker.ietf.org/doc/html/rfc7517#section-4.1
Algorithms are used with keys which have a type. Like:
Key Type | Signature Algorithm |
---|---|
Ed25519 | EdDSA |
secp256k1 | ES256K |
P-256 | ES256 |
P-384 | ES384 |
P-521 | ES512 |
RSA | PS256 |
BLS | BBS+ |
Dilithium Mode 2 | CRYDI2 |
Dilithium Mode 3 | CRYDI3 |
Dilithium Mode 5 | CRYDI5 |
Point taken, closing! |
Thanks for sharing @decentralgabe, I wasn't aware of the distinction |
KeyType
was found to be a bit confusing: #20 (comment)We originally had it as
KeyAlgorithm
, but changed it here: #17 (comment)This PR switches it back to
KeyAlgorithm
!