Skip to content

Commit f7f39bf

Browse files
committed
Fix DTLS 1.2 signature hash mismatch for P-384 keys
1 parent dda2935 commit f7f39bf

8 files changed

Lines changed: 189 additions & 49 deletions

File tree

src/crypto/aws_lc_rs/sign.rs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,14 @@ impl std::fmt::Debug for EcdsaSigningKey {
3131
}
3232

3333
impl SigningKey for EcdsaSigningKey {
34-
fn sign(&mut self, data: &[u8], buf: &mut Buf) -> Result<(), String> {
34+
fn sign(&mut self, data: &[u8], hash_alg: HashAlgorithm, buf: &mut Buf) -> Result<(), String> {
35+
let key_hash = self.hash_algorithm();
36+
if hash_alg != key_hash {
37+
return Err(format!(
38+
"aws-lc-rs ECDSA key is locked to {:?} but {:?} was requested",
39+
key_hash, hash_alg
40+
));
41+
}
3542
let rng = aws_lc_rs::rand::SystemRandom::new();
3643
let signature = self
3744
.key_pair
@@ -55,6 +62,17 @@ impl SigningKey for EcdsaSigningKey {
5562
panic!("Unsupported signing algorithm")
5663
}
5764
}
65+
66+
fn supported_hash_algorithms(&self) -> &[HashAlgorithm] {
67+
// aws-lc-rs locks the hash at key-load time; only one is supported.
68+
if self.signing_algorithm == &ECDSA_P256_SHA256_ASN1_SIGNING {
69+
&[HashAlgorithm::SHA256]
70+
} else if self.signing_algorithm == &ECDSA_P384_SHA384_ASN1_SIGNING {
71+
&[HashAlgorithm::SHA384]
72+
} else {
73+
panic!("Unsupported signing algorithm")
74+
}
75+
}
5876
}
5977

6078
/// Key provider implementation.

src/crypto/provider.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,14 +201,22 @@ pub trait HashContext: CryptoSafe {
201201

202202
/// Signing key for generating digital signatures.
203203
pub trait SigningKey: CryptoSafe {
204-
/// Sign data and return the signature.
205-
fn sign(&mut self, data: &[u8], out: &mut Buf) -> Result<(), String>;
204+
/// Sign data using the specified hash algorithm and return the signature.
205+
fn sign(&mut self, data: &[u8], hash_alg: HashAlgorithm, out: &mut Buf) -> Result<(), String>;
206206

207207
/// Signature algorithm used by this key.
208208
fn algorithm(&self) -> SignatureAlgorithm;
209209

210210
/// Default hash algorithm for this key.
211211
fn hash_algorithm(&self) -> HashAlgorithm;
212+
213+
/// Hash algorithms this key can sign with.
214+
///
215+
/// Used during negotiation to intersect with the peer's offered
216+
/// algorithms. Backends that lock the hash at key-load time (e.g.
217+
/// aws-lc-rs) return only the locked hash; backends that support
218+
/// arbitrary prehash signing (e.g. RustCrypto) may return several.
219+
fn supported_hash_algorithms(&self) -> &[HashAlgorithm];
212220
}
213221

214222
/// Active key exchange instance (ephemeral keypair for one handshake).

src/crypto/rust_crypto/sign.rs

Lines changed: 45 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -32,42 +32,53 @@ impl std::fmt::Debug for EcdsaSigningKey {
3232
}
3333

3434
impl SigningKeyTrait for EcdsaSigningKey {
35-
fn sign(&mut self, data: &[u8], out: &mut Buf) -> Result<(), String> {
35+
fn sign(&mut self, data: &[u8], hash_alg: HashAlgorithm, out: &mut Buf) -> Result<(), String> {
36+
use ecdsa::signature::hazmat::PrehashSigner;
37+
use sha2::Digest;
38+
3639
match self {
3740
EcdsaSigningKey::P256(key) => {
38-
use ecdsa::signature::hazmat::PrehashSigner;
39-
use sha2::{Digest, Sha256};
40-
41-
// Hash the data before signing (PrehashSigner expects a hash digest)
42-
let mut hasher = Sha256::new();
43-
hasher.update(data);
44-
let hash = hasher.finalize();
45-
46-
let signature: Signature<NistP256> = key
47-
.sign_prehash(&hash)
48-
.map_err(|_| "Signing failed".to_string())?;
49-
let sig_der = signature.to_der();
50-
let sig_bytes = sig_der.as_bytes();
41+
let signature: Signature<NistP256> = match hash_alg {
42+
HashAlgorithm::SHA256 => {
43+
let hash = sha2::Sha256::digest(data);
44+
key.sign_prehash(&hash)
45+
}
46+
HashAlgorithm::SHA384 => {
47+
let hash = sha2::Sha384::digest(data);
48+
key.sign_prehash(&hash)
49+
}
50+
_ => {
51+
return Err(format!(
52+
"P-256 key does not support hash algorithm {:?}",
53+
hash_alg
54+
));
55+
}
56+
}
57+
.map_err(|_| "Signing failed".to_string())?;
5158
out.clear();
52-
out.extend_from_slice(sig_bytes);
59+
out.extend_from_slice(signature.to_der().as_bytes());
5360
Ok(())
5461
}
5562
EcdsaSigningKey::P384(key) => {
56-
use ecdsa::signature::hazmat::PrehashSigner;
57-
use sha2::{Digest, Sha384};
58-
59-
// Hash the data before signing (PrehashSigner expects a hash digest)
60-
let mut hasher = Sha384::new();
61-
hasher.update(data);
62-
let hash = hasher.finalize();
63-
64-
let signature: Signature<NistP384> = key
65-
.sign_prehash(&hash)
66-
.map_err(|_| "Signing failed".to_string())?;
67-
let sig_der = signature.to_der();
68-
let sig_bytes = sig_der.as_bytes();
63+
let signature: Signature<NistP384> = match hash_alg {
64+
HashAlgorithm::SHA256 => {
65+
let hash = sha2::Sha256::digest(data);
66+
key.sign_prehash(&hash)
67+
}
68+
HashAlgorithm::SHA384 => {
69+
let hash = sha2::Sha384::digest(data);
70+
key.sign_prehash(&hash)
71+
}
72+
_ => {
73+
return Err(format!(
74+
"P-384 key does not support hash algorithm {:?}",
75+
hash_alg
76+
));
77+
}
78+
}
79+
.map_err(|_| "Signing failed".to_string())?;
6980
out.clear();
70-
out.extend_from_slice(sig_bytes);
81+
out.extend_from_slice(signature.to_der().as_bytes());
7182
Ok(())
7283
}
7384
}
@@ -83,6 +94,11 @@ impl SigningKeyTrait for EcdsaSigningKey {
8394
EcdsaSigningKey::P384(_) => HashAlgorithm::SHA384,
8495
}
8596
}
97+
98+
fn supported_hash_algorithms(&self) -> &[HashAlgorithm] {
99+
// PrehashSigner accepts any hash digest, so both work for either curve.
100+
&[HashAlgorithm::SHA256, HashAlgorithm::SHA384]
101+
}
86102
}
87103

88104
/// Key provider implementation.

src/dtls12/context.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -393,10 +393,10 @@ impl CryptoContext {
393393
pub fn sign_data(
394394
&mut self,
395395
data: &[u8],
396-
_hash_alg: HashAlgorithm,
396+
hash_alg: HashAlgorithm,
397397
out: &mut Buf,
398398
) -> Result<(), String> {
399-
self.private_key.sign(data, out)
399+
self.private_key.sign(data, hash_alg, out)
400400
}
401401

402402
/// Generate verify data for a Finished message using PRF
@@ -513,6 +513,11 @@ impl CryptoContext {
513513
self.private_key.hash_algorithm()
514514
}
515515

516+
/// Hash algorithms the configured private key can sign with
517+
pub fn private_key_supported_hash_algorithms(&self) -> &[HashAlgorithm] {
518+
self.private_key.supported_hash_algorithms()
519+
}
520+
516521
/// Create a hash context for the given algorithm
517522
pub fn create_hash(&self, algorithm: HashAlgorithm) -> Box<dyn crypto::HashContext> {
518523
self.provider().hash_provider.create_hash(algorithm)

src/dtls12/server.rs

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -488,10 +488,18 @@ impl State {
488488
})?;
489489

490490
// Select signature/hash for SKE by intersecting client's list
491-
// with our key type (prefer SHA256, then SHA384)
491+
// with our key type, preferring the key's native hash algorithm
492492
let selected_signature = select_ske_signature_algorithm(
493493
server.client_signature_algorithms.as_ref(),
494494
server.engine.crypto_context().signature_algorithm(),
495+
server
496+
.engine
497+
.crypto_context()
498+
.private_key_default_hash_algorithm(),
499+
server
500+
.engine
501+
.crypto_context()
502+
.private_key_supported_hash_algorithms(),
495503
);
496504

497505
debug!(
@@ -1182,12 +1190,21 @@ mod tests {
11821190
fn select_ske_signature_algorithm(
11831191
client_algs: Option<&SignatureAndHashAlgorithmVec>,
11841192
our_sig: SignatureAlgorithm,
1193+
our_hash: HashAlgorithm,
1194+
supported_hashes: &[HashAlgorithm],
11851195
) -> SignatureAndHashAlgorithm {
1186-
// Our hash preference order
1187-
let hash_pref = [HashAlgorithm::SHA256, HashAlgorithm::SHA384];
1196+
// Prefer the key's native hash first, then fall back to the other
1197+
let hash_pref = match our_hash {
1198+
HashAlgorithm::SHA384 => [HashAlgorithm::SHA384, HashAlgorithm::SHA256],
1199+
_ => [HashAlgorithm::SHA256, HashAlgorithm::SHA384],
1200+
};
11881201

11891202
if let Some(list) = client_algs {
11901203
for h in hash_pref.iter() {
1204+
// Only consider hash algorithms the backend can actually sign with
1205+
if !supported_hashes.contains(h) {
1206+
continue;
1207+
}
11911208
if let Some(chosen) = list
11921209
.iter()
11931210
.find(|alg| alg.signature == our_sig && alg.hash == *h)
@@ -1197,17 +1214,8 @@ fn select_ske_signature_algorithm(
11971214
}
11981215
}
11991216

1200-
// Fallback to our default hash for our key type
1201-
let hash = engine_default_hash_for_sig(our_sig);
1202-
SignatureAndHashAlgorithm::new(hash, our_sig)
1203-
}
1204-
1205-
fn engine_default_hash_for_sig(sig: SignatureAlgorithm) -> HashAlgorithm {
1206-
match sig {
1207-
SignatureAlgorithm::RSA => HashAlgorithm::SHA256,
1208-
SignatureAlgorithm::ECDSA => HashAlgorithm::SHA256,
1209-
_ => HashAlgorithm::SHA256,
1210-
}
1217+
// Fallback: use the key's native hash
1218+
SignatureAndHashAlgorithm::new(our_hash, our_sig)
12111219
}
12121220

12131221
fn select_certificate_request_sig_algs(

src/dtls13/client.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1284,7 +1284,7 @@ pub(crate) fn handshake_create_certificate_verify(
12841284
let mut signature = Buf::new();
12851285
engine
12861286
.signing_key()
1287-
.sign(&signed_content, &mut signature)
1287+
.sign(&signed_content, hash_alg, &mut signature)
12881288
.map_err(|e| Error::CryptoError(format!("Failed to sign CertificateVerify: {}", e)))?;
12891289

12901290
// Determine the SignatureScheme from hash_alg + sig_alg

tests/dtls12/handshake.rs

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -793,3 +793,80 @@ fn dtls12_handshake_timeout_expires() {
793793
"Client handshake should eventually time out when no packets are delivered"
794794
);
795795
}
796+
797+
#[test]
798+
fn dtls12_handshake_p384_certificate() {
799+
//! DTLS 1.2 handshake where both sides use P-384 ECDSA certificates.
800+
//! Regression test: the server's ServerKeyExchange must sign with the
801+
//! same hash algorithm it advertises on the wire (SHA-384 for P-384).
802+
803+
use crate::ossl_helper::{DtlsCertOptions, DtlsPKeyType, OsslDtlsCert};
804+
805+
let client_cert = OsslDtlsCert::new(DtlsCertOptions {
806+
common_name: "WebRTC".into(),
807+
pkey_type: DtlsPKeyType::EcDsaP384,
808+
});
809+
let server_cert = OsslDtlsCert::new(DtlsCertOptions {
810+
common_name: "WebRTC".into(),
811+
pkey_type: DtlsPKeyType::EcDsaP384,
812+
});
813+
814+
let config = dtls12_config();
815+
let mut now = Instant::now();
816+
817+
let mut client = Dtls::new_12(
818+
Arc::clone(&config),
819+
dimpl::DtlsCertificate {
820+
certificate: client_cert.x509.to_der().expect("client cert der"),
821+
private_key: client_cert
822+
.pkey
823+
.private_key_to_der()
824+
.expect("client key der"),
825+
},
826+
now,
827+
);
828+
client.set_active(true);
829+
830+
let mut server = Dtls::new_12(
831+
config,
832+
dimpl::DtlsCertificate {
833+
certificate: server_cert.x509.to_der().expect("server cert der"),
834+
private_key: server_cert
835+
.pkey
836+
.private_key_to_der()
837+
.expect("server key der"),
838+
},
839+
now,
840+
);
841+
server.set_active(false);
842+
843+
let mut client_connected = false;
844+
let mut server_connected = false;
845+
846+
for _ in 0..30 {
847+
client.handle_timeout(now).expect("client timeout");
848+
server.handle_timeout(now).expect("server timeout");
849+
850+
let client_out = drain_outputs(&mut client);
851+
let server_out = drain_outputs(&mut server);
852+
853+
if client_out.connected {
854+
client_connected = true;
855+
}
856+
if server_out.connected {
857+
server_connected = true;
858+
}
859+
860+
deliver_packets(&client_out.packets, &mut server);
861+
deliver_packets(&server_out.packets, &mut client);
862+
863+
if client_connected && server_connected {
864+
break;
865+
}
866+
867+
now += Duration::from_millis(10);
868+
}
869+
870+
assert!(client_connected, "Client should connect with P-384 cert");
871+
assert!(server_connected, "Server should connect with P-384 cert");
872+
}

tests/ossl/cert.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ pub enum DtlsPKeyType {
3030
/// Generate an EC-DSA key pair using the NIST P-256 curve
3131
#[default]
3232
EcDsaP256,
33+
/// Generate an EC-DSA key pair using the NIST P-384 curve
34+
EcDsaP384,
3335
}
3436

3537
/// Controls certificate generation options.
@@ -73,6 +75,12 @@ impl OsslDtlsCert {
7375
let key = EcKey::generate(&group)?;
7476
PKey::from_ec_key(key)?
7577
}
78+
DtlsPKeyType::EcDsaP384 => {
79+
let nid = Nid::SECP384R1;
80+
let group = EcGroup::from_curve_name(nid)?;
81+
let key = EcKey::generate(&group)?;
82+
PKey::from_ec_key(key)?
83+
}
7684
};
7785

7886
let mut x509b = X509::builder()?;

0 commit comments

Comments
 (0)