Fix load_private_key for ECDSA keys (#16)

Previously it would return an incorrect signature verification
algorithm, which would cause verification to fail.

Add tests using rcgen for a variety of key algorithm

Author: tofay

.github/workflows/ci.yml

@@ -45,6 +45,8 @@ jobs:
         run: cargo fmt -- --check -l
       - name: cargo clippy (warnings)
         run: cargo clippy --all-targets -- -D warnings
+      - name: cargo clippy --no-default-features (warnings)
+        run: cargo clippy --no-default-features --all-targets -- -D warnings
 
   test-fips:
     name: Test using FIPS openssl

Cargo.toml

@@ -26,6 +26,9 @@ antidote = "1.0.0"
 hex = "0.4.3"
 lazy_static = "1.4.0"
 once_cell = "1.8.0"
+rcgen = { version = "0.13.1", default-features = false, features = [
+    "aws_lc_rs",
+] }
 rstest = "0.23.0"
 # Use aws_lc_rs to test our provider
 rustls = { version = "0.23.0", features = ["aws_lc_rs"] }

src/lib.rs

@@ -201,7 +201,7 @@ pub fn custom_provider(
 }
 
 /// All supported cipher suites in descending order of preference:
-/// * [tls13::TLS13_AES_256_GCM_SHA384]
+/// * TLS13_AES_256_GCM_SHA384
 /// * TLS13_AES_128_GCM_SHA256
 /// * TLS13_CHACHA20_POLY1305_SHA256
 /// * TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384

src/signer.rs

@@ -21,13 +21,6 @@ pub(crate) static RSA_SCHEMES: &[SignatureScheme] = &[
     SignatureScheme::RSA_PKCS1_SHA256,
 ];
 
-/// All ECDSA signature schemes in descending order of preference
-pub(crate) static ECDSA_SCHEMES: &[SignatureScheme] = &[
-    SignatureScheme::ECDSA_NISTP521_SHA512,
-    SignatureScheme::ECDSA_NISTP384_SHA384,
-    SignatureScheme::ECDSA_NISTP256_SHA256,
-];
-
 #[derive(Debug)]
 struct Signer {
     key: Arc<openssl::pkey::PKey<Private>>,
@@ -134,10 +127,36 @@ impl SigningKey for PKey {
                     None
                 }
             }
-            SignatureAlgorithm::ECDSA => ECDSA_SCHEMES
-                .iter()
-                .find(|scheme| offered.contains(scheme))
-                .map(|scheme| Box::new(self.signer(*scheme)) as Box<dyn rustls::sign::Signer>),
+            SignatureAlgorithm::ECDSA => {
+                // First determine our scheme
+                self.0
+                    .ec_key()
+                    .ok()
+                    .and_then(|ec_key| {
+                        let nid = ec_key.group().curve_name();
+                        let scheme = match nid {
+                            Some(openssl::nid::Nid::X9_62_PRIME256V1) => {
+                                SignatureScheme::ECDSA_NISTP256_SHA256
+                            }
+                            Some(openssl::nid::Nid::SECP384R1) => {
+                                SignatureScheme::ECDSA_NISTP384_SHA384
+                            }
+                            Some(openssl::nid::Nid::SECP521R1) => {
+                                SignatureScheme::ECDSA_NISTP521_SHA512
+                            }
+                            _ => return None,
+                        };
+                        Some(scheme)
+                    })
+                    // Now see if that was offered
+                    .and_then(|scheme| {
+                        if offered.contains(&scheme) {
+                            Some(Box::new(self.signer(scheme)) as Box<dyn rustls::sign::Signer>)
+                        } else {
+                            None
+                        }
+                    })
+            }
             _ => None,
         }
     }

src/tls12.rs

@@ -1,21 +1,29 @@
 use crate::aead::{self, TAG_LEN};
 use crate::hash::{SHA256, SHA384};
 use crate::prf::Prf;
-use crate::signer::{ECDSA_SCHEMES, RSA_SCHEMES};
+use crate::signer::RSA_SCHEMES;
 use rustls::crypto::cipher::{
     make_tls12_aad, AeadKey, InboundOpaqueMessage, InboundPlainMessage, Iv, KeyBlockShape,
     MessageDecrypter, MessageEncrypter, Nonce, OutboundOpaqueMessage, OutboundPlainMessage,
     PrefixedPayload, Tls12AeadAlgorithm, UnsupportedOperationError, NONCE_LEN,
 };
