diff --git a/tests/unit/s2n_client_auth_handshake_test.c b/tests/unit/s2n_client_auth_handshake_test.c index 64b994104b8..850b3c001f0 100644 --- a/tests/unit/s2n_client_auth_handshake_test.c +++ b/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(); } diff --git a/tests/unit/s2n_connection_test.c b/tests/unit/s2n_connection_test.c index 61c03d7ac0c..ad7f95967b8 100644 --- a/tests/unit/s2n_connection_test.c +++ b/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(); diff --git a/tests/unit/s2n_handshake_test.c b/tests/unit/s2n_handshake_test.c index c5e0ca314eb..c017d337ae7 100644 --- a/tests/unit/s2n_handshake_test.c +++ b/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; diff --git a/tests/unit/s2n_tls13_handshake_state_machine_test.c b/tests/unit/s2n_tls13_handshake_state_machine_test.c index 963d195e11f..b11d060a157 100644 --- a/tests/unit/s2n_tls13_handshake_state_machine_test.c +++ b/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; diff --git a/tls/s2n_config.c b/tls/s2n_config.c index 45bb6790995..1a181e840d9 100644 --- a/tls/s2n_config.c +++ b/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; } diff --git a/tls/s2n_config.h b/tls/s2n_config.h index 22aea637449..03954d273c0 100644 --- a/tls/s2n_config.h +++ b/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 diff --git a/tls/s2n_connection.c b/tls/s2n_connection.c index cabc2ea9f15..c2238cfe06a 100644 --- a/tls/s2n_connection.c +++ b/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)