make CertificateParams::signed_by accept impl PublicKeyData rather than KeyPair for the key being signed
#288
Conversation
| @@ -36,7 +36,7 @@ x509-parser = { workspace = true, features = ["verify"], optional = true } | |||
| zeroize = { version = "1.2", optional = true } | |||
|
|
|||
| [features] | |||
| default = ["crypto", "pem", "ring"] | |||
| default = ["crypto", "pem", "ring", "x509-parser"] | |||
| self, | ||
| key_pair: &KeyPair, | ||
| key_pair: &K, |
There was a problem hiding this comment.
Nit: I'd prefer to spell this as &impl PublicKeyData.
| @@ -57,6 +57,90 @@ impl fmt::Debug for KeyPairKind { | |||
| } | |||
| } | |||
|
|
|||
| /// A public key | |||
| #[cfg(feature = "x509-parser")] | |||
There was a problem hiding this comment.
There's no need to guard the type directly.
| } | ||
|
|
||
| /// Create a `SubjectPublicKey` value from DER-encoded SubjectPublicKeyInfo bytes | ||
| pub fn from_der(spki_der: &[u8]) -> Result<Self, Error> { |
| #[cfg(feature = "x509-parser")] | ||
| impl SubjectPublicKey { | ||
| /// Create a `SubjectPublicKey` value from a PEM-encoded SubjectPublicKeyInfo string | ||
| pub fn from_pem(pem_str: &str) -> Result<Self, Error> { |
There was a problem hiding this comment.
This needs the feature guards from from_der() and also another one for pem.
| } | ||
| } | ||
|
|
||
| #[cfg(feature = "x509-parser")] |
| @@ -689,7 +773,7 @@ impl<T> ExternalError<T> for Result<T, pem::PemError> { | |||
| } | |||
| } | |||
|
|
|||
| pub(crate) trait PublicKeyData { | |||
| pub trait PublicKeyData { | |||
There was a problem hiding this comment.
Needs some tweaks, as discussed here, please put those in a separate commit.
Also needs documentation.
| @@ -45,6 +45,9 @@ pub enum Error { | |||
| #[cfg(not(feature = "crypto"))] | |||
| /// Missing serial number | |||
| MissingSerialNumber, | |||
| /// X509 parsing error | |||
| #[cfg(feature = "x509-parser")] | |||
| X509, | |||
There was a problem hiding this comment.
I think we might want to include a String representation of the nested error here.
| } | ||
| fn raw_bytes(&self) -> &[u8] { |
| /// A public key | ||
| #[cfg(feature = "x509-parser")] | ||
| #[derive(Debug)] | ||
| pub struct SubjectPublicKey { |
There was a problem hiding this comment.
bike-shedding: I think this would be better called SubjectPublicKeyInfo. It's the pairing of an algorithm identifier and the public key.
|
@kwantam Are you interested in completing this pull-request by addressing the feedback received? |
|
I've taken this branch and addressed the comments so far in #291. |
|
Closing in favour of #291. Thanks for getting the ball rolling Kwantam |
|
Thanks for the comments and @djc for getting this across the finish line! Apologies that I've been buried in other stuff... |
CertificateParams::signed_bydoes not need the secret corresponding to the public key being certified (though of course it needs the secret for the CA key signing the certificate!). More discussion about this in #282.This PR:
signed_bytoimpl PublicKeyData(which maintains compatibility becauseKeyPairimplements this trait)SubjectPublicKey, that can be parsed from a PEM or DER output by this crate (or, e.g., by openssl) and also implsPublicKeyData.This type is only available when thex509-parserfeature is enabled (because that's what is required to parse).I added a quick test that the
SubjectPublicKeytype'sPublicKeyDataimpl produces the same output asKeyPairwould for the same key, at least for the key types that can be generated using the crate. (So this means, no test right now for RSA keys, since those seemed to give an error when I tried to generate a keypair.)