Make S2N_CERT_AUTH_OPTIONAL the default for clients

tests/unit/s2n_client_auth_handshake_test.c

@@ -379,6 +379,7 @@ int main(int argc, char **argv)
         EXPECT_SUCCESS(s2n_connection_set_config(server, config));
 
         /* Enable client auth on the server, but not on the client */
+        EXPECT_SUCCESS(s2n_connection_set_client_auth_type(client, S2N_CERT_AUTH_NONE));
         EXPECT_SUCCESS(s2n_connection_set_client_auth_type(server, S2N_CERT_AUTH_OPTIONAL));
 
         DEFER_CLEANUP(struct s2n_test_io_pair io_pair = { 0 }, s2n_io_pair_close);
@@ -389,5 +390,31 @@ int main(int argc, char **argv)
                 S2N_ERR_UNEXPECTED_CERT_REQUEST);
     };
 
+    /* By default, client accepts certificate requests */
+    {
+        DEFER_CLEANUP(struct s2n_config *client_config = s2n_config_new(), s2n_config_ptr_free);
+        EXPECT_SUCCESS(s2n_config_set_unsafe_for_testing(client_config));
+        DEFER_CLEANUP(struct s2n_connection *client = s2n_connection_new(S2N_CLIENT),
+                s2n_connection_ptr_free);
+        EXPECT_NOT_NULL(client);
+        EXPECT_SUCCESS(s2n_connection_set_config(client, client_config));
+
+        DEFER_CLEANUP(struct s2n_config *server_config = s2n_config_new(), s2n_config_ptr_free);
+        EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(server_config, chain_and_key));
+        DEFER_CLEANUP(struct s2n_connection *server = s2n_connection_new(S2N_SERVER),
+                s2n_connection_ptr_free);
+        EXPECT_NOT_NULL(server);
+        EXPECT_SUCCESS(s2n_connection_set_config(server, server_config));
+
+        /* Enable client auth on the server */
+        EXPECT_SUCCESS(s2n_connection_set_client_auth_type(server, S2N_CERT_AUTH_OPTIONAL));
+
+        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_connections_set_io_pair(client, server, &io_pair));
+
+        EXPECT_SUCCESS(s2n_negotiate_test_server_and_client(server, client));
+    };
+
     END_TEST();
 }

tests/unit/s2n_connection_test.c

@@ -906,6 +906,59 @@ int main(int argc, char **argv)
         EXPECT_EQUAL(recv_count, expected_recv_count);
     }
 
+    /* Test s2n_connection_get_client_auth_type */
+    {
+        DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free);
+
+        DEFER_CLEANUP(struct s2n_connection *client = s2n_connection_new(S2N_CLIENT),
+                s2n_connection_ptr_free);
+        EXPECT_NOT_NULL(client);
+        EXPECT_SUCCESS(s2n_connection_set_config(client, config));
+
+        DEFER_CLEANUP(struct s2n_connection *server = s2n_connection_new(S2N_SERVER),
+                s2n_connection_ptr_free);
+        EXPECT_NOT_NULL(server);
+        EXPECT_SUCCESS(s2n_connection_set_config(server, config));
+
+        /* Test: use defaults if no overrides set */
+        {
+            /* Ensure that defaults are still used if the override flags are not set,
+             * even if client_cert_auth_type is somehow set.
+             */
+            server->client_cert_auth_type = S2N_CERT_AUTH_REQUIRED;
+            client->client_cert_auth_type = S2N_CERT_AUTH_REQUIRED;
+
+            s2n_cert_auth_type auth_type = S2N_CERT_AUTH_REQUIRED;
+            EXPECT_SUCCESS(s2n_connection_get_client_auth_type(client, &auth_type));
+            EXPECT_EQUAL(auth_type, S2N_CERT_AUTH_OPTIONAL);
+            EXPECT_SUCCESS(s2n_connection_get_client_auth_type(server, &auth_type));
+            EXPECT_EQUAL(auth_type, S2N_CERT_AUTH_NONE);
+        };
+
+        /* Test: use config overrides if set */
+        {
+            EXPECT_SUCCESS(s2n_config_set_client_auth_type(config, S2N_CERT_AUTH_REQUIRED));
+
+            s2n_cert_auth_type auth_type = S2N_CERT_AUTH_NONE;
+            EXPECT_SUCCESS(s2n_connection_get_client_auth_type(client, &auth_type));
+            EXPECT_EQUAL(auth_type, S2N_CERT_AUTH_REQUIRED);
+            EXPECT_SUCCESS(s2n_connection_get_client_auth_type(server, &auth_type));
+            EXPECT_EQUAL(auth_type, S2N_CERT_AUTH_REQUIRED);
+        };
+
+        /* Test: use connection overrides if set */
+        {
+            EXPECT_SUCCESS(s2n_connection_set_client_auth_type(client, S2N_CERT_AUTH_NONE));
+            EXPECT_SUCCESS(s2n_connection_set_client_auth_type(server, S2N_CERT_AUTH_OPTIONAL));
+
+            s2n_cert_auth_type auth_type = S2N_CERT_AUTH_REQUIRED;
+            EXPECT_SUCCESS(s2n_connection_get_client_auth_type(client, &auth_type));
+            EXPECT_EQUAL(auth_type, S2N_CERT_AUTH_NONE);
+            EXPECT_SUCCESS(s2n_connection_get_client_auth_type(server, &auth_type));
+            EXPECT_EQUAL(auth_type, S2N_CERT_AUTH_OPTIONAL);
+        };
+    };
+
     EXPECT_SUCCESS(s2n_cert_chain_and_key_free(ecdsa_chain_and_key));
     EXPECT_SUCCESS(s2n_cert_chain_and_key_free(rsa_chain_and_key));
     END_TEST();

