fix(connection): Make set_config validator replacement transactional

s2n_connection_set_config wiped the connection's x509_validator
before initializing a replacement from the new config. If the
replacement init failed (e.g. X509_STORE_CTX_new returning NULL
under OOM, or an invalid max chain depth), the connection was left
with a wiped validator while conn->config still referenced the old
config. A subsequent handshake would fail in cert validation.

Stage the new validator into a stack-local struct. If any init step
fails, wipe the local and propagate the error, leaving the
connection's existing validator intact. Only swap on full success.
Add a regression test that triggers a validator init failure via an
invalid max chain depth and verifies the old validator is preserved.

Author: alexw91

tests/unit/s2n_config_test.c

@@ -32,6 +32,7 @@
 #include "tls/s2n_security_policies.h"
 #include "tls/s2n_tls.h"
 #include "tls/s2n_tls13.h"
+#include "tls/s2n_x509_validator.h"
 #include "unstable/npn.h"
 #include "utils/s2n_map.h"
 
@@ -92,7 +93,7 @@ int main(int argc, char **argv)
 
         /* s2n_config_new() matches s2n_fetch_default_config() */
         if (default_config->security_policy != config->security_policy) {
-            /* one possible cause for this is attempting to includes s2n_config.c 
+            /* one possible cause for this is attempting to includes s2n_config.c
              * for access to internal `static` functions. This causes two copies
              * of the default config to be created. The default_config in *this*
              * unit of translation doesn't get properly initialized. */
@@ -1107,7 +1108,7 @@ int main(int argc, char **argv)
         struct s2n_security_policy rfc9151_applied_locally = security_policy_20250429;
         rfc9151_applied_locally.certificate_preferences_apply_locally = true;
 
-        /* rfc9151 doesn't allow SHA256 signatures, but does allow SHA384 signatures, 
+        /* rfc9151 doesn't allow SHA256 signatures, but does allow SHA384 signatures,
          * so ecdsa_p384_sha256 is invalid and ecdsa_p384_sha384 is valid */
 
         /* valid certs are accepted */
@@ -1134,8 +1135,8 @@ int main(int argc, char **argv)
 
         /* certs in default_certs_by_type are validated */
         {
-            /* s2n_config_set_cert_chain_and_key_defaults populates default_certs_by_type 
-             * but doesn't populate domain_name_to_cert_map 
+            /* s2n_config_set_cert_chain_and_key_defaults populates default_certs_by_type
+             * but doesn't populate domain_name_to_cert_map
              */
             DEFER_CLEANUP(struct s2n_config *config = s2n_config_new_minimal(), s2n_config_ptr_free);
             EXPECT_SUCCESS(s2n_config_set_cert_chain_and_key_defaults(config, &invalid_cert, 1));
@@ -1296,5 +1297,52 @@ int main(int argc, char **argv)
         EXPECT_FAILURE_WITH_ERRNO(s2n_config_set_subscriber(NULL, (s2n_event_on_handshake_cb) &fake_callback), S2N_ERR_NULL);
     };
 
+    /* Test: s2n_connection_set_config preserves the old validator on failure
+     *
+     * Regression test: s2n_connection_set_config wipes the connection's
+     * x509_validator before initializing a new one from the incoming config.
+     * If the new validator's initialization fails (e.g. set_max_chain_depth
+     * with an invalid depth), the connection was left with a wiped validator
+     * while conn->config still referenced the old config. The fix stages
+     * the new validator into a local and only swaps on full success.
+     */
+    {
+        DEFER_CLEANUP(struct s2n_config *config_old = s2n_config_new(), s2n_config_ptr_free);
+        EXPECT_NOT_NULL(config_old);
+
+        DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_SERVER),
+                s2n_connection_ptr_free);
+        EXPECT_NOT_NULL(conn);
+        EXPECT_SUCCESS(s2n_connection_set_config(conn, config_old));
+        EXPECT_EQUAL(conn->config, config_old);
+
+        /* The validator should be in the INIT state after a successful set_config. */
+        EXPECT_EQUAL(conn->x509_validator.state, INIT);
+
+        /* Create a second config that will cause set_config to fail during
+         * validator initialization. Setting max_verify_cert_chain_depth to 0
+         * (invalid) with the flag enabled triggers the failure inside
+         * s2n_x509_validator_set_max_chain_depth.
+         */
+        DEFER_CLEANUP(struct s2n_config *config_bad = s2n_config_new(), s2n_config_ptr_free);
+        EXPECT_NOT_NULL(config_bad);
+        config_bad->max_verify_cert_chain_depth = 0;
+        config_bad->max_verify_cert_chain_depth_set = 1;
+
+        /* set_config should fail because of the invalid chain depth. */
+        EXPECT_FAILURE_WITH_ERRNO(s2n_connection_set_config(conn, config_bad),
+                S2N_ERR_INVALID_ARGUMENT);
+
+        /* The connection must still reference the old config. */
+        EXPECT_EQUAL(conn->config, config_old);
+
+        /* The validator must still be functional (INIT state, not UNINIT).
+         * Before the fix, the validator was wiped to UNINIT before the
+         * failed init attempt, leaving the connection in an inconsistent
+         * state.
+         */
+        EXPECT_EQUAL(conn->x509_validator.state, INIT);
+    };
+
     END_TEST();
 }