+use rustls::crypto::KeyExchangeAlgorithm;
 use rustls::{
-    crypto::KeyExchangeAlgorithm, CipherSuite, CipherSuiteCommon, SupportedCipherSuite,
-    Tls12CipherSuite,
+    CipherSuite, CipherSuiteCommon, ConnectionTrafficSecrets, Error, SignatureScheme,
+    SupportedCipherSuite, Tls12CipherSuite,
 };
-use rustls::{ConnectionTrafficSecrets, Error};
 
 const GCM_EXPLICIT_NONCE_LENGTH: usize = 8;
 const GCM_IMPLICIT_NONCE_LENGTH: usize = 4;
 
+static ECDSA_SCHEMES: &[SignatureScheme] = &[
+    #[cfg(not(feature = "fips"))]
+    SignatureScheme::ED25519,
+    SignatureScheme::ECDSA_NISTP521_SHA512,
+    SignatureScheme::ECDSA_NISTP384_SHA384,
+    SignatureScheme::ECDSA_NISTP256_SHA256,
+];
+
 /// The TLS1.2 ciphersuite `TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256`.
 #[cfg(all(chacha, not(feature = "fips")))]
 pub static TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256: SupportedCipherSuite =

src/verify.rs

@@ -58,100 +58,116 @@ pub static SUPPORTED_SIG_ALGS: WebPkiSupportedAlgorithms = WebPkiSupportedAlgori
 
 /// RSA PKCS#1 1.5 signatures using SHA-256.
 pub(crate) static RSA_PKCS1_SHA256: &dyn SignatureVerificationAlgorithm = &OpenSslAlgorithm {
+    display_name: "RSA_PKCS1_SHA256",
     public_key_alg_id: alg_id::RSA_ENCRYPTION,
     signature_alg_id: alg_id::RSA_PKCS1_SHA256,
 };
 
 /// RSA PKCS#1 1.5 signatures using SHA-384.
 pub(crate) static RSA_PKCS1_SHA384: &dyn SignatureVerificationAlgorithm = &OpenSslAlgorithm {
+    display_name: "RSA_PKCS1_SHA384",
     public_key_alg_id: alg_id::RSA_ENCRYPTION,
     signature_alg_id: alg_id::RSA_PKCS1_SHA384,
 };
 
 /// RSA PKCS#1 1.5 signatures using SHA-512.
 pub(crate) static RSA_PKCS1_SHA512: &dyn SignatureVerificationAlgorithm = &OpenSslAlgorithm {
+    display_name: "RSA_PKCS1_SHA512",
     public_key_alg_id: alg_id::RSA_ENCRYPTION,
     signature_alg_id: alg_id::RSA_PKCS1_SHA512,
 };
 
 /// RSA PSS signatures using SHA-256.
 pub(crate) static RSA_PSS_SHA256: &dyn SignatureVerificationAlgorithm = &OpenSslAlgorithm {
+    display_name: "RSA_PSS_SHA256",
     public_key_alg_id: alg_id::RSA_ENCRYPTION,
     signature_alg_id: alg_id::RSA_PSS_SHA256,
 };
 
 /// RSA PSS signatures using SHA-384.
 pub(crate) static RSA_PSS_SHA384: &dyn SignatureVerificationAlgorithm = &OpenSslAlgorithm {
+    display_name: "RSA_PSS_SHA384",
     public_key_alg_id: alg_id::RSA_ENCRYPTION,
     signature_alg_id: alg_id::RSA_PSS_SHA384,
 };
 
 /// RSA PSS signatures using SHA-512.
 pub(crate) static RSA_PSS_SHA512: &dyn SignatureVerificationAlgorithm = &OpenSslAlgorithm {
+    display_name: "RSA_PSS_SHA512",
     public_key_alg_id: alg_id::RSA_ENCRYPTION,
     signature_alg_id: alg_id::RSA_PSS_SHA512,
 };
 
 #[cfg(not(feature = "fips"))]
 /// ED25519 signatures according to RFC 8410
 pub(crate) static ED25519: &dyn SignatureVerificationAlgorithm = &OpenSslAlgorithm {
+    display_name: "ED25519",
     public_key_alg_id: alg_id::ED25519,
     signature_alg_id: alg_id::ED25519,
 };
 
 /// ECDSA signatures using the P-256 curve and SHA-256.
 pub(crate) static ECDSA_P256_SHA256: &dyn SignatureVerificationAlgorithm = &OpenSslAlgorithm {
+    display_name: "ECDSA_P256_SHA256",
     public_key_alg_id: alg_id::ECDSA_P256,
     signature_alg_id: alg_id::ECDSA_SHA256,
 };
 
 /// ECDSA signatures using the P-256 curve and SHA-384. Deprecated.
 pub(crate) static ECDSA_P256_SHA384: &dyn SignatureVerificationAlgorithm = &OpenSslAlgorithm {
+    display_name: "ECDSA_P256_SHA384",
     public_key_alg_id: alg_id::ECDSA_P256,
     signature_alg_id: alg_id::ECDSA_SHA384,
 };
 
 /// ECDSA signatures using the P-384 curve and SHA-256. Deprecated.
 pub(crate) static ECDSA_P384_SHA256: &dyn SignatureVerificationAlgorithm = &OpenSslAlgorithm {
+    display_name: "ECDSA_P384_SHA256",
     public_key_alg_id: alg_id::ECDSA_P384,
     signature_alg_id: alg_id::ECDSA_SHA256,
 };
 
 /// ECDSA signatures using the P-384 curve and SHA-384.
 pub(crate) static ECDSA_P384_SHA384: &dyn SignatureVerificationAlgorithm = &OpenSslAlgorithm {
+    display_name: "ECDSA_P384_SHA384",
     public_key_alg_id: alg_id::ECDSA_P384,
     signature_alg_id: alg_id::ECDSA_SHA384,
 };
 
 /// ECDSA signatures using the P-521 curve and SHA-256.
 pub(crate) static ECDSA_P521_SHA256: &dyn SignatureVerificationAlgorithm = &OpenSslAlgorithm {
+    display_name: "ECDSA_P521_SHA256",
     public_key_alg_id: alg_id::ECDSA_P521,
     signature_alg_id: alg_id::ECDSA_SHA256,
 };
 
 /// ECDSA signatures using the P-521 curve and SHA-384.
 pub(crate) static ECDSA_P521_SHA384: &dyn SignatureVerificationAlgorithm = &OpenSslAlgorithm {
+    display_name: "ECDSA_P521_SHA384",
     public_key_alg_id: alg_id::ECDSA_P521,
     signature_alg_id: alg_id::ECDSA_SHA384,
 };
 
 /// ECDSA signatures using the P-521 curve and SHA-512.
 pub(crate) static ECDSA_P521_SHA512: &dyn SignatureVerificationAlgorithm = &OpenSslAlgorithm {
+    display_name: "ECDSA_P521_SHA512",
     public_key_alg_id: alg_id::ECDSA_P521,
     signature_alg_id: alg_id::ECDSA_SHA512,
 };
 
 struct OpenSslAlgorithm {
+    display_name: &'static str,
     public_key_alg_id: AlgorithmIdentifier,
     signature_alg_id: AlgorithmIdentifier,
 }
 
 impl fmt::Debug for OpenSslAlgorithm {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.debug_struct("OpenSSLAlgorithm")
-            .field("public_key_alg_id", &self.public_key_alg_id)
-            .field("signature_alg_id", &self.signature_alg_id)
-            .finish()
+        write!(
+            f,
+            "rustls_openssl Signature Verification Algorithm: {}",
+            self.display_name
+        )
     }
 }
 
@@ -293,3 +309,20 @@ impl SignatureVerificationAlgorithm for OpenSslAlgorithm {
         crate::fips()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_open_ssl_algorithm_debug() {
+        assert_eq!(
+            format!("{:?}", ECDSA_P256_SHA256),
+            "rustls_openssl Signature Verification Algorithm: ECDSA_P256_SHA256"
+        );
+        assert_eq!(
+            format!("{:?}", RSA_PSS_SHA256),
+            "rustls_openssl Signature Verification Algorithm: RSA_PSS_SHA256"
+        );
+    }
+}