diff --git a/tests/saw/spec/handshake/handshake_io_lowlevel.saw b/tests/saw/spec/handshake/handshake_io_lowlevel.saw index 90b6ad2aecd..32a89af0866 100644 --- a/tests/saw/spec/handshake/handshake_io_lowlevel.saw +++ b/tests/saw/spec/handshake/handshake_io_lowlevel.saw @@ -154,8 +154,13 @@ let setup_connection_common chosen_psk_null = do { cork_val <- crucible_fresh_var "corked" (llvm_int 2); crucible_ghost_value corked cork_val; + // We assume that the client_cert_auth_type_overridden flag in s2n_connection is set. + // If that flag is not set, auth_type / "cca_type" could be determined by the config + // or just set to a default value. Since we're primarily interested in the handshake + // here and the end result is the same, just consider the connection auth_type. cca_ov <- crucible_fresh_var "cca_ov" (llvm_int 8); crucible_points_to (cca_type_ov pconn) (crucible_term cca_ov); + crucible_equal (crucible_term cca_ov) (crucible_term {{1 : [8]}}); cca <- crucible_fresh_var "cca" (llvm_int 32); crucible_points_to (cca_type pconn) (crucible_term cca); @@ -175,9 +180,6 @@ let setup_connection_common chosen_psk_null = do { config <- crucible_alloc (llvm_struct "struct.s2n_config"); crucible_points_to (conn_config pconn) config; - config_cca <- crucible_fresh_var "config_cca" (llvm_int 32); - crucible_points_to (config_cca_type config) (crucible_term config_cca); - cak <- crucible_alloc (llvm_struct "struct.s2n_cert_chain_and_key"); crucible_points_to (conn_chain_and_key pconn) cak; @@ -226,8 +228,6 @@ let setup_connection_common chosen_psk_null = do { no_client_cert <- crucible_fresh_var "no_client_cert" (llvm_int 8); - let client_cert_auth_type = {{ if cca_ov != 0 then cca else config_cca }}; - npn_negotiated <- llvm_fresh_var "npn_negotiated" (llvm_int 1); llvm_points_to_bitfield pconn "npn_negotiated" (llvm_term npn_negotiated); let npn_negotiated_bit = {{ bv1_to_bit npn_negotiated }}; @@ -246,8 +246,8 @@ let setup_connection_common chosen_psk_null = do { ((ocsp_flag == 1) && (status_size > 0)) || ((mode == 1) && (ocsp_flag == 1)) ,resume_from_cache = False - ,client_auth_flag = if mode == S2N_CLIENT then client_cert_auth_type == S2N_CERT_AUTH_REQUIRED else - if mode == S2N_SERVER then client_cert_auth_type != S2N_CERT_AUTH_NONE else False + ,client_auth_flag = if mode == S2N_CLIENT then cca == S2N_CERT_AUTH_REQUIRED else + if mode == S2N_SERVER then cca != S2N_CERT_AUTH_NONE else False ,no_client_cert = no_client_cert != zero ,actual_protocol_version = version ,early_data_state = early_data_state @@ -292,9 +292,7 @@ let s2n_allowed_to_cache_connection_spec = do { crucible_return (crucible_term {{ 0 : [32] }}); }; - -let s2n_connection_get_client_auth_type_spec = do{ - +let s2n_connection_get_client_auth_type_spec = do { pconn <- crucible_alloc_readonly (llvm_struct "struct.s2n_connection"); auth_type <- crucible_alloc (llvm_int 32); @@ -304,18 +302,28 @@ let s2n_connection_get_client_auth_type_spec = do{ config <- crucible_alloc_readonly (llvm_struct "struct.s2n_config"); crucible_points_to (conn_config pconn) config; + config_cca_ov <- llvm_fresh_var "config_cca_ov" (llvm_int 8); + crucible_points_to (crucible_field config "client_cert_auth_type_overridden") (crucible_term config_cca_ov); + config_cca <- crucible_fresh_var "config_cca" (llvm_int 32); crucible_points_to (config_cca_type config) (crucible_term config_cca); cca <- crucible_fresh_var "cca" (llvm_int 32); crucible_points_to (cca_type pconn) (crucible_term cca); + mode <- crucible_fresh_var "mode" (llvm_int 32); + crucible_points_to (conn_mode pconn) (crucible_term mode); + crucible_execute_func [pconn, auth_type]; - crucible_points_to (auth_type) (crucible_term {{if cca_ov != zero then cca else config_cca}}); + crucible_points_to (auth_type) (crucible_term {{ + if cca_ov != zero then cca else + if config_cca_ov != zero then config_cca else + if mode == S2N_CLIENT then S2N_CERT_AUTH_OPTIONAL else + S2N_CERT_AUTH_NONE + }}); crucible_return (crucible_term {{ 0 : [32] }}); - }; // Specification for s2n_conn_set_handshake_type that sets up simulation of it diff --git a/tests/saw/spec/handshake/s2n_handshake_io.cry b/tests/saw/spec/handshake/s2n_handshake_io.cry index 46fe869ddd3..91faf76f05a 100644 --- a/tests/saw/spec/handshake/s2n_handshake_io.cry +++ b/tests/saw/spec/handshake/s2n_handshake_io.cry @@ -1112,6 +1112,7 @@ S2N_CLIENT = 1 //S2N cert auth types S2N_CERT_AUTH_NONE = 0 S2N_CERT_AUTH_REQUIRED = 1 +S2N_CERT_AUTH_OPTIONAL = 2 //S2N early data states S2N_EARLY_DATA_ACCEPTED = 3 diff --git a/tests/unit/s2n_client_auth_handshake_test.c b/tests/unit/s2n_client_auth_handshake_test.c index 6e0b082bf7d..3b29c02604c 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 b0afd60f695..cad18ab1b64 100644 --- a/tests/unit/s2n_connection_test.c +++ b/tests/unit/s2n_connection_test.c @@ -929,6 +929,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 ddbb04f81c0..72c8faea66a 100644 --- a/tests/unit/s2n_handshake_test.c +++ b/tests/unit/s2n_handshake_test.c @@ -384,7 +384,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; @@ -420,7 +419,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 b8bc8395fc2..b9848188772 100644 --- a/tests/unit/s2n_tls13_handshake_state_machine_test.c +++ b/tests/unit/s2n_tls13_handshake_state_machine_test.c @@ -762,7 +762,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 25098325ea0..aea8689a217 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; @@ -222,7 +218,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; @@ -415,6 +410,7 @@ int s2n_config_get_client_auth_type(struct s2n_config *config, s2n_cert_auth_typ int s2n_config_set_client_auth_type(struct s2n_config *config, s2n_cert_auth_type client_auth_type) { POSIX_ENSURE_REF(config); + config->client_cert_auth_type_overridden = 1; config->client_cert_auth_type = client_auth_type; return 0; } diff --git a/tls/s2n_config.h b/tls/s2n_config.h index 078cc4b7b25..10da5ce8f1a 100644 --- a/tls/s2n_config.h +++ b/tls/s2n_config.h @@ -144,6 +144,16 @@ struct s2n_config { s2n_ct_support_level ct_type; + /* Track whether the application has overriden the default client auth type. + * Clients and servers have different default client auth behavior, and this + * config could apply to either. + * This should be a bitflag, but that change is blocked on the SAW proofs. + */ + uint8_t client_cert_auth_type_overridden; + + /* Whether or not the client should authenticate itself to the server. + * Only used if client_cert_auth_type_overridden is true. + */ s2n_cert_auth_type client_cert_auth_type; s2n_alert_behavior alert_behavior; diff --git a/tls/s2n_connection.c b/tls/s2n_connection.c index 2b68fcedbed..1a555bdc054 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(const struct s2n_connection *conn, + const 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 @@ -296,11 +299,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); @@ -748,19 +748,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(const struct s2n_connection *conn, + const 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) diff --git a/tls/s2n_connection.h b/tls/s2n_connection.h index 1f8c735f127..44f3e892345 100644 --- a/tls/s2n_connection.h +++ b/tls/s2n_connection.h @@ -224,15 +224,15 @@ struct s2n_connection { /* The PRF needs some storage elements to work with */ struct s2n_prf_working_space *prf_space; - /* Whether to use client_cert_auth_type stored in s2n_config or in this s2n_connection. - * - * By default the s2n_connection will defer to s2n_config->client_cert_auth_type on whether or not to use Client Auth. - * But users can override Client Auth at the connection level using s2n_connection_set_client_auth_type() without mutating - * s2n_config since s2n_config can be shared between multiple s2n_connections. */ + /* Indicates whether the application has overridden the client auth behavior + * inherited from the config. + * This should be a bitflag, but that change is blocked on the SAW proofs. + */ uint8_t client_cert_auth_type_overridden; - /* Whether or not the s2n_connection should require the Client to authenticate itself to the server. Only used if - * client_cert_auth_type_overridden is non-zero. */ + /* Whether or not the client should authenticate itself to the server. + * Only used if client_cert_auth_type_overridden is true. + */ s2n_cert_auth_type client_cert_auth_type; /* Our workhorse stuffers, used for buffering the plaintext