Merge branch 'main' into nix_libcrypto_helpers

aws · Mar 14, 2024 · 40ea526 · 40ea526
2 parents 8eda2a1 + ec12b52
commit 40ea526
Show file tree

Hide file tree

Showing 41 changed files with 250 additions and 146 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -493,9 +493,7 @@ if (BUILD_TESTING)
         string(REGEX REPLACE ".+\\/(.+)\\.c" "\\1" test_case_name ${test_case})
 
         add_executable(${test_case_name} ${test_case})
-        target_include_directories(${test_case_name} PRIVATE api)
         target_include_directories(${test_case_name} PRIVATE ./)
-        target_include_directories(${test_case_name} PRIVATE tests)
         target_link_libraries(${test_case_name} PRIVATE testss2n)
         if (S2N_INTERN_LIBCRYPTO)
             # if libcrypto was interned, rewrite libcrypto symbols so use of internal functions will link correctly
@@ -521,15 +519,11 @@ if (BUILD_TESTING)
 
     add_executable(s2nc "bin/s2nc.c" "bin/echo.c" "bin/https.c" "bin/common.c")
     target_link_libraries(s2nc ${PROJECT_NAME})
-
-    target_include_directories(s2nc PRIVATE api)
     target_compile_options(s2nc PRIVATE -std=gnu99)
 
 
     add_executable(s2nd "bin/s2nd.c" "bin/echo.c" "bin/https.c" "bin/common.c")
     target_link_libraries(s2nd ${PROJECT_NAME})
-
-    target_include_directories(s2nd PRIVATE api)
     target_compile_options(s2nd PRIVATE -std=gnu99)
 
     if(S2N_LTO)

diff --git a/api/s2n.h b/api/s2n.h
@@ -241,6 +241,27 @@ S2N_API extern int s2n_init(void);
  */
 S2N_API extern int s2n_cleanup(void);
 
+typedef enum {
+    S2N_FIPS_MODE_DISABLED = 0,
+    S2N_FIPS_MODE_ENABLED,
+} s2n_fips_mode;
+
+/**
+ * Determines whether s2n-tls is operating in FIPS mode.
+ *
+ * s2n-tls enters FIPS mode on initialization when the linked libcrypto has FIPS mode enabled. Some
+ * libcryptos, such as AWS-LC-FIPS, have FIPS mode enabled by default. With other libcryptos, such
+ * as OpenSSL, FIPS mode must be enabled before initialization by calling `FIPS_mode_set()`.
+ *
+ * s2n-tls MUST be linked to a FIPS libcrypto and MUST be in FIPS mode in order to comply with FIPS
+ * requirements. Applications desiring FIPS compliance should use this API to ensure that s2n-tls
+ * has been properly linked with a FIPS libcrypto and has successfully entered FIPS mode.
+ *
+ * @param fips_mode Set to the FIPS mode of s2n-tls.
+ * @returns S2N_SUCCESS on success. S2N_FAILURE on failure.
+ */
+S2N_API extern int s2n_get_fips_mode(s2n_fips_mode *fips_mode);
+
 /**
  * Creates a new s2n_config object. This object can (and should) be associated with many connection
  * objects.
@@ -1882,6 +1903,30 @@ S2N_API extern uint64_t s2n_connection_get_delay(struct s2n_connection *conn);
  */
 S2N_API extern int s2n_connection_set_cipher_preferences(struct s2n_connection *conn, const char *version);
 
