pkcs1+sec1: remove pkcs8 integration/feature (#1927)

Both `pkcs1` and `sec1` used some "clever" tricks to write blanket impls
of their traits for any types which impl the `pkcs8` traits.

However, we've received some complaints this makes the impls of these
traits more confusing and less discoverable (#1611), and adds a
maintenance burden in that the blanket impl relationship is
fundamentally "backwards" in that PKCS#8 builds on PKCS#1 and SEC1, not
the other way around.

These could potentially be replaced with a macro that writes impls of
the `pkcs8` traits for types which impl the `pkcs1`/`sec1` traits.
However, for now, each of these is only used in one place (i.e. the
`rsa` and `elliptic-curve` crates respectively), so we can start by
moving the relevant code there for now.

Author: tarcieri

Cargo.lock

@@ -62,3 +62,4 @@ x509-ocsp = { path = "./x509-ocsp" }
 
 elliptic-curve = { git = "https://github.com/RustCrypto/traits.git" }
 ecdsa = { git = "https://github.com/RustCrypto/signatures.git" }
+rsa = { git = "https://github.com/RustCrypto/RSA.git", branch = "pkcs1/remove-blanket-impl" }

Cargo.toml

@@ -19,20 +19,21 @@ rust-version = "1.85"
 der = { version = "0.8.0-rc.6", features = ["oid"] }
 spki = { version = "0.8.0-rc.3" }
 
-# optional dependencies
-pkcs8 = { version = "0.11.0-rc.5", optional = true, default-features = false }
-
 [dev-dependencies]
 const-oid = { version = "0.10", features = ["db"] }
 hex-literal = "1"
 tempfile = "3"
 
 [features]
-zeroize = ["der/zeroize"]
-alloc = ["der/alloc", "zeroize", "pkcs8?/alloc"]
-pem = ["alloc", "der/pem", "pkcs8?/pem"]
+alloc = ["der/alloc", "zeroize"]
 std = ["der/std", "alloc"]
 
+pem = ["alloc", "der/pem"]
+zeroize = ["der/zeroize"]
+
+# TODO(tarcieri): stub! (for `rsa` crate for now) remove before release
+pkcs8 = []
+
 [package.metadata.docs.rs]
 all-features = true
 rustdoc-args = ["--cfg", "docsrs"]

pkcs1/Cargo.toml

@@ -23,21 +23,30 @@ pub enum Error {
     /// a number expected to be a prime was not a prime.
     Crypto,
 
-    /// PKCS#8 errors.
-    #[cfg(feature = "pkcs8")]
-    Pkcs8(pkcs8::Error),
+    /// Malformed cryptographic key contained in a PKCS#1 document.
+    ///
+    /// This is intended for relaying errors when decoding fields of a PKCS#1 document.
+    KeyMalformed,
 
     /// Version errors
     Version,
 }
 
+impl core::error::Error for Error {
+    fn source(&self) -> Option<&(dyn core::error::Error + 'static)> {
+        match self {
+            Error::Asn1(err) => Some(err),
+            _ => None,
+        }
+    }
+}
+
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             Error::Asn1(err) => write!(f, "PKCS#1 ASN.1 error: {err}"),
+            Error::KeyMalformed => f.write_str("PKCS#1 cryptographic key data malformed"),
             Error::Crypto => f.write_str("PKCS#1 cryptographic error"),
-            #[cfg(feature = "pkcs8")]
-            Error::Pkcs8(err) => write!(f, "{err}"),
             Error::Version => f.write_str("PKCS#1 version error"),
         }
     }
@@ -55,41 +64,3 @@ impl From<pem::Error> for Error {
         der::Error::from(err).into()
     }
 }
