Stop advertising unsupported RSA signatures in DTLS 1.2

* Stop advertising unsupported RSA signatures

* Fix record parsing Clippy warnings

* Size signature algorithm vectors to supported set

---------

Co-authored-by: Martin Algesten <[email protected]>

Author: xnorpx

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

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..];

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]

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]

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::<Dtls12CipherSuite>(), 2);

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());
+    }
 }

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..];