+/**
+ * Used to indicate the type of key update that is being requested. For further 
+ * information refer to `s2n_connection_request_key_update`.
+*/
+typedef enum {
+    S2N_KEY_UPDATE_NOT_REQUESTED = 0,
+    S2N_KEY_UPDATE_REQUESTED
+} s2n_peer_key_update;
+
+/**
+ * Signals the connection to do a key_update at the next possible opportunity. Note that the resulting key update message
+ * will not be sent until `s2n_send` is called.
+ * 
+ * @param conn The connection object to trigger the key update on.
+ * @param peer_request Indicates if a key update should also be requested 
+ * of the peer. When set to `S2N_KEY_UPDATE_NOT_REQUESTED`, then only the sending
+ * key of `conn` will be updated. If set to `S2N_KEY_UPDATE_REQUESTED`, then 
+ * the sending key of conn will be updated AND the peer will be requested to 
+ * update their sending key. Note that s2n-tls currently only supports 
+ * `peer_request` being set to `S2N_KEY_UPDATE_NOT_REQUESTED` and will return
+ *  S2N_FAILURE if any other value is used.
+ * @returns S2N_SUCCESS on success. S2N_FAILURE on failure
+*/
+S2N_API extern int s2n_connection_request_key_update(struct s2n_connection *conn, s2n_peer_key_update peer_request);
 /**
  * Appends the provided application protocol to the preference list
  *

diff --git a/bindings/rust/s2n-tls/src/config.rs b/bindings/rust/s2n-tls/src/config.rs
@@ -151,7 +151,6 @@ impl Drop for Config {
     }
 }
 
-#[derive(Default)]
 pub struct Builder {
     config: Config,
     load_system_certs: bool,
@@ -743,6 +742,12 @@ impl Builder {
     }
 }
 
+impl Default for Builder {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 pub(crate) struct Context {
     refcount: AtomicUsize,
     pub(crate) client_hello_callback: Option<Box<dyn ClientHelloCallback>>,

diff --git a/bindings/rust/s2n-tls/src/testing/s2n_tls.rs b/bindings/rust/s2n-tls/src/testing/s2n_tls.rs
@@ -756,32 +756,36 @@ mod tests {
         // Load the server certificate into the trust store by overriding the OpenSSL default
         // certificate location.
         temp_env::with_var("SSL_CERT_FILE", Some(keypair.cert_path()), || {
-            let mut builder = Builder::new();
-            builder
-                .load_pem(keypair.cert(), keypair.key())
-                .unwrap()
-                .set_security_policy(&security::DEFAULT_TLS13)
-                .unwrap()
-                .set_verify_host_callback(InsecureAcceptAllCertificatesHandler {})
-                .unwrap();
+            // Test the Builder itself, and also the Builder produced by the Config builder() API.
+            for mut builder in [Builder::new(), Config::builder()] {
+                builder
+                    .load_pem(keypair.cert(), keypair.key())
+                    .unwrap()
+                    .set_security_policy(&security::DEFAULT_TLS13)
+                    .unwrap()
+                    .set_verify_host_callback(InsecureAcceptAllCertificatesHandler {})
+                    .unwrap();
 
-            // Disable loading system certificates
-            builder.with_system_certs(false).unwrap();
+                // Disable loading system certificates
+                builder.with_system_certs(false).unwrap();
 
-            let config = builder.build().unwrap();
-            let mut config_with_system_certs = config.clone();
+                let config = builder.build().unwrap();
+                let mut config_with_system_certs = config.clone();
 
-            let mut pair = tls_pair(config);
+                let mut pair = tls_pair(config);
 
-            // System certificates should not be loaded into the trust store. The handshake
-            // should fail since the certificate should not be trusted.
-            assert!(poll_tls_pair_result(&mut pair).is_err());
+                // System certificates should not be loaded into the trust store. The handshake
+                // should fail since the certificate should not be trusted.
+                assert!(poll_tls_pair_result(&mut pair).is_err());
 
-            // The handshake should succeed after trusting the certificate.
-            unsafe {
-                s2n_tls_sys::s2n_config_load_system_certs(config_with_system_certs.as_mut_ptr());
+                // The handshake should succeed after trusting the certificate.
+                unsafe {
+                    s2n_tls_sys::s2n_config_load_system_certs(
+                        config_with_system_certs.as_mut_ptr(),
+                    );
+                }
+                establish_connection(config_with_system_certs);
             }
-            establish_connection(config_with_system_certs);
         });
     }
 

diff --git a/crypto/s2n_fips.c b/crypto/s2n_fips.c
@@ -17,11 +17,14 @@
 
 #include <openssl/crypto.h>
 
+#include "utils/s2n_init.h"
+#include "utils/s2n_safety.h"
+
 #if defined(S2N_INTERN_LIBCRYPTO) && defined(OPENSSL_FIPS)
     #error "Interning with OpenSSL fips-validated libcrypto is not currently supported. See https://github.com/aws/s2n-tls/issues/2741"
 #endif
 
-static int s2n_fips_mode = 0;
+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.
  *
@@ -46,17 +49,25 @@ bool s2n_libcrypto_is_fips(void)
 
 int s2n_fips_init(void)
 {
-    s2n_fips_mode = 0;
-
-    if (s2n_libcrypto_is_fips()) {
-        s2n_fips_mode = 1;
-    }
-
-    return 0;
+    s2n_fips_mode_enabled = s2n_libcrypto_is_fips();
+    return S2N_SUCCESS;
 }
 
 /* Return 1 if FIPS mode is enabled, 0 otherwise. FIPS mode must be enabled prior to calling s2n_init(). */