-
-#[cfg(feature = "pkcs8")]
-impl From<Error> for pkcs8::Error {
-    fn from(err: Error) -> pkcs8::Error {
-        match err {
-            Error::Asn1(e) => pkcs8::Error::Asn1(e),
-            Error::Crypto | Error::Version => pkcs8::Error::KeyMalformed,
-            Error::Pkcs8(e) => e,
-        }
-    }
-}
-
-#[cfg(feature = "pkcs8")]
-impl From<pkcs8::Error> for Error {
-    fn from(err: pkcs8::Error) -> Error {
-        Error::Pkcs8(err)
-    }
-}
-
-#[cfg(feature = "pkcs8")]
-impl From<Error> for spki::Error {
-    fn from(err: Error) -> spki::Error {
-        match err {
-            Error::Asn1(e) => spki::Error::Asn1(e),
-            _ => spki::Error::KeyMalformed,
-        }
-    }
-}
-
-#[cfg(feature = "pkcs8")]
-impl From<spki::Error> for Error {
-    fn from(err: spki::Error) -> Error {
-        Error::Pkcs8(pkcs8::Error::PublicKey(err))
-    }
-}
-
-#[cfg(feature = "std")]
-impl std::error::Error for Error {}

pkcs1/src/error.rs

@@ -51,12 +51,10 @@ pub use crate::{
 pub use der::pem::{self, LineEnding};
 
 /// `rsaEncryption` Object Identifier (OID)
-#[cfg(feature = "pkcs8")]
 pub const ALGORITHM_OID: ObjectIdentifier = ObjectIdentifier::new_unwrap("1.2.840.113549.1.1.1");
 
 /// `AlgorithmIdentifier` for RSA.
-#[cfg(feature = "pkcs8")]
-pub const ALGORITHM_ID: pkcs8::AlgorithmIdentifierRef<'static> = pkcs8::AlgorithmIdentifierRef {
+pub const ALGORITHM_ID: spki::AlgorithmIdentifierRef<'static> = spki::AlgorithmIdentifierRef {
     oid: ALGORITHM_OID,
     parameters: Some(der::asn1::AnyRef::NULL),
 };

pkcs1/src/lib.rs

@@ -12,19 +12,10 @@ use {
     der::{pem::PemLabel, zeroize::Zeroizing},
 };
 
-#[cfg(feature = "pkcs8")]
-use {
-    crate::{ALGORITHM_ID, ALGORITHM_OID},
-    der::asn1::{BitStringRef, OctetStringRef},
-};
-
 #[cfg(feature = "std")]
 use std::path::Path;
 
-#[cfg(all(feature = "alloc", feature = "pkcs8"))]
-use der::Decode;
-
-#[cfg(all(feature = "alloc", any(feature = "pem", feature = "pkcs8")))]
+#[cfg(all(feature = "alloc", feature = "pem"))]
 use crate::{RsaPrivateKey, RsaPublicKey};
 
 /// Parse an [`RsaPrivateKey`] from a PKCS#1-encoded document.
@@ -153,51 +144,3 @@ pub trait EncodeRsaPublicKey {
         Ok(doc.write_pem_file(path, RsaPublicKey::PEM_LABEL, line_ending)?)
     }
 }
