Merge commit from fork

Co-authored-by: Lindsay Stewart <slindsay@amazon.com>

tests/unit/s2n_cert_status_extension_test.c

@@ -108,8 +108,13 @@ int main(int argc, char **argv)
 
     /* Test recv */
     {
+        /* Disable x509 validation so that the OCSP test data can be successfully received. */
+        DEFER_CLEANUP(struct s2n_config *config = s2n_config_new_minimal(), s2n_config_ptr_free);
+        EXPECT_SUCCESS(s2n_config_set_unsafe_for_testing(config));
+
         struct s2n_connection *conn = NULL;
         EXPECT_NOT_NULL(conn = s2n_connection_new(S2N_SERVER));
+        EXPECT_SUCCESS(s2n_connection_set_config(conn, config));
         EXPECT_SUCCESS(s2n_test_enable_sending_extension(conn, chain_and_key));
 
         struct s2n_stuffer stuffer = { 0 };

tests/unit/s2n_certificate_extensions_test.c

@@ -77,6 +77,11 @@ static int s2n_setup_connection_for_ocsp_validate_test(struct s2n_connection **c
     nconn->actual_protocol_version = S2N_TLS13;
     nconn->handshake_params.our_chain_and_key = chain_and_key;
 
+    /* The OCSP tests ensure that an OCSP response can be successfully sent/received. They do NOT
+     * ensure that the OCSP response is correctly validated.
+     */
+    nconn->x509_validator.skip_cert_validation = true;
+
     POSIX_GUARD(s2n_connection_allow_all_response_extensions(nconn));
     nconn->status_type = S2N_STATUS_REQUEST_OCSP;
 
@@ -194,6 +199,7 @@ int main(int argc, char **argv)
             EXPECT_NOT_NULL(conn = s2n_connection_new(S2N_SERVER));
             conn->actual_protocol_version = S2N_TLS13;
             conn->handshake_params.our_chain_and_key = chain_and_key;
+            conn->x509_validator.skip_cert_validation = true;
 
             DEFER_CLEANUP(struct s2n_stuffer stuffer, s2n_stuffer_free);
             EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&stuffer, 0));
@@ -210,6 +216,7 @@ int main(int argc, char **argv)
             EXPECT_NOT_NULL(conn = s2n_connection_new(S2N_SERVER));
             conn->actual_protocol_version = S2N_TLS13;
             conn->handshake_params.our_chain_and_key = chain_and_key;
+            conn->x509_validator.skip_cert_validation = true;
 
             DEFER_CLEANUP(struct s2n_stuffer stuffer, s2n_stuffer_free);
             EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&stuffer, 0));

tests/unit/s2n_mutual_auth_test.c

@@ -306,6 +306,84 @@ int main(int argc, char **argv)
         EXPECT_SUCCESS(s2n_stuffer_free(&client_to_server));
     }
 
