Remove commented-out code and clarify ED25519 key support in TryFrom implementation and restore sec1 support for EcdsaSigningKey

Author: stevefan1999-personal

Cargo.toml

@@ -25,7 +25,6 @@ aes-gcm = { version = "0.11.0-rc.1", default-features = false, optional = true }
 ccm = { version = "0.6.0-pre", default-features = false, optional = true, git = "https://github.com/RustCrypto/AEADs/" }
 chacha20poly1305 = { version = "0.11.0-rc.1", default-features = false, optional = true }
 cipher = "0.5.0-rc.1"
-# crrl = { git = "https://github.com/stevefan1999-personal/crrl", version = "0.9.0", default-features = false, optional = true }
 crypto-common = { version = "0.2.0-rc.4", default-features = false }
 der = { version = "0.8.0-rc.8", default-features = false, optional = true }
 digest = { version = "0.11.0-rc.1", default-features = false }

src/sign/ecdsa/nist.rs

@@ -3,9 +3,6 @@ use alloc::{boxed::Box, format, sync::Arc};
 use core::fmt::Debug;
 use core::marker::PhantomData;
 
-// #[cfg(feature = "sec1")]
-// use sec1::DecodeEcPrivateKey;
-
 use crate::sign::rand::GenericRandomizedSigner;
 use rustls::sign::SigningKey;
 use rustls::{SignatureAlgorithm, SignatureScheme};
@@ -17,52 +14,57 @@ trait EcdsaKey: Sized {
     const SCHEME: SignatureScheme;
 }
 
-// #[cfg(all(feature = "pkcs8", not(feature = "sec1")))]
-// trait DecodePrivateKey: ::pkcs8::DecodePrivateKey {}
+#[cfg(all(feature = "pkcs8", not(feature = "sec1")))]
+trait DecodePrivateKey: ::pkcs8::DecodePrivateKey {}
 
-// #[cfg(all(feature = "sec1", not(feature = "pkcs8")))]
-// trait DecodePrivateKey: ::sec1::DecodeEcPrivateKey {}
+#[cfg(all(feature = "sec1", not(feature = "pkcs8")))]
+trait DecodePrivateKey: ::sec1::DecodeEcPrivateKey {}
 
-// #[cfg(all(feature = "pkcs8", feature = "sec1"))]
-// trait DecodePrivateKey: ::pkcs8::DecodePrivateKey + ::sec1::DecodeEcPrivateKey {}
+#[cfg(all(feature = "pkcs8", feature = "sec1"))]
+trait DecodePrivateKey: ::pkcs8::DecodePrivateKey + ::sec1::DecodeEcPrivateKey {}
 
 #[cfg(feature = "der")]
-impl<SK, SIG> TryFrom<&PrivateKeyDer<'_>> for EcdsaSigningKey<SK, SIG>
+impl<SecretKey, SigningKey, Signature> TryFrom<&PrivateKeyDer<'_>>
+    for EcdsaSigningKey<SecretKey, SigningKey, Signature>
 where
-    SK: EcdsaKey + ::pkcs8::DecodePrivateKey + Send + Sync + 'static,
-    SIG: Send + Sync + 'static,
+    SecretKey: Debug + DecodePrivateKey,
+    SigningKey: EcdsaKey + Send + Sync + 'static + From<SecretKey>,
+    Signature: Send + Sync + 'static,
 {
     type Error = rustls::Error;
 
     fn try_from(value: &PrivateKeyDer<'_>) -> Result<Self, Self::Error> {
         let pkey = match value {
             #[cfg(feature = "pkcs8")]
-            PrivateKeyDer::Pkcs8(der) => SK::from_pkcs8_der(der.secret_pkcs8_der())
+            PrivateKeyDer::Pkcs8(der) => SecretKey::from_pkcs8_der(der.secret_pkcs8_der())
+                .map_err(|e| format!("failed to decrypt private key: {e}")),
+            #[cfg(feature = "sec1")]
+            PrivateKeyDer::Sec1(sec1) => SecretKey::from_sec1_der(sec1.secret_sec1_der())
                 .map_err(|e| format!("failed to decrypt private key: {e}")),
-            // #[cfg(feature = "sec1")]
-            // PrivateKeyDer::Sec1(sec1) => SK::from_sec1_der(sec1.secret_sec1_der())
-            //     .map_err(|e| format!("failed to decrypt private key: {e}")),
             PrivateKeyDer::Pkcs1(_) => Err("ECDSA does not support PKCS#1 key".into()),
             _ => Err("not supported".into()),
         };
         pkey.map(|kp| Self {
-            key: Arc::new(kp),
-            scheme: SK::SCHEME,
+            key: Arc::new(kp.into()),
+            scheme: SigningKey::SCHEME,
             _phantom: PhantomData,
+            _phantom_sk: PhantomData,
         })
         .map_err(rustls::Error::General)
     }
 }
 
 #[derive(Debug)]