-int s2n_is_in_fips_mode(void)
+bool s2n_is_in_fips_mode(void)
+{
+    return s2n_fips_mode_enabled;
+}
+
+int s2n_get_fips_mode(s2n_fips_mode *fips_mode)
 {
-    return s2n_fips_mode;
+    POSIX_ENSURE_REF(fips_mode);
+    *fips_mode = S2N_FIPS_MODE_DISABLED;
+    POSIX_ENSURE(s2n_is_initialized(), S2N_ERR_NOT_INITIALIZED);
+
+    if (s2n_is_in_fips_mode()) {
+        *fips_mode = S2N_FIPS_MODE_ENABLED;
+    }
+
+    return S2N_SUCCESS;
 }
diff --git a/crypto/s2n_fips.h b/crypto/s2n_fips.h
@@ -21,7 +21,7 @@
 #pragma once
 
 int s2n_fips_init(void);
-int s2n_is_in_fips_mode(void);
+bool s2n_is_in_fips_mode(void);
 bool s2n_libcrypto_is_fips(void);
 
 struct s2n_cipher_suite;

diff --git a/crypto/s2n_pkey.c b/crypto/s2n_pkey.c
@@ -115,7 +115,11 @@ int s2n_pkey_match(const struct s2n_pkey *pub_key, const struct s2n_pkey *priv_k
 
 int s2n_pkey_free(struct s2n_pkey *key)
 {
-    if (key != NULL && key->free != NULL) {
+    if (key == NULL) {
+        return S2N_SUCCESS;
+    }
+
+    if (key->free != NULL) {
         POSIX_GUARD(key->free(key));
     }
 

diff --git a/scripts/s2n_safety_macros.py b/scripts/s2n_safety_macros.py
@@ -539,7 +539,7 @@ def cmp_check(op):
         * The size of the data pointed to by both the `destination` and `source` parameters,
           shall be at least `len` bytes.
         ''',
-        impl='__S2N_ENSURE_SAFE_MEMCPY((destination), (source), (len), {prefix}ENSURE_REF)',
+        impl='__S2N_ENSURE_SAFE_MEMMOVE((destination), (source), (len), {prefix}ENSURE_REF)',
         harness='''
         static {ret} {prefix}CHECKED_MEMCPY_harness(uint32_t* dest, uint32_t* source, size_t len)
         {{

diff --git a/stuffer/s2n_stuffer_network_order.c b/stuffer/s2n_stuffer_network_order.c
@@ -21,10 +21,14 @@
 /* Writes length bytes of input to stuffer, in network order, starting from the smallest byte of input. */
 int s2n_stuffer_write_network_order(struct s2n_stuffer *stuffer, const uint64_t input, const uint8_t length)
 {
+    if (length == 0) {
+        return S2N_SUCCESS;
+    }
+    POSIX_ENSURE_REF(stuffer);
     POSIX_ENSURE(length <= sizeof(input), S2N_ERR_SAFETY);
     POSIX_GUARD(s2n_stuffer_skip_write(stuffer, length));
-    uint8_t *data = (stuffer->blob.data) ? (stuffer->blob.data + stuffer->write_cursor - length) : NULL;
-
+    POSIX_ENSURE_REF(stuffer->blob.data);
+    uint8_t *data = stuffer->blob.data + stuffer->write_cursor - length;
     for (int i = 0; i < length; i++) {
         S2N_INVARIANT(i <= length);
         uint8_t shift = (length - i - 1) * CHAR_BIT;

diff --git a/tests/cbmc/proofs/s2n_alloc/Makefile b/tests/cbmc/proofs/s2n_alloc/Makefile
@@ -40,7 +40,7 @@ PROJECT_SOURCES += $(SRCDIR)/utils/s2n_result.c
 # We abstract these functions because manual inspection demonstrates they are unreachable.
 REMOVE_FUNCTION_BODY += __CPROVER_file_local_s2n_mem_c_s2n_mem_cleanup_impl
 REMOVE_FUNCTION_BODY += s2n_blob_slice
-REMOVE_FUNCTION_BODY += s2n_ensure_memcpy_trace
+REMOVE_FUNCTION_BODY += s2n_ensure_memmove_trace
 
 UNWINDSET +=
 

diff --git a/tests/cbmc/proofs/s2n_array_init/Makefile b/tests/cbmc/proofs/s2n_array_init/Makefile
@@ -35,7 +35,7 @@ PROJECT_SOURCES += $(SRCDIR)/utils/s2n_safety.c
 
 # We abstract these functions because manual inspection demonstrates they are unreachable.
 REMOVE_FUNCTION_BODY += s2n_blob_slice
-REMOVE_FUNCTION_BODY += s2n_ensure_memcpy_trace
+REMOVE_FUNCTION_BODY += s2n_ensure_memmove_trace
 REMOVE_FUNCTION_BODY += __CPROVER_file_local_s2n_mem_c_s2n_mem_cleanup_impl
 
 UNWINDSET +=

diff --git a/tests/cbmc/proofs/s2n_array_new/Makefile b/tests/cbmc/proofs/s2n_array_new/Makefile
@@ -38,7 +38,7 @@ PROJECT_SOURCES += $(SRCDIR)/utils/s2n_safety.c
 
 # We abstract these functions because manual inspection demonstrates they are unreachable.
 REMOVE_FUNCTION_BODY += s2n_blob_slice
-REMOVE_FUNCTION_BODY += s2n_ensure_memcpy_trace
+REMOVE_FUNCTION_BODY += s2n_ensure_memmove_trace
 REMOVE_FUNCTION_BODY += __CPROVER_file_local_s2n_mem_c_s2n_mem_cleanup_impl
 
 UNWINDSET +=

diff --git a/tests/cbmc/proofs/s2n_dh_compute_shared_secret_as_client/Makefile b/tests/cbmc/proofs/s2n_dh_compute_shared_secret_as_client/Makefile
@@ -39,7 +39,7 @@ PROJECT_SOURCES += $(SRCDIR)/utils/s2n_mem.c
 PROJECT_SOURCES += $(SRCDIR)/utils/s2n_result.c
 PROJECT_SOURCES += $(SRCDIR)/utils/s2n_safety.c
 
-UNWINDSET += s2n_stuffer_write_network_order.4:9
+UNWINDSET += s2n_stuffer_write_network_order.10:9
 
 # We abstract this function because manual inspection demonstrates it is unreachable.
 REMOVE_FUNCTION_BODY += __CPROVER_file_local_s2n_mem_c_s2n_mem_cleanup_impl

diff --git a/tests/cbmc/proofs/s2n_dh_params_to_p_g_Ys/Makefile b/tests/cbmc/proofs/s2n_dh_params_to_p_g_Ys/Makefile
@@ -42,6 +42,6 @@ REMOVE_FUNCTION_BODY += s2n_free
 REMOVE_FUNCTION_BODY += s2n_realloc
 REMOVE_FUNCTION_BODY += s2n_stuffer_wipe
 
-UNWINDSET += s2n_stuffer_write_network_order.4:9
+UNWINDSET += s2n_stuffer_write_network_order.10:9
 
 include ../Makefile.common
diff --git a/tests/cbmc/proofs/s2n_set_new/Makefile b/tests/cbmc/proofs/s2n_set_new/Makefile
@@ -42,7 +42,7 @@ PROJECT_SOURCES += $(SRCDIR)/utils/s2n_set.c
 
 # We abstract these functions because manual inspection demonstrates they are unreachable.
 REMOVE_FUNCTION_BODY += s2n_blob_slice
-REMOVE_FUNCTION_BODY += s2n_ensure_memcpy_trace
+REMOVE_FUNCTION_BODY += s2n_ensure_memmove_trace
 REMOVE_FUNCTION_BODY += __CPROVER_file_local_s2n_mem_c_s2n_mem_cleanup_impl
 
 UNWINDSET +=

diff --git a/tests/cbmc/proofs/s2n_stuffer_alloc/Makefile b/tests/cbmc/proofs/s2n_stuffer_alloc/Makefile
@@ -40,7 +40,7 @@ PROJECT_SOURCES += $(SRCDIR)/utils/s2n_safety.c
 # We abstract these functions because manual inspection demonstrates they are unreachable.
 REMOVE_FUNCTION_BODY += __CPROVER_file_local_s2n_mem_c_s2n_mem_cleanup_impl
 REMOVE_FUNCTION_BODY += s2n_blob_slice
-REMOVE_FUNCTION_BODY += s2n_ensure_memcpy_trace
+REMOVE_FUNCTION_BODY += s2n_ensure_memmove_trace
 
 UNWINDSET +=
 

diff --git a/tests/cbmc/proofs/s2n_stuffer_growable_alloc/Makefile b/tests/cbmc/proofs/s2n_stuffer_growable_alloc/Makefile
@@ -38,7 +38,7 @@ PROJECT_SOURCES += $(SRCDIR)/utils/s2n_safety.c
 # We abstract these functions because manual inspection demonstrates they are unreachable.
 REMOVE_FUNCTION_BODY += __CPROVER_file_local_s2n_mem_c_s2n_mem_cleanup_impl
 REMOVE_FUNCTION_BODY += s2n_blob_slice
-REMOVE_FUNCTION_BODY += s2n_ensure_memcpy_trace
+REMOVE_FUNCTION_BODY += s2n_ensure_memmove_trace
 
 UNWINDSET +=
 

diff --git a/tests/cbmc/proofs/s2n_stuffer_resize_if_empty/Makefile b/tests/cbmc/proofs/s2n_stuffer_resize_if_empty/Makefile
@@ -35,7 +35,7 @@ PROJECT_SOURCES += $(SRCDIR)/utils/s2n_result.c
 # We abstract these functions because manual inspection demonstrates they are unreachable.
 REMOVE_FUNCTION_BODY += s2n_calculate_stacktrace
 REMOVE_FUNCTION_BODY += s2n_blob_slice
-REMOVE_FUNCTION_BODY += s2n_ensure_memcpy_trace
+REMOVE_FUNCTION_BODY += s2n_ensure_memmove_trace
 REMOVE_FUNCTION_BODY += __CPROVER_file_local_s2n_mem_c_s2n_mem_cleanup_impl
 
 UNWINDSET +=

diff --git a/tests/cbmc/proofs/s2n_stuffer_write_network_order/Makefile b/tests/cbmc/proofs/s2n_stuffer_write_network_order/Makefile
@@ -38,7 +38,7 @@ REMOVE_FUNCTION_BODY += __CPROVER_file_local_s2n_mem_c_s2n_mem_cleanup_impl
 REMOVE_FUNCTION_BODY += s2n_blob_slice
 REMOVE_FUNCTION_BODY += s2n_stuffer_wipe
 
-UNWINDSET += s2n_stuffer_write_network_order.4:9
+UNWINDSET += s2n_stuffer_write_network_order.10:9
 UNWINDSET += s2n_stuffer_write_network_order_harness.0:9
 
 include ../Makefile.common
diff --git a/tests/cbmc/proofs/s2n_stuffer_write_reservation/Makefile b/tests/cbmc/proofs/s2n_stuffer_write_reservation/Makefile
@@ -48,13 +48,6 @@ REMOVE_FUNCTION_BODY += s2n_stuffer_wipe
 REMOVE_FUNCTION_BODY += s2n_stuffer_wipe_n
 REMOVE_FUNCTION_BODY += __CPROVER_file_local_s2n_mem_c_s2n_mem_cleanup_impl
 
-UNWINDSET += s2n_stuffer_write_network_order.0:$(MAX_WRITE_NETWORK_ORDER)
-UNWINDSET += s2n_stuffer_write_network_order.1:$(MAX_WRITE_NETWORK_ORDER)
-UNWINDSET += s2n_stuffer_write_network_order.2:$(MAX_WRITE_NETWORK_ORDER)
-UNWINDSET += s2n_stuffer_write_network_order.3:$(MAX_WRITE_NETWORK_ORDER)
-UNWINDSET += s2n_stuffer_write_network_order.4:$(MAX_WRITE_NETWORK_ORDER)
-UNWINDSET += s2n_stuffer_write_network_order.5:$(MAX_WRITE_NETWORK_ORDER)
-UNWINDSET += s2n_stuffer_write_network_order.6:$(MAX_WRITE_NETWORK_ORDER)
-UNWINDSET += s2n_stuffer_write_network_order.7:$(MAX_WRITE_NETWORK_ORDER)
+UNWINDSET += s2n_stuffer_write_network_order.10:$(MAX_WRITE_NETWORK_ORDER)
 
 include ../Makefile.common
diff --git a/tests/cbmc/proofs/s2n_stuffer_write_uint16/Makefile b/tests/cbmc/proofs/s2n_stuffer_write_uint16/Makefile
@@ -36,6 +36,6 @@ PROJECT_SOURCES += $(SRCDIR)/utils/s2n_result.c
 REMOVE_FUNCTION_BODY += s2n_blob_slice
 REMOVE_FUNCTION_BODY += s2n_stuffer_wipe
 
-UNWINDSET += s2n_stuffer_write_network_order.4:$(shell echo $$((1 + $(SIZEOF_UINT16))))
+UNWINDSET += s2n_stuffer_write_network_order.10:$(shell echo $$((1 + $(SIZEOF_UINT16))))
 
 include ../Makefile.common
diff --git a/tests/cbmc/proofs/s2n_stuffer_write_uint24/Makefile b/tests/cbmc/proofs/s2n_stuffer_write_uint24/Makefile
@@ -36,6 +36,6 @@ PROJECT_SOURCES += $(SRCDIR)/utils/s2n_result.c
 REMOVE_FUNCTION_BODY += s2n_blob_slice
 REMOVE_FUNCTION_BODY += s2n_stuffer_wipe
 
-UNWINDSET += s2n_stuffer_write_network_order.4:$(shell echo $$((1 + $(SIZEOF_UINT24))))
+UNWINDSET += s2n_stuffer_write_network_order.10:$(shell echo $$((1 + $(SIZEOF_UINT24))))
 
 include ../Makefile.common
diff --git a/tests/cbmc/proofs/s2n_stuffer_write_uint32/Makefile b/tests/cbmc/proofs/s2n_stuffer_write_uint32/Makefile
@@ -36,6 +36,6 @@ PROJECT_SOURCES += $(SRCDIR)/utils/s2n_result.c
 REMOVE_FUNCTION_BODY += s2n_blob_slice
 REMOVE_FUNCTION_BODY += s2n_stuffer_wipe
 
-UNWINDSET += s2n_stuffer_write_network_order.4:$(shell echo $$((1 + $(SIZEOF_UINT32))))
+UNWINDSET += s2n_stuffer_write_network_order.10:$(shell echo $$((1 + $(SIZEOF_UINT32))))
 
 include ../Makefile.common
diff --git a/tests/cbmc/proofs/s2n_stuffer_write_uint64/Makefile b/tests/cbmc/proofs/s2n_stuffer_write_uint64/Makefile
@@ -36,6 +36,6 @@ PROJECT_SOURCES += $(SRCDIR)/utils/s2n_result.c
 REMOVE_FUNCTION_BODY += s2n_blob_slice
 REMOVE_FUNCTION_BODY += s2n_stuffer_wipe
 
-UNWINDSET += s2n_stuffer_write_network_order.4:$(shell echo $$((1 + $(SIZEOF_UINT64))))
+UNWINDSET += s2n_stuffer_write_network_order.10:$(shell echo $$((1 + $(SIZEOF_UINT64))))
 
 include ../Makefile.common