+    /* Ensure that the client's certificate is validated, regardless of how client auth was enabled */
+    {
+        typedef enum {
+            TEST_ENABLE_WITH_CONFIG,
+            TEST_ENABLE_WITH_CONN_BEFORE_CONFIG,
+            TEST_ENABLE_WITH_CONN_AFTER_CONFIG,
+            TEST_COUNT,
+        } test_case;
+
+        for (test_case test = TEST_ENABLE_WITH_CONFIG; test < TEST_COUNT; test++) {
+            DEFER_CLEANUP(struct s2n_config *client_config = s2n_config_new(), s2n_config_ptr_free);
+            EXPECT_NOT_NULL(client_config);
+            EXPECT_SUCCESS(s2n_config_set_cipher_preferences(client_config, "default_tls13"));
+            EXPECT_SUCCESS(s2n_config_set_client_auth_type(client_config, S2N_CERT_AUTH_OPTIONAL));
+
+            /* The client trusts the server's cert, and sends the same cert to the server. */
+            EXPECT_SUCCESS(s2n_config_set_verification_ca_location(client_config, S2N_DEFAULT_TEST_CERT_CHAIN, NULL));
+            EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(client_config, chain_and_key));
+
+            DEFER_CLEANUP(struct s2n_config *server_config = s2n_config_new(), s2n_config_ptr_free);
+            EXPECT_NOT_NULL(server_config);
+            EXPECT_SUCCESS(s2n_config_set_cipher_preferences(server_config, "default_tls13"));
+            struct host_verify_data verify_data_allow = { .allow = 1 };
+            EXPECT_SUCCESS(s2n_config_set_verify_host_callback(server_config, verify_host_fn, &verify_data_allow));
+
+            /* The server sends its cert, but does NOT trust the client's cert. This should always
+             * cause certificate validation to fail on the server.
+             */
+            EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(server_config, chain_and_key));
+
+            DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER),
+                    s2n_connection_ptr_free);
+            EXPECT_NOT_NULL(server_conn);
+            EXPECT_SUCCESS(s2n_connection_set_blinding(server_conn, S2N_SELF_SERVICE_BLINDING));
+
+            switch (test) {
+                case TEST_ENABLE_WITH_CONFIG:
+                    EXPECT_SUCCESS(s2n_config_set_client_auth_type(server_config, S2N_CERT_AUTH_REQUIRED));
+                    EXPECT_SUCCESS(s2n_connection_set_config(server_conn, server_config));
+                    break;
+                case TEST_ENABLE_WITH_CONN_BEFORE_CONFIG:
+                    EXPECT_SUCCESS(s2n_connection_set_client_auth_type(server_conn, S2N_CERT_AUTH_REQUIRED));
+                    EXPECT_SUCCESS(s2n_connection_set_config(server_conn, server_config));
+                    break;
+                case TEST_ENABLE_WITH_CONN_AFTER_CONFIG:
+                    EXPECT_SUCCESS(s2n_connection_set_config(server_conn, server_config));
+                    EXPECT_SUCCESS(s2n_connection_set_client_auth_type(server_conn, S2N_CERT_AUTH_REQUIRED));
+                    break;
+                default:
+                    FAIL_MSG("Invalid test case");
+            }
+
+            DEFER_CLEANUP(struct s2n_connection *client_conn = s2n_connection_new(S2N_CLIENT),
+                    s2n_connection_ptr_free);
+            EXPECT_NOT_NULL(client_conn);
+            EXPECT_SUCCESS(s2n_connection_set_config(client_conn, client_config));
+            EXPECT_SUCCESS(s2n_set_server_name(client_conn, "localhost"));
+
+            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_connection_set_io_pair(client_conn, &io_pair));
+            EXPECT_SUCCESS(s2n_connection_set_io_pair(server_conn, &io_pair));
+
+            EXPECT_FAILURE_WITH_ERRNO(s2n_negotiate_test_server_and_client(server_conn, client_conn),
+                    S2N_ERR_CERT_UNTRUSTED);
+
+            /* Ensure that a client certificate was received on the server, indicating that the
+             * validation error occurred when processing the client's certificate, rather than the
+             * server's.
+             */
+            uint8_t *client_cert_chain = NULL;
+            uint32_t client_cert_chain_len = 0;
+            EXPECT_SUCCESS(s2n_connection_get_client_cert_chain(server_conn,
+                    &client_cert_chain, &client_cert_chain_len));
+            EXPECT_TRUE(client_cert_chain_len > 0);
+        }
+    }
+
     EXPECT_SUCCESS(s2n_cert_chain_and_key_free(chain_and_key));
     EXPECT_SUCCESS(s2n_config_free(config));
     free(cert_chain_pem);

tls/s2n_connection.c

@@ -303,12 +303,7 @@ int s2n_connection_set_config(struct s2n_connection *conn, struct s2n_config *co
 
     s2n_x509_validator_wipe(&conn->x509_validator);
 
-    s2n_cert_auth_type auth_type = S2N_CERT_AUTH_NONE;
-    POSIX_GUARD_RESULT(s2n_connection_and_config_get_client_auth_type(conn, config, &auth_type));
-
-    int8_t dont_need_x509_validation = (conn->mode == S2N_SERVER) && (auth_type == S2N_CERT_AUTH_NONE);
-
-    if (config->disable_x509_validation || dont_need_x509_validation) {
+    if (config->disable_x509_validation) {
         POSIX_GUARD(s2n_x509_validator_init_no_x509_validation(&conn->x509_validator));
     } else {
         POSIX_GUARD(s2n_x509_validator_init(&conn->x509_validator, &config->trust_store, config->check_ocsp));