From f7f39bff1dc95641b398de0177e11c2db58abc08 Mon Sep 17 00:00:00 2001 From: Olav Kongas Date: Tue, 7 Apr 2026 00:13:44 +0300 Subject: [PATCH] Fix DTLS 1.2 signature hash mismatch for P-384 keys --- src/crypto/aws_lc_rs/sign.rs | 20 ++++++++- src/crypto/provider.rs | 12 +++++- src/crypto/rust_crypto/sign.rs | 74 +++++++++++++++++++------------- src/dtls12/context.rs | 9 +++- src/dtls12/server.rs | 36 +++++++++------- src/dtls13/client.rs | 2 +- tests/dtls12/handshake.rs | 77 ++++++++++++++++++++++++++++++++++ tests/ossl/cert.rs | 8 ++++ 8 files changed, 189 insertions(+), 49 deletions(-) diff --git a/src/crypto/aws_lc_rs/sign.rs b/src/crypto/aws_lc_rs/sign.rs index 365b4da6..d1cc2b88 100644 --- a/src/crypto/aws_lc_rs/sign.rs +++ b/src/crypto/aws_lc_rs/sign.rs @@ -31,7 +31,14 @@ impl std::fmt::Debug for EcdsaSigningKey { } impl SigningKey for EcdsaSigningKey { - fn sign(&mut self, data: &[u8], buf: &mut Buf) -> Result<(), String> { + fn sign(&mut self, data: &[u8], hash_alg: HashAlgorithm, buf: &mut Buf) -> Result<(), String> { + let key_hash = self.hash_algorithm(); + if hash_alg != key_hash { + return Err(format!( + "aws-lc-rs ECDSA key is locked to {:?} but {:?} was requested", + key_hash, hash_alg + )); + } let rng = aws_lc_rs::rand::SystemRandom::new(); let signature = self .key_pair @@ -55,6 +62,17 @@ impl SigningKey for EcdsaSigningKey { panic!("Unsupported signing algorithm") } } + + fn supported_hash_algorithms(&self) -> &[HashAlgorithm] { + // aws-lc-rs locks the hash at key-load time; only one is supported. + if self.signing_algorithm == &ECDSA_P256_SHA256_ASN1_SIGNING { + &[HashAlgorithm::SHA256] + } else if self.signing_algorithm == &ECDSA_P384_SHA384_ASN1_SIGNING { + &[HashAlgorithm::SHA384] + } else { + panic!("Unsupported signing algorithm") + } + } } /// Key provider implementation. diff --git a/src/crypto/provider.rs b/src/crypto/provider.rs index 5def5fd5..6cf94887 100644 --- a/src/crypto/provider.rs +++ b/src/crypto/provider.rs @@ -201,14 +201,22 @@ pub trait HashContext: CryptoSafe { /// Signing key for generating digital signatures. pub trait SigningKey: CryptoSafe { - /// Sign data and return the signature. - fn sign(&mut self, data: &[u8], out: &mut Buf) -> Result<(), String>; + /// Sign data using the specified hash algorithm and return the signature. + fn sign(&mut self, data: &[u8], hash_alg: HashAlgorithm, out: &mut Buf) -> Result<(), String>; /// Signature algorithm used by this key. fn algorithm(&self) -> SignatureAlgorithm; /// Default hash algorithm for this key. fn hash_algorithm(&self) -> HashAlgorithm; + + /// Hash algorithms this key can sign with. + /// + /// Used during negotiation to intersect with the peer's offered + /// algorithms. Backends that lock the hash at key-load time (e.g. + /// aws-lc-rs) return only the locked hash; backends that support + /// arbitrary prehash signing (e.g. RustCrypto) may return several. + fn supported_hash_algorithms(&self) -> &[HashAlgorithm]; } /// Active key exchange instance (ephemeral keypair for one handshake). diff --git a/src/crypto/rust_crypto/sign.rs b/src/crypto/rust_crypto/sign.rs index 15cf044b..42f01f17 100644 --- a/src/crypto/rust_crypto/sign.rs +++ b/src/crypto/rust_crypto/sign.rs @@ -32,42 +32,53 @@ impl std::fmt::Debug for EcdsaSigningKey { } impl SigningKeyTrait for EcdsaSigningKey { - fn sign(&mut self, data: &[u8], out: &mut Buf) -> Result<(), String> { + fn sign(&mut self, data: &[u8], hash_alg: HashAlgorithm, out: &mut Buf) -> Result<(), String> { + use ecdsa::signature::hazmat::PrehashSigner; + use sha2::Digest; + match self { EcdsaSigningKey::P256(key) => { - use ecdsa::signature::hazmat::PrehashSigner; - use sha2::{Digest, Sha256}; - - // Hash the data before signing (PrehashSigner expects a hash digest) - let mut hasher = Sha256::new(); - hasher.update(data); - let hash = hasher.finalize(); - - let signature: Signature = key - .sign_prehash(&hash) - .map_err(|_| "Signing failed".to_string())?; - let sig_der = signature.to_der(); - let sig_bytes = sig_der.as_bytes(); + let signature: Signature = match hash_alg { + HashAlgorithm::SHA256 => { + let hash = sha2::Sha256::digest(data); + key.sign_prehash(&hash) + } + HashAlgorithm::SHA384 => { + let hash = sha2::Sha384::digest(data); + key.sign_prehash(&hash) + } + _ => { + return Err(format!( + "P-256 key does not support hash algorithm {:?}", + hash_alg + )); + } + } + .map_err(|_| "Signing failed".to_string())?; out.clear(); - out.extend_from_slice(sig_bytes); + out.extend_from_slice(signature.to_der().as_bytes()); Ok(()) } EcdsaSigningKey::P384(key) => { - use ecdsa::signature::hazmat::PrehashSigner; - use sha2::{Digest, Sha384}; - - // Hash the data before signing (PrehashSigner expects a hash digest) - let mut hasher = Sha384::new(); - hasher.update(data); - let hash = hasher.finalize(); - - let signature: Signature = key - .sign_prehash(&hash) - .map_err(|_| "Signing failed".to_string())?; - let sig_der = signature.to_der(); - let sig_bytes = sig_der.as_bytes(); + let signature: Signature = match hash_alg { + HashAlgorithm::SHA256 => { + let hash = sha2::Sha256::digest(data); + key.sign_prehash(&hash) + } + HashAlgorithm::SHA384 => { + let hash = sha2::Sha384::digest(data); + key.sign_prehash(&hash) + } + _ => { + return Err(format!( + "P-384 key does not support hash algorithm {:?}", + hash_alg + )); + } + } + .map_err(|_| "Signing failed".to_string())?; out.clear(); - out.extend_from_slice(sig_bytes); + out.extend_from_slice(signature.to_der().as_bytes()); Ok(()) } } @@ -83,6 +94,11 @@ impl SigningKeyTrait for EcdsaSigningKey { EcdsaSigningKey::P384(_) => HashAlgorithm::SHA384, } } + + fn supported_hash_algorithms(&self) -> &[HashAlgorithm] { + // PrehashSigner accepts any hash digest, so both work for either curve. + &[HashAlgorithm::SHA256, HashAlgorithm::SHA384] + } } /// Key provider implementation. diff --git a/src/dtls12/context.rs b/src/dtls12/context.rs index 59a7fcda..6ba60692 100644 --- a/src/dtls12/context.rs +++ b/src/dtls12/context.rs @@ -393,10 +393,10 @@ impl CryptoContext { pub fn sign_data( &mut self, data: &[u8], - _hash_alg: HashAlgorithm, + hash_alg: HashAlgorithm, out: &mut Buf, ) -> Result<(), String> { - self.private_key.sign(data, out) + self.private_key.sign(data, hash_alg, out) } /// Generate verify data for a Finished message using PRF @@ -513,6 +513,11 @@ impl CryptoContext { self.private_key.hash_algorithm() } + /// Hash algorithms the configured private key can sign with + pub fn private_key_supported_hash_algorithms(&self) -> &[HashAlgorithm] { + self.private_key.supported_hash_algorithms() + } + /// Create a hash context for the given algorithm pub fn create_hash(&self, algorithm: HashAlgorithm) -> Box { self.provider().hash_provider.create_hash(algorithm) diff --git a/src/dtls12/server.rs b/src/dtls12/server.rs index b1579088..84e4e241 100644 --- a/src/dtls12/server.rs +++ b/src/dtls12/server.rs @@ -488,10 +488,18 @@ impl State { })?; // Select signature/hash for SKE by intersecting client's list - // with our key type (prefer SHA256, then SHA384) + // with our key type, preferring the key's native hash algorithm let selected_signature = select_ske_signature_algorithm( server.client_signature_algorithms.as_ref(), server.engine.crypto_context().signature_algorithm(), + server + .engine + .crypto_context() + .private_key_default_hash_algorithm(), + server + .engine + .crypto_context() + .private_key_supported_hash_algorithms(), ); debug!( @@ -1182,12 +1190,21 @@ mod tests { fn select_ske_signature_algorithm( client_algs: Option<&SignatureAndHashAlgorithmVec>, our_sig: SignatureAlgorithm, + our_hash: HashAlgorithm, + supported_hashes: &[HashAlgorithm], ) -> SignatureAndHashAlgorithm { - // Our hash preference order - let hash_pref = [HashAlgorithm::SHA256, HashAlgorithm::SHA384]; + // Prefer the key's native hash first, then fall back to the other + let hash_pref = match our_hash { + HashAlgorithm::SHA384 => [HashAlgorithm::SHA384, HashAlgorithm::SHA256], + _ => [HashAlgorithm::SHA256, HashAlgorithm::SHA384], + }; if let Some(list) = client_algs { for h in hash_pref.iter() { + // Only consider hash algorithms the backend can actually sign with + if !supported_hashes.contains(h) { + continue; + } if let Some(chosen) = list .iter() .find(|alg| alg.signature == our_sig && alg.hash == *h) @@ -1197,17 +1214,8 @@ fn select_ske_signature_algorithm( } } - // Fallback to our default hash for our key type - let hash = engine_default_hash_for_sig(our_sig); - SignatureAndHashAlgorithm::new(hash, our_sig) -} - -fn engine_default_hash_for_sig(sig: SignatureAlgorithm) -> HashAlgorithm { - match sig { - SignatureAlgorithm::RSA => HashAlgorithm::SHA256, - SignatureAlgorithm::ECDSA => HashAlgorithm::SHA256, - _ => HashAlgorithm::SHA256, - } + // Fallback: use the key's native hash + SignatureAndHashAlgorithm::new(our_hash, our_sig) } fn select_certificate_request_sig_algs( diff --git a/src/dtls13/client.rs b/src/dtls13/client.rs index 77e388fd..9188a47a 100644 --- a/src/dtls13/client.rs +++ b/src/dtls13/client.rs @@ -1284,7 +1284,7 @@ pub(crate) fn handshake_create_certificate_verify( let mut signature = Buf::new(); engine .signing_key() - .sign(&signed_content, &mut signature) + .sign(&signed_content, hash_alg, &mut signature) .map_err(|e| Error::CryptoError(format!("Failed to sign CertificateVerify: {}", e)))?; // Determine the SignatureScheme from hash_alg + sig_alg diff --git a/tests/dtls12/handshake.rs b/tests/dtls12/handshake.rs index e4b342f7..a6f19907 100644 --- a/tests/dtls12/handshake.rs +++ b/tests/dtls12/handshake.rs @@ -793,3 +793,80 @@ fn dtls12_handshake_timeout_expires() { "Client handshake should eventually time out when no packets are delivered" ); } + +#[test] +fn dtls12_handshake_p384_certificate() { + //! DTLS 1.2 handshake where both sides use P-384 ECDSA certificates. + //! Regression test: the server's ServerKeyExchange must sign with the + //! same hash algorithm it advertises on the wire (SHA-384 for P-384). + + use crate::ossl_helper::{DtlsCertOptions, DtlsPKeyType, OsslDtlsCert}; + + let client_cert = OsslDtlsCert::new(DtlsCertOptions { + common_name: "WebRTC".into(), + pkey_type: DtlsPKeyType::EcDsaP384, + }); + let server_cert = OsslDtlsCert::new(DtlsCertOptions { + common_name: "WebRTC".into(), + pkey_type: DtlsPKeyType::EcDsaP384, + }); + + let config = dtls12_config(); + let mut now = Instant::now(); + + let mut client = Dtls::new_12( + Arc::clone(&config), + dimpl::DtlsCertificate { + certificate: client_cert.x509.to_der().expect("client cert der"), + private_key: client_cert + .pkey + .private_key_to_der() + .expect("client key der"), + }, + now, + ); + client.set_active(true); + + let mut server = Dtls::new_12( + config, + dimpl::DtlsCertificate { + certificate: server_cert.x509.to_der().expect("server cert der"), + private_key: server_cert + .pkey + .private_key_to_der() + .expect("server key der"), + }, + now, + ); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + + for _ in 0..30 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + + let client_out = drain_outputs(&mut client); + let server_out = drain_outputs(&mut server); + + if client_out.connected { + client_connected = true; + } + if server_out.connected { + server_connected = true; + } + + deliver_packets(&client_out.packets, &mut server); + deliver_packets(&server_out.packets, &mut client); + + if client_connected && server_connected { + break; + } + + now += Duration::from_millis(10); + } + + assert!(client_connected, "Client should connect with P-384 cert"); + assert!(server_connected, "Server should connect with P-384 cert"); +} diff --git a/tests/ossl/cert.rs b/tests/ossl/cert.rs index a595ced6..af9721ce 100644 --- a/tests/ossl/cert.rs +++ b/tests/ossl/cert.rs @@ -30,6 +30,8 @@ pub enum DtlsPKeyType { /// Generate an EC-DSA key pair using the NIST P-256 curve #[default] EcDsaP256, + /// Generate an EC-DSA key pair using the NIST P-384 curve + EcDsaP384, } /// Controls certificate generation options. @@ -73,6 +75,12 @@ impl OsslDtlsCert { let key = EcKey::generate(&group)?; PKey::from_ec_key(key)? } + DtlsPKeyType::EcDsaP384 => { + let nid = Nid::SECP384R1; + let group = EcGroup::from_curve_name(nid)?; + let key = EcKey::generate(&group)?; + PKey::from_ec_key(key)? + } }; let mut x509b = X509::builder()?;