Remove PKCS#8 blanket impls for PKCS#1 traits (#538)

Companion PR to RustCrypto/formats#1927 which properly composes the
PKCS#8 impl in terms of the PKCS#1 impl

Author: tarcieri

Cargo.lock

@@ -23,7 +23,7 @@ subtle = { version = "2.6.1", default-features = false }
 zeroize = { version = "1.5", features = ["alloc"] }
 
 # optional dependencies
-pkcs1 = { version = "0.8.0-rc.2", optional = true, default-features = false, features = ["alloc", "pem", "pkcs8"] }
+pkcs1 = { version = "0.8.0-rc.2", optional = true, default-features = false, features = ["alloc", "pem"] }
 pkcs8 = { version = "0.11.0-rc.4", optional = true, default-features = false, features = ["alloc", "pem"] }
 serdect = { version = "0.3.0", optional = true }
 sha1 = { version = "0.11.0-rc.0", optional = true, default-features = false, features = ["oid"] }
@@ -68,3 +68,6 @@ opt-level = 2
 
 [profile.bench]
 debug = true
+
+[patch.crates-io]
+pkcs1 = { git = "https://github.com/RustCrypto/formats/" }

Cargo.toml

@@ -12,56 +12,48 @@ use crate::{
 use core::convert::{TryFrom, TryInto};
 use crypto_bigint::{BoxedUint, NonZero, Resize};
 use pkcs8::{
-    der::{asn1::OctetStringRef, Encode},
+    der::{asn1::OctetStringRef, Decode},
     Document, EncodePrivateKey, EncodePublicKey, ObjectIdentifier, SecretDocument,
 };
 use zeroize::Zeroizing;
 
 /// ObjectID for the RSA PSS keys
 pub const ID_RSASSA_PSS: ObjectIdentifier = ObjectIdentifier::new_unwrap("1.2.840.113549.1.1.10");
 
-/// Verify that the `AlgorithmIdentifier` for a key is correct.
-pub(crate) fn verify_algorithm_id(algorithm: &pkcs8::AlgorithmIdentifierRef) -> spki::Result<()> {
-    match algorithm.oid {
-        pkcs1::ALGORITHM_OID => {
-            if algorithm.parameters_any()? != pkcs8::der::asn1::Null.into() {
-                return Err(spki::Error::KeyMalformed);
-            }
-        }
-        ID_RSASSA_PSS => {
-            if algorithm.parameters.is_some() {
-                return Err(spki::Error::KeyMalformed);
-            }
-        }
-        _ => return Err(spki::Error::OidUnknown { oid: algorithm.oid }),
-    };
+// PKCS#1
 
-    Ok(())
+fn uint_from_slice(data: &[u8], bits: u32) -> pkcs1::Result<BoxedUint> {
+    BoxedUint::from_be_slice(data, bits).map_err(|_| pkcs1::Error::KeyMalformed)
 }
 
-fn uint_from_slice(data: &[u8], bits: u32) -> pkcs8::Result<BoxedUint> {
-    BoxedUint::from_be_slice(data, bits).map_err(|_| pkcs8::Error::KeyMalformed)
+impl pkcs1::DecodeRsaPrivateKey for RsaPrivateKey {
+    fn from_pkcs1_der(bytes: &[u8]) -> pkcs1::Result<Self> {
+        pkcs1::RsaPrivateKey::from_der(bytes)?.try_into()
+    }
 }
 
-impl TryFrom<pkcs8::PrivateKeyInfoRef<'_>> for RsaPrivateKey {
-    type Error = pkcs8::Error;
+impl pkcs1::DecodeRsaPublicKey for RsaPublicKey {
+    fn from_pkcs1_der(bytes: &[u8]) -> pkcs1::Result<Self> {
+        pkcs1::RsaPublicKey::from_der(bytes)?.try_into()
+    }
+}
 
-    fn try_from(private_key_info: pkcs8::PrivateKeyInfoRef<'_>) -> pkcs8::Result<Self> {
-        use pkcs8::Error::KeyMalformed;
-        verify_algorithm_id(&private_key_info.algorithm)?;
+impl TryFrom<pkcs1::RsaPrivateKey<'_>> for RsaPrivateKey {
+    type Error = pkcs1::Error;
 
-        let pkcs1_key = pkcs1::RsaPrivateKey::try_from(private_key_info.private_key)?;
+    fn try_from(pkcs1_key: pkcs1::RsaPrivateKey<'_>) -> pkcs1::Result<RsaPrivateKey> {
+        use pkcs1::Error::KeyMalformed;
 
         // Multi-prime RSA keys not currently supported
         if pkcs1_key.version() != pkcs1::Version::TwoPrime {
-            return Err(pkcs1::Error::Version.into());
+            return Err(pkcs1::Error::Version);
         }
 
         let bits = u32::try_from(pkcs1_key.modulus.as_bytes().len()).map_err(|_| KeyMalformed)? * 8;
 
         let n = uint_from_slice(pkcs1_key.modulus.as_bytes(), bits)?;
         let bits_e = u32::try_from(pkcs1_key.public_exponent.as_bytes().len())
-            .map_err(|_| KeyMalformed)?
+            .map_err(|_| pkcs1::Error::KeyMalformed)?
             * 8;
         let e = uint_from_slice(pkcs1_key.public_exponent.as_bytes(), bits_e)?;
         let e = Option::from(e).ok_or(KeyMalformed)?;
@@ -75,16 +67,11 @@ impl TryFrom<pkcs8::PrivateKeyInfoRef<'_>> for RsaPrivateKey {
     }
 }
 
-impl TryFrom<pkcs8::SubjectPublicKeyInfoRef<'_>> for RsaPublicKey {
-    type Error = spki::Error;
-
-    fn try_from(spki: pkcs8::SubjectPublicKeyInfoRef<'_>) -> spki::Result<Self> {
-        use spki::Error::KeyMalformed;
-
-        verify_algorithm_id(&spki.algorithm)?;
+impl TryFrom<pkcs1::RsaPublicKey<'_>> for RsaPublicKey {
+    type Error = pkcs1::Error;
 
-        let pkcs1_key =
-            pkcs1::RsaPublicKey::try_from(spki.subject_public_key.as_bytes().ok_or(KeyMalformed)?)?;
+    fn try_from(pkcs1_key: pkcs1::RsaPublicKey<'_>) -> pkcs1::Result<Self> {
+        use pkcs1::Error::KeyMalformed;
 
         let bits = u32::try_from(pkcs1_key.modulus.as_bytes().len()).map_err(|_| KeyMalformed)? * 8;
         let n = uint_from_slice(pkcs1_key.modulus.as_bytes(), bits)?;
@@ -98,11 +85,11 @@ impl TryFrom<pkcs8::SubjectPublicKeyInfoRef<'_>> for RsaPublicKey {
     }
 }
 
-impl EncodePrivateKey for RsaPrivateKey {
-    fn to_pkcs8_der(&self) -> pkcs8::Result<SecretDocument> {
+impl pkcs1::EncodeRsaPrivateKey for RsaPrivateKey {
+    fn to_pkcs1_der(&self) -> pkcs1::Result<SecretDocument> {
         // Check if the key is multi prime
         if self.primes.len() > 2 {
-            return Err(pkcs1::Error::Version.into());
+            return Err(pkcs1::Error::Crypto);
         }
 
         let modulus = self.n().to_be_bytes();
@@ -134,7 +121,7 @@ impl EncodePrivateKey for RsaPrivateKey {
                 .to_be_bytes(),
         );
 
-        let private_key = pkcs1::RsaPrivateKey {
+        Ok(SecretDocument::encode_msg(&pkcs1::RsaPrivateKey {
             modulus: pkcs1::UintRef::new(&modulus)?,
             public_exponent: pkcs1::UintRef::new(&public_exponent)?,
             private_exponent: pkcs1::UintRef::new(&private_exponent)?,
@@ -144,27 +131,86 @@ impl EncodePrivateKey for RsaPrivateKey {
             exponent2: pkcs1::UintRef::new(&exponent2)?,
             coefficient: pkcs1::UintRef::new(&coefficient)?,
             other_prime_infos: None,
+        })?)
+    }
+}
+
+impl pkcs1::EncodeRsaPublicKey for RsaPublicKey {
+    fn to_pkcs1_der(&self) -> pkcs1::Result<Document> {
+        let modulus = self.n().to_be_bytes();
+        let public_exponent = self.e().to_be_bytes();
+
+        Ok(Document::encode_msg(&pkcs1::RsaPublicKey {
+            modulus: pkcs1::UintRef::new(&modulus)?,
+            public_exponent: pkcs1::UintRef::new(&public_exponent)?,
+        })?)
+    }
+}
+
+// PKCS#8
+
+/// Verify that the `AlgorithmIdentifier` for a key is correct.
+pub(crate) fn verify_algorithm_id(algorithm: &spki::AlgorithmIdentifierRef) -> spki::Result<()> {
+    match algorithm.oid {
+        pkcs1::ALGORITHM_OID => {
+            if algorithm.parameters_any()? != pkcs8::der::asn1::Null.into() {
+                return Err(spki::Error::KeyMalformed);
+            }
+        }
+        ID_RSASSA_PSS => {
+            if algorithm.parameters.is_some() {
+                return Err(spki::Error::KeyMalformed);
+            }
         }
-        .to_der()?;
+        _ => return Err(spki::Error::OidUnknown { oid: algorithm.oid }),
+    };
+
+    Ok(())
+}
+
+impl TryFrom<pkcs8::PrivateKeyInfoRef<'_>> for RsaPrivateKey {
+    type Error = pkcs8::Error;
+
+    fn try_from(private_key_info: pkcs8::PrivateKeyInfoRef<'_>) -> pkcs8::Result<Self> {
+        verify_algorithm_id(&private_key_info.algorithm)?;
+
+        pkcs1::RsaPrivateKey::try_from(private_key_info.private_key)
+            .and_then(TryInto::try_into)
+            .map_err(pkcs1_error_to_pkcs8)
+    }
+}
+
+impl TryFrom<pkcs8::SubjectPublicKeyInfoRef<'_>> for RsaPublicKey {
+    type Error = spki::Error;
+
+    fn try_from(spki: pkcs8::SubjectPublicKeyInfoRef<'_>) -> spki::Result<Self> {
+        use spki::Error::KeyMalformed;
+
+        verify_algorithm_id(&spki.algorithm)?;
+
+        pkcs1::RsaPublicKey::try_from(spki.subject_public_key.as_bytes().ok_or(KeyMalformed)?)
+            .and_then(TryInto::try_into)
+            .map_err(pkcs1_error_to_spki)
+    }
+}
+
+impl EncodePrivateKey for RsaPrivateKey {
+    fn to_pkcs8_der(&self) -> pkcs8::Result<SecretDocument> {
+        let private_key =
+            pkcs1::EncodeRsaPrivateKey::to_pkcs1_der(self).map_err(pkcs1_error_to_pkcs8)?;
 
         pkcs8::PrivateKeyInfoRef::new(
             pkcs1::ALGORITHM_ID,
-            OctetStringRef::new(private_key.as_ref())?,
+            OctetStringRef::new(private_key.as_bytes())?,
         )
         .try_into()
     }
 }
 
 impl EncodePublicKey for RsaPublicKey {
     fn to_public_key_der(&self) -> spki::Result<Document> {
-        let modulus = self.n().to_be_bytes();
-        let public_exponent = self.e().to_be_bytes();
-
-        let subject_public_key = pkcs1::RsaPublicKey {
-            modulus: pkcs1::UintRef::new(&modulus)?,
-            public_exponent: pkcs1::UintRef::new(&public_exponent)?,
-        }
-        .to_der()?;
+        let subject_public_key =
+            pkcs1::EncodeRsaPublicKey::to_pkcs1_der(self).map_err(pkcs1_error_to_spki)?;
 
         pkcs8::SubjectPublicKeyInfoRef {
             algorithm: pkcs1::ALGORITHM_ID,
@@ -176,3 +222,19 @@ impl EncodePublicKey for RsaPublicKey {
         .try_into()
     }
 }
+
+/// Convert `pkcs1::Result` to `pkcs8::Result`.
+fn pkcs1_error_to_pkcs8(error: pkcs1::Error) -> pkcs8::Error {
+    match error {
+        pkcs1::Error::Asn1(e) => pkcs8::Error::Asn1(e),
+        _ => pkcs8::Error::KeyMalformed,
+    }
+}
+
+/// Convert `pkcs1::Result` to `spki::Result`.
+fn pkcs1_error_to_spki(error: pkcs1::Error) -> spki::Error {
+    match error {
+        pkcs1::Error::Asn1(e) => spki::Error::Asn1(e),
+        _ => spki::Error::KeyMalformed,
+    }
+}

src/encoding.rs

@@ -130,16 +130,8 @@ impl From<crypto_bigint::DecodeError> for Error {
     }
 }
 
-#[cfg(feature = "std")]
 impl From<Error> for signature::Error {
     fn from(err: Error) -> Self {
         Self::from_source(err)
     }
 }
-
-#[cfg(not(feature = "std"))]
-impl From<Error> for signature::Error {
-    fn from(_err: Error) -> Self {
-        Self::new()
-    }
-}