Merge branch 'main' into ubsan

aws · Mar 14, 2024 · e05fa56 · e05fa56
2 parents 003abe7 + f14659d
commit e05fa56
Show file tree

Hide file tree

Showing 61 changed files with 285 additions and 263 deletions.
diff --git a/api/s2n.h b/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
  *
@@ -2008,7 +2032,7 @@ S2N_API extern int s2n_negotiate(struct s2n_connection *conn, s2n_blocked_status
  * @param buf A pointer to a buffer that s2n will write data from
  * @param size The size of buf
  * @param blocked A pointer which will be set to the blocked status if an `S2N_ERR_T_BLOCKED` error is returned.
- * @returns The number of bytes written, and may indicate a partial write
+ * @returns The number of bytes written on success, which may indicate a partial write. S2N_FAILURE on failure.
  */
 S2N_API extern ssize_t s2n_send(struct s2n_connection *conn, const void *buf, ssize_t size, s2n_blocked_status *blocked);
 
@@ -2021,7 +2045,7 @@ S2N_API extern ssize_t s2n_send(struct s2n_connection *conn, const void *buf, ss
  * @param bufs A pointer to a vector of buffers that s2n will write data from.
  * @param count The number of buffers in `bufs`
  * @param blocked A pointer which will be set to the blocked status if an `S2N_ERR_T_BLOCKED` error is returned.
- * @returns The number of bytes written, and may indicate a partial write. 
+ * @returns The number of bytes written on success, which may indicate a partial write. S2N_FAILURE on failure.
  */
 S2N_API extern ssize_t s2n_sendv(struct s2n_connection *conn, const struct iovec *bufs, ssize_t count, s2n_blocked_status *blocked);
 
@@ -2040,7 +2064,7 @@ S2N_API extern ssize_t s2n_sendv(struct s2n_connection *conn, const struct iovec
  * @param count The number of buffers in `bufs`
  * @param offs The write cursor offset. This should be updated as data is written. See the example code.
  * @param blocked A pointer which will be set to the blocked status if an `S2N_ERR_T_BLOCKED` error is returned.
- * @returns The number of bytes written, and may indicate a partial write. 
+ * @returns The number of bytes written on success, which may indicate a partial write. S2N_FAILURE on failure.
  */
 S2N_API extern ssize_t s2n_sendv_with_offset(struct s2n_connection *conn, const struct iovec *bufs, ssize_t count, ssize_t offs, s2n_blocked_status *blocked);
 
@@ -2057,7 +2081,7 @@ S2N_API extern ssize_t s2n_sendv_with_offset(struct s2n_connection *conn, const
  * @param buf A pointer to a buffer that s2n will place read data into.
  * @param size Size of `buf`
  * @param blocked A pointer which will be set to the blocked status if an `S2N_ERR_T_BLOCKED` error is returned.
- * @returns number of bytes read. 0 if the connection was shutdown by peer.
+ * @returns The number of bytes read on success. 0 if the connection was shutdown by the peer. S2N_FAILURE on failure.
  */
 S2N_API extern ssize_t s2n_recv(struct s2n_connection *conn, void *buf, ssize_t size, s2n_blocked_status *blocked);
 

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