-
-#[cfg(feature = "pkcs8")]
-impl<T> DecodeRsaPrivateKey for T
-where
-    T: for<'a> TryFrom<pkcs8::PrivateKeyInfoRef<'a>, Error = pkcs8::Error>,
-{
-    fn from_pkcs1_der(private_key: &[u8]) -> Result<Self> {
-        let private_key = OctetStringRef::new(private_key)?;
-        Ok(Self::try_from(pkcs8::PrivateKeyInfoRef {
-            algorithm: ALGORITHM_ID,
-            private_key,
-            public_key: None,
-        })?)
-    }
-}
-
-#[cfg(feature = "pkcs8")]
-impl<T> DecodeRsaPublicKey for T
-where
-    T: for<'a> TryFrom<pkcs8::SubjectPublicKeyInfoRef<'a>, Error = spki::Error>,
-{
-    fn from_pkcs1_der(public_key: &[u8]) -> Result<Self> {
-        Ok(Self::try_from(pkcs8::SubjectPublicKeyInfoRef {
-            algorithm: ALGORITHM_ID,
-            subject_public_key: BitStringRef::from_bytes(public_key)?,
-        })?)
-    }
-}
-
-#[cfg(all(feature = "alloc", feature = "pkcs8"))]
-impl<T: pkcs8::EncodePrivateKey> EncodeRsaPrivateKey for T {
-    fn to_pkcs1_der(&self) -> Result<SecretDocument> {
-        let pkcs8_doc = self.to_pkcs8_der()?;
-        let pkcs8_key = pkcs8::PrivateKeyInfoRef::from_der(pkcs8_doc.as_bytes())?;
-        pkcs8_key.algorithm.assert_algorithm_oid(ALGORITHM_OID)?;
-        RsaPrivateKey::from_der(pkcs8_key.private_key.as_bytes())?.try_into()
-    }
-}
-
-#[cfg(all(feature = "alloc", feature = "pkcs8"))]
-impl<T: pkcs8::EncodePublicKey> EncodeRsaPublicKey for T {
-    fn to_pkcs1_der(&self) -> Result<Document> {
-        let doc = self.to_public_key_der()?;
-        let spki = pkcs8::SubjectPublicKeyInfoRef::from_der(doc.as_bytes())?;
-        spki.algorithm.assert_algorithm_oid(ALGORITHM_OID)?;
-        RsaPublicKey::from_der(spki.subject_public_key.raw_bytes())?.try_into()
-    }
-}

pkcs1/src/traits.rs

@@ -20,7 +20,6 @@ rust-version = "1.85"
 base16ct = { version = "0.2", optional = true, default-features = false }
 der = { version = "0.8.0-rc.6", optional = true, features = ["oid"] }
 hybrid-array = { version = "0.3", optional = true, default-features = false }
-pkcs8 = { version = "0.11.0-rc.5", optional = true, default-features = false }
 serdect = { version = "0.3", optional = true, default-features = false, features = ["alloc"] }
 subtle = { version = "2", optional = true, default-features = false }
 zeroize = { version = "1", optional = true, default-features = false }
@@ -31,11 +30,11 @@ tempfile = "3"
 
 [features]
 default = ["der", "point"]
-alloc = ["der?/alloc", "pkcs8?/alloc", "zeroize?/alloc"]
+alloc = ["der?/alloc", "zeroize?/alloc"]
 std = ["alloc", "der?/std"]
 
 der = ["dep:der", "zeroize"]
-pem = ["alloc", "der/pem", "pkcs8/pem"]
+pem = ["alloc", "der/pem"]
 point = ["dep:base16ct", "dep:hybrid-array"]
 serde = ["dep:serdect"]
 zeroize = ["dep:zeroize", "der?/zeroize"]

sec1/Cargo.toml

@@ -24,10 +24,6 @@ pub enum Error {
     /// a number expected to be a prime was not a prime.
     Crypto,
 
-    /// PKCS#8 errors.
-    #[cfg(feature = "pkcs8")]
-    Pkcs8(pkcs8::Error),
-
     /// Errors relating to the `Elliptic-Curve-Point-to-Octet-String` or
     /// `Octet-String-to-Elliptic-Curve-Point` encodings.
     PointEncoding,
@@ -44,8 +40,6 @@ impl fmt::Display for Error {
             #[cfg(feature = "der")]
             Error::Asn1(err) => write!(f, "SEC1 ASN.1 error: {err}"),
             Error::Crypto => f.write_str("SEC1 cryptographic error"),
-            #[cfg(feature = "pkcs8")]
-            Error::Pkcs8(err) => write!(f, "{err}"),
             Error::PointEncoding => f.write_str("elliptic curve point encoding error"),
             Error::Version => f.write_str("SEC1 version error"),
         }
@@ -65,17 +59,3 @@ impl From<pem::Error> for Error {
         der::Error::from(err).into()
     }
 }
-
-#[cfg(feature = "pkcs8")]
-impl From<pkcs8::Error> for Error {
-    fn from(err: pkcs8::Error) -> Error {
-        Error::Pkcs8(err)
-    }
-}
-
-#[cfg(feature = "pkcs8")]
-impl From<pkcs8::spki::Error> for Error {
-    fn from(err: pkcs8::spki::Error) -> Error {
-        Error::Pkcs8(pkcs8::Error::PublicKey(err))
-    }
-}