tests/unit/s2n_handshake_test.c

@@ -383,7 +383,6 @@ int main(int argc, char **argv)
             server_config->security_policy = &security_policy;
 
             EXPECT_NOT_NULL(client_config = s2n_config_new());
-            client_config->client_cert_auth_type = S2N_CERT_AUTH_NONE;
             client_config->check_ocsp = 0;
             client_config->disable_x509_validation = 1;
             client_config->security_policy = &security_policy;
@@ -419,7 +418,6 @@ int main(int argc, char **argv)
 
             EXPECT_NOT_NULL(client_config = s2n_config_new());
             EXPECT_SUCCESS(s2n_config_set_cipher_preferences(client_config, "20200207"));
-            client_config->client_cert_auth_type = S2N_CERT_AUTH_NONE;
             client_config->check_ocsp = 0;
             client_config->disable_x509_validation = 1;
 

tests/unit/s2n_tls13_handshake_state_machine_test.c

@@ -766,7 +766,7 @@ int main(int argc, char **argv)
         conn->session_ticket_status = S2N_NEW_TICKET;
 
         /* Ensure CLIENT_AUTH is set */
-        conn->config->client_cert_auth_type = S2N_CERT_AUTH_REQUIRED;
+        EXPECT_SUCCESS(s2n_connection_set_client_auth_type(conn, S2N_CERT_AUTH_REQUIRED));
 
         /* Ensure TLS12_PERFECT_FORWARD_SECRECY is set by choosing a cipher suite with is_ephemeral=1 on the kex */
         conn->secure->cipher_suite = &s2n_dhe_rsa_with_chacha20_poly1305_sha256;

tls/s2n_config.c

@@ -96,10 +96,6 @@ static int s2n_config_init(struct s2n_config *config)
     config->encrypt_decrypt_key_lifetime_in_nanos = S2N_TICKET_ENCRYPT_DECRYPT_KEY_LIFETIME_IN_NANOS;
     config->decrypt_key_lifetime_in_nanos = S2N_TICKET_DECRYPT_KEY_LIFETIME_IN_NANOS;
     config->async_pkey_validation_mode = S2N_ASYNC_PKEY_VALIDATION_FAST;
-
-    /* By default, only the client will authenticate the Server's Certificate. The Server does not request or
-     * authenticate any client certificates. */
-    config->client_cert_auth_type = S2N_CERT_AUTH_NONE;
     config->check_ocsp = 1;
 
     config->client_hello_cb_mode = S2N_CLIENT_HELLO_CB_BLOCKING;
