diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 9c9ee516393..b870ca17aae 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -10,13 +10,27 @@ updates: directory: "/.github/workflows" schedule: interval: "daily" + groups: + all-gha-updates: + patterns: + - "*" - # Maintain dependencies for cargo + # permissive-MSRV, batch updates are acceptable - package-ecosystem: "cargo" directories: - - "/bindings/rust" - "/bindings/rust-examples" - "/tests/pcap" - "/tests/regression" schedule: interval: "daily" + groups: + all-cargo-updates: + patterns: + - "*" + + # restricted-MSRV, so don't do batch updates + - package-ecosystem: "cargo" + directories: + - "/bindings/rust" + schedule: + interval: "daily" diff --git a/.github/workflows/ci_linting.yml b/.github/workflows/ci_linting.yml index 181aabe581b..f4f63b6c9eb 100644 --- a/.github/workflows/ci_linting.yml +++ b/.github/workflows/ci_linting.yml @@ -22,7 +22,7 @@ jobs: - name: Cache id: cache - uses: actions/cache@v2.1.4 + uses: actions/cache@v4 continue-on-error: true with: path: ${{ env.CPPCHECK_INSTALL_DIR }} diff --git a/.github/workflows/ci_openbsd.yml b/.github/workflows/ci_openbsd.yml index 3bd2292a762..97bf1ebd932 100644 --- a/.github/workflows/ci_openbsd.yml +++ b/.github/workflows/ci_openbsd.yml @@ -15,7 +15,7 @@ jobs: - uses: actions/checkout@v4 - name: Build and test in OpenBSD id: test - uses: cross-platform-actions/action@v0.23.0 + uses: cross-platform-actions/action@v0.26.0 with: operating_system: openbsd architecture: x86-64 diff --git a/.github/workflows/ci_rust.yml b/.github/workflows/ci_rust.yml index 6c7f7d74fb5..c16b794e12d 100644 --- a/.github/workflows/ci_rust.yml +++ b/.github/workflows/ci_rust.yml @@ -11,7 +11,7 @@ on: env: # Pin the nightly toolchain to prevent breakage. # This should be occasionally updated. - RUST_NIGHTLY_TOOLCHAIN: nightly-2024-01-01 + RUST_NIGHTLY_TOOLCHAIN: nightly-2024-12-01 ROOT_PATH: bindings/rust EXAMPLE_WORKSPACE: bindings/rust-examples PCAP_TEST_PATH: tests/pcap @@ -138,7 +138,7 @@ jobs: - name: Cache OpenSSL 1.0.2 id: cache-openssl - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: ~/openssl-102/install key: ${{ runner.os }}-openssl-102 @@ -213,6 +213,48 @@ jobs: run: | cargo test --tests --all-features + # Run the rust unit tests under address sanitizer. + # + # Rust is generally memory safe, but our bindings contain a large amount of unsafe + # code. Additionally, "safe" code doesn't guarentee that the code is free of + # memory leaks. + asan-unit-tests: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Install Rust toolchain + id: toolchain + run: | + rustup toolchain install ${{env.RUST_NIGHTLY_TOOLCHAIN }} \ + --profile minimal \ + --component rust-src \ + --target x86_64-unknown-linux-gnu + rustup override set ${{ env.RUST_NIGHTLY_TOOLCHAIN }} + + - name: Generate + run: ./${{env.ROOT_PATH}}/generate.sh --skip-tests + + # asan expects a binary at /usr/bin/llvm-symbolizer but GHA runners include + # multiple versioned binaries, like /usr/bin/llvm-symbolizer-13. This step + # finds the latest symbolizer and use it as the "base" llvm-symbolizer binary. + # + # llvm-symbolizer is necessary to get nice stack traces from asan errors. + # Otherwise the stack trace just contains a hex address like "0x55bc6a28a9b6" + - name: set llvm symbolizer + run: | + sudo ln -s $(find /usr/bin/ -maxdepth 1 -name "llvm-symbolizer-*" | sort -V | tail -n 1) /usr/bin/llvm-symbolizer + + - name: Run Unit Tests under ASAN + env: + RUSTDOCFLAGS: -Zsanitizer=address + RUSTFLAGS: -Zsanitizer=address + run: | + cargo test \ + -Zbuild-std \ + --manifest-path ${{ env.ROOT_PATH}}/Cargo.toml \ + --target x86_64-unknown-linux-gnu + rustfmt: runs-on: ubuntu-latest steps: diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 240256c35de..babcab6dac7 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -35,17 +35,17 @@ jobs: uses: actions/checkout@v4 - name: Initialize CodeQL - uses: github/codeql-action/init@v2 + uses: github/codeql-action/init@v3 with: languages: ${{ matrix.language }} queries: +security-and-quality config-file: ./.github/codeql-config.yml - name: Autobuild - uses: github/codeql-action/autobuild@v2 + uses: github/codeql-action/autobuild@v3 if: ${{ matrix.language == 'c' || matrix.language == 'python' }} - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v2 + uses: github/codeql-action/analyze@v3 with: category: "/language:${{ matrix.language }}" diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index c17f4e41726..5a8b230f47b 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -28,7 +28,7 @@ jobs: path: | docs/doxygen/output - name: Deploy documentation to gh-pages - uses: peaceiris/actions-gh-pages@v3 + uses: peaceiris/actions-gh-pages@v4 if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} with: github_token: ${{ secrets.GITHUB_TOKEN }} diff --git a/bindings/rust/s2n-tls/src/callbacks/pkey.rs b/bindings/rust/s2n-tls/src/callbacks/pkey.rs index bbf44d6e8cf..b946617fd8f 100644 --- a/bindings/rust/s2n-tls/src/callbacks/pkey.rs +++ b/bindings/rust/s2n-tls/src/callbacks/pkey.rs @@ -128,7 +128,7 @@ mod tests { testing::{self, *}, }; use core::task::{Poll, Waker}; - use futures_test::task::new_count_waker; + use futures_test::task::{new_count_waker, noop_waker}; use openssl::{ec::EcKey, ecdsa::EcdsaSig}; type Error = Box; @@ -350,4 +350,52 @@ mod tests { assert_test_error(err, ERROR); Ok(()) } + + /// pkey offload should also work with public certs created from + /// [CertificateChain::from_public_pems]. + #[test] + fn app_owned_public_cert() -> Result<(), Error> { + struct TestPkeyCallback; + impl PrivateKeyCallback for TestPkeyCallback { + fn handle_operation( + &self, + conn: &mut connection::Connection, + op: PrivateKeyOperation, + ) -> Result>>, error::Error> { + ecdsa_sign(op, conn, KEY)?; + Ok(None) + } + } + + let public_chain = { + let mut chain = crate::cert_chain::Builder::new()?; + chain.load_public_pem(CERT)?; + chain.build()? + }; + + let server_config = { + let mut config = config::Builder::new(); + config + .set_security_policy(&security::DEFAULT_TLS13)? + .load_chain(public_chain)? + .set_private_key_callback(TestPkeyCallback)?; + config.build()? + }; + + let client_config = { + let mut config = config::Builder::new(); + config + .set_security_policy(&security::DEFAULT_TLS13)? + .set_verify_host_callback(InsecureAcceptAllCertificatesHandler {})? + .trust_pem(CERT)?; + config.build()? + }; + + let mut pair = TestPair::from_configs(&client_config, &server_config); + pair.server.set_waker(Some(&noop_waker()))?; + + assert!(pair.handshake().is_ok()); + + Ok(()) + } } diff --git a/bindings/rust/s2n-tls/src/cert_chain.rs b/bindings/rust/s2n-tls/src/cert_chain.rs index e95502c1fc9..189383cfb15 100644 --- a/bindings/rust/s2n-tls/src/cert_chain.rs +++ b/bindings/rust/s2n-tls/src/cert_chain.rs @@ -6,34 +6,160 @@ use s2n_tls_sys::*; use std::{ marker::PhantomData, ptr::{self, NonNull}, + sync::Arc, }; +/// Internal wrapper type used for a convenient drop implementation. +/// +/// [CertificateChain] is internally reference counted. The reference counted `T` +/// must have a drop implementation. +struct CertificateChainHandle { + cert: NonNull, + is_owned: bool, +} + +// # Safety +// +// s2n_cert_chain_and_key objects can be sent across threads. +unsafe impl Send for CertificateChainHandle {} +unsafe impl Sync for CertificateChainHandle {} + +impl CertificateChainHandle { + fn from_owned(cert: NonNull) -> Self { + Self { + cert, + is_owned: true, + } + } + + fn from_reference(cert: NonNull) -> Self { + Self { + cert, + is_owned: false, + } + } +} + +impl Drop for CertificateChainHandle { + fn drop(&mut self) { + // ignore failures since there's not much we can do about it + if self.is_owned { + unsafe { + let _ = s2n_cert_chain_and_key_free(self.cert.as_ptr()).into_result(); + } + } + } +} + +pub struct Builder { + cert: CertificateChain<'static>, +} + +impl Builder { + pub fn new() -> Result { + Ok(Self { + cert: CertificateChain::allocate_owned()?, + }) + } + + /// Corresponds to [s2n_cert_chain_and_key_load_pem_bytes] + /// + /// This can be used with [crate::config::Builder::load_chain] to share a + /// single cert across multiple configs. + pub fn load_pem(&mut self, chain: &[u8], key: &[u8]) -> Result<&mut Self, Error> { + unsafe { + // SAFETY: manual audit of load_pem_bytes shows that `chain_pem` and + // `private_key_pem` are not modified. + // https://github.com/aws/s2n-tls/issues/4140 + s2n_cert_chain_and_key_load_pem_bytes( + self.cert.as_mut_ptr(), + chain.as_ptr() as *mut _, + chain.len() as u32, + key.as_ptr() as *mut _, + key.len() as u32, + ) + .into_result() + }?; + + Ok(self) + } + + /// Corresponds to [s2n_cert_chain_and_key_load_public_pem_bytes]. + /// + /// This method is only used when performing private-key offloading. For standard + /// use-cases see [CertificateChain::from_pem]. + pub fn load_public_pem(&mut self, chain: &[u8]) -> Result<&mut Self, Error> { + unsafe { + // SAFETY: manual audit of load_public_pem_bytes shows that `chain_pem` + // is not modified + // https://github.com/aws/s2n-tls/issues/4140 + s2n_cert_chain_and_key_load_public_pem_bytes( + self.cert.as_mut_ptr(), + chain.as_ptr() as *mut _, + chain.len() as u32, + ) + .into_result() + }?; + + Ok(self) + } + + /// Corresponds to [s2n_cert_chain_and_key_set_ocsp_data]. + pub fn set_ocsp_data(&mut self, data: &[u8]) -> Result<&mut Self, Error> { + unsafe { + s2n_cert_chain_and_key_set_ocsp_data( + self.cert.as_mut_ptr(), + data.as_ptr(), + data.len() as u32, + ) + .into_result() + }?; + Ok(self) + } + + /// Return an immutable, internally-reference counted CertificateChain. + pub fn build(self) -> Result, Error> { + // This method is currently infalliable, but returning a result allows + // us to add validation in the future. + Ok(self.cert) + } +} + /// A CertificateChain represents a chain of X.509 certificates. +/// +/// Certificate chains are internally reference counted and are cheaply cloneable. +// +// SAFETY: it is important that no CertificateChain methods operate on mutable +// references. Because CertificateChains can be shared across threads, it is not +// safe to mutate CertificateChains. +#[derive(Clone)] pub struct CertificateChain<'a> { - ptr: NonNull, - is_owned: bool, + ptr: Arc, _lifetime: PhantomData<&'a s2n_cert_chain_and_key>, } impl CertificateChain<'_> { /// This allocates a new certificate chain from s2n. - pub(crate) fn new() -> Result, Error> { + pub(crate) fn allocate_owned() -> Result, Error> { + crate::init::init(); unsafe { let ptr = s2n_cert_chain_and_key_new().into_result()?; Ok(CertificateChain { - ptr, - is_owned: true, + ptr: Arc::new(CertificateChainHandle::from_owned(ptr)), _lifetime: PhantomData, }) } } + /// This is used to create a CertificateChain "reference" backed by memory + /// on some external struct, where the external struct has some lifetime `'a`. pub(crate) unsafe fn from_ptr_reference<'a>( ptr: NonNull, ) -> CertificateChain<'a> { + let handle = Arc::new(CertificateChainHandle::from_reference(ptr)); + CertificateChain { - ptr, - is_owned: false, + ptr: handle, _lifetime: PhantomData, } } @@ -54,8 +180,7 @@ impl CertificateChain<'_> { /// expensive API to call. pub fn len(&self) -> usize { let mut length: u32 = 0; - let res = - unsafe { s2n_cert_chain_get_length(self.ptr.as_ptr(), &mut length).into_result() }; + let res = unsafe { s2n_cert_chain_get_length(self.as_ptr(), &mut length).into_result() }; if res.is_err() { // Errors should only happen on empty chains (we guarantee that `ptr` is a valid chain). return 0; @@ -72,24 +197,16 @@ impl CertificateChain<'_> { self.len() == 0 } - pub(crate) fn as_mut_ptr(&mut self) -> NonNull { - self.ptr + /// 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() } -} - -// # Safety -// -// s2n_cert_chain_and_key objects can be sent across threads. -unsafe impl Send for CertificateChain<'_> {} -impl Drop for CertificateChain<'_> { - fn drop(&mut self) { - if self.is_owned { - // ignore failures since there's not much we can do about it - unsafe { - let _ = s2n_cert_chain_and_key_free(self.ptr.as_ptr()).into_result(); - } - } + pub(crate) fn as_ptr(&self) -> *const s2n_cert_chain_and_key { + self.ptr.cert.as_ptr() as *const _ } } @@ -112,7 +229,7 @@ impl<'a> Iterator for CertificateChainIter<'a> { let mut out = ptr::null_mut(); unsafe { if let Err(e) = - s2n_cert_chain_get_cert(self.chain.ptr.as_ptr(), &mut out, idx).into_result() + s2n_cert_chain_get_cert(self.chain.as_ptr(), &mut out, idx).into_result() { return Some(Err(e)); } @@ -152,3 +269,231 @@ impl Certificate<'_> { // // Certificates just reference data in the chain, so share the Send-ness of the chain. unsafe impl Send for Certificate<'_> {} + +#[cfg(test)] +mod tests { + use crate::{ + config, + error::{ErrorSource, ErrorType}, + security::DEFAULT_TLS13, + testing::{InsecureAcceptAllCertificatesHandler, SniTestCerts, TestPair}, + }; + + use super::*; + + /// Create a test pair using SNI certs + /// * `certs`: takes references to already created cert chains. This is useful + /// to assert on expected reference counts. + /// * `types`: Used to find the CA paths for the client configs + fn sni_test_pair( + certs: Vec>, + defaults: Option>>, + types: &[SniTestCerts], + ) -> Result { + let mut server_config = config::Builder::new(); + server_config + .with_system_certs(false)? + .set_security_policy(&DEFAULT_TLS13)?; + for cert in certs.into_iter() { + server_config.load_chain(cert)?; + } + if let Some(defaults) = defaults { + server_config.set_default_chains(defaults)?; + } + + let mut client_config = config::Builder::new(); + client_config + .with_system_certs(false)? + .set_security_policy(&DEFAULT_TLS13)? + .set_verify_host_callback(InsecureAcceptAllCertificatesHandler {})?; + for t in types { + client_config.trust_pem(t.get().cert())?; + } + Ok(TestPair::from_configs( + &client_config.build()?, + &server_config.build()?, + )) + } + + /// This is a useful (but inefficient) test utility to check if CertificateChain + /// structs are equal. It does this by comparing the serialized `der` representation. + fn cert_chains_are_equal(this: &CertificateChain<'_>, that: &CertificateChain<'_>) -> bool { + let this: Vec> = this + .iter() + .map(|cert| cert.unwrap().der().unwrap().to_owned()) + .collect(); + let that: Vec> = that + .iter() + .map(|cert| cert.unwrap().der().unwrap().to_owned()) + .collect(); + this == that + } + + #[test] + fn reference_count_increment() -> Result<(), crate::error::Error> { + let cert = SniTestCerts::AlligatorRsa.get().into_certificate_chain(); + assert_eq!(Arc::strong_count(&cert.ptr), 1); + + { + let mut server = config::Builder::new(); + server.load_chain(cert.clone())?; + + // after being added, the reference count should have increased + assert_eq!(Arc::strong_count(&cert.ptr), 2); + } + + // after the config goes out of scope and is dropped, the ref count should + // decrement + assert_eq!(Arc::strong_count(&cert.ptr), 1); + Ok(()) + } + + #[test] + fn cert_is_dropped() { + let weak_ref = { + let cert = SniTestCerts::AlligatorEcdsa.get().into_certificate_chain(); + assert_eq!(Arc::strong_count(&cert.ptr), 1); + Arc::downgrade(&cert.ptr) + }; + assert_eq!(weak_ref.strong_count(), 0); + assert!(weak_ref.upgrade().is_none()); + } + + // a cert can be successfully shared across multiple configs + #[test] + fn shared_certs() -> Result<(), crate::error::Error> { + let test_key_pair = SniTestCerts::AlligatorRsa.get(); + let cert = test_key_pair.into_certificate_chain(); + + let mut test_pair_1 = + sni_test_pair(vec![cert.clone()], None, &[SniTestCerts::AlligatorRsa])?; + let mut test_pair_2 = + sni_test_pair(vec![cert.clone()], None, &[SniTestCerts::AlligatorRsa])?; + + assert_eq!(Arc::strong_count(&cert.ptr), 3); + + assert!(test_pair_1.handshake().is_ok()); + assert!(test_pair_2.handshake().is_ok()); + + assert_eq!(Arc::strong_count(&cert.ptr), 3); + + drop(test_pair_1); + assert_eq!(Arc::strong_count(&cert.ptr), 2); + drop(test_pair_2); + assert_eq!(Arc::strong_count(&cert.ptr), 1); + Ok(()) + } + + #[test] + fn too_many_certs_in_default() -> Result<(), crate::error::Error> { + // 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(); + assert_eq!(err.kind(), ErrorType::UsageError); + assert_eq!(err.source(), ErrorSource::Bindings); + + // The config should not hold a reference when the error was detected + // in the bindings + assert_eq!(Arc::strong_count(&certs[0].ptr), FAILING_NUMBER); + + Ok(()) + } + + #[test] + fn default_selection() -> Result<(), crate::error::Error> { + let alligator_cert = SniTestCerts::AlligatorRsa.get().into_certificate_chain(); + let beaver_cert = SniTestCerts::BeaverRsa.get().into_certificate_chain(); + + // when no default is explicitly set, the first loaded cert is the default + { + let mut test_pair = sni_test_pair( + vec![alligator_cert.clone(), beaver_cert.clone()], + None, + &[SniTestCerts::AlligatorRsa, SniTestCerts::BeaverRsa], + )?; + + assert!(test_pair.handshake().is_ok()); + + assert!(cert_chains_are_equal( + &alligator_cert, + &test_pair.client.peer_cert_chain().unwrap() + )); + + assert_eq!(Arc::strong_count(&alligator_cert.ptr), 2); + assert_eq!(Arc::strong_count(&beaver_cert.ptr), 2); + } + + // set an explicit default + { + let mut test_pair = sni_test_pair( + vec![alligator_cert.clone(), beaver_cert.clone()], + Some(vec![beaver_cert.clone()]), + &[SniTestCerts::AlligatorRsa, SniTestCerts::BeaverRsa], + )?; + + assert!(test_pair.handshake().is_ok()); + + assert!(cert_chains_are_equal( + &beaver_cert, + &test_pair.client.peer_cert_chain().unwrap() + )); + + assert_eq!(Arc::strong_count(&alligator_cert.ptr), 2); + // beaver has an additional reference because it was used in multiple + // calls + assert_eq!(Arc::strong_count(&beaver_cert.ptr), 3); + } + + // set a default without adding it to the store + { + let mut test_pair = sni_test_pair( + vec![alligator_cert.clone()], + Some(vec![beaver_cert.clone()]), + &[SniTestCerts::AlligatorRsa, SniTestCerts::BeaverRsa], + )?; + + assert!(test_pair.handshake().is_ok()); + + assert!(cert_chains_are_equal( + &beaver_cert, + &test_pair.client.peer_cert_chain().unwrap() + )); + + assert_eq!(Arc::strong_count(&alligator_cert.ptr), 2); + assert_eq!(Arc::strong_count(&beaver_cert.ptr), 2); + } + + Ok(()) + } + + #[test] + fn cert_ownership_error() -> Result<(), crate::error::Error> { + let application_owned_cert = SniTestCerts::AlligatorRsa.get().into_certificate_chain(); + let cert_for_lib = SniTestCerts::BeaverRsa.get(); + + let mut config = config::Builder::new(); + + // library owned certs can not be used with application owned certs + config.load_chain(application_owned_cert)?; + let err = config + .load_pem(cert_for_lib.cert(), cert_for_lib.key()) + .err() + .unwrap(); + + assert_eq!(err.kind(), ErrorType::UsageError); + assert_eq!(err.name(), "S2N_ERR_CERT_OWNERSHIP"); + + Ok(()) + } + + // ensure the certificates are send and sync + #[test] + fn certificate_send_sync_test() { + fn assert_send_sync() {} + assert_send_sync::>(); + } +} diff --git a/bindings/rust/s2n-tls/src/config.rs b/bindings/rust/s2n-tls/src/config.rs index c7e6353347e..81c0c4db0a4 100644 --- a/bindings/rust/s2n-tls/src/config.rs +++ b/bindings/rust/s2n-tls/src/config.rs @@ -5,8 +5,9 @@ use crate::renegotiate::RenegotiateCallback; use crate::{ callbacks::*, + cert_chain::CertificateChain, enums::*, - error::{Error, Fallible}, + error::{Error, ErrorType, Fallible}, security, }; use core::{convert::TryInto, ptr::NonNull}; @@ -277,6 +278,12 @@ impl Builder { Ok(self) } + /// Associate a `certificate` and corresponding `private_key` with a config. + /// Using this method, at most one certificate per auth type (ECDSA, RSA, RSA-PSS) + /// can be loaded. + /// + /// For more advanced cert use cases such as sharing certs across configs or + /// serving differents certs based on the client SNI, see [Builder::load_chain]. pub fn load_pem(&mut self, certificate: &[u8], private_key: &[u8]) -> Result<&mut Self, Error> { let certificate = CString::new(certificate).map_err(|_| Error::INVALID_INPUT)?; let private_key = CString::new(private_key).map_err(|_| Error::INVALID_INPUT)?; @@ -291,6 +298,77 @@ impl Builder { Ok(self) } + /// Corresponds to [s2n_config_add_cert_chain_and_key_to_store]. + pub fn load_chain(&mut self, chain: CertificateChain<'static>) -> Result<&mut Self, Error> { + // 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 + // is not mutated. https://github.com/aws/s2n-tls/issues/4140 + chain.as_ptr() as *mut _, + ) + .into_result() + }; + self.context_mut().application_owned_certs.push(chain); + result?; + + Ok(self) + } + + /// Corresponds to [s2n_config_set_cert_chain_and_key_defaults]. + pub fn set_default_chains>>( + &mut self, + chains: T, + ) -> Result<&mut Self, Error> { + // Must be equal to S2N_CERT_TYPE_COUNT in s2n_certificate.h. + const CHAINS_MAX_COUNT: usize = 3; + + let mut chain_arrays: [Option>; CHAINS_MAX_COUNT] = + [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 cert_chain_count >= CHAINS_MAX_COUNT { + return Err(Error::bindings( + ErrorType::UsageError, + "InvalidInput", + "A single default can be specified for RSA, ECDSA, + and RSA-PSS auth types, but more than 3 certs were supplied", + )); + } + + // SAFETY: manual inspection of set_defaults shows that certificates + // are not mutated. https://github.com/aws/s2n-tls/issues/4140 + pointer_array[cert_chain_count] = chain.as_ptr() as *mut _; + chain_arrays[cert_chain_count] = Some(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(), + cert_chain_count as u32, + ) + .into_result() + }?; + + Ok(self) + } + pub fn load_public_pem(&mut self, certificate: &[u8]) -> Result<&mut Self, Error> { let size: u32 = certificate .len() @@ -811,6 +889,16 @@ impl Default for Builder { pub(crate) struct Context { refcount: AtomicUsize, + /// This is a container for reference counts. + /// + /// In the bindings, application owned certificate chains are reference counted. + /// The C library is not aware of the reference counts, so a naive implementation + /// would result in certs being prematurely freed because the "reference" + /// held by the C library wouldn't be accounted for. + /// + /// Storing the CertificateChains in this Vec ensures that reference counts + /// behave as expected when stored in an s2n-tls config. + application_owned_certs: Vec>, pub(crate) client_hello_callback: Option>, pub(crate) private_key_callback: Option>, pub(crate) verify_host_callback: Option>, @@ -830,6 +918,7 @@ impl Default for Context { Self { refcount, + application_owned_certs: Vec::new(), client_hello_callback: None, private_key_callback: None, verify_host_callback: None, diff --git a/bindings/rust/s2n-tls/src/connection.rs b/bindings/rust/s2n-tls/src/connection.rs index e65225cf111..7db69da50bf 100644 --- a/bindings/rust/s2n-tls/src/connection.rs +++ b/bindings/rust/s2n-tls/src/connection.rs @@ -1098,13 +1098,10 @@ impl Connection { // chain, so the lifetime is independent of the connection. pub fn peer_cert_chain(&self) -> Result, Error> { unsafe { - let mut chain = CertificateChain::new()?; - s2n_connection_get_peer_cert_chain( - self.connection.as_ptr(), - chain.as_mut_ptr().as_ptr(), - ) - .into_result() - .map(|_| ())?; + let mut chain = CertificateChain::allocate_owned()?; + s2n_connection_get_peer_cert_chain(self.connection.as_ptr(), chain.as_mut_ptr()) + .into_result() + .map(|_| ())?; Ok(chain) } } diff --git a/bindings/rust/s2n-tls/src/renegotiate.rs b/bindings/rust/s2n-tls/src/renegotiate.rs index 493cf746a95..0a3cb841ec4 100644 --- a/bindings/rust/s2n-tls/src/renegotiate.rs +++ b/bindings/rust/s2n-tls/src/renegotiate.rs @@ -514,11 +514,12 @@ mod tests { // // openssl also requires a properly configured CA cert, which the // default TestPair does not include. - let certs_dir = concat!( - env!("CARGO_MANIFEST_DIR"), - "/../../../tests/pems/permutations/rsae_pkcs_4096_sha384/" + let certs = CertKeyPair::from_path( + "permutations/rsae_pkcs_4096_sha384/", + "server-chain", + "server-key", + "ca-cert", ); - let certs = CertKeyPair::from(certs_dir, "server-chain", "server-key", "ca-cert"); // Build the s2n-tls client. builder.load_pem(certs.cert(), certs.key())?; @@ -1021,9 +1022,11 @@ mod tests { // Perform the pkey operation with the selected cert / key pair. let op = this.op.take().unwrap(); let opt_ptr = op.as_ptr(); - let chain_ptr = conn.selected_cert().unwrap().as_mut_ptr().as_ptr(); + let chain_ptr = conn.selected_cert().unwrap().as_ptr(); unsafe { - let key_ptr = s2n_cert_chain_and_key_get_private_key(chain_ptr) + // SAFETY, mut cast: get_private_key does not modify the + // chain, and it is invalid to modify key through `key_ptr` + let key_ptr = s2n_cert_chain_and_key_get_private_key(chain_ptr as *mut _) .into_result()? .as_ptr(); s2n_async_pkey_op_perform(opt_ptr, key_ptr).into_result()?; diff --git a/bindings/rust/s2n-tls/src/testing.rs b/bindings/rust/s2n-tls/src/testing.rs index feaab38acba..4eb74fa396b 100644 --- a/bindings/rust/s2n-tls/src/testing.rs +++ b/bindings/rust/s2n-tls/src/testing.rs @@ -3,6 +3,7 @@ use crate::{ callbacks::VerifyHostNameCallback, + cert_chain::{self, CertificateChain}, config::{self, *}, connection, enums::{self, Blinding}, @@ -59,6 +60,26 @@ impl Default for Counter { } } +#[allow(non_camel_case_types)] +// allow non camel case types because the mixture of letters and numbers is easier +// to read with snake_case. +pub enum SniTestCerts { + AlligatorRsa, + AlligatorEcdsa, + BeaverRsa, +} + +impl SniTestCerts { + pub fn get(&self) -> CertKeyPair { + let prefix = match *self { + SniTestCerts::AlligatorRsa => "alligator_", + SniTestCerts::AlligatorEcdsa => "alligator_ecdsa_", + SniTestCerts::BeaverRsa => "beaver_", + }; + CertKeyPair::from_path(&format!("sni/{prefix}"), "cert", "key", "cert") + } +} + pub struct CertKeyPair { cert_path: String, key_path: String, @@ -69,19 +90,40 @@ pub struct CertKeyPair { impl Default for CertKeyPair { fn default() -> Self { - let prefix = concat!( - env!("CARGO_MANIFEST_DIR"), - "/../../../tests/pems/rsa_4096_sha512_client_" - ); - Self::from(prefix, "cert", "key", "cert") + Self::from_path("rsa_4096_sha512_client_", "cert", "key", "cert") } } impl CertKeyPair { - pub fn from(prefix: &str, chain: &str, key: &str, ca: &str) -> Self { - let cert_path = format!("{prefix}{chain}.pem"); - let key_path = format!("{prefix}{key}.pem"); - let ca_path = format!("{prefix}{ca}.pem"); + /// This is the directory holding all of the pems used for s2n-tls unit tests + const TEST_PEMS_PATH: &'static str = + concat!(env!("CARGO_MANIFEST_DIR"), "/../../../tests/pems/"); + + /// Create a test CertKeyPair + /// * `prefix`: The *relative* prefix from the s2n-tls/tests/pems/ folder. + /// * `chain`: The suffix indicate the full chain. + /// * `key`: The suffix indicate the private key. + /// * `ca`: The suffix indicating the CA. + /// + /// ### Example + /// Assuming the relevant files are at + /// - s2n-tls/tests/pems/permutations/rsae_pkcs_4096_sha384/server-chain.pem + /// - s2n-tls/tests/pems/permutations/rsae_pkcs_4096_sha384/server-key.pem + /// - s2n-tls/tests/pems/permutations/rsae_pkcs_4096_sha384/ca-cert.pem + /// + /// ```ignore + /// let cert = CertKeyPair::from( + /// "permutations/rsae_pkcs_4096_sha384/", + /// "server-chain", + /// "server-key", + /// "ca-cert" + /// ); + /// ``` + pub fn from_path(prefix: &str, chain: &str, key: &str, ca: &str) -> Self { + let cert_path = format!("{}{prefix}{chain}.pem", Self::TEST_PEMS_PATH); + println!("{:?}", cert_path); + let key_path = format!("{}{prefix}{key}.pem", Self::TEST_PEMS_PATH); + let ca_path = format!("{}{prefix}{ca}.pem", Self::TEST_PEMS_PATH); let cert = std::fs::read(&cert_path) .unwrap_or_else(|_| panic!("Failed to read cert at {cert_path}")); let key = @@ -95,6 +137,12 @@ impl CertKeyPair { } } + pub fn into_certificate_chain(&self) -> CertificateChain<'static> { + let mut chain = cert_chain::Builder::new().unwrap(); + chain.load_pem(&self.cert, &self.key).unwrap(); + chain.build().unwrap() + } + pub fn cert_path(&self) -> &str { &self.cert_path } diff --git a/codebuild/bin/s2n_codebuild.sh b/codebuild/bin/s2n_codebuild.sh index b42f1428f1a..507f4fa6e92 100755 --- a/codebuild/bin/s2n_codebuild.sh +++ b/codebuild/bin/s2n_codebuild.sh @@ -102,7 +102,6 @@ if [[ "$TESTS" == "ALL" || "$TESTS" == "sawHMACPlus" ]] && [[ "$OS_NAME" == "lin if [[ "$TESTS" == "ALL" || "$TESTS" == "unit" ]]; then run_unit_tests; fi if [[ "$TESTS" == "ALL" || "$TESTS" == "interning" ]]; then ./codebuild/bin/test_libcrypto_interning.sh; fi if [[ "$TESTS" == "ALL" || "$TESTS" == "exec_leak" ]]; then ./codebuild/bin/test_exec_leak.sh; fi -if [[ "$TESTS" == "ALL" || "$TESTS" == "asan" ]]; then make clean; S2N_ADDRESS_SANITIZER=1 make -j $JOBS ; fi if [[ "$TESTS" == "ALL" || "$TESTS" == "integrationv2" ]]; then run_integration_v2_tests; fi if [[ "$TESTS" == "ALL" || "$TESTS" == "crt" ]]; then ./codebuild/bin/build_aws_crt_cpp.sh $(mktemp -d) $(mktemp -d); fi if [[ "$TESTS" == "ALL" || "$TESTS" == "sharedandstatic" ]]; then ./codebuild/bin/test_install_shared_and_static.sh $(mktemp -d); fi diff --git a/codebuild/spec/buildspec_32bit_cross_compile.yml b/codebuild/spec/buildspec_32bit_cross_compile.yml index 6fbf04e2f79..67eb9450fac 100644 --- a/codebuild/spec/buildspec_32bit_cross_compile.yml +++ b/codebuild/spec/buildspec_32bit_cross_compile.yml @@ -14,6 +14,12 @@ version: 0.2 phases: + pre_build: + commands: + - | + if [ -d "third-party-src" ]; then + cd third-party-src; + fi build: on-failure: ABORT commands: diff --git a/codebuild/spec/buildspec_generalbatch.yml b/codebuild/spec/buildspec_generalbatch.yml index 7fbf6de6f83..192449a331e 100644 --- a/codebuild/spec/buildspec_generalbatch.yml +++ b/codebuild/spec/buildspec_generalbatch.yml @@ -53,51 +53,6 @@ batch: BUILD_S2N: 'true' TESTS: exec_leak identifier: s2nExecLeak - - identifier: s2nAsanOpenSSL111Coverage - buildspec: codebuild/spec/buildspec_ubuntu.yml - env: - compute-type: BUILD_GENERAL1_LARGE - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild - privileged-mode: true - variables: - BUILD_S2N: 'true' - GCC_VERSION: '9' - S2N_COVERAGE: 'true' - S2N_LIBCRYPTO: 'openssl-1.1.1' - TESTS: asan - - identifier: s2nAsanAwslc - buildspec: codebuild/spec/buildspec_ubuntu.yml - env: - compute-type: BUILD_GENERAL1_LARGE - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild - privileged-mode: true - variables: - BUILD_S2N: 'true' - GCC_VERSION: '9' - S2N_LIBCRYPTO: 'awslc' - TESTS: asan - - identifier: s2nAsanOpenssl3 - buildspec: codebuild/spec/buildspec_ubuntu.yml - env: - privileged-mode: true - compute-type: BUILD_GENERAL1_LARGE - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild - variables: - TESTS: asan - GCC_VERSION: '9' - S2N_LIBCRYPTO: 'openssl-3.0' - BUILD_S2N: 'true' - - identifier: s2nAsanOpenssl102 - buildspec: codebuild/spec/buildspec_ubuntu.yml - env: - compute-type: BUILD_GENERAL1_LARGE - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild - privileged-mode: true - variables: - BUILD_S2N: 'true' - GCC_VERSION: '9' - S2N_LIBCRYPTO: 'openssl-1.0.2' - TESTS: asan - buildspec: codebuild/spec/buildspec_ubuntu.yml env: compute-type: BUILD_GENERAL1_SMALL @@ -222,10 +177,10 @@ batch: - buildspec: codebuild/spec/buildspec_ubuntu.yml env: compute-type: BUILD_GENERAL1_LARGE - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild + image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu24 privileged-mode: true variables: - GCC_VERSION: '6' + GCC_VERSION: '13' TESTS: crt identifier: s2nUnitCRT - buildspec: codebuild/spec/buildspec_ubuntu.yml diff --git a/codebuild/spec/buildspec_omnibus.yml b/codebuild/spec/buildspec_omnibus.yml deleted file mode 100644 index 863b78c9fab..00000000000 --- a/codebuild/spec/buildspec_omnibus.yml +++ /dev/null @@ -1,241 +0,0 @@ ---- -# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"). You may not use -# this file except in compliance with the License. A copy of the License is -# located at -# -# http://aws.amazon.com/apache2.0/ -# -# or in the "license" file accompanying this file. This file is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or -# implied. See the License for the specific language governing permissions and -# limitations under the License. -version: 0.2 - -# Doc for batch https://docs.aws.amazon.com/codebuild/latest/userguide/batch-build-buildspec.html#build-spec.batch.build-list -batch: - build-list: - - identifier: sawHMACPlus - buildspec: codebuild/spec/buildspec_ubuntu.yml - env: - privileged-mode: true - compute-type: BUILD_GENERAL1_LARGE - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild - variables: - TESTS: sawHMACPlus - SAW: true - GCC_VERSION: NONE - - - identifier: s2nSawTls - buildspec: codebuild/spec/buildspec_ubuntu.yml - env: - privileged-mode: true - compute-type: BUILD_GENERAL1_LARGE - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild - variables: - TESTS: tls - SAW: true - GCC_VERSION: NONE - - # Other - - identifier: s2nSidetrail - buildspec: codebuild/spec/buildspec_sidetrail.yml - env: - privileged-mode: true - compute-type: BUILD_GENERAL1_2XLARGE - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu14codebuild - variables: - TESTS: sidetrail - - - identifier: s2nAsanOpenSSL111Coverage - buildspec: codebuild/spec/buildspec_ubuntu.yml - env: - privileged-mode: true - compute-type: BUILD_GENERAL1_LARGE - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild - variables: - TESTS: asan - GCC_VERSION: '6' - S2N_LIBCRYPTO: 'openssl-1.1.1' - BUILD_S2N: 'true' - S2N_COVERAGE: 'true' - - - identifier: s2nAsanOpenssl102 - buildspec: codebuild/spec/buildspec_ubuntu.yml - env: - privileged-mode: true - compute-type: BUILD_GENERAL1_LARGE - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild - variables: - TESTS: asan - GCC_VERSION: '6' - S2N_LIBCRYPTO: 'openssl-1.0.2' - BUILD_S2N: 'true' - - - identifier: s2nUnitNoPQ - buildspec: codebuild/spec/buildspec_ubuntu.yml - env: - privileged-mode: true - compute-type: BUILD_GENERAL1_SMALL - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild - variables: - TESTS: unit - GCC_VERSION: '9' - S2N_LIBCRYPTO: 'openssl-1.1.1' - BUILD_S2N: 'true' - - - identifier: s2nUnitOpenSSL3GCC9 - buildspec: codebuild/spec/buildspec_ubuntu.yml - env: - compute-type: BUILD_GENERAL1_LARGE - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild - privileged-mode: true - variables: - BUILD_S2N: true - GCC_VERSION: 9 - S2N_COVERAGE: true - S2N_LIBCRYPTO: openssl-3.0 - TESTS: unit - - - identifier: s2nUnitAl2Arm - buildspec: codebuild/spec/buildspec_amazonlinux.yml - env: - type: ARM_CONTAINER - compute-type: BUILD_GENERAL1_LARGE - image: aws/codebuild/amazonlinux2-aarch64-standard:2.0 - privileged-mode: true - variables: - TESTS: unit - - - identifier: s2nUnitAl2 - buildspec: codebuild/spec/buildspec_amazonlinux.yml - env: - image: aws/codebuild/amazonlinux2-x86_64-standard:3.0 - privileged-mode: true - compute-type: BUILD_GENERAL1_SMALL - variables: - TESTS: unit - - - identifier: s2nLibcryptoInterningOpenSSL - buildspec: codebuild/spec/buildspec_ubuntu.yml - env: - privileged-mode: true - compute-type: BUILD_GENERAL1_LARGE - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild - variables: - TESTS: interning - BUILD_S2N: 'true' - - - identifier: s2nLibcryptoInterningAwslc - buildspec: codebuild/spec/buildspec_ubuntu.yml - env: - privileged-mode: true - compute-type: BUILD_GENERAL1_LARGE - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild - variables: - TESTS: interning - BUILD_S2N: 'true' - S2N_LIBCRYPTO: awslc - - - identifier: s2nUnitCRT - buildspec: codebuild/spec/buildspec_ubuntu.yml - env: - compute-type: BUILD_GENERAL1_LARGE - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild - privileged-mode: true - variables: - GCC_VERSION: '6' - TESTS: crt - - - identifier: s2nInstallSharedAndStatic - buildspec: codebuild/spec/buildspec_ubuntu.yml - env: - compute-type: BUILD_GENERAL1_SMALL - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild - privileged-mode: true - variables: - TESTS: sharedandstatic - # must use the libcrypto that's actually installed on the system - S2N_LIBCRYPTO: openssl-1.1.1 - - - identifier: s2nDynamicLoad - buildspec: codebuild/spec/buildspec_ubuntu.yml - env: - compute-type: BUILD_GENERAL1_SMALL - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild - privileged-mode: true - variables: - TESTS: dynamicload - GCC_VERSION: '9' - S2N_LIBCRYPTO: openssl-1.1.1 - - - buildspec: codebuild/spec/buildspec_ubuntu.yml - env: - compute-type: BUILD_GENERAL1_LARGE - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild - privileged-mode: true - variables: - BUILD_S2N: true - GCC_VERSION: 6 - S2N_LIBCRYPTO: openssl-1.1.1 - TESTS: unit - identifier: s2nUnitOpenSSL111Gcc6 - - - buildspec: codebuild/spec/buildspec_ubuntu.yml - env: - compute-type: BUILD_GENERAL1_SMALL - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild - privileged-mode: true - variables: - BUILD_S2N: 'true' - GCC_VERSION: '6' - S2N_LIBCRYPTO: 'libressl' - TESTS: unit - identifier: s2nUnitLibressl - - - buildspec: codebuild/spec/buildspec_ubuntu.yml - env: - compute-type: BUILD_GENERAL1_SMALL - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild - privileged-mode: true - variables: - BUILD_S2N: 'true' - GCC_VERSION: '9' - S2N_LIBCRYPTO: 'boringssl' - TESTS: unit - identifier: s2nUnitBoringssl - - - identifier: s2nFuzzerAWSLC - buildspec: codebuild/spec/buildspec_fuzz.yml - env: - privileged-mode: true - compute-type: BUILD_GENERAL1_XLARGE - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu22codebuild - variables: - S2N_LIBCRYPTO: awslc - COMPILER: clang - FUZZ_TIMEOUT_SEC: 60 - - - identifier: s2nFuzzerOpenSSL111 - buildspec: codebuild/spec/buildspec_fuzz.yml - env: - privileged-mode: true - compute-type: BUILD_GENERAL1_XLARGE - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu22codebuild - variables: - S2N_LIBCRYPTO: openssl-1.1.1 - COMPILER: clang - FUZZ_TIMEOUT_SEC: 60 - - - identifier: s2nFuzzerOpenSSL102FIPS - buildspec: codebuild/spec/buildspec_fuzz.yml - env: - privileged-mode: true - compute-type: BUILD_GENERAL1_XLARGE - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu22codebuild - variables: - S2N_LIBCRYPTO: openssl-1.0.2-fips - COMPILER: clang - FUZZ_TIMEOUT_SEC: 60 - diff --git a/codebuild/spec/buildspec_tsan.yml b/codebuild/spec/buildspec_tsan.yml index 6aaf00efaa9..460d9cbc4c0 100644 --- a/codebuild/spec/buildspec_tsan.yml +++ b/codebuild/spec/buildspec_tsan.yml @@ -14,6 +14,12 @@ version: 0.2 phases: + pre_build: + commands: + - | + if [ -d "third-party-src" ]; then + cd third-party-src; + fi build: on-failure: ABORT commands: diff --git a/codebuild/spec/buildspec_ubuntu_cmake.yml b/codebuild/spec/buildspec_ubuntu_cmake.yml index 88360356bda..0ce71aa558e 100644 --- a/codebuild/spec/buildspec_ubuntu_cmake.yml +++ b/codebuild/spec/buildspec_ubuntu_cmake.yml @@ -14,6 +14,12 @@ version: 0.2 phases: + pre_build: + commands: + - | + if [ -d "third-party-src" ]; then + cd third-party-src; + fi build: on-failure: ABORT commands: diff --git a/crypto/s2n_fips.c b/crypto/s2n_fips.c index ff73433a6ce..1b2c0307069 100644 --- a/crypto/s2n_fips.c +++ b/crypto/s2n_fips.c @@ -26,17 +26,24 @@ static bool s2n_fips_mode_enabled = false; -/* FIPS mode can be checked if OpenSSL was configured and built for FIPS which then defines OPENSSL_FIPS. +/* Check if the linked libcrypto has FIPS mode enabled. * - * AWS-LC always defines FIPS_mode() that you can call and check what the library was built with. It does not define - * a public OPENSSL_FIPS/AWSLC_FIPS macro that we can (or need to) check here + * This method indicates the state of the libcrypto, NOT the state + * of s2n-tls and should ONLY be called during library initialization (i.e. + * s2n_init()). For example, if s2n-tls is using Openssl and FIPS_mode_set(1) + * is called after s2n_init() is called, then this method will return true + * while s2n_is_in_fips_mode() will return false and s2n-tls will not operate + * in FIPS mode. * - * Safeguard with macro's, for example because Libressl dosn't define - * FIPS_mode() by default. + * For AWS-LC, the FIPS_mode() method is always defined. If AWS-LC was built to + * support FIPS, FIPS_mode() always returns 1. * - * Note: FIPS_mode() does not change the FIPS state of libcrypto. This only returns the current state. Applications - * using s2n must call FIPS_mode_set(1) prior to s2n_init. - * */ + * For OpenSSL, OPENSSL_FIPS is defined if the libcrypto was built to support + * FIPS. The FIPS_mode() method is only present if OPENSSL_FIPS is defined, and + * only returns 1 if FIPS_mode_set(1) was used to enable FIPS mode. + * Applications wanting to enable FIPS mode with OpenSSL must call + * FIPS_mode_set(1) prior to calling s2n_init(). + */ bool s2n_libcrypto_is_fips(void) { #if defined(OPENSSL_FIPS) || defined(OPENSSL_IS_AWSLC) diff --git a/crypto/s2n_fips.h b/crypto/s2n_fips.h index f9eafef5907..9cce9b50bfa 100644 --- a/crypto/s2n_fips.h +++ b/crypto/s2n_fips.h @@ -23,7 +23,6 @@ int s2n_fips_init(void); bool s2n_is_in_fips_mode(void); -bool s2n_libcrypto_is_fips(void); struct s2n_cipher_suite; S2N_RESULT s2n_fips_validate_cipher_suite(const struct s2n_cipher_suite *cipher_suite, bool *valid); diff --git a/crypto/s2n_libcrypto.c b/crypto/s2n_libcrypto.c index 166e3bf86e3..a3172dc5d93 100644 --- a/crypto/s2n_libcrypto.c +++ b/crypto/s2n_libcrypto.c @@ -99,16 +99,32 @@ static S2N_RESULT s2n_libcrypto_validate_expected_version_number(void) } /* s2n_libcrypto_is_*() encodes the libcrypto version used at build-time. - * Currently only captures AWS-LC and BoringSSL. When a libcrypto-dependent - * branch is required, we prefer these functions where possible to reduce - # #ifs and avoid potential bugs where the header containing the #define is not - * included. + * + * When a libcrypto-dependent branch is required, we prefer these functions + * where possible to reduce #ifs and avoid potential bugs where the header + * containing the #define is not included. */ #if defined(OPENSSL_IS_AWSLC) && defined(OPENSSL_IS_BORINGSSL) #error "Both OPENSSL_IS_AWSLC and OPENSSL_IS_BORINGSSL are defined at the same time!" #endif +/* Attempt to detect if the libcrypto is OpenSSL. + * + * This check should be updated if s2n-tls adds support for a new libcrypto. + * + * Since several libcrypto implementations (such as BoringSSL and AWS-LC) are + * ABI compatible forks of OpenSSL, detecting OpenSSL is done by checking the + * absence of other known libcrypto variants. + */ +bool s2n_libcrypto_is_openssl(void) +{ + bool is_other_libcrypto_variant = + s2n_libcrypto_is_boringssl() || s2n_libcrypto_is_libressl() || s2n_libcrypto_is_awslc(); + + return !is_other_libcrypto_variant; +} + bool s2n_libcrypto_is_awslc() { #if defined(OPENSSL_IS_AWSLC) diff --git a/crypto/s2n_openssl.h b/crypto/s2n_openssl.h index cd43cb8f4e1..eaee87700c3 100644 --- a/crypto/s2n_openssl.h +++ b/crypto/s2n_openssl.h @@ -50,12 +50,7 @@ #define RESULT_EVP_CTX_INIT(ctx) EVP_CIPHER_CTX_init(ctx) #endif -#if !defined(OPENSSL_IS_BORINGSSL) && !defined(OPENSSL_FIPS) && !defined(LIBRESSL_VERSION_NUMBER) && !defined(OPENSSL_IS_AWSLC) && !defined(OPENSSL_NO_ENGINE) - #define S2N_LIBCRYPTO_SUPPORTS_CUSTOM_RAND 1 -#else - #define S2N_LIBCRYPTO_SUPPORTS_CUSTOM_RAND 0 -#endif - +bool s2n_libcrypto_is_openssl(void); bool s2n_libcrypto_is_awslc(); bool s2n_libcrypto_is_boringssl(); bool s2n_libcrypto_is_libressl(); diff --git a/tests/features/S2N_LIBCRYPTO_SUPPORTS_ENGINE.c b/tests/features/S2N_LIBCRYPTO_SUPPORTS_ENGINE.c new file mode 100644 index 00000000000..781ddae4540 --- /dev/null +++ b/tests/features/S2N_LIBCRYPTO_SUPPORTS_ENGINE.c @@ -0,0 +1,79 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +/* + * This feature probe checks if the linked libcrypto has ENGINE support. + * https://docs.openssl.org/1.0.2/man3/engine/ + */ + +/* + * We would always expect the `openssl/engine.h` header to be available. + * However, some platforms (CentOS 10, Fedora 41, and RHEL 10) have reportedly + * been removing the `openssl/engine.h` header. + * + * See the related issues: + * - https://github.com/aws/s2n-tls/pull/4705 + * - https://github.com/aws/s2n-tls/pull/4873 + */ +#include +/* LibreSSL requires include. + * https://github.com/aws/s2n-tls/issues/153#issuecomment-129651643 + */ +#include + +int s2n_noop_rand(unsigned char *buf, int num) +{ + return 1; +} + +int main() +{ + /* Init usage in utils/s2n_random.c */ + ENGINE *e = ENGINE_new(); + ENGINE_set_id(e, "id"); + ENGINE_set_name(e, "name"); + ENGINE_set_flags(e, ENGINE_FLAGS_NO_REGISTER_ALL); + ENGINE_set_init_function(e, NULL); + ENGINE_set_RAND(e, NULL); + ENGINE_add(e); + ENGINE_init(e); + ENGINE_set_default(e, ENGINE_METHOD_RAND); + + /* Cleanup usage in utils/s2n_random.c */ + ENGINE_remove(e); + ENGINE_finish(e); + ENGINE_unregister_RAND(e); + ENGINE_free(e); + ENGINE_cleanup(); + RAND_set_rand_engine(NULL); + RAND_set_rand_method(NULL); + + /* RAND_METHOD is gated behind S2N_LIBCRYPTO_SUPPORTS_ENGINE because AWS-LC has + * a different signature for RAND_METHOD and fails to compile. + * + * - AWS-LC: https://github.com/aws/aws-lc/blob/main/include/openssl/rand.h#L124 + * - OpenSSL: https://github.com/openssl/openssl/blob/master/include/openssl/rand.h#L42 + */ + RAND_METHOD s2n_noop_rand_method = { + .seed = NULL, + .bytes = s2n_noop_rand, + .cleanup = NULL, + .add = NULL, + .pseudorand = s2n_noop_rand, + .status = NULL + }; + + return 0; +} diff --git a/tests/features/S2N_LIBCRYPTO_SUPPORTS_ENGINE.flags b/tests/features/S2N_LIBCRYPTO_SUPPORTS_ENGINE.flags new file mode 100644 index 00000000000..9d0b10a71d9 --- /dev/null +++ b/tests/features/S2N_LIBCRYPTO_SUPPORTS_ENGINE.flags @@ -0,0 +1 @@ +-Wincompatible-pointer-types diff --git a/tests/integrationv2/common.py b/tests/integrationv2/common.py index b550d463659..35ba091cd79 100644 --- a/tests/integrationv2/common.py +++ b/tests/integrationv2/common.py @@ -111,7 +111,20 @@ def __init__(self, name, prefix, location=TEST_CERT_DIRECTORY): self.algorithm = 'RSAPSS' def compatible_with_cipher(self, cipher): - return (self.algorithm == cipher.algorithm) or (cipher.algorithm == 'ANY') + if self.algorithm == cipher.algorithm: + return True + # TLS1.3 cipher suites do not specify auth method, so allow any auth method + if cipher.algorithm == 'ANY': + return True + if self.algorithm == 'RSAPSS': + # RSA-PSS certs can only be used by ciphers with RSA auth + if cipher.algorithm != 'RSA': + return False + # RSA-PSS certs do not support RSA key exchange, only RSA auth + # "DHE" here is intended to capture both "DHE" and "ECDHE" + if 'DHE' in cipher.name: + return True + return False def compatible_with_curve(self, curve): if self.algorithm != 'EC': @@ -442,7 +455,7 @@ class Signatures(object): RSA_PSS_PSS_SHA256 = Signature( 'rsa_pss_pss_sha256', - min_protocol=Protocols.TLS13, + min_protocol=Protocols.TLS12, sig_type='RSA-PSS-PSS', sig_digest='SHA256') diff --git a/tests/integrationv2/providers.py b/tests/integrationv2/providers.py index ae203af6e6c..185d4f08be1 100644 --- a/tests/integrationv2/providers.py +++ b/tests/integrationv2/providers.py @@ -148,22 +148,24 @@ def get_send_marker(cls): @classmethod def supports_protocol(cls, protocol, with_cert=None): - # TLS 1.3 is unsupported for openssl-1.0 + # RSA-PSS is unsupported for openssl-1.0 # libressl and boringssl are disabled because of configuration issues # see https://github.com/aws/s2n-tls/issues/3250 - TLS_13_UNSUPPORTED_LIBCRYPTOS = { + PSS_UNSUPPORTED_LIBCRYPTOS = { "libressl", "boringssl", "openssl-1.0" } - - # Disable TLS 1.3 tests for all libcryptos that don't support 1.3 - if protocol == Protocols.TLS13: - current_libcrypto = get_flag(S2N_PROVIDER_VERSION) - for unsupported_lc in TLS_13_UNSUPPORTED_LIBCRYPTOS: - # e.g. "openssl-1.0" in "openssl-1.0.2-fips" - if unsupported_lc in current_libcrypto: - return False + pss_is_unsupported = any([ + # e.g. "openssl-1.0" in "openssl-1.0.2-fips" + libcrypto in get_flag(S2N_PROVIDER_VERSION) + for libcrypto in PSS_UNSUPPORTED_LIBCRYPTOS + ]) + if pss_is_unsupported: + if protocol == Protocols.TLS13: + return False + if with_cert and with_cert.algorithm == 'RSAPSS': + return False # SSLv3 cannot be negotiated in FIPS mode with libcryptos other than AWS-LC. if all([ diff --git a/tests/integrationv2/utils.py b/tests/integrationv2/utils.py index 3d7c2022120..47842fd3bdb 100644 --- a/tests/integrationv2/utils.py +++ b/tests/integrationv2/utils.py @@ -72,9 +72,8 @@ def invalid_test_parameters(*args, **kwargs): # Always consider S2N providers.append(S2N) - # Only TLS1.3 supports RSA-PSS-PSS certificates - # (Earlier versions support RSA-PSS signatures, just via RSA-PSS-RSAE) - if protocol and protocol is not Protocols.TLS13: + # Older versions do not support RSA-PSS-PSS certificates + if protocol and protocol < Protocols.TLS12: if client_certificate and client_certificate.algorithm == 'RSAPSS': return True if certificate and certificate.algorithm == 'RSAPSS': diff --git a/tests/unit/s2n_build_test.c b/tests/unit/s2n_build_test.c index 87001ab3078..5acffb576f7 100644 --- a/tests/unit/s2n_build_test.c +++ b/tests/unit/s2n_build_test.c @@ -65,6 +65,43 @@ S2N_RESULT s2n_test_lowercase_copy(const char *input, char *destination, size_t return S2N_RESULT_OK; } +S2N_RESULT s2n_check_supported_libcrypto(const char *s2n_libcrypto) +{ + RESULT_ENSURE_REF(s2n_libcrypto); + + /* List of supported libcrypto variants we test with */ + const struct { + const char *libcrypto; + bool is_openssl; + } supported_libcrypto[] = { + { .libcrypto = "awslc", .is_openssl = false }, + { .libcrypto = "awslc-fips", .is_openssl = false }, + { .libcrypto = "awslc-fips-2022", .is_openssl = false }, + { .libcrypto = "boringssl", .is_openssl = false }, + { .libcrypto = "libressl", .is_openssl = false }, + { .libcrypto = "openssl-1.0.2", .is_openssl = true }, + { .libcrypto = "openssl-1.0.2-fips", .is_openssl = true }, + { .libcrypto = "openssl-1.1.1", .is_openssl = true }, + { .libcrypto = "openssl-3.0", .is_openssl = true }, + }; + + for (size_t i = 0; i < s2n_array_len(supported_libcrypto); i++) { + /* The linked libcryto is one of the known supported libcrypto variants */ + if (strcmp(s2n_libcrypto, supported_libcrypto[i].libcrypto) == 0) { + if (supported_libcrypto[i].is_openssl) { + EXPECT_TRUE(s2n_libcrypto_is_openssl()); + } else { + EXPECT_FALSE(s2n_libcrypto_is_openssl()); + } + + return S2N_RESULT_OK; + } + } + + /* Testing with an unexpected libcrypto. */ + return S2N_RESULT_ERROR; +} + int main() { BEGIN_TEST(); @@ -131,6 +168,15 @@ int main() } }; + /* Ensure we are testing with supported libcryto variants. + * + * We need to update s2n_libcrypto_is_openssl() when adding support + * for a new libcrypto. + */ + { + EXPECT_OK(s2n_check_supported_libcrypto(s2n_libcrypto)); + }; + END_TEST(); return 0; } diff --git a/tests/unit/s2n_certificate_test.c b/tests/unit/s2n_certificate_test.c index 48797611486..cdc555e23d3 100644 --- a/tests/unit/s2n_certificate_test.c +++ b/tests/unit/s2n_certificate_test.c @@ -1055,5 +1055,14 @@ int main(int argc, char **argv) EXPECT_EQUAL(len, 2); }; + /* cert type count is equal to 3 */ + { + /* The rust bindings have a constant - CHAINS_MAX_COUNT - which must be + * equal to S2N_CERT_TYPE_COUNT. If this test fails, CHAINS_MAX_COUNT in + * config.rs must be updated. + */ + EXPECT_EQUAL(S2N_CERT_TYPE_COUNT, 3); + } + END_TEST(); } diff --git a/tests/unit/s2n_openssl_test.c b/tests/unit/s2n_openssl_test.c index 11b14fa26a8..d48cc8b4bd9 100644 --- a/tests/unit/s2n_openssl_test.c +++ b/tests/unit/s2n_openssl_test.c @@ -26,15 +26,31 @@ int main(int argc, char** argv) END_TEST(); } - if (strcmp(env_libcrypto, "boringssl") == 0) { + /* Confirm "S2N_LIBCRYPTO" env variable matches the linked libcrypto. */ + if (strstr(env_libcrypto, "awslc") != NULL) { + EXPECT_TRUE(s2n_libcrypto_is_awslc()); + EXPECT_FALSE(s2n_libcrypto_is_boringssl()); + EXPECT_FALSE(s2n_libcrypto_is_libressl()); + EXPECT_FALSE(s2n_libcrypto_is_openssl()); + } else if (strcmp(env_libcrypto, "boringssl") == 0) { EXPECT_FALSE(s2n_libcrypto_is_awslc()); EXPECT_TRUE(s2n_libcrypto_is_boringssl()); - } else if (strstr(env_libcrypto, "awslc") != NULL) { - EXPECT_TRUE(s2n_libcrypto_is_awslc()); + EXPECT_FALSE(s2n_libcrypto_is_libressl()); + EXPECT_FALSE(s2n_libcrypto_is_openssl()); + } else if (strcmp(env_libcrypto, "libressl") == 0) { + EXPECT_FALSE(s2n_libcrypto_is_awslc()); EXPECT_FALSE(s2n_libcrypto_is_boringssl()); - } else { + EXPECT_TRUE(s2n_libcrypto_is_libressl()); + EXPECT_FALSE(s2n_libcrypto_is_openssl()); + } else if (strstr(env_libcrypto, "openssl") != NULL) { EXPECT_FALSE(s2n_libcrypto_is_awslc()); EXPECT_FALSE(s2n_libcrypto_is_boringssl()); + EXPECT_FALSE(s2n_libcrypto_is_libressl()); + EXPECT_TRUE(s2n_libcrypto_is_openssl()); + } else if (strcmp(env_libcrypto, "default") == 0) { + /* running with the default libcrypto on path */ + } else { + FAIL_MSG("Testing with an unexpected libcrypto."); } END_TEST(); diff --git a/tests/unit/s2n_override_openssl_random_test.c b/tests/unit/s2n_override_openssl_random_test.c index ed6d13cb54c..5c820912f55 100644 --- a/tests/unit/s2n_override_openssl_random_test.c +++ b/tests/unit/s2n_override_openssl_random_test.c @@ -14,7 +14,6 @@ */ #include -#include #include "api/s2n.h" #include "crypto/s2n_dhe.h" @@ -26,7 +25,6 @@ #include "utils/s2n_random.h" #include "utils/s2n_safety.h" -#if S2N_LIBCRYPTO_SUPPORTS_CUSTOM_RAND const char reference_entropy_hex[] = "00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" "00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" "00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" @@ -70,6 +68,12 @@ int main(int argc, char **argv) uint64_t bytes_used = 0; BEGIN_TEST(); + + if (!s2n_supports_custom_rand()) { + /* Skip when custom rand is not supported */ + END_TEST(); + } + EXPECT_SUCCESS(s2n_disable_tls13_in_test()); EXPECT_NOT_NULL(dhparams_pem = malloc(S2N_MAX_TEST_PEM_SIZE)); @@ -137,14 +141,3 @@ int main(int argc, char **argv) END_TEST(); } - -#else - -int main(int argc, char **argv) -{ - BEGIN_TEST(); - - END_TEST(); -} - -#endif diff --git a/tests/unit/s2n_pq_kem_test.c b/tests/unit/s2n_pq_kem_test.c index 355dbd0b83a..4c6d84f243a 100644 --- a/tests/unit/s2n_pq_kem_test.c +++ b/tests/unit/s2n_pq_kem_test.c @@ -41,7 +41,7 @@ int main() #if defined(OPENSSL_IS_AWSLC) && defined(AWSLC_API_VERSION) /* If using non-FIPS AWS-LC >= v1.6 (API vers. 21), expect Kyber512 KEM from AWS-LC */ - if (!s2n_libcrypto_is_fips() && AWSLC_API_VERSION >= 21) { + if (!s2n_is_in_fips_mode() && AWSLC_API_VERSION >= 21) { EXPECT_TRUE(s2n_libcrypto_supports_evp_kem()); } #endif diff --git a/tests/unit/s2n_random_test.c b/tests/unit/s2n_random_test.c index 802693d6f81..0fbbfb823e8 100644 --- a/tests/unit/s2n_random_test.c +++ b/tests/unit/s2n_random_test.c @@ -48,6 +48,8 @@ #define NUMBER_OF_RANGE_FUNCTION_CALLS 200 #define MAX_REPEATED_OUTPUT 4 +bool s2n_libcrypto_is_fips(void); +bool s2n_libcrypto_is_openssl(void); S2N_RESULT s2n_rand_device_validate(struct s2n_rand_device *device); S2N_RESULT s2n_rand_get_urandom_for_test(struct s2n_rand_device **device); S2N_RESULT s2n_rand_set_urandom_for_test(); @@ -793,25 +795,25 @@ static int s2n_random_rand_bytes_after_cleanup_cb(struct random_test_case *test_ static int s2n_random_rand_bytes_before_init(struct random_test_case *test_case) { -#if S2N_LIBCRYPTO_SUPPORTS_CUSTOM_RAND - /* Calling RAND_bytes will set a global random method */ - unsigned char rndbytes[16] = { 0 }; - EXPECT_EQUAL(RAND_bytes(rndbytes, sizeof(rndbytes)), 1); - const RAND_METHOD *rand_method = RAND_get_rand_method(); - EXPECT_NOT_NULL(rand_method); - EXPECT_NOT_EQUAL(rand_method->bytes, s2n_openssl_compat_rand); - - EXPECT_SUCCESS(s2n_init()); + /* s2n_libcrypto_is_fips() is used since we are testing `s2n_init()` */ + if (s2n_supports_custom_rand() && !s2n_libcrypto_is_fips()) { + /* Calling RAND_bytes will set a global random method */ + unsigned char rndbytes[16] = { 0 }; + EXPECT_EQUAL(RAND_bytes(rndbytes, sizeof(rndbytes)), 1); + const RAND_METHOD *rand_method = RAND_get_rand_method(); + EXPECT_NOT_NULL(rand_method); + EXPECT_NOT_EQUAL((void (*)(void)) rand_method->bytes, (void (*)(void)) s2n_openssl_compat_rand); - /* The global random method is overridden after calling s2n_init() */ - const RAND_METHOD *custom_rand_method = RAND_get_rand_method(); - EXPECT_NOT_NULL(custom_rand_method); - EXPECT_EQUAL(custom_rand_method->bytes, s2n_openssl_compat_rand); + EXPECT_SUCCESS(s2n_init()); - /* RAND_bytes is still successful */ - EXPECT_EQUAL(RAND_bytes(rndbytes, sizeof(rndbytes)), 1); + /* The global random method is overridden after calling s2n_init() */ + const RAND_METHOD *custom_rand_method = RAND_get_rand_method(); + EXPECT_NOT_NULL(custom_rand_method); + EXPECT_EQUAL((void (*)(void)) custom_rand_method->bytes, (void (*)(void)) s2n_openssl_compat_rand); -#endif + /* RAND_bytes is still successful */ + EXPECT_EQUAL(RAND_bytes(rndbytes, sizeof(rndbytes)), 1); + } return S2N_SUCCESS; } @@ -894,6 +896,29 @@ int main(int argc, char **argv) { BEGIN_TEST_NO_INIT(); + /* Feature probe: Negative test */ + { + if (s2n_libcrypto_is_awslc()) { +#if defined(S2N_LIBCRYPTO_SUPPORTS_ENGINE) + FAIL_MSG("Expected ENGINE feature probe to be disabled with AWS-LC"); +#endif + } + }; + + /* Feature probe: Positive test + * + * TODO: Test missing due to unrelated feature probe failure on AL2. + * https://github.com/aws/s2n-tls/issues/4900 + */ + + /* s2n_supports_custom_rand */ + { + if (s2n_supports_custom_rand()) { + EXPECT_TRUE(s2n_libcrypto_is_openssl()); + EXPECT_FALSE(s2n_is_in_fips_mode()); + } + }; + /* For each test case, creates a child process that runs the test case. * * Fork detection is lazily initialised on first invocation of diff --git a/tests/unit/s2n_security_policies_test.c b/tests/unit/s2n_security_policies_test.c index efaf6901642..a4ae4e515c2 100644 --- a/tests/unit/s2n_security_policies_test.c +++ b/tests/unit/s2n_security_policies_test.c @@ -151,9 +151,12 @@ int main(int argc, char **argv) for (size_t i = 0; i < security_policy->signature_preferences->count; i++) { int min = security_policy->signature_preferences->signature_schemes[i]->minimum_protocol_version; int max = security_policy->signature_preferences->signature_schemes[i]->maximum_protocol_version; + if (max == S2N_UNKNOWN_PROTOCOL_VERSION) { + max = S2N_TLS13; + } s2n_signature_algorithm sig_alg = security_policy->signature_preferences->signature_schemes[i]->sig_alg; - if (min == S2N_TLS13 || max >= S2N_TLS13) { + if (min <= S2N_TLS13 && max >= S2N_TLS13) { has_tls_13_sig_alg = true; } @@ -825,8 +828,8 @@ int main(int argc, char **argv) } } - /* If scheme will be used for pre-tls1.3 */ - if (min_version < S2N_TLS13) { + /* If scheme will be used for legacy versions */ + if (min_version < S2N_TLS12) { EXPECT_NOT_EQUAL(scheme->sig_alg, S2N_SIGNATURE_RSA_PSS_PSS); } } diff --git a/tests/unit/s2n_self_talk_certificates_test.c b/tests/unit/s2n_self_talk_certificates_test.c new file mode 100644 index 00000000000..c3e8fb34d8a --- /dev/null +++ b/tests/unit/s2n_self_talk_certificates_test.c @@ -0,0 +1,141 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +#include "crypto/s2n_rsa_pss.h" +#include "s2n_test.h" +#include "testlib/s2n_testlib.h" +#include "tls/s2n_tls.h" + +static uint8_t s2n_noop_verify_host_fn(const char *name, size_t len, void *data) +{ + return 1; +} + +static S2N_RESULT s2n_test_load_certificate(struct s2n_cert_chain_and_key **chain_out, + char *chain_pem_out, const char *chain_pem_path, const char *key_pem_path) +{ + RESULT_ENSURE_REF(chain_out); + + char key_pem[S2N_MAX_TEST_PEM_SIZE] = { 0 }; + RESULT_GUARD_POSIX(s2n_read_test_pem(chain_pem_path, chain_pem_out, S2N_MAX_TEST_PEM_SIZE)); + RESULT_GUARD_POSIX(s2n_read_test_pem(key_pem_path, key_pem, S2N_MAX_TEST_PEM_SIZE)); + + *chain_out = s2n_cert_chain_and_key_new(); + RESULT_ENSURE_REF(*chain_out); + RESULT_GUARD_POSIX(s2n_cert_chain_and_key_load_pem(*chain_out, chain_pem_out, key_pem)); + + return S2N_RESULT_OK; +} + +int main(int argc, char **argv) +{ + BEGIN_TEST(); + + if (!s2n_is_rsa_pss_certs_supported() || !s2n_is_tls13_fully_supported()) { + END_TEST(); + } + + const uint8_t test_versions[] = { S2N_TLS13, S2N_TLS12, S2N_TLS11, S2N_TLS10 }; + struct { + const char *cert; + const char *key; + uint8_t min_version; + } test_certs[] = { + { + .cert = S2N_RSA_2048_PKCS1_SHA256_CERT_CHAIN, + .key = S2N_RSA_2048_PKCS1_SHA256_CERT_KEY, + }, + { + .cert = S2N_ECDSA_P384_PKCS1_CERT_CHAIN, + .key = S2N_ECDSA_P384_PKCS1_KEY, + }, + { + .cert = S2N_ECDSA_P512_CERT_CHAIN, + .key = S2N_ECDSA_P512_KEY, + }, + { + .cert = S2N_RSA_PSS_2048_SHA256_LEAF_CERT, + .key = S2N_RSA_PSS_2048_SHA256_LEAF_KEY, + .min_version = S2N_TLS12, + }, + }; + + for (size_t cert_i = 0; cert_i < s2n_array_len(test_certs); cert_i++) { + DEFER_CLEANUP(struct s2n_cert_chain_and_key *chain = NULL, + s2n_cert_chain_and_key_ptr_free); + char pem[S2N_MAX_TEST_PEM_SIZE] = { 0 }; + EXPECT_OK(s2n_test_load_certificate(&chain, pem, + test_certs[cert_i].cert, test_certs[cert_i].key)); + + for (size_t version_i = 0; version_i < s2n_array_len(test_versions); version_i++) { + uint8_t version = test_versions[version_i]; + bool expect_success = (version >= test_certs[cert_i].min_version); + + /* We intentionally use the default policies. + * The default policies should support all certificate types. + */ + const char *security_policy = "default"; + if (version >= S2N_TLS13) { + security_policy = "default_tls13"; + } else if (version < S2N_TLS12) { + /* The default policies don't support legacy versions */ + security_policy = "test_all"; + } + + DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free); + EXPECT_NOT_NULL(config); + EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(config, chain)); + EXPECT_SUCCESS(s2n_config_add_pem_to_trust_store(config, pem)); + EXPECT_SUCCESS(s2n_config_set_verify_host_callback(config, s2n_noop_verify_host_fn, NULL)); + EXPECT_SUCCESS(s2n_config_set_max_blinding_delay(config, 0)); + EXPECT_SUCCESS(s2n_config_set_cipher_preferences(config, security_policy)); + + DEFER_CLEANUP(struct s2n_connection *server = s2n_connection_new(S2N_SERVER), + s2n_connection_ptr_free); + EXPECT_NOT_NULL(server); + EXPECT_SUCCESS(s2n_connection_set_config(server, config)); + server->server_protocol_version = version; + + DEFER_CLEANUP(struct s2n_connection *client = s2n_connection_new(S2N_CLIENT), + s2n_connection_ptr_free); + EXPECT_NOT_NULL(client); + EXPECT_SUCCESS(s2n_connection_set_config(client, config)); + client->client_protocol_version = version; + + DEFER_CLEANUP(struct s2n_test_io_pair io_pair = { 0 }, s2n_io_pair_close); + EXPECT_SUCCESS(s2n_io_pair_init_non_blocking(&io_pair)); + EXPECT_SUCCESS(s2n_connections_set_io_pair(client, server, &io_pair)); + + bool handshake_success = + (s2n_negotiate_test_server_and_client(server, client) == S2N_SUCCESS); + if (handshake_success) { + EXPECT_EQUAL(server->actual_protocol_version, version); + EXPECT_EQUAL(client->actual_protocol_version, version); + } + + const char *error_message = "Handshake failed"; + if (!expect_success) { + error_message = "Handshake unexpectedly succeeded"; + } + if (handshake_success != expect_success) { + fprintf(stderr, "%s version=%i cert=%s\n", + error_message, version, test_certs[cert_i].cert); + FAIL_MSG(error_message); + } + } + } + + END_TEST(); +} diff --git a/tls/s2n_signature_algorithms.c b/tls/s2n_signature_algorithms.c index bfb5b6b7d22..2710bed1a09 100644 --- a/tls/s2n_signature_algorithms.c +++ b/tls/s2n_signature_algorithms.c @@ -76,8 +76,6 @@ static S2N_RESULT s2n_signature_scheme_validate_for_recv(struct s2n_connection * if (conn->actual_protocol_version >= S2N_TLS13) { RESULT_ENSURE_NE(scheme->hash_alg, S2N_HASH_SHA1); RESULT_ENSURE_NE(scheme->sig_alg, S2N_SIGNATURE_RSA); - } else { - RESULT_ENSURE_NE(scheme->sig_alg, S2N_SIGNATURE_RSA_PSS_PSS); } return S2N_RESULT_OK; diff --git a/tls/s2n_signature_scheme.c b/tls/s2n_signature_scheme.c index 498085e602b..93ca30fb602 100644 --- a/tls/s2n_signature_scheme.c +++ b/tls/s2n_signature_scheme.c @@ -181,7 +181,7 @@ const struct s2n_signature_scheme s2n_rsa_pss_pss_sha256 = { .sig_alg = S2N_SIGNATURE_RSA_PSS_PSS, .libcrypto_nid = NID_rsassaPss, .signature_curve = NULL, /* Elliptic Curve not needed for RSA */ - .minimum_protocol_version = S2N_TLS13, + .minimum_protocol_version = S2N_TLS12, }; const struct s2n_signature_scheme s2n_rsa_pss_pss_sha384 = { @@ -191,7 +191,7 @@ const struct s2n_signature_scheme s2n_rsa_pss_pss_sha384 = { .sig_alg = S2N_SIGNATURE_RSA_PSS_PSS, .libcrypto_nid = NID_rsassaPss, .signature_curve = NULL, /* Elliptic Curve not needed for RSA */ - .minimum_protocol_version = S2N_TLS13, + .minimum_protocol_version = S2N_TLS12, }; const struct s2n_signature_scheme s2n_rsa_pss_pss_sha512 = { @@ -201,7 +201,7 @@ const struct s2n_signature_scheme s2n_rsa_pss_pss_sha512 = { .sig_alg = S2N_SIGNATURE_RSA_PSS_PSS, .libcrypto_nid = NID_rsassaPss, .signature_curve = NULL, /* Elliptic Curve not needed for RSA */ - .minimum_protocol_version = S2N_TLS13, + .minimum_protocol_version = S2N_TLS12, }; /* Chosen based on AWS server recommendations as of 05/24. diff --git a/utils/s2n_random.c b/utils/s2n_random.c index ac8f9817b1f..233a76b3060 100644 --- a/utils/s2n_random.c +++ b/utils/s2n_random.c @@ -47,7 +47,12 @@ #endif #include #include -#include +#if S2N_LIBCRYPTO_SUPPORTS_ENGINE + #include +#endif +/* LibreSSL requires include. + * https://github.com/aws/s2n-tls/issues/153#issuecomment-129651643 + */ #include #include #include @@ -76,6 +81,8 @@ #include "utils/s2n_result.h" #include "utils/s2n_safety.h" +const char s2n_rand_engine_id[] = "s2n_rand"; + #if defined(O_CLOEXEC) #define ENTROPY_FLAGS O_RDONLY | O_CLOEXEC #else @@ -492,10 +499,6 @@ S2N_RESULT s2n_public_random(int64_t bound, uint64_t *output) } } -#if S2N_LIBCRYPTO_SUPPORTS_CUSTOM_RAND - - #define S2N_RAND_ENGINE_ID "s2n_rand" - int s2n_openssl_compat_rand(unsigned char *buf, int num) { struct s2n_blob out = { 0 }; @@ -512,11 +515,18 @@ int s2n_openssl_compat_status(void) return 1; } +#if S2N_LIBCRYPTO_SUPPORTS_ENGINE int s2n_openssl_compat_init(ENGINE *unused) { return 1; } +/* RAND_METHOD is gated behind S2N_LIBCRYPTO_SUPPORTS_ENGINE because AWS-LC has + * a different signature for RAND_METHOD and fails to compile. + * + * - AWS-LC: https://github.com/aws/aws-lc/blob/main/include/openssl/rand.h#L124 + * - OpenSSL: https://github.com/openssl/openssl/blob/master/include/openssl/rand.h#L42 + */ RAND_METHOD s2n_openssl_rand_method = { .seed = NULL, .bytes = s2n_openssl_compat_rand, @@ -542,38 +552,46 @@ static int s2n_rand_init_cb_impl(void) return S2N_SUCCESS; } +bool s2n_supports_custom_rand(void) +{ +#if !defined(S2N_LIBCRYPTO_SUPPORTS_ENGINE) + return false; +#else + return s2n_libcrypto_is_openssl() && !s2n_is_in_fips_mode(); +#endif +} + S2N_RESULT s2n_rand_init(void) { RESULT_ENSURE(s2n_rand_init_cb() >= S2N_SUCCESS, S2N_ERR_CANCELLED); RESULT_GUARD(s2n_ensure_initialized_drbgs()); -#if S2N_LIBCRYPTO_SUPPORTS_CUSTOM_RAND - if (s2n_is_in_fips_mode()) { - return S2N_RESULT_OK; +#if S2N_LIBCRYPTO_SUPPORTS_ENGINE + if (s2n_supports_custom_rand()) { + /* Unset any existing random engine */ + RESULT_GUARD_OSSL(RAND_set_rand_engine(NULL), S2N_ERR_OPEN_RANDOM); + + /* Create an engine */ + ENGINE *e = ENGINE_new(); + + /* Initialize the engine */ + RESULT_ENSURE(e != NULL, S2N_ERR_OPEN_RANDOM); + RESULT_GUARD_OSSL(ENGINE_set_id(e, s2n_rand_engine_id), S2N_ERR_OPEN_RANDOM); + RESULT_GUARD_OSSL(ENGINE_set_name(e, "s2n entropy generator"), S2N_ERR_OPEN_RANDOM); + RESULT_GUARD_OSSL(ENGINE_set_flags(e, ENGINE_FLAGS_NO_REGISTER_ALL), S2N_ERR_OPEN_RANDOM); + RESULT_GUARD_OSSL(ENGINE_set_init_function(e, s2n_openssl_compat_init), S2N_ERR_OPEN_RANDOM); + RESULT_GUARD_OSSL(ENGINE_set_RAND(e, &s2n_openssl_rand_method), S2N_ERR_OPEN_RANDOM); + RESULT_GUARD_OSSL(ENGINE_add(e), S2N_ERR_OPEN_RANDOM); + RESULT_GUARD_OSSL(ENGINE_free(e), S2N_ERR_OPEN_RANDOM); + + /* Use that engine for rand() */ + e = ENGINE_by_id(s2n_rand_engine_id); + RESULT_ENSURE(e != NULL, S2N_ERR_OPEN_RANDOM); + RESULT_GUARD_OSSL(ENGINE_init(e), S2N_ERR_OPEN_RANDOM); + RESULT_GUARD_OSSL(ENGINE_set_default(e, ENGINE_METHOD_RAND), S2N_ERR_OPEN_RANDOM); + RESULT_GUARD_OSSL(ENGINE_free(e), S2N_ERR_OPEN_RANDOM); } - - /* Unset any existing random engine */ - RESULT_GUARD_OSSL(RAND_set_rand_engine(NULL), S2N_ERR_OPEN_RANDOM); - - /* Create an engine */ - ENGINE *e = ENGINE_new(); - - RESULT_ENSURE(e != NULL, S2N_ERR_OPEN_RANDOM); - RESULT_GUARD_OSSL(ENGINE_set_id(e, S2N_RAND_ENGINE_ID), S2N_ERR_OPEN_RANDOM); - RESULT_GUARD_OSSL(ENGINE_set_name(e, "s2n entropy generator"), S2N_ERR_OPEN_RANDOM); - RESULT_GUARD_OSSL(ENGINE_set_flags(e, ENGINE_FLAGS_NO_REGISTER_ALL), S2N_ERR_OPEN_RANDOM); - RESULT_GUARD_OSSL(ENGINE_set_init_function(e, s2n_openssl_compat_init), S2N_ERR_OPEN_RANDOM); - RESULT_GUARD_OSSL(ENGINE_set_RAND(e, &s2n_openssl_rand_method), S2N_ERR_OPEN_RANDOM); - RESULT_GUARD_OSSL(ENGINE_add(e), S2N_ERR_OPEN_RANDOM); - RESULT_GUARD_OSSL(ENGINE_free(e), S2N_ERR_OPEN_RANDOM); - - /* Use that engine for rand() */ - e = ENGINE_by_id(S2N_RAND_ENGINE_ID); - RESULT_ENSURE(e != NULL, S2N_ERR_OPEN_RANDOM); - RESULT_GUARD_OSSL(ENGINE_init(e), S2N_ERR_OPEN_RANDOM); - RESULT_GUARD_OSSL(ENGINE_set_default(e, ENGINE_METHOD_RAND), S2N_ERR_OPEN_RANDOM); - RESULT_GUARD_OSSL(ENGINE_free(e), S2N_ERR_OPEN_RANDOM); #endif return S2N_RESULT_OK; @@ -595,17 +613,19 @@ S2N_RESULT s2n_rand_cleanup(void) { RESULT_ENSURE(s2n_rand_cleanup_cb() >= S2N_SUCCESS, S2N_ERR_CANCELLED); -#if S2N_LIBCRYPTO_SUPPORTS_CUSTOM_RAND - /* Cleanup our rand ENGINE in libcrypto */ - ENGINE *rand_engine = ENGINE_by_id(S2N_RAND_ENGINE_ID); - if (rand_engine) { - ENGINE_remove(rand_engine); - ENGINE_finish(rand_engine); - ENGINE_unregister_RAND(rand_engine); - ENGINE_free(rand_engine); - ENGINE_cleanup(); - RAND_set_rand_engine(NULL); - RAND_set_rand_method(NULL); +#if S2N_LIBCRYPTO_SUPPORTS_ENGINE + if (s2n_supports_custom_rand()) { + /* Cleanup our rand ENGINE in libcrypto */ + ENGINE *rand_engine = ENGINE_by_id(s2n_rand_engine_id); + if (rand_engine) { + ENGINE_remove(rand_engine); + ENGINE_finish(rand_engine); + ENGINE_unregister_RAND(rand_engine); + ENGINE_free(rand_engine); + ENGINE_cleanup(); + RAND_set_rand_engine(NULL); + RAND_set_rand_method(NULL); + } } #endif diff --git a/utils/s2n_random.h b/utils/s2n_random.h index 5284e6271a8..2d093804948 100644 --- a/utils/s2n_random.h +++ b/utils/s2n_random.h @@ -28,6 +28,7 @@ struct s2n_rand_device { dev_t rdev; }; +bool s2n_supports_custom_rand(void); S2N_RESULT s2n_rand_init(void); S2N_RESULT s2n_rand_cleanup(void); S2N_RESULT s2n_get_seed_entropy(struct s2n_blob *blob);