tls/s2n_connection.c

@@ -84,11 +84,11 @@ struct s2n_connection *s2n_connection_new(s2n_mode mode)
     conn->mode = mode;
 
     /* Allocate the fixed-size stuffers */
-    blob = (struct s2n_blob){ 0 };
+    blob = (struct s2n_blob) { 0 };
     PTR_GUARD_POSIX(s2n_blob_init(&blob, conn->alert_in_data, S2N_ALERT_LENGTH));
     PTR_GUARD_POSIX(s2n_stuffer_init(&conn->alert_in, &blob));
 
-    blob = (struct s2n_blob){ 0 };
+    blob = (struct s2n_blob) { 0 };
     PTR_GUARD_POSIX(s2n_blob_init(&blob, conn->ticket_ext_data, S2N_TLS12_TICKET_SIZE_IN_BYTES));
     PTR_GUARD_POSIX(s2n_stuffer_init(&conn->client_ticket_to_decrypt, &blob));
 
@@ -99,7 +99,7 @@ struct s2n_connection *s2n_connection_new(s2n_mode mode)
     /* Initialize the growable stuffers. Zero length at first, but the resize
      * in _wipe will fix that
      */
-    blob = (struct s2n_blob){ 0 };
+    blob = (struct s2n_blob) { 0 };
     PTR_GUARD_POSIX(s2n_blob_init(&blob, conn->header_in_data, S2N_TLS_RECORD_HEADER_LENGTH));
     PTR_GUARD_POSIX(s2n_stuffer_init(&conn->header_in, &blob));
     PTR_GUARD_POSIX(s2n_stuffer_growable_alloc(&conn->out, 0));
@@ -309,12 +309,24 @@ int s2n_connection_set_config(struct s2n_connection *conn, struct s2n_config *co
         POSIX_BAIL(S2N_ERR_TOO_MANY_CERTIFICATES);
     }
 