-pub struct EcdsaSigningKey<SK, SIG> {
+pub struct EcdsaSigningKey<SecretKey, SK, SIG> {
     key: Arc<SK>,
     scheme: SignatureScheme,
     _phantom: PhantomData<SIG>,
+    _phantom_sk: PhantomData<SecretKey>,
 }
 
-impl<SK, SIG> SigningKey for EcdsaSigningKey<SK, SIG>
+impl<SecretKey, SK, SIG> SigningKey for EcdsaSigningKey<SecretKey, SK, SIG>
 where
+    SecretKey: Debug + Send + Sync,
     SK: Send + Sync + 'static + Debug + ecdsa::signature::RandomizedSigner<SIG>,
     SIG: Send + Sync + 'static + Debug + ecdsa::signature::SignatureEncoding,
 {
@@ -83,35 +85,39 @@ where
     }
 }
 
-#[cfg(feature = "ecdsa-p256")]
-pub type EcdsaSigningKeyP256 =
-    EcdsaSigningKey<::p256::ecdsa::SigningKey, ::p256::ecdsa::DerSignature>;
-
-#[cfg(all(feature = "ecdsa-p256", feature = "hash-sha256"))]
-impl EcdsaKey for ::p256::ecdsa::SigningKey {
-    const SCHEME: SignatureScheme = SignatureScheme::ECDSA_NISTP256_SHA256;
-}
-
-// #[cfg(feature = "ecdsa-p384")]
-// impl DecodePrivateKey for ::p384::ecdsa::SigningKey {}
+macro_rules! impl_ecdsa_curve {
+    ($curve:ident, $scheme:expr, $type_name:ident) => {
+        pub type $type_name = EcdsaSigningKey<
+            ::$curve::SecretKey,
+            ::$curve::ecdsa::SigningKey,
+            ::$curve::ecdsa::DerSignature,
+        >;
 
-#[cfg(feature = "ecdsa-p384")]
-pub type EcdsaSigningKeyP384 =
-    EcdsaSigningKey<::p384::ecdsa::SigningKey, ::p384::ecdsa::DerSignature>;
+        impl EcdsaKey for ::$curve::ecdsa::SigningKey {
+            const SCHEME: SignatureScheme = $scheme;
+        }
 
-#[cfg(feature = "ecdsa-p521")]
-impl EcdsaKey for ::p384::ecdsa::SigningKey {
-    const SCHEME: SignatureScheme = SignatureScheme::ECDSA_NISTP384_SHA384;
+        impl DecodePrivateKey for ::$curve::SecretKey {}
+    };
 }
 
-// #[cfg(feature = "ecdsa-p521")]
-// impl DecodePrivateKey for ::p521::ecdsa::SigningKey {}
-
-#[cfg(feature = "ecdsa-p521")]
-pub type EcdsaSigningKeyP521 =
-    EcdsaSigningKey<::p521::ecdsa::SigningKey, ::p521::ecdsa::DerSignature>;
+#[cfg(all(feature = "ecdsa-p256", feature = "hash-sha256"))]
+impl_ecdsa_curve!(
+    p256,
+    SignatureScheme::ECDSA_NISTP256_SHA256,
+    EcdsaSigningKeyP256
+);
+
+#[cfg(all(feature = "ecdsa-p384", feature = "hash-sha384"))]
+impl_ecdsa_curve!(
+    p384,
+    SignatureScheme::ECDSA_NISTP384_SHA384,
+    EcdsaSigningKeyP384
+);
 
 #[cfg(all(feature = "ecdsa-p521", feature = "hash-sha512"))]
-impl EcdsaKey for ::p521::ecdsa::SigningKey {
-    const SCHEME: SignatureScheme = SignatureScheme::ECDSA_NISTP521_SHA512;
-}
+impl_ecdsa_curve!(
+    p521,
+    SignatureScheme::ECDSA_NISTP521_SHA512,
+    EcdsaSigningKeyP521
+);

src/sign/eddsa/ed25519.rs

@@ -26,12 +26,12 @@ impl TryFrom<&PrivateKeyDer<'_>> for Ed25519SigningKey {
                 SigningKey::from_pkcs8_der(der.secret_pkcs8_der())
                     .map_err(|e| format!("failed to decrypt private key: {e}"))
             }
-            // #[cfg(feature = "sec1")]
-            // PrivateKeyDer::Sec1(sec1) => {
-            //     use sec1::DecodeEcPrivateKey;
-            //     SigningKey::from_sec1_der(sec1.secret_sec1_der())
-            //         .map_err(|e| format!("failed to decrypt private key: {e}"))
-            // }
+            
+            // (chat log from tony in zulip)
+            // Per RFC 8410, only PKCS#8 is supported for ED25519 keys
+            // https://datatracker.ietf.org/doc/html/rfc8410#section-7
+            // So no SEC 1 support for ED25519 (despite we do have it compile before?!)
+            PrivateKeyDer::Sec1(_) => Err("ED25519 does not support SEC 1 key".to_string()),
             PrivateKeyDer::Pkcs1(_) => Err("ED25519 does not support PKCS#1 key".to_string()),
             _ => Err("not supported".into()),
         };