diff --git a/CHANGELOG.md b/CHANGELOG.md index 2eacc7a8..d13d41c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Unreleased + * Stop advertising unsupported RSA signatures in DTLS 1.2 CertificateRequest + # 0.7.1 * Add `Dtls::is_closing()` and `Dtls::is_closed()` shutdown predicates #155 diff --git a/src/dtls12/incoming.rs b/src/dtls12/incoming.rs index ca21b6c0..1083af4d 100644 --- a/src/dtls12/incoming.rs +++ b/src/dtls12/incoming.rs @@ -90,17 +90,13 @@ impl Records { // This is the ONLY copy: packet -> record buffer let record_slice = &packet[..record_end]; - match Record::parse(record_slice, decrypt, cs) { - Ok(record) => { - if let Some(record) = record { - if parsed_records.try_push(record).is_err() { - return Err(InternalError::too_many_records()); - } - } else { - trace!("Discarding replayed rec"); - } + let record = Record::parse(record_slice, decrypt, cs)?; + if let Some(record) = record { + if parsed_records.try_push(record).is_err() { + return Err(InternalError::too_many_records()); } - Err(e) => return Err(e), + } else { + trace!("Discarding replayed rec"); } packet = &packet[record_end..]; diff --git a/src/dtls12/message/certificate_request.rs b/src/dtls12/message/certificate_request.rs index 120fbdfd..6391d2c6 100644 --- a/src/dtls12/message/certificate_request.rs +++ b/src/dtls12/message/certificate_request.rs @@ -118,7 +118,7 @@ mod test { use super::*; use crate::buffer::Buf; - // Test message with supported values: + // Test message with recognized values: // - Certificate type: 0x40 (ECDSA_SIGN) // - Signature algorithms: SHA256/ECDSA (0x04, 0x03), SHA256/RSA (0x04, 0x01) const MESSAGE: &[u8] = &[ @@ -135,15 +135,34 @@ mod test { ]; #[test] - fn roundtrip() { + fn filters_unsupported_rsa_signature_algorithm() { // Parse the message with base_offset 0 let (rest, parsed) = CertificateRequest::parse(MESSAGE, 0).unwrap(); assert!(rest.is_empty()); + assert_eq!(parsed.supported_signature_algorithms.len(), 1); + assert_eq!( + parsed.supported_signature_algorithms[0], + SignatureAndHashAlgorithm::new( + super::super::HashAlgorithm::SHA256, + super::super::SignatureAlgorithm::ECDSA, + ) + ); - // Serialize and compare to MESSAGE + // Serialization must not re-advertise the filtered RSA algorithm. let mut serialized = Buf::new(); parsed.serialize(MESSAGE, &mut serialized); - assert_eq!(&*serialized, MESSAGE); + let expected = [ + 0x01, // Certificate types length (1 byte) + 0x40, // Certificate type: ECDSA_SIGN + 0x00, 0x02, // Signature algorithms length (2 bytes = 1 algorithm) + 0x04, 0x03, // SHA256/ECDSA + 0x00, 0x0C, // Certificate authorities length + 0x00, 0x04, // Distinguished name 1 length + 0x01, 0x02, 0x03, 0x04, // Distinguished name 1 data + 0x00, 0x04, // Distinguished name 2 length + 0x05, 0x06, 0x07, 0x08, // Distinguished name 2 data + ]; + assert_eq!(&*serialized, expected); } #[test] diff --git a/src/dtls12/message/extensions/signature_algorithms.rs b/src/dtls12/message/extensions/signature_algorithms.rs index 7fe2d67c..258bdd52 100644 --- a/src/dtls12/message/extensions/signature_algorithms.rs +++ b/src/dtls12/message/extensions/signature_algorithms.rs @@ -101,7 +101,33 @@ mod tests { let (_, parsed) = SignatureAlgorithmsExtension::parse(&serialized).unwrap(); - assert_eq!(parsed.supported_signature_algorithms, algorithms); + assert_eq!(parsed.supported_signature_algorithms.len(), 1); + assert_eq!( + parsed.supported_signature_algorithms[0], + SignatureAndHashAlgorithm::new(HashAlgorithm::SHA256, SignatureAlgorithm::ECDSA,) + ); + } + + #[test] + fn default_does_not_advertise_rsa() { + let extension = SignatureAlgorithmsExtension::default(); + + assert!( + extension + .supported_signature_algorithms + .iter() + .all(|algorithm| algorithm.signature != SignatureAlgorithm::RSA) + ); + } + + #[test] + fn capacity_matches_supported() { + let extension = SignatureAlgorithmsExtension::default(); + + assert_eq!( + extension.supported_signature_algorithms.capacity(), + SignatureAndHashAlgorithm::supported().len() + ); } #[test] diff --git a/src/dtls12/message/mod.rs b/src/dtls12/message/mod.rs index d4c3a513..1d254aa3 100644 --- a/src/dtls12/message/mod.rs +++ b/src/dtls12/message/mod.rs @@ -335,19 +335,24 @@ impl SignatureAndHashAlgorithm { Ok((input, SignatureAndHashAlgorithm::from_u16(value))) } - /// All recognized signature+hash combinations (same as `supported()`). + /// All recognized signature+hash combinations. #[allow(dead_code)] pub const fn all() -> &'static [SignatureAndHashAlgorithm; 4] { - Self::supported() + const ALL: &[SignatureAndHashAlgorithm; 4] = &[ + SignatureAndHashAlgorithm::new(HashAlgorithm::SHA256, SignatureAlgorithm::ECDSA), + SignatureAndHashAlgorithm::new(HashAlgorithm::SHA384, SignatureAlgorithm::ECDSA), + SignatureAndHashAlgorithm::new(HashAlgorithm::SHA256, SignatureAlgorithm::RSA), + SignatureAndHashAlgorithm::new(HashAlgorithm::SHA384, SignatureAlgorithm::RSA), + ]; + + ALL } /// Supported signature+hash combinations. - pub const fn supported() -> &'static [SignatureAndHashAlgorithm; 4] { - const SUPPORTED: &[SignatureAndHashAlgorithm; 4] = &[ + pub const fn supported() -> &'static [SignatureAndHashAlgorithm; 2] { + const SUPPORTED: &[SignatureAndHashAlgorithm; 2] = &[ SignatureAndHashAlgorithm::new(HashAlgorithm::SHA256, SignatureAlgorithm::ECDSA), SignatureAndHashAlgorithm::new(HashAlgorithm::SHA384, SignatureAlgorithm::ECDSA), - SignatureAndHashAlgorithm::new(HashAlgorithm::SHA256, SignatureAlgorithm::RSA), - SignatureAndHashAlgorithm::new(HashAlgorithm::SHA384, SignatureAlgorithm::RSA), ]; SUPPORTED @@ -363,6 +368,16 @@ impl SignatureAndHashAlgorithm { mod tests { use super::*; + #[test] + fn rsa_signature_algorithms_are_recognized_but_not_supported() { + let rsa_sha256 = + SignatureAndHashAlgorithm::new(HashAlgorithm::SHA256, SignatureAlgorithm::RSA); + + assert!(SignatureAndHashAlgorithm::all().contains(&rsa_sha256)); + assert!(!rsa_sha256.is_supported()); + assert_eq!(SignatureAndHashAlgorithm::supported().len(), 2); + } + #[test] fn dtls12_cipher_suite_newtype_shape() { assert_eq!(std::mem::size_of::(), 2); diff --git a/src/dtls12/server.rs b/src/dtls12/server.rs index fce3d508..1e0a765a 100644 --- a/src/dtls12/server.rs +++ b/src/dtls12/server.rs @@ -1395,7 +1395,7 @@ fn select_ske_signature_algorithm( fn select_certificate_request_sig_algs( client_algs: Option<&SignatureAndHashAlgorithmVec>, ) -> SignatureAndHashAlgorithmVec { - // Our supported set (RSA/ECDSA with SHA256/384) + // Only select algorithms supported by dimpl's signature verifiers. let ours = SignatureAndHashAlgorithm::supported(); // Build intersection preserving client preference order @@ -1467,4 +1467,36 @@ mod tests { assert_eq!(selected, None); } + + #[test] + fn certificate_request_does_not_select_rsa_signatures() { + let mut client = SignatureAndHashAlgorithmVec::new(); + client.push(SignatureAndHashAlgorithm::new( + HashAlgorithm::SHA256, + SignatureAlgorithm::ECDSA, + )); + client.push(SignatureAndHashAlgorithm::new( + HashAlgorithm::SHA256, + SignatureAlgorithm::RSA, + )); + + let selected = select_certificate_request_sig_algs(Some(&client)); + + assert_eq!(selected.len(), 1); + assert_eq!(selected[0].signature, SignatureAlgorithm::ECDSA); + assert_eq!(selected[0].hash, HashAlgorithm::SHA256); + } + + #[test] + fn certificate_request_rejects_rsa_only_signatures() { + let mut client = SignatureAndHashAlgorithmVec::new(); + client.push(SignatureAndHashAlgorithm::new( + HashAlgorithm::SHA256, + SignatureAlgorithm::RSA, + )); + + let selected = select_certificate_request_sig_algs(Some(&client)); + + assert!(selected.is_empty()); + } } diff --git a/src/dtls13/incoming.rs b/src/dtls13/incoming.rs index f617a416..6dfeb6f2 100644 --- a/src/dtls13/incoming.rs +++ b/src/dtls13/incoming.rs @@ -129,17 +129,13 @@ impl Records { // This is the ONLY copy: packet -> record buffer let record_slice = &packet[..record_end]; - match Record::parse(record_slice, decrypt, cs) { - Ok(record) => { - if let Some(record) = record { - if parsed_records.try_push(record).is_err() { - return Err(InternalError::too_many_records()); - } - } else { - trace!("Discarding replayed rec"); - } + let record = Record::parse(record_slice, decrypt, cs)?; + if let Some(record) = record { + if parsed_records.try_push(record).is_err() { + return Err(InternalError::too_many_records()); } - Err(e) => return Err(e), + } else { + trace!("Discarding replayed rec"); } packet = &packet[record_end..];