-    s2n_x509_validator_wipe(&conn->x509_validator);
+    /* Build the new validator into a local so that the connection's existing
+     * validator is not destroyed until the new one is fully initialized.
+     * Without this staging, a failure mid-init (e.g. X509_STORE_CTX_new
+     * returning NULL under OOM) would leave conn->x509_validator wiped
+     * while conn->config still references the old config.
+     */
+    struct s2n_x509_validator new_validator = { 0 };
 
     if (config->disable_x509_validation) {
-        POSIX_GUARD(s2n_x509_validator_init_no_x509_validation(&conn->x509_validator));
+        POSIX_GUARD(s2n_x509_validator_init_no_x509_validation(&new_validator));
     } else {
-        POSIX_GUARD(s2n_x509_validator_init(&conn->x509_validator, &config->trust_store, config->check_ocsp));
+        int ret = s2n_x509_validator_init(&new_validator, &config->trust_store, config->check_ocsp);
+        if (ret != S2N_SUCCESS) {
+            /* init may have partially populated new_validator before failing */
+            s2n_x509_validator_wipe(&new_validator);
+            POSIX_GUARD(ret);
+        }
+
         if (!conn->verify_host_fn_overridden) {
             if (config->verify_host_fn != NULL) {
                 conn->verify_host_fn = config->verify_host_fn;
@@ -326,9 +338,18 @@ int s2n_connection_set_config(struct s2n_connection *conn, struct s2n_config *co
         }
 
         if (config->max_verify_cert_chain_depth_set) {
-            POSIX_GUARD(s2n_x509_validator_set_max_chain_depth(&conn->x509_validator, config->max_verify_cert_chain_depth));
+            ret = s2n_x509_validator_set_max_chain_depth(&new_validator, config->max_verify_cert_chain_depth);
+            if (ret != S2N_SUCCESS) {
+                s2n_x509_validator_wipe(&new_validator);
+                POSIX_GUARD(ret);
+            }
         }
     }
+
+    /* New validator is fully initialized. Swap it in. */
+    s2n_x509_validator_wipe(&conn->x509_validator);
+    conn->x509_validator = new_validator;
+
     conn->tickets_to_send = config->initial_tickets_to_send;
 
     if (conn->psk_params.psk_list.len == 0 && !conn->psk_mode_overridden) {
@@ -369,7 +390,7 @@ int s2n_connection_set_config(struct s2n_connection *conn, struct s2n_config *co
      * However, the s2n_config_set_verification_ca_location behavior predates client authentication
      * support for OCSP stapling, so could only affect whether clients requested OCSP stapling. We
      * therefore only have to maintain the legacy behavior for clients, not servers.
-     * 
+     *
      * Note: The Rust bindings do not maintain the legacy behavior.
      */
     conn->request_ocsp_status = config->ocsp_status_requested_by_user;
@@ -1020,7 +1041,7 @@ int s2n_connection_get_key_exchange_group(struct s2n_connection *conn, const cha
     POSIX_ENSURE_REF(conn);
     POSIX_ENSURE_REF(group_name);
 
-    /* s2n_connection_get_curve returns only the ECDH curve portion of a named group, even if 
+    /* s2n_connection_get_curve returns only the ECDH curve portion of a named group, even if
        the negotiated group was a hybrid PQ key exchange also containing a KEM. Therefore,
        we use the result of s2n_connection_get_kem_group_name if the connection supports PQ. */
     if (s2n_tls13_pq_hybrid_supported(conn)) {
@@ -1256,11 +1277,11 @@ uint64_t s2n_connection_get_delay(struct s2n_connection *conn)
 
 /* s2n-tls has a random delay that will trigger for sensitive errors. This is a mitigation
  * for possible timing sidechannels.
- * 
+ *
  * The historical sidechannel that inspired s2n-tls blinding was the Lucky 13 attack, which takes
  * advantage of potential timing differences when removing padding from a record encrypted in CBC mode.
- * The attack is only theoretical in TLS; the attack criteria is unlikely to ever occur 
- * (See: Fardan, N. J. A., & Paterson, K. G. (2013, May 1). Lucky Thirteen: Breaking the TLS and 
+ * The attack is only theoretical in TLS; the attack criteria is unlikely to ever occur
+ * (See: Fardan, N. J. A., & Paterson, K. G. (2013, May 1). Lucky Thirteen: Breaking the TLS and
  * DTLS Record Protocols.) However, we still include blinding to provide a defense in depth mitigation.
  */
 S2N_RESULT s2n_connection_calculate_blinding(struct s2n_connection *conn, int64_t *min, int64_t *max)