feat(bindings): enable application owned certs

bindings/rust/s2n-tls/src/callbacks/pkey.rs

@@ -124,7 +124,6 @@ pub trait PrivateKeyCallback: 'static + Send + Sync {
 mod tests {
     use super::*;
     use crate::{
-        cert_chain::CertificateChain,
         config, connection, error, security,
         testing::{self, *},
     };

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

@@ -197,9 +197,10 @@ impl CertificateChain<'_> {
         self.len() == 0
     }
 
-    pub(crate) fn as_mut_ptr(&mut self) -> *mut s2n_cert_chain_and_key {
-        // this private method is only safe to use if the CertificateChain is
-        // not shared across multiple threads
+    /// SAFETY: Only one instance of `CertificateChain` may exist when this method
+    /// is called. s2n_cert_chain_and_key is not thread-safe, so it is not safe
+    /// to mutate the certificate chain if references are held across multiple  threads.
+    pub(crate) unsafe fn as_mut_ptr(&mut self) -> *mut s2n_cert_chain_and_key {
         debug_assert_eq!(Arc::strong_count(&self.ptr), 1);
         self.ptr.cert.as_ptr()
     }
@@ -385,13 +386,10 @@ mod tests {
 
     #[test]
     fn too_many_certs_in_default() -> Result<(), crate::error::Error> {
-        // 3 certs in the maximum allowed, 4 should error.
-        let certs = vec![
-            SniTestCerts::AlligatorRsa.get().into_certificate_chain(),
-            SniTestCerts::AlligatorRsa.get().into_certificate_chain(),
-            SniTestCerts::AlligatorRsa.get().into_certificate_chain(),
-            SniTestCerts::AlligatorRsa.get().into_certificate_chain(),
-        ];
+        // 5 certs in the maximum allowed, 6 should error.
+        const FAILING_NUMBER: usize = 6;
+        let certs = vec![SniTestCerts::AlligatorRsa.get().into_certificate_chain(); FAILING_NUMBER];
+        assert_eq!(Arc::strong_count(&certs[0].ptr), FAILING_NUMBER);
 
         let mut config = config::Builder::new();
         let err = config.set_default_chains(certs.clone()).err().unwrap();
@@ -400,7 +398,7 @@ mod tests {
 
         // The config should not hold a reference when the error was detected
         // in the bindings
-        assert_eq!(Arc::strong_count(&certs[0].ptr), 1);
+        assert_eq!(Arc::strong_count(&certs[0].ptr), FAILING_NUMBER);
 
         Ok(())
     }

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

@@ -300,7 +300,12 @@ impl Builder {
 
     /// Corresponds to [s2n_config_add_cert_chain_and_key_to_store].
     pub fn load_chain(&mut self, chain: CertificateChain<'static>) -> Result<&mut Self, Error> {
-        let add_result = unsafe {
+        // Out of an abudance of caution, we hold a reference to the CertificateChain
+        // regardless of whether add_to_store fails or succeeds. We have limited
+        // visibility into the failure modes, so this behavior ensures that _if_
+        // the C library held the reference despite the failure, it would continue
+        // to be valid memory.
+        let result = unsafe {
             s2n_config_add_cert_chain_and_key_to_store(
                 self.as_mut_ptr(),
                 // SAFETY: audit of add_to_store shows that the certificate chain
@@ -309,15 +314,8 @@ impl Builder {
             )
             .into_result()
         };
-
-        // Out of an abudance of caution, we hold a reference to the CertificateChain
-        // regardless of whether add_to_store fails or succeeds. We have limited
-        // visibility into the failure modes, so this behavior ensures that _if_
-        // the C library held the reference despite the failure, it would continue
-        // to be valid memory.
         self.context_mut().application_owned_certs.push(chain);
-
-        add_result?;
+        result?;
 
         Ok(self)
     }
@@ -327,19 +325,18 @@ impl Builder {
         &mut self,
         chains: T,
     ) -> Result<&mut Self, Error> {
-        // Must match S2N_CERT_TYPE_COUNT in s2n_certificate.h
-        const CERT_TYPE_COUNT: usize = 3;
+        // Must be greater than or equal to S2N_CERT_TYPE_COUNT in s2n_certificate.h,
+        // and small enough that stack allocating an array of this length is safe.
+        const CHAINS_MAX_COUNT: usize = 5;
 
-        let mut pointer_array = [std::ptr::null_mut(); CERT_TYPE_COUNT];
-        let mut pointer_array_length = 0;
+        let mut chain_arrays: [Option<CertificateChain<'static>>; CHAINS_MAX_COUNT] =
+            [None, None, None, None, None];
+        let mut pointer_array = [std::ptr::null_mut(); CHAINS_MAX_COUNT];
+        let mut cert_chain_count = 0;
 
         for chain in chains.into_iter() {
-            if pointer_array_length >= CERT_TYPE_COUNT {
+            if cert_chain_count >= CHAINS_MAX_COUNT {
                 // drop the reference counts that we previously had taken
-                for _ in 0..CERT_TYPE_COUNT {
-                    self.context_mut().application_owned_certs.pop();
-                }
-
                 return Err(Error::bindings(
                     ErrorType::UsageError,
                     "InvalidInput",
@@ -350,17 +347,23 @@ impl Builder {
 
             // SAFETY: manual inspection of set_defaults shows that certificates
             // are not mutated. https://github.com/aws/s2n-tls/issues/4140
-            pointer_array[pointer_array_length] = chain.as_ptr() as *mut _;
-            pointer_array_length += 1;
+            pointer_array[cert_chain_count] = chain.as_ptr() as *mut _;
+            chain_arrays[cert_chain_count] = Some(chain);
 
-            self.context_mut().application_owned_certs.push(chain);
+            cert_chain_count += 1;
         }
 
+        let collected_chains = chain_arrays.into_iter().take(cert_chain_count).flatten();
+
+        self.context_mut()
+            .application_owned_certs
+            .extend(collected_chains);
+
         unsafe {
             s2n_config_set_cert_chain_and_key_defaults(
                 self.as_mut_ptr(),
                 pointer_array.as_mut_ptr(),
-                pointer_array_length as u32,
+                cert_chain_count as u32,
             )
             .into_result()
         }?;

tests/unit/s2n_certificate_test.c

@@ -1055,13 +1055,13 @@ int main(int argc, char **argv)
         EXPECT_EQUAL(len, 2);
     };
 
-    /* C and Rust constants should match */
+    /* cert type count is less than 5 */
     {
-        /* The rust bindings have a separate definition of cert type count, which
-         * must be kept in sync with the C library. If this test ever fails, 
-         * CERT_TYPE_COUNT in config.rs must be updated. 
+        /* The rust bindings have constant - CHAINS_MAX_COUNT - which be 
+         * greather than or equal to CERT_TYPE_COUNT. If this test fails, 
+         * CHAINS_MAX_COUNT in config.rs must be updated. 
          */
-        EXPECT_EQUAL(S2N_CERT_TYPE_COUNT, 3);
+        EXPECT_TRUE(S2N_CERT_TYPE_COUNT <= 5);
     }
 
     END_TEST();