Merge branch 'main' into cmake-unnecessary-include

api/s2n.h

@@ -1882,6 +1882,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
  *

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>>,

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);
         });
     }
 

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)
         {{

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 +=
 

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 +=

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 +=

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 +=

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 +=
 

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 +=
 

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 +=

tests/cbmc/stubs/s2n_ensure.c

@@ -15,7 +15,7 @@
 
 #include "utils/s2n_safety.h"
 
-void* s2n_ensure_memcpy_trace(void *restrict to, const void *restrict from, size_t size)
+void* s2n_ensure_memmove_trace(void *to, const void *from, size_t size)
 {
     if (to == NULL || from == NULL) {
         return NULL;

tests/sidetrail/working/patches/cbc.patch

@@ -35,7 +35,7 @@ index 401ab760..f39cc7e2 100644
   */
  int s2n_verify_cbc(struct s2n_connection *conn, struct s2n_hmac_state *hmac, struct s2n_blob *decrypted)
  {
--    uint8_t mac_digest_size;
+-    uint8_t mac_digest_size = 0;
 -    POSIX_GUARD(s2n_hmac_digest_size(hmac->alg, &mac_digest_size));
 +    uint8_t mac_digest_size = DIGEST_SIZE;
 +    //POSIX_GUARD(s2n_hmac_digest_size(hmac->alg, &mac_digest_size));

tests/sidetrail/working/stubs/s2n_ensure.h

@@ -31,7 +31,10 @@ void *s2n_sidetrail_memset(void * ptr, int value, size_t num);
 #define __S2N_ENSURE_PRECONDITION( result )                S2N_RESULT_OK
 #define __S2N_ENSURE_POSTCONDITION( result )               S2N_RESULT_OK
 
-#define __S2N_ENSURE_SAFE_MEMCPY( d , s , n , guard )      do { memcpy((d), (s), (n)); } while(0)
+/* memmove isn't supported, so use memcpy instead.
+ * For the purposes of these proofs, there should be no useful difference.
+ */
+#define __S2N_ENSURE_SAFE_MEMMOVE( d , s , n , guard )      do { memcpy((d), (s), (n)); } while(0)
 
 #define __S2N_ENSURE_SAFE_MEMSET( d , c , n , guard )      \
   do {                                                     \

tests/unit/s2n_key_update_test.c

@@ -333,7 +333,7 @@ int main(int argc, char **argv)
             EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&stuffer, 0));
             EXPECT_SUCCESS(s2n_connection_set_io_stuffers(&stuffer, &stuffer, client_conn));
 
-            s2n_atomic_flag_set(&client_conn->key_update_pending);
+            EXPECT_SUCCESS(s2n_connection_request_key_update(client_conn, S2N_KEY_UPDATE_NOT_REQUESTED));
 
             s2n_blocked_status blocked = 0;
             EXPECT_SUCCESS(s2n_key_update_send(client_conn, &blocked));
@@ -677,5 +677,27 @@ int main(int argc, char **argv)
         }
     }
 
+    /* s2n_connection_key_update_requested */
+    {
+        /* null safety */
+        {
+            EXPECT_FAILURE_WITH_ERRNO(s2n_connection_request_key_update(NULL, S2N_KEY_UPDATE_NOT_REQUESTED), S2N_ERR_NULL);
+        };
+
+        /* usage */
+        {
+            DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_SERVER), s2n_connection_ptr_free);
+            EXPECT_FAILURE_WITH_ERRNO(s2n_connection_request_key_update(conn, S2N_KEY_UPDATE_REQUESTED), S2N_ERR_INVALID_ARGUMENT);
+        };
+
+        /* happy path */
+        {
+            DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_SERVER), s2n_connection_ptr_free);
+            EXPECT_FALSE(s2n_atomic_flag_test(&conn->key_update_pending));
+            EXPECT_SUCCESS(s2n_connection_request_key_update(conn, S2N_KEY_UPDATE_NOT_REQUESTED));
+            EXPECT_TRUE(s2n_atomic_flag_test(&conn->key_update_pending));
+        };
+    };
+
     END_TEST();
 }

tests/unit/s2n_key_update_threads_test.c

@@ -35,7 +35,7 @@
     _##name##_record_alg.encryption_limit = limit;                       \
     name.record_alg = &_##name##_record_alg;
 
