Allow server to choose root cert for client auth based on SNI

rustls/examples/internal/bogo_shim.rs

@@ -170,14 +170,14 @@ struct DummyClientAuth {
 impl rustls::ClientCertVerifier for DummyClientAuth {
     fn offer_client_auth(&self) -> bool { true }
 
-    fn client_auth_mandatory(&self) -> bool { self.mandatory }
+    fn client_auth_mandatory(&self, _sni: Option<&webpki::DNSName>) -> Option<bool> { Some(self.mandatory) }
 
-    fn client_auth_root_subjects(&self) -> rustls::DistinguishedNames {
-        rustls::DistinguishedNames::new()
+    fn client_auth_root_subjects(&self, _sni: Option<&webpki::DNSName>) -> Option<rustls::DistinguishedNames> {
+        Some(rustls::DistinguishedNames::new())
     }
 
     fn verify_client_cert(&self,
-                          _certs: &[rustls::Certificate]) -> Result<rustls::ClientCertVerified, rustls::TLSError> {
+                          _certs: &[rustls::Certificate], _sni: Option<&webpki::DNSName>) -> Result<rustls::ClientCertVerified, rustls::TLSError> {
         Ok(rustls::ClientCertVerified::assertion())
     }
 }

rustls/src/server/hs.rs

@@ -463,14 +463,18 @@ impl ExpectClientHello {
         Ok(kx)
     }
 
-    fn emit_certificate_req(&mut self, sess: &mut ServerSessionImpl) -> bool {
+    fn emit_certificate_req(&mut self, sess: &mut ServerSessionImpl) -> Result<bool, TLSError> {
         let client_auth = &sess.config.verifier;
 
         if !client_auth.offer_client_auth() {
-            return false;
+            return Ok(false);
         }
 
-        let names = client_auth.client_auth_root_subjects();
+        let names = client_auth.client_auth_root_subjects(sess.get_sni()).ok_or_else(|| {
+                debug!("could not determine root subjects based on SNI");
+                sess.common.send_fatal_alert(AlertDescription::UnrecognisedName);
+                TLSError::AlertReceived(AlertDescription::UnrecognisedName)
+            })?;
 
         let cr = CertificateRequestPayload {
             certtypes: vec![ ClientCertificateType::RSASign,
@@ -491,7 +495,7 @@ impl ExpectClientHello {
         trace!("Sending CertificateRequest {:?}", m);
         self.handshake.transcript.add_message(&m);
         sess.common.send_msg(m, false);
-        true
+        Ok(true)
     }
 
     fn emit_server_hello_done(&mut self, sess: &mut ServerSessionImpl) {
@@ -799,7 +803,7 @@ impl State for ExpectClientHello {
         self.emit_certificate(sess, &mut certkey);
         self.emit_cert_status(sess, &mut certkey);
         let kx = self.emit_server_kx(sess, sigschemes, group, &mut certkey)?;
-        let doing_client_auth = self.emit_certificate_req(sess);
+        let doing_client_auth = self.emit_certificate_req(sess)?;
         self.emit_server_hello_done(sess);
 
         if doing_client_auth {

rustls/src/server/tls12.rs

@@ -48,16 +48,26 @@ impl hs::State for ExpectCertificate {
         let cert_chain = extract_handshake!(m, HandshakePayload::Certificate).unwrap();
         self.handshake.transcript.add_message(&m);
 
-        if cert_chain.is_empty() &&
-           !sess.config.verifier.client_auth_mandatory() {
-            debug!("client auth requested but no certificate supplied");
-            self.handshake.transcript.abandon_client_auth();
-            return Ok(self.into_expect_tls12_client_kx(None));
+        // If we can't determine if the auth is mandatory, abort
+        let mandatory = sess.config.verifier.client_auth_mandatory(sess.get_sni()).ok_or_else(|| {
+                debug!("could not determine if client auth is mandatory based on SNI");
+                sess.common.send_fatal_alert(AlertDescription::UnrecognisedName);
+                TLSError::AlertReceived(AlertDescription::UnrecognisedName)
+            })?;
+
+        if cert_chain.is_empty() {
+            if !mandatory {
+                debug!("client auth requested but no certificate supplied");
+                self.handshake.transcript.abandon_client_auth();
+                return Ok(self.into_expect_tls12_client_kx(None));
+            }
+            sess.common.send_fatal_alert(AlertDescription::CertificateRequired);
+            return Err(TLSError::NoCertificatesPresented);
         }
 
         trace!("certs {:?}", cert_chain);
 
-        sess.config.verifier.verify_client_cert(cert_chain)
+        sess.config.verifier.verify_client_cert(cert_chain, sess.get_sni())
             .or_else(|err| {
                      hs::incompatible(sess, "certificate invalid");
                      Err(err)

rustls/src/server/tls13.rs

@@ -277,9 +277,9 @@ impl CompleteClientHelloHandling {
         Ok(())
     }
 
-    fn emit_certificate_req_tls13(&mut self, sess: &mut ServerSessionImpl) -> bool {
+    fn emit_certificate_req_tls13(&mut self, sess: &mut ServerSessionImpl) -> Result<bool, TLSError> {
         if !sess.config.verifier.offer_client_auth() {
-            return false;
+            return Ok(false);
         }
 
         let mut cr = CertificateRequestPayloadTLS13 {
@@ -290,7 +290,12 @@ impl CompleteClientHelloHandling {
         let schemes = verify::supported_verify_schemes();
         cr.extensions.push(CertReqExtension::SignatureAlgorithms(schemes.to_vec()));
 
-        let names = sess.config.verifier.client_auth_root_subjects();
+        let names = sess.config.verifier.client_auth_root_subjects(sess.get_sni()).ok_or_else(|| {
+                debug!("could not determine root subjects based on SNI");
+                sess.common.send_fatal_alert(AlertDescription::UnrecognisedName);
+                TLSError::AlertReceived(AlertDescription::UnrecognisedName)
+            })?;
+
         if !names.is_empty() {
             cr.extensions.push(CertReqExtension::AuthorityNames(names));
         }
@@ -307,7 +312,7 @@ impl CompleteClientHelloHandling {
         trace!("Sending CertificateRequest {:?}", m);
         self.handshake.transcript.add_message(&m);
         sess.common.send_msg(m, true);
-        true
+        Ok(true)
     }
 
     fn emit_certificate_tls13(&mut self,
@@ -586,7 +591,7 @@ impl CompleteClientHelloHandling {
         self.emit_encrypted_extensions(sess, &mut server_key, client_hello, resumedata.as_ref())?;
 
         let doing_client_auth = if full_handshake {
-            let client_auth = self.emit_certificate_req_tls13(sess);
+            let client_auth = self.emit_certificate_req_tls13(sess)?;
             self.emit_certificate_tls13(sess, &mut server_key);
             self.emit_certificate_verify_tls13(sess, &mut server_key, &sigschemes_ext)?;
             client_auth
@@ -649,8 +654,14 @@ impl hs::State for ExpectCertificate {
 
         let cert_chain = certp.convert();
 
+        let mandatory = sess.config.verifier.client_auth_mandatory(sess.get_sni()).ok_or_else(|| {
+                debug!("could not determine if client auth is mandatory based on SNI");
+                sess.common.send_fatal_alert(AlertDescription::UnrecognisedName);
+                TLSError::AlertReceived(AlertDescription::UnrecognisedName)
+            })?;
+
         if cert_chain.is_empty() {
-            if !sess.config.verifier.client_auth_mandatory() {
+            if !mandatory {
                 debug!("client auth requested but no certificate supplied");
                 self.handshake.transcript.abandon_client_auth();
                 return Ok(self.into_expect_finished());
@@ -660,7 +671,7 @@ impl hs::State for ExpectCertificate {
             return Err(TLSError::NoCertificatesPresented);
         }
 
-        sess.config.get_verifier().verify_client_cert(&cert_chain)
+        sess.config.get_verifier().verify_client_cert(&cert_chain, sess.get_sni())
             .or_else(|err| {
                      hs::incompatible(sess, "certificate invalid");
                      Err(err)

rustls/src/verify.rs

@@ -80,16 +80,18 @@ pub trait ClientCertVerifier : Send + Sync {
 
     /// Returns `true` to require a client certificate and `false` to make client
     /// authentication optional. Defaults to `self.offer_client_auth()`.
-    fn client_auth_mandatory(&self) -> bool { self.offer_client_auth() }
+    fn client_auth_mandatory(&self, _sni: Option<&webpki::DNSName>) -> Option<bool> {
+        Some(self.offer_client_auth())
+    }
 
     /// Returns the subject names of the client authentication trust anchors to
     /// share with the client when requesting client authentication.
-    fn client_auth_root_subjects(&self) -> DistinguishedNames;
+    fn client_auth_root_subjects(&self, sni: Option<&webpki::DNSName>) -> Option<DistinguishedNames>;
 
-    /// Verify a certificate chain `presented_certs` is rooted in `roots`.
+    /// Verify a certificate chain `presented_certs` is rooted in `roots` when the client indicates the `sni`.
     /// Does no further checking of the certificate.
     fn verify_client_cert(&self,
-                          presented_certs: &[Certificate]) -> Result<ClientCertVerified, TLSError>;
+                          presented_certs: &[Certificate], sni: Option<&webpki::DNSName>) -> Result<ClientCertVerified, TLSError>;
 }
 
 /// Default `ServerCertVerifier`, see the trait impl for more information.
@@ -185,13 +187,13 @@ impl AllowAnyAuthenticatedClient {
 impl ClientCertVerifier for AllowAnyAuthenticatedClient {
     fn offer_client_auth(&self) -> bool { true }
 
-    fn client_auth_mandatory(&self) -> bool { true }
+    fn client_auth_mandatory(&self, _sni: Option<&webpki::DNSName>) -> Option<bool> { Some(true) }
 
-    fn client_auth_root_subjects(&self) -> DistinguishedNames {
-        self.roots.get_subjects()
+    fn client_auth_root_subjects(&self, _sni: Option<&webpki::DNSName>) -> Option<DistinguishedNames> {
+        Some(self.roots.get_subjects())
     }
 
-    fn verify_client_cert(&self, presented_certs: &[Certificate])
+    fn verify_client_cert(&self, presented_certs: &[Certificate], _sni: Option<&webpki::DNSName>)
                           -> Result<ClientCertVerified, TLSError> {
         let (cert, chain, trustroots) = prepare(&self.roots, presented_certs)?;
         let now = try_now()?;
@@ -227,15 +229,15 @@ impl AllowAnyAnonymousOrAuthenticatedClient {
 impl ClientCertVerifier for AllowAnyAnonymousOrAuthenticatedClient {
     fn offer_client_auth(&self) -> bool { self.inner.offer_client_auth() }
 
-    fn client_auth_mandatory(&self) -> bool { false }
+    fn client_auth_mandatory(&self, _sni: Option<&webpki::DNSName>) -> Option<bool> { Some(false) }
 
-    fn client_auth_root_subjects(&self) -> DistinguishedNames {
-        self.inner.client_auth_root_subjects()
+    fn client_auth_root_subjects(&self, sni: Option<&webpki::DNSName>) -> Option<DistinguishedNames> {
+        self.inner.client_auth_root_subjects(sni)
     }
 
-    fn verify_client_cert(&self, presented_certs: &[Certificate])
+    fn verify_client_cert(&self, presented_certs: &[Certificate], sni: Option<&webpki::DNSName>)
             -> Result<ClientCertVerified, TLSError> {
-        self.inner.verify_client_cert(presented_certs)
+        self.inner.verify_client_cert(presented_certs, sni)
     }
 }
 
@@ -250,11 +252,11 @@ impl NoClientAuth {
 impl ClientCertVerifier for NoClientAuth {
     fn offer_client_auth(&self) -> bool { false }
 
-    fn client_auth_root_subjects(&self) -> DistinguishedNames {
+    fn client_auth_root_subjects(&self, _sni: Option<&webpki::DNSName>) -> Option<DistinguishedNames> {
         unimplemented!();
     }
 
-    fn verify_client_cert(&self, _presented_certs: &[Certificate])
+    fn verify_client_cert(&self,_presented_certs: &[Certificate], _sni: Option<&webpki::DNSName>)
                           -> Result<ClientCertVerified, TLSError> {
         unimplemented!();
     }

rustls/tests/api.rs

@@ -24,6 +24,10 @@ use rustls::ClientHello;
 use rustls::quic::{self, QuicExt, ClientQuicExt, ServerQuicExt};
 #[cfg(feature = "quic")]
 use ring::hkdf;
+use rustls::internal::msgs::enums::AlertDescription;
+
+#[cfg(feature = "dangerous_configuration")]
+use rustls::ClientCertVerified;
 
 use webpki;
 
@@ -564,6 +568,183 @@ fn client_auth_works() {
     }
 }
 
+#[cfg(feature = "dangerous_configuration")]
+mod test_verifier {
+    use super::*;
+    use crate::common::MockClientVerifier;
+
+    // Client is authorized!
+    fn ver_ok() -> Result<ClientCertVerified, TLSError> {
+        Ok(rustls::ClientCertVerified::assertion())
+    }
+
+    // Use when we shouldn't even attempt verification
+    fn ver_unreachable() -> Result<ClientCertVerified, TLSError> {
+        unreachable!()
+    }
+
+    // Verifier that returns an error that we can expect
+    fn ver_err() -> Result<ClientCertVerified, TLSError> {
+        Err(TLSError::General("test err".to_string()))
+    }
+
+    #[test]
+    // Happy path, we resolve to a root, it is verified OK, should be able to connect
+    fn client_verifier_works() {
+        for kt in ALL_KEY_TYPES.iter() {
+            let client_verifier = MockClientVerifier {
+                verified: ver_ok,
+                subjects: Some(get_client_root_store(*kt).get_subjects()),
+                mandatory: Some(true),
+            };
+
+            let mut server_config = ServerConfig::new(Arc::new(client_verifier));
+            server_config.set_single_cert(kt.get_chain(), kt.get_key()).unwrap();
+
+            let server_config = Arc::new(server_config);
+            let client_config = make_client_config_with_auth(*kt);
+
+            for client_config in AllClientVersions::new(client_config) {
+                let (mut client, mut server) = make_pair_for_arc_configs(&Arc::new(client_config.clone()),
+                                                                     &server_config);
+                let err = do_handshake_until_error(&mut client, &mut server);
+                assert_eq!(err, Ok(()));
+            }
+        }
+    }
+
+    // Common case, we do not find a root store to resolve to
+    #[test]
+    fn client_verifier_no_root() {
+        for kt in ALL_KEY_TYPES.iter() {
+            let client_verifier = MockClientVerifier {
+                verified: ver_ok,
+                subjects: None,
+                mandatory: Some(true),
+            };
+
+            let mut server_config = ServerConfig::new(Arc::new(client_verifier));
+            server_config.set_single_cert(kt.get_chain(), kt.get_key()).unwrap();
+
+            let server_config = Arc::new(server_config);
+            let client_config = make_client_config_with_auth(*kt);
+
+            for client_config in AllClientVersions::new(client_config) {
+                let mut server = ServerSession::new(&server_config);
+                let mut client = ClientSession::new(&Arc::new(client_config), dns_name("notlocalhost"));
+                let err = do_handshake_until_error(&mut client, &mut server);
+                assert_eq!(err, Err(TLSErrorFromPeer::Server(
+                            TLSError::AlertReceived(AlertDescription::UnrecognisedName))));
+            }
+        }
+    }
+
+    // If we cannot resolve a root, we cannot decide if auth is mandatory
+    #[test]
+    fn client_verifier_no_auth_no_root() {
+        for kt in ALL_KEY_TYPES.iter() {
+            let client_verifier = MockClientVerifier {
+                verified: ver_unreachable,
+                subjects: None,
+                mandatory: Some(true),
+            };
+
+            let mut server_config = ServerConfig::new(Arc::new(client_verifier));
+            server_config.set_single_cert(kt.get_chain(), kt.get_key()).unwrap();
+
+            let server_config = Arc::new(server_config);
+            let client_config = make_client_config(*kt);
+
+            for client_config in AllClientVersions::new(client_config) {
+                let mut server = ServerSession::new(&server_config);
+                let mut client = ClientSession::new(&Arc::new(client_config), dns_name("notlocalhost"));
+                let err = do_handshake_until_error(&mut client, &mut server);
+                assert_eq!(err, Err(TLSErrorFromPeer::Server(
+                            TLSError::AlertReceived(AlertDescription::UnrecognisedName))));
+            }
+        }
+    }
+
+        // If we do have a root, we must do auth
+    #[test]
+    fn client_verifier_no_auth_yes_root() {
+        for kt in ALL_KEY_TYPES.iter() {
+            let client_verifier = MockClientVerifier {
+                verified: ver_unreachable,
+                subjects: Some(get_client_root_store(*kt).get_subjects()),
+                mandatory: Some(true),
+            };
+
+            let mut server_config = ServerConfig::new(Arc::new(client_verifier));
+            server_config.set_single_cert(kt.get_chain(), kt.get_key()).unwrap();
+
+            let server_config = Arc::new(server_config);
+            let client_config = make_client_config(*kt);
+
+            for client_config in AllClientVersions::new(client_config) {
+                println!("Failing: {:?}", client_config.versions);
+                let mut server = ServerSession::new(&server_config);
+                let mut client = ClientSession::new(&Arc::new(client_config), dns_name("localhost"));
+                let err = do_handshake_until_error(&mut client, &mut server);
+                assert_eq!(err, Err(TLSErrorFromPeer::Server(TLSError::NoCertificatesPresented)));
+            }
+        }
+    }
+
+    #[test]
+    // Triple checks we propagate the TLSError through
+    fn client_verifier_fails_properly() {
+        for kt in ALL_KEY_TYPES.iter() {
+            let client_verifier = MockClientVerifier {
+                verified: ver_err,
+                subjects: Some(get_client_root_store(*kt).get_subjects()),
+                mandatory: Some(true),
+            };
+
+            let mut server_config = ServerConfig::new(Arc::new(client_verifier));
+            server_config.set_single_cert(kt.get_chain(), kt.get_key()).unwrap();
+
+            let server_config = Arc::new(server_config);
+            let client_config = make_client_config_with_auth(*kt);
+
+            for client_config in AllClientVersions::new(client_config) {
+                let mut server = ServerSession::new(&server_config);
+                let mut client = ClientSession::new(&Arc::new(client_config), dns_name("localhost"));
+                let err = do_handshake_until_error(&mut client, &mut server);
+                assert_eq!(err, Err(TLSErrorFromPeer::Server(
+                            TLSError::General("test err".into()))));
+            }
+        }
+    }
+
+
+    #[test]
+    // If a verifier returns a None on Mandatory-ness, then we error out
+    fn client_verifier_must_determine_client_auth_requirement_to_continue() {
+        for kt in ALL_KEY_TYPES.iter() {
+            let client_verifier = MockClientVerifier {
+                verified: ver_ok,
+                subjects: Some(get_client_root_store(*kt).get_subjects()),
+                mandatory: None,
+            };
+
+            let mut server_config = ServerConfig::new(Arc::new(client_verifier));
+            server_config.set_single_cert(kt.get_chain(), kt.get_key()).unwrap();
+
+            let server_config = Arc::new(server_config);
+            let client_config = make_client_config_with_auth(*kt);
+
+            for client_config in AllClientVersions::new(client_config) {
+                let mut server = ServerSession::new(&server_config);
+                let mut client = ClientSession::new(&Arc::new(client_config), dns_name("localhost"));
+                let err = do_handshake_until_error(&mut client, &mut server);
+                assert_eq!(err, Err(TLSErrorFromPeer::Server(
+                            TLSError::AlertReceived(AlertDescription::UnrecognisedName))));
+            }
+        }
+    }
+} // mod test_verifier
+
 #[test]
 fn client_error_is_sticky() {
     let (mut client, _) = make_pair(KeyType::RSA);

rustls/tests/common/mod.rs

@@ -17,6 +17,9 @@ use rustls::internal::pemfile;
 use rustls::{RootCertStore, NoClientAuth, AllowAnyAuthenticatedClient};
 use rustls::internal::msgs::{codec::Codec, codec::Reader, message::Message};
 
+#[cfg(feature = "dangerous_configuration")]
+use rustls::{ClientCertVerified, ClientCertVerifier, DistinguishedNames};
+
 use webpki;
 
 macro_rules! embed_files {
@@ -199,12 +202,17 @@ pub fn make_server_config(kt: KeyType) -> ServerConfig {
     cfg
 }
 
-pub fn make_server_config_with_mandatory_client_auth(kt: KeyType) -> ServerConfig {
+pub fn get_client_root_store(kt: KeyType) -> RootCertStore {
     let roots = kt.get_chain();
     let mut client_auth_roots = RootCertStore::empty();
     for root in roots {
         client_auth_roots.add(&root).unwrap();
     }
+    client_auth_roots
+}
+
+pub fn make_server_config_with_mandatory_client_auth(kt: KeyType) -> ServerConfig {
+    let client_auth_roots = get_client_root_store(kt);
 
     let client_auth = AllowAnyAuthenticatedClient::new(client_auth_roots);
     let mut cfg = ServerConfig::new(NoClientAuth::new());
@@ -290,6 +298,33 @@ impl Iterator for AllClientVersions {
     }
 }
 
+#[cfg(feature = "dangerous_configuration")]
+pub struct MockClientVerifier {
+    pub verified: fn() -> Result<ClientCertVerified, TLSError>,
+    pub subjects: Option<DistinguishedNames>,
+    pub mandatory: Option<bool>,
+}
+
+#[cfg(feature = "dangerous_configuration")]
+impl ClientCertVerifier for MockClientVerifier {
+    fn client_auth_mandatory(&self, sni: Option<&webpki::DNSName>) -> Option<bool> {
+        // This is just an added 'test' to make sure we plumb through the SNI,
+        // although its valid for it to be None, its just our tests should (as of now) always provide it
+        assert!(sni.is_some());
+        self.mandatory
+    }
+
+    fn client_auth_root_subjects(&self, sni: Option<&webpki::DNSName>) -> Option<DistinguishedNames> {
+        assert!(sni.is_some());
+        self.subjects.as_ref().cloned()
+    }
+
+    fn verify_client_cert(&self, _presented_certs: &[Certificate], sni: Option<&webpki::DNSName>) -> Result<ClientCertVerified, TLSError> {
+        assert!(sni.is_some());
+        (self.verified)()
+    }
+}
+
 #[derive(PartialEq, Debug)]
 pub enum TLSErrorFromPeer { Client(TLSError), Server(TLSError) }