@@ -220,7 +216,6 @@ struct s2n_config *s2n_fetch_default_config(void)
 int s2n_config_set_unsafe_for_testing(struct s2n_config *config)
 {
     POSIX_ENSURE(s2n_in_test(), S2N_ERR_NOT_IN_TEST);
-    config->client_cert_auth_type = S2N_CERT_AUTH_NONE;
     config->check_ocsp = 0;
     config->disable_x509_validation = 1;
 
@@ -409,6 +404,7 @@ int s2n_config_set_client_auth_type(struct s2n_config *config, s2n_cert_auth_typ
 {
     POSIX_ENSURE_REF(config);
     config->client_cert_auth_type = client_auth_type;
+    config->client_cert_auth_type_overridden = true;
     return 0;
 }
 

tls/s2n_config.h

@@ -105,6 +105,8 @@ struct s2n_config {
     /* TLS1.3 can be dangerous with kTLS. Require it to be explicitly enabled. */
     unsigned ktls_tls13_enabled : 1;
 
+    unsigned client_cert_auth_type_overridden : 1;
+
     struct s2n_dh_params *dhparams;
     /* Needed until we can deprecate s2n_config_add_cert_chain_and_key. This is
      * used to release memory allocated only in the deprecated API that the application 

tls/s2n_connection.c

@@ -59,6 +59,9 @@
 #define S2N_SET_KEY_SHARE_LIST_EMPTY(keyshares) (keyshares |= 1)
 #define S2N_SET_KEY_SHARE_REQUEST(keyshares, i) (keyshares |= (1 << (i + 1)))
 
+static S2N_RESULT s2n_connection_and_config_get_client_auth_type(struct s2n_connection *conn,
+        struct s2n_config *config, s2n_cert_auth_type *client_cert_auth_type);
+
 /* Allocates and initializes memory for a new connection.
  *
  * Since customers can reuse a connection, ensure that values on the connection are
@@ -290,11 +293,8 @@ 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 = config->client_cert_auth_type;
-
-    if (conn->client_cert_auth_type_overridden) {
-        auth_type = conn->client_cert_auth_type;
-    }
+    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);
 
@@ -742,19 +742,37 @@ int s2n_connection_get_protocol_preferences(struct s2n_connection *conn, struct
     return 0;
 }
 
-int s2n_connection_get_client_auth_type(struct s2n_connection *conn, s2n_cert_auth_type *client_cert_auth_type)
+static S2N_RESULT s2n_connection_and_config_get_client_auth_type(struct s2n_connection *conn,
+        struct s2n_config *config, s2n_cert_auth_type *client_cert_auth_type)
 {
-    POSIX_ENSURE_REF(conn);
-    POSIX_ENSURE_REF(client_cert_auth_type);
+    RESULT_ENSURE_REF(conn);
+    RESULT_ENSURE_REF(config);
+    RESULT_ENSURE_REF(client_cert_auth_type);
 
     if (conn->client_cert_auth_type_overridden) {
         *client_cert_auth_type = conn->client_cert_auth_type;
+    } else if (config->client_cert_auth_type_overridden) {
+        *client_cert_auth_type = config->client_cert_auth_type;
+    } else if (conn->mode == S2N_CLIENT) {
+        /* Clients should default to "Optional" so that they handle any
+         * CertificateRequests sent by the server.
+         */
+        *client_cert_auth_type = S2N_CERT_AUTH_OPTIONAL;
     } else {
-        POSIX_ENSURE_REF(conn->config);
-        *client_cert_auth_type = conn->config->client_cert_auth_type;
+        /* Servers should default to "None" so that they send no CertificateRequests. */
+        *client_cert_auth_type = S2N_CERT_AUTH_NONE;
     }
 
-    return 0;
+    return S2N_RESULT_OK;
+}
+
+int s2n_connection_get_client_auth_type(struct s2n_connection *conn,
+        s2n_cert_auth_type *client_cert_auth_type)
+{
+    POSIX_ENSURE_REF(conn);
+    POSIX_GUARD_RESULT(s2n_connection_and_config_get_client_auth_type(
+            conn, conn->config, client_cert_auth_type));
+    return S2N_SUCCESS;
 }
 
 int s2n_connection_set_client_auth_type(struct s2n_connection *conn, s2n_cert_auth_type client_cert_auth_type)