fix: support SEC1 EC keys with full domain parameters

russh/src/keys/format/pkcs8.rs

@@ -24,23 +24,22 @@ pub fn decode_pkcs8(
         doc
     };
 
-    match doc.decode_msg::<sec1::EcPrivateKey>() {
-        Ok(key) => {
-            // X9.62 EC private key
-            let Some(curve) = key.parameters.and_then(|x| x.named_curve()) else {
-                return Err(Error::CouldNotReadKey);
-            };
-            let kp = ec_key_data_into_keypair(curve, key)?;
-            Ok(PrivateKey::new(KeypairData::Ecdsa(kp), "")?)
-        }
-        Err(_) => {
-            // ASN.1 key
-            Ok(
-                pkcs8_pki_into_keypair_data(doc.decode_msg::<PrivateKeyInfoRef<'_>>()?)?
-                    .try_into()?,
-            )
-        }
+    if let Ok(key) = doc.decode_msg::<sec1::EcPrivateKey>() {
+        // X9.62 EC private key
+        let Some(curve) = key.parameters.and_then(|x| x.named_curve()) else {
+            return Err(Error::CouldNotReadKey);
+        };
+        let kp = ec_key_data_into_keypair(curve, key)?;
+        return Ok(PrivateKey::new(KeypairData::Ecdsa(kp), "")?);
+    }
+
+    // SEC1 key with full domain parameters (not a named curve OID)
+    if let Ok(kp) = explicit_curve_params::decode_sec1_with_full_domain_params(ciphertext) {
+        return Ok(PrivateKey::new(KeypairData::Ecdsa(kp), "")?);
     }
+
+    // ASN.1 key (PKCS#8)
+    Ok(pkcs8_pki_into_keypair_data(doc.decode_msg::<PrivateKeyInfoRef<'_>>()?)?.try_into()?)
 }
 
 fn pkcs8_pki_into_keypair_data(pki: PrivateKeyInfoRef<'_>) -> Result<KeypairData, Error> {
@@ -112,6 +111,105 @@ where
     })
 }
 
+mod explicit_curve_params {
+    use super::*;
+
+    use der::{
+        Reader, SliceReader, Tag, TagNumber, Tagged,
+        asn1::{AnyRef, ContextSpecific, UintRef},
+    };
+
+    /// Try to parse an SEC1 EC key with full domain parameters.
+    ///
+    /// Some key generators (e.g. OpenSSL with certain options) produce SEC1 keys
+    /// where the `[0]` parameters field contains full EC domain parameters instead
+    /// of a named curve OID. The `sec1` crate does not support this format.
+    pub fn decode_sec1_with_full_domain_params(der_bytes: &[u8]) -> Result<EcdsaKeypair, Error> {
+        let mut reader = SliceReader::new(der_bytes)?;
+        reader.sequence(|seq| {
+            let version: u8 = seq.decode()?;
+            if version < 1 {
+                return Err(Error::CouldNotReadKey);
+            }
+
+            let priv_key: AnyRef = seq.decode()?;
+            priv_key.tag().assert_eq(Tag::OctetString)?;
+
+            let params = ContextSpecific::<AnyRef>::decode_explicit(seq, TagNumber(0))?
+                .ok_or(Error::CouldNotReadKey)?;
+
+            let curve_oid = extract_curve_from_domain_params(params.value)?;
+
+            let keypair = build_ec_keypair_from_bytes(curve_oid, priv_key.value())?;
+
+            // Drain any remaining optional fields (e.g. [1] publicKey) so finish() succeeds
+            seq.drain(seq.remaining_len())?;
+            Ok(keypair)
+        })
+    }
+
+    /// Extract the named curve OID from full EC domain parameters.
+    /// Handles two formats:
+    /// 1. Standard ECParameters: SEQUENCE { FieldID, Curve, base, order, cofactor }
+    /// 2. Wrapped ECParameters: SEQUENCE { INTEGER version, SEQUENCE { FieldID, ... } }
+    fn extract_curve_from_domain_params(params: AnyRef<'_>) -> Result<ObjectIdentifier, Error> {
+        params.tag().assert_eq(Tag::Sequence)?;
+
+        // Use a standalone SliceReader so we aren't required to consume all of ECParams
+        // (Curve, base, order, cofactor follow FieldID but are irrelevant here).
+        let mut seq = SliceReader::new(params.value())?;
+
+        // Skip optional ECParameters version INTEGER
+        if Tag::peek(&seq)? == Tag::Integer {
+            seq.decode::<u8>()?;
+        }
+
+        // FieldID ::= SEQUENCE { fieldType OID, parameters ANY }
+        seq.sequence(|field_id| {
+            let _field_oid: ObjectIdentifier = field_id.decode()?;
+            // prime INTEGER — as_bytes() strips DER sign-extension leading zero
+            let prime: UintRef = field_id.decode()?;
+            Ok(match prime.as_bytes().len() {
+                32 => NistP256::OID,
+                48 => NistP384::OID,
+                66 => NistP521::OID,
+                _ => return Err(Error::CouldNotReadKey),
+            })
+        })
+    }
+
+    /// Build an EcdsaKeypair from raw private key bytes and a curve OID.
+    fn build_ec_keypair_from_bytes(
+        curve_oid: ObjectIdentifier,
+        private_key_bytes: &[u8],
+    ) -> Result<EcdsaKeypair, Error> {
+        if curve_oid == NistP256::OID {
+            let sk = p256::SecretKey::from_slice(private_key_bytes)
+                .map_err(|_| Error::CouldNotReadKey)?;
+            Ok(EcdsaKeypair::NistP256 {
+                public: sk.public_key().into(),
+                private: sk.into(),
+            })
+        } else if curve_oid == NistP384::OID {
+            let sk = p384::SecretKey::from_slice(private_key_bytes)
+                .map_err(|_| Error::CouldNotReadKey)?;
+            Ok(EcdsaKeypair::NistP384 {
+                public: sk.public_key().into(),
+                private: sk.into(),
+            })
+        } else if curve_oid == NistP521::OID {
+            let sk = p521::SecretKey::from_slice(private_key_bytes)
+                .map_err(|_| Error::CouldNotReadKey)?;
+            Ok(EcdsaKeypair::NistP521 {
+                public: sk.public_key().into(),
+                private: sk.into(),
+            })
+        } else {
+            Err(Error::UnknownAlgorithm(curve_oid))
+        }
+    }
+}
+
 /// Encode into a password-protected PKCS#8-encoded private key.
 pub fn encode_pkcs8_encrypted(
     pass: &[u8],

russh/src/keys/format/tests.rs

@@ -24,3 +24,66 @@ fn test_pkcs8_roundtrip() {
     let decrypted = decode_pkcs8(&encrypted, Some(password)).unwrap();
     assert_eq!(decrypted, original_key);
 }
+
+#[test]
+fn test_ec_private_key_with_full_domain_params() {
+    // This key uses full EC domain parameters instead of a named curve OID.
+    // Generated with: openssl ecparam -name prime256v1 -genkey -param_enc explicit -noout
+    // The sec1 crate cannot parse this format; our fallback parser handles it.
+    let key = "-----BEGIN EC PRIVATE KEY-----\n\
+MIIBaAIBAQQguoBiuFhw88aF2jBBK9zZAxFL3fTXmSnjUt2usONDS+SggfowgfcC\n\
+AQEwLAYHKoZIzj0BAQIhAP////8AAAABAAAAAAAAAAAAAAAA////////////////\n\
+MFsEIP////8AAAABAAAAAAAAAAAAAAAA///////////////8BCBaxjXYqjqT57Pr\n\
+vVV2mIa8ZR0GsMxTsPY7zjw+J9JgSwMVAMSdNgiG5wSTamZ44ROdJreBn36QBEEE\n\
+axfR8uEsQkf4vOblY6RA8ncDfYEt6zOg9KE5RdiYwpZP40Li/hp/m47n60p8D54W\n\
+K84zV2sxXs7LtkBoN79R9QIhAP////8AAAAA//////////+85vqtpxeehPO5ysL8\n\
+YyVRAgEBoUQDQgAEqZqnpFc/+9yfQh7B/sx5dms/sccAtE+PoGTqAa4y399K1S0H\n\
+b6KBhA+L9No0qBbsdpwaMewJChyf5AIft0Un3A==\n\
+-----END EC PRIVATE KEY-----";
+    let result = decode_secret_key(key, None);
+    assert!(result.is_ok(), "Failed to parse EC key with full domain params: {:?}", result.err());
+    let pk = result.unwrap();
+    assert_eq!(pk.algorithm(), Algorithm::Ecdsa {
+        curve: ssh_key::EcdsaCurve::NistP256,
+    });
+}
+
+#[test]
+fn test_ec_p521_private_key_with_full_domain_params() {
+    // P-521 key with full EC domain parameters (not named curve OID).
+    // Generated with: openssl ecparam -name secp521r1 -genkey -param_enc explicit -noout
+    let key = "-----BEGIN EC PRIVATE KEY-----\n\
+MIICngIBAQRCAH2esSsV6PlGdsc5TekzHNtyj0vhHfok5t0UCXu08hL3ZYqe7JKP\n\
+EjUiuV0NWoo++Zy3juTEB+nssQhOBd4DOBpmoIIBxzCCAcMCAQEwTQYHKoZIzj0B\n\
+AQJCAf//////////////////////////////////////////////////////////\n\
+////////////////////////////MIGfBEIB////////////////////////////\n\
+//////////////////////////////////////////////////////////wEQgBR\n\
+lT65YY4cmh+SmiGgtoVA7qLacluZsxXzuLSJkY7xCeFWGTlR7H6TexZSwL07sb8H\n\
+NXPfiD0sNPHvRR/Ua1A/AAMVANCeiAApHLhTlsxnFzkyhKqg2mS6BIGFBADGhY4G\n\
+twQE6c2ePstmI5W0QpxkgTkFP7Uh+CivYGtNPbqhS1537+dZKP4dwSei/6jeM0iz\n\
+wYVqQpv5fn4xwuW9ZgEYOSlqeJo7wARcil+0LH0b2Zj1RElXm0RoF6+9Fyc+ZiyX\n\
+7nKZXvQmQMVQuQE/rQdhNTxwhqJywkCIvpR2n9FmUAJCAf//////////////////\n\
+////////////////////////+lGGh4O/L5Zrf8wBSPcJpdA7tcm4iZxHrrtvtx6R\n\
+OGQJAgEBoYGJA4GGAAQAtGyQsquetaPetft29sZ1SxWcegQj59V3cSLSaQYpjesA\n\
+ERfIfoSaPbVtCanBcJ4xIPvxaarrGhWCj1B3mjmSDv4B1DDMQiD6jggxZrzg+kRC\n\
+vVH8f9/FHwjQjBWEtctiQzPShusqnD5I3hTBBbX/qh0XaLcLMz0bA0o/HHQ+0xUw\n\
+Aak=\n\
+-----END EC PRIVATE KEY-----";
+    let result = decode_secret_key(key, None);
+    assert!(result.is_ok(), "Failed to parse P-521 key with full domain params: {:?}", result.err());
+    let pk = result.unwrap();
+    assert_eq!(pk.algorithm(), Algorithm::Ecdsa {
+        curve: ssh_key::EcdsaCurve::NistP521,
+    });
+}
+
+#[test]
+fn test_ec_malformed_der_returns_error() {
+    // Completely invalid data — not valid SEC1 or PKCS#8
+    let result = decode_secret_key("-----BEGIN EC PRIVATE KEY-----\nAAAA\n-----END EC PRIVATE KEY-----", None);
+    assert!(result.is_err(), "Should fail on malformed DER");
+
+    // Truncated SEC1 key
+    let result = decode_secret_key("-----BEGIN EC PRIVATE KEY-----\nMIIBaAIBAQQg\n-----END EC PRIVATE KEY-----", None);
+    assert!(result.is_err(), "Should fail on truncated key");
+}