-S2N_RESULT s2n_set_key_update_request_for_testing(keyupdate_request request);
+S2N_RESULT s2n_set_key_update_request_for_testing(s2n_peer_key_update request);
 
 static void *s2n_send_random_data(void *arg)
 {

tls/s2n_cbc.c

@@ -45,7 +45,7 @@
  */
 int s2n_verify_cbc(struct s2n_connection *conn, struct s2n_hmac_state *hmac, struct s2n_blob *decrypted)
 {
-    uint8_t mac_digest_size;
+    uint8_t mac_digest_size = 0;
     POSIX_GUARD(s2n_hmac_digest_size(hmac->alg, &mac_digest_size));
 
     /* The record has to be at least big enough to contain the MAC,

tls/s2n_key_update.c

@@ -24,12 +24,12 @@
 #include "utils/s2n_atomic.h"
 #include "utils/s2n_safety.h"
 
-static keyupdate_request key_update_request_val = S2N_KEY_UPDATE_NOT_REQUESTED;
+static s2n_peer_key_update key_update_request_val = S2N_KEY_UPDATE_NOT_REQUESTED;
 
 int s2n_key_update_write(struct s2n_blob *out);
 int s2n_check_record_limit(struct s2n_connection *conn, struct s2n_blob *sequence_number);
 
-S2N_RESULT s2n_set_key_update_request_for_testing(keyupdate_request request)
+S2N_RESULT s2n_set_key_update_request_for_testing(s2n_peer_key_update request)
 {
     RESULT_ENSURE(s2n_in_unit_test(), S2N_ERR_NOT_IN_UNIT_TEST);
     key_update_request_val = request;
@@ -146,3 +146,12 @@ int s2n_check_record_limit(struct s2n_connection *conn, struct s2n_blob *sequenc
 
     return S2N_SUCCESS;
 }
+
+int s2n_connection_request_key_update(struct s2n_connection *conn, s2n_peer_key_update peer_request)
+{
+    POSIX_ENSURE_REF(conn);
+    /* s2n-tls does not currently support requesting key updates from peers */
+    POSIX_ENSURE(peer_request == S2N_KEY_UPDATE_NOT_REQUESTED, S2N_ERR_INVALID_ARGUMENT);
+    s2n_atomic_flag_set(&conn->key_update_pending);
+    return S2N_SUCCESS;
+}

tls/s2n_key_update.h

@@ -25,10 +25,5 @@ typedef enum {
     RECEIVING
 } keyupdate_status;
 
-typedef enum {
-    S2N_KEY_UPDATE_NOT_REQUESTED = 0,
-    S2N_KEY_UPDATE_REQUESTED
-} keyupdate_request;
-
 int s2n_key_update_recv(struct s2n_connection *conn, struct s2n_stuffer *request);
 int s2n_key_update_send(struct s2n_connection *conn, s2n_blocked_status *blocked);

utils/s2n_ensure.c

@@ -15,7 +15,7 @@
 
 #include "utils/s2n_safety.h"
 
-void *s2n_ensure_memcpy_trace(void *restrict to, const void *restrict from, size_t size)
+void *s2n_ensure_memmove_trace(void *to, const void *from, size_t size)
 {
     PTR_ENSURE_REF(to);
     PTR_ENSURE_REF(from);

utils/s2n_ensure.h

@@ -60,13 +60,13 @@
     #define __S2N_ENSURE_POSTCONDITION(result) (s2n_likely(s2n_result_is_ok(result)) ? S2N_RESULT_OK : S2N_RESULT_ERROR)
 #endif
 
-#define __S2N_ENSURE_SAFE_MEMCPY(d, s, n, guard)                    \
-    do {                                                            \
-        __typeof(n) __tmp_n = (n);                                  \
-        if (s2n_likely(__tmp_n)) {                                  \
-            void *r = s2n_ensure_memcpy_trace((d), (s), (__tmp_n)); \
-            guard(r);                                               \
-        }                                                           \
+#define __S2N_ENSURE_SAFE_MEMMOVE(d, s, n, guard)                    \
+    do {                                                             \
+        __typeof(n) __tmp_n = (n);                                   \
+        if (s2n_likely(__tmp_n)) {                                   \
+            void *r = s2n_ensure_memmove_trace((d), (s), (__tmp_n)); \
+            guard(r);                                                \
+        }                                                            \
     } while (0)
 
 #define __S2N_ENSURE_SAFE_MEMSET(d, c, n, guard) \
@@ -90,23 +90,7 @@
     #define __S2N_ENSURE_CHECKED_RETURN(v) return v
 #endif
 
-/**
- * `restrict` is a part of the c99 standard and will work with any C compiler. If you're trying to
- * compile with a C++ compiler `restrict` is invalid. However some C++ compilers support the behavior
- * of `restrict` using the `__restrict__` keyword. Therefore if the compiler supports `__restrict__`
- * use it.
- *
- * This is helpful for the benchmarks in tests/benchmark which use Google's Benchmark library and
- * are all written in C++.
- *
- * https://gcc.gnu.org/onlinedocs/gcc/Restricted-Pointers.html
- *
- */
-#if defined(S2N___RESTRICT__SUPPORTED)
-void *s2n_ensure_memcpy_trace(void *__restrict__ to, const void *__restrict__ from, size_t size);
-#else
-void *s2n_ensure_memcpy_trace(void *restrict to, const void *restrict from, size_t size);
-#endif
+void *s2n_ensure_memmove_trace(void *to, const void *from, size_t size);
 
 /**
  * These macros should not be used in validate functions.