Reject empty required client certificates

CHANGELOG.md

@@ -1,5 +1,7 @@
 # Unreleased
 
+  * Reject empty required client certificates in DTLS 1.2 and 1.3 #170
+
 # 0.7.5
 
   * Reject internally reordered DTLS 1.2 datagrams #169

src/dtls12/server.rs

@@ -5,7 +5,7 @@
 //    - Client resends ClientHello with cookie
 // 3. Server sends ServerHello, Certificate, ServerKeyExchange,
 //    CertificateRequest (required), ServerHelloDone
-// 4. Client sends Certificate (optional), ClientKeyExchange,
+// 4. Client sends Certificate (required when requested), ClientKeyExchange,
 //    CertificateVerify (if client cert), ChangeCipherSpec, Finished
 // 5. Server verifies Finished, then sends ChangeCipherSpec, Finished
 // 6. Handshake complete, application data can flow
@@ -724,7 +724,19 @@ impl State {
         drop(maybe);
 
         if cert_ranges.is_empty() {
-            // Client didn't provide a certificate (allowed), skip
+            // The handshake has no key material yet, so send a fatal alert in epoch 0.
+            server.engine.abort();
+            server
+                .engine
+                .create_record(ContentType::Alert, 0, false, |body| {
+                    body.push(2); // fatal
+                    body.push(40); // handshake_failure
+                })?;
+            server.state = State::Closed;
+            return Err(Error::CertificateError(
+                crate::CertificateError::NoClientCertificateReceived,
+            )
+            .into());
         } else {
             // Store and verify via callback
             debug!(

src/dtls13/server.rs

@@ -972,13 +972,20 @@ impl State {
         };
 
         if certificate.certificate_list.is_empty() {
-            debug!("Client sent empty certificate list");
-            drop(maybe);
-            server.engine.advance_peer_handshake_seq();
-            // Client's Certificate means our flight was received; stop retransmitting
-            server.engine.flight_stop_resend_timers();
-            // Empty cert list is acceptable - proceed without client auth verification
-            return Ok(Self::AwaitFinished);
+            server.engine.abort();
+            // Server Finished was already sent, so the client now reads epoch 3.
+            let epoch = server.engine.app_send_epoch();
+            server
+                .engine
+                .create_ciphertext_record(ContentType::Alert, epoch, false, |body| {
+                    body.push(2); // legacy level (ignored in DTLS 1.3)
+                    body.push(116); // certificate_required
+                })?;
+            server.state = State::Closed;
+            return Err(Error::CertificateError(
+                crate::CertificateError::NoClientCertificateReceived,
+            )
+            .into());
         }
 
         debug!(

src/error.rs

@@ -390,6 +390,8 @@ pub enum CryptoOperation {
 pub enum CertificateError {
     /// The peer did not send a server certificate.
     NoServerCertificateReceived,
+    /// The client sent an empty certificate list when a certificate was required.
+    NoClientCertificateReceived,
     /// Client certificate verification was requested with no client certificate.
     NoClientCertificateForVerification,
     /// Server certificate verification was requested with no server certificate.
@@ -1050,6 +1052,7 @@ impl fmt::Display for CertificateError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             Self::NoServerCertificateReceived => write!(f, "no server certificate received"),
+            Self::NoClientCertificateReceived => write!(f, "no client certificate received"),
             Self::NoClientCertificateForVerification => {
                 write!(f, "no client certificate for verification")
             }

tests/dtls12/handshake.rs

@@ -7,6 +7,81 @@ use dimpl::{Config, Dtls, SrtpProfile};
 
 use crate::common::*;
 
+#[test]
+#[cfg(feature = "rcgen")]
+fn dtls12_client_certificate_not_required() {
+    use dimpl::certificate::generate_self_signed_certificate;
+
+    let now = Instant::now();
+    let client_cert = generate_self_signed_certificate().expect("gen client cert");
+    let server_cert = generate_self_signed_certificate().expect("gen server cert");
+    let config = Arc::new(
+        Config::builder()
+            .require_client_certificate(false)
+            .build()
+            .expect("build config"),
+    );
+    let mut client = Dtls::new_12(Arc::clone(&config), client_cert, now);
+    client.set_active(true);
+    let mut server = Dtls::new_12(config, server_cert, now);
+    complete_dtls12_handshake(&mut client, &mut server, now);
+}
+
+#[test]
+#[cfg(feature = "rcgen")]
+fn dtls12_empty_client_certificate_is_rejected_when_required() {
+    use dimpl::certificate::generate_self_signed_certificate;
+    use dimpl::{CertificateError, Error};
+
+    let mut now = Instant::now();
+    let client_cert = generate_self_signed_certificate().expect("gen client cert");
+    let server_cert = generate_self_signed_certificate().expect("gen server cert");
+    let config = Arc::new(Config::builder().build().expect("build config"));
+    let mut client = Dtls::new_12(Arc::clone(&config), client_cert, now);
+    client.set_active(true);
+    let mut server = Dtls::new_12(config, server_cert, now);
+
+    for step in 0..60 {
+        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);
+        for packet in &client_out.packets {
+            if packet.len() >= 25 && packet[0] == 22 && packet[13] == CERTIFICATE {
+                // Replace the client's plaintext Certificate record with an
+                // empty certificate list, preserving record and handshake sequence numbers.
+                let mut empty = packet[..25].to_vec();
+                empty[11..13].copy_from_slice(&15u16.to_be_bytes());
+                empty[14..17].copy_from_slice(&[0, 0, 3]);
+                empty[22..25].copy_from_slice(&[0, 0, 3]);
+                empty.extend_from_slice(&[0, 0, 0]);
+                assert!(matches!(
+                    server.handle_packet(&empty),
+                    Err(Error::CertificateError(
+                        CertificateError::NoClientCertificateReceived
+                    ))
+                ));
+                let output = drain_outputs(&mut server);
+                assert_eq!(output.packets.len(), 1, "one fatal alert packet");
+                let alert = &output.packets[0];
+                assert_eq!(alert[0], 21, "DTLS Alert record");
+                assert_eq!(&alert[3..5], &[0, 0], "pre-key-exchange epoch");
+                assert_eq!(&alert[13..], &[2, 40], "fatal handshake_failure");
+                assert!(server.is_closed());
+                return;
+            }
+            server.handle_packet(packet).expect("server packet");
+        }
+        deliver_packets(&server_out.packets, &mut client);
+        if step % 5 == 4 {
+            now += Duration::from_secs(2);
+        } else {
+            now += Duration::from_millis(50);
+        }
+    }
+    panic!("client did not send Certificate");
+}
+
 #[test]
 #[cfg(feature = "rcgen")]
 fn dtls12_cookie_retry_proceeds_to_server_hello() {

tests/dtls13/handshake.rs

@@ -3,10 +3,84 @@
 use std::sync::Arc;
 use std::time::{Duration, Instant};
 
-use dimpl::{Dtls, SrtpProfile};
+use dimpl::{CertificateError, Config, Dtls, DtlsCertificate, Error, SrtpProfile};
 
 use crate::common::*;
 
+#[test]
+#[cfg(feature = "rcgen")]
+fn dtls13_empty_client_certificate_is_rejected_when_required() {
+    use dimpl::certificate::generate_self_signed_certificate;
+
+    let now = Instant::now();
+    let client_cert = generate_self_signed_certificate().expect("gen client cert");
+    let server_cert = generate_self_signed_certificate().expect("gen server cert");
+    let config = Arc::new(Config::builder().build().expect("build config"));
+    let mut client = Dtls::new_13(
+        Arc::clone(&config),
+        DtlsCertificate {
+            certificate: Vec::new(),
+            private_key: client_cert.private_key,
+        },
+        now,
+    );
+    client.set_active(true);
+    let mut server = Dtls::new_13(config, server_cert, now);
+
+    let mut now = now;
+    for _ in 0..40 {
+        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);
+        for packet in &client_out.packets {
+            match server.handle_packet(packet) {
+                Err(Error::CertificateError(CertificateError::NoClientCertificateReceived)) => {
+                    let output = drain_outputs(&mut server);
+                    assert_eq!(output.packets.len(), 1, "one fatal alert packet");
+                    let alert_result = client.handle_packet(&output.packets[0]);
+                    assert!(
+                        matches!(
+                            alert_result,
+                            Err(Error::SecurityError(dimpl::SecurityError::FatalAlert {
+                                description: 116
+                            }))
+                        ),
+                        "client result: {alert_result:?}"
+                    );
+                    assert!(server.is_closed());
+                    return;
+                }
+                Err(error) => panic!("unexpected server error: {error}"),
+                Ok(()) => {}
+            }
+        }
+        deliver_packets(&server_out.packets, &mut client);
+        now += Duration::from_millis(10);
+    }
+    panic!("server accepted an empty client Certificate");
+}
+
+#[test]
+#[cfg(feature = "rcgen")]
+fn dtls13_client_certificate_not_required() {
+    use dimpl::certificate::generate_self_signed_certificate;
+
+    let now = Instant::now();
+    let client_cert = generate_self_signed_certificate().expect("gen client cert");
+    let server_cert = generate_self_signed_certificate().expect("gen server cert");
+    let config = Arc::new(
+        Config::builder()
+            .require_client_certificate(false)
+            .build()
+            .expect("build config"),
+    );
+    let mut client = Dtls::new_13(Arc::clone(&config), client_cert, now);
+    client.set_active(true);
+    let mut server = Dtls::new_13(config, server_cert, now);
+    complete_dtls13_handshake(&mut client, &mut server, now);
+}
+
 #[test]
 #[cfg(feature = "rcgen")]
 fn dtls13_basic_handshake() {