bindings: do not enable OCSP when calling trust_location()

bindings/rust/s2n-tls/src/config.rs

@@ -152,6 +152,7 @@ impl Drop for Config {
 pub struct Builder {
     config: Config,
     load_system_certs: bool,
+    enable_ocsp: bool,
 }
 
 impl Builder {
@@ -181,6 +182,7 @@ impl Builder {
         Self {
             config: Config(config),
             load_system_certs: true,
+            enable_ocsp: false,
         }
     }
 
@@ -288,6 +290,10 @@ impl Builder {
         Ok(self)
     }
 
+    /// Adds to the trust store from a CA file or directory containing trusted certificates.
+    ///
+    /// NOTE: This function is equivalent to `s2n_config_set_verification_ca_location` except it does
+    /// not automatically enable the client to request OCSP stapling from the server.
     pub fn trust_location(
         &mut self,
         file: Option<&Path>,
@@ -353,6 +359,7 @@ impl Builder {
             s2n_config_set_status_request_type(self.as_mut_ptr(), s2n_status_request_type::OCSP)
                 .into_result()
         }?;
+        self.enable_ocsp = true;
         Ok(self)
     }
 
@@ -585,6 +592,15 @@ impl Builder {
             }
         }
 
+        // If OCSP has not been explicitly requested, turn off OCSP. This is to prevent the `trust_location()` function
+        // from automatically enabling `OCSP` due to the legacy behavior of `s2n_config_set_verification_ca_location`
+        if !self.enable_ocsp {
+            unsafe {
+                s2n_config_set_status_request_type(self.as_mut_ptr(), s2n_status_request_type::NONE)
+                    .into_result()?
+            };
+        }
+
         Ok(self.config)
     }
 

bindings/rust/s2n-tls/src/testing/client_hello.rs

@@ -7,6 +7,7 @@ use crate::{
 };
 use alloc::sync::Arc;
 use core::{sync::atomic::Ordering, task::Poll};
+use s2n_tls_sys::{s2n_client_hello_has_extension, s2n_connection_get_client_hello};
 use std::{fmt, io, pin::Pin, sync::atomic::AtomicUsize};
 
 // The Future returned by MockClientHelloHandler.
@@ -132,6 +133,42 @@ impl Drop for FailingCHFuture {
         assert!(self.invoked.load(Ordering::SeqCst) >= 1);
     }
 }
+/// A client hello handler that asserts that the extension with the given
+/// IANA code is either present or not present in the client hello
+pub struct HasExtensionClientHelloHandler {
+    pub extension_iana: u16,
+    pub extension_expected: bool,
+}
+
+impl ClientHelloCallback for HasExtensionClientHelloHandler {
+    fn on_client_hello(
+        &self,
+        connection: &mut crate::connection::Connection,
+    ) -> Result<Option<Pin<Box<dyn ConnectionFuture>>>, error::Error> {
+        let mut exists = false;
+
+        unsafe {
+            let client_hello = s2n_connection_get_client_hello(connection.as_ptr());
+            s2n_client_hello_has_extension(client_hello, self.extension_iana, &mut exists as _);
+        }
+
+        if self.extension_expected {
+            assert!(
+                exists,
+                "Extension {} was not found in the client hello",
+                self.extension_iana
+            );
+        } else {
+            assert!(
+                !exists,
+                "Unexpected extension {} found in the client hello",
+                self.extension_iana
+            )
+        }
+
+        Ok(None)
+    }
+}
 
 #[derive(Debug)]
 pub struct CustomError;

bindings/rust/s2n-tls/src/testing/s2n_tls.rs

@@ -208,7 +208,7 @@ mod tests {
     };
     use alloc::sync::Arc;
     use core::sync::atomic::Ordering;
-    use futures_test::task::new_count_waker;
+    use futures_test::task::{new_count_waker, noop_waker};
     use std::{fs, path::Path, pin::Pin, sync::atomic::AtomicUsize};
 
     #[test]
@@ -536,6 +536,62 @@ mod tests {
         Ok(())
     }
 
+    /// `trust_location()` calls `s2n_config_set_verification_ca_location()`, which has the legacy behavior
+    /// of enabling OCSP on clients. Since we do not want to replicate that behavior in the Rust bindings,
+    /// this test verifies that `trust_location()` does not turn on OCSP. It also verifies that turning
+    /// on OCSP explicitly still works when `trust_location()` is called.
+    #[test]
+    fn trust_location_does_not_change_ocsp_status() -> Result<(), Error> {
+        let pem_dir = Path::new(concat!(env!("CARGO_MANIFEST_DIR"), "/../../../tests/pems"));
+        let mut cert = pem_dir.to_path_buf();
+        cert.push("rsa_4096_sha512_client_cert.pem");
+        let mut key = pem_dir.to_path_buf();
+        key.push("rsa_4096_sha512_client_key.pem");
+
+        const OCSP_IANA_EXTENSION_ID: u16 = 5;
+
+        for enable_ocsp in [true, false] {
+            let config = {
+                let mut config = crate::config::Builder::new();
+
+                if enable_ocsp {
+                    config.enable_ocsp()?;
+                }
+
+                config.set_security_policy(&security::DEFAULT_TLS13)?;
+                config.set_verify_host_callback(InsecureAcceptAllCertificatesHandler {})?;
+                config.set_client_hello_callback(HasExtensionClientHelloHandler {
+                    // This client hello handler will check for the OCSP extension
+                    extension_iana: OCSP_IANA_EXTENSION_ID,
+                    extension_expected: enable_ocsp,
+                })?;
+                config.load_pem(&fs::read(&cert)?, &fs::read(&key)?)?;
+                config.trust_location(Some(&cert), None)?;
+                config.build()?
+            };
+
+            let server = {
+                // create and configure a server connection
+                let mut server = crate::connection::Connection::new_server();
+                server.set_config(config.clone())?;
+                server.set_waker(Some(&noop_waker()))?;
+                Harness::new(server)
+            };
+
+            let client = {
+                // create a client connection
+                let mut client = crate::connection::Connection::new_client();
+                client.set_config(config)?;
+                Harness::new(client)
+            };
+
+            let pair = Pair::new(server, client);
+
+            poll_tls_pair(pair);
+        }
+        Ok(())
+    }
+
     #[test]
     fn connection_level_verify_host_callback() -> Result<(), Error> {
         let reject_config = {

tls/s2n_connection.c

@@ -352,6 +352,8 @@ int s2n_connection_set_config(struct s2n_connection *conn, struct s2n_config *co
      * However, the s2n_config_set_verification_ca_location behavior predates client authentication
      * support for OCSP stapling, so could only affect whether clients requested OCSP stapling. We
      * therefore only have to maintain the legacy behavior for clients, not servers.
+     * 
+     * Note: The Rust bindings do not maintain the legacy behavior.
      */
     conn->request_ocsp_status = config->ocsp_status_requested_by_user;
     if (config->ocsp_status_requested_by_s2n && conn->mode == S2N_CLIENT) {