sec1/src/error.rs

@@ -60,18 +60,14 @@ pub use crate::traits::EncodeEcPrivateKey;
 #[cfg(feature = "pem")]
 pub use der::pem::{self, LineEnding};
 
-#[cfg(feature = "pkcs8")]
-pub use pkcs8;
-
-#[cfg(feature = "pkcs8")]
-use pkcs8::ObjectIdentifier;
+#[cfg(feature = "der")]
+use der::asn1::ObjectIdentifier;
 
 #[cfg(all(doc, feature = "serde"))]
 use serdect::serde;
 
-/// Algorithm [`ObjectIdentifier`] for elliptic curve public key cryptography
-/// (`id-ecPublicKey`).
+/// Algorithm [`ObjectIdentifier`] for elliptic curve public key cryptography (`id-ecPublicKey`).
 ///
 /// <http://oid-info.com/get/1.2.840.10045.2.1>
-#[cfg(feature = "pkcs8")]
+#[cfg(feature = "der")]
 pub const ALGORITHM_OID: ObjectIdentifier = ObjectIdentifier::new_unwrap("1.2.840.10045.2.1");

sec1/src/lib.rs

@@ -8,17 +8,11 @@ use der::SecretDocument;
 #[cfg(feature = "pem")]
 use {crate::LineEnding, alloc::string::String, der::pem::PemLabel};
 
-#[cfg(feature = "pkcs8")]
-use {
-    crate::{ALGORITHM_OID, EcPrivateKey},
-    der::{Decode, asn1::OctetStringRef},
-};
-
 #[cfg(feature = "std")]
 use std::path::Path;
 
 #[cfg(feature = "pem")]
-use zeroize::Zeroizing;
+use {crate::EcPrivateKey, zeroize::Zeroizing};
 
 /// Parse an [`EcPrivateKey`] from a SEC1-encoded document.
 pub trait DecodeEcPrivateKey: Sized {
@@ -84,41 +78,3 @@ pub trait EncodeEcPrivateKey {
         Ok(doc.write_pem_file(path, EcPrivateKey::PEM_LABEL, line_ending)?)
     }
 }
-
-#[cfg(feature = "pkcs8")]
-impl<T> DecodeEcPrivateKey for T
-where
-    T: for<'a> TryFrom<pkcs8::PrivateKeyInfoRef<'a>, Error = pkcs8::Error>,
-{
-    fn from_sec1_der(private_key: &[u8]) -> Result<Self> {
-        let params_oid = EcPrivateKey::from_der(private_key)?
-            .parameters
-            .and_then(|params| params.named_curve());
-
-        let algorithm = pkcs8::AlgorithmIdentifierRef {
-            oid: ALGORITHM_OID,
-            parameters: params_oid.as_ref().map(Into::into),
-        };
-
-        let private_key = OctetStringRef::new(private_key)?;
-
-        Ok(Self::try_from(pkcs8::PrivateKeyInfoRef {
-            algorithm,
-            private_key,
-            public_key: None,
-        })?)
-    }
-}
-
-#[cfg(all(feature = "alloc", feature = "pkcs8"))]
-impl<T: pkcs8::EncodePrivateKey> EncodeEcPrivateKey for T {
-    fn to_sec1_der(&self) -> Result<SecretDocument> {
-        let doc = self.to_pkcs8_der()?;
-        let pkcs8_key = pkcs8::PrivateKeyInfoRef::from_der(doc.as_bytes())?;
-        pkcs8_key.algorithm.assert_algorithm_oid(ALGORITHM_OID)?;
-
-        let mut pkcs1_key = EcPrivateKey::from_der(pkcs8_key.private_key.as_bytes())?;
-        pkcs1_key.parameters = Some(pkcs8_key.algorithm.parameters_oid()?.into());
-        pkcs1_key.try_into()
-    }
-}