diff --git a/tests/unit/s2n_client_hello_recv_test.c b/tests/unit/s2n_client_hello_recv_test.c index 2422559e9e9..ed8c19bcc2f 100644 --- a/tests/unit/s2n_client_hello_recv_test.c +++ b/tests/unit/s2n_client_hello_recv_test.c @@ -102,7 +102,7 @@ int main(int argc, char **argv) EXPECT_SUCCESS(s2n_stuffer_write(&server_conn->handshake.io, &client_hello)); EXPECT_SUCCESS(s2n_client_hello_recv(server_conn)); - EXPECT_EQUAL(server_conn->server_protocol_version, i == 0 ? S2N_TLS12 : S2N_TLS13); + EXPECT_EQUAL(server_conn->server_protocol_version, S2N_TLS12); EXPECT_EQUAL(server_conn->actual_protocol_version, S2N_TLS12); EXPECT_EQUAL(server_conn->client_protocol_version, S2N_TLS12); EXPECT_EQUAL(server_conn->client_hello_version, S2N_SSLv2); diff --git a/tests/unit/s2n_config_test.c b/tests/unit/s2n_config_test.c index e43930bb188..a1996f260f2 100644 --- a/tests/unit/s2n_config_test.c +++ b/tests/unit/s2n_config_test.c @@ -16,6 +16,7 @@ #include "tls/s2n_config.h" #include +#include #include "api/s2n.h" #include "crypto/s2n_fips.h" @@ -27,6 +28,7 @@ #include "tls/s2n_internal.h" #include "tls/s2n_record.h" #include "tls/s2n_security_policies.h" +#include "tls/s2n_tls.h" #include "tls/s2n_tls13.h" #include "unstable/npn.h" #include "utils/s2n_map.h" @@ -1232,5 +1234,44 @@ int main(int argc, char **argv) } } + /* Checks that servers don't use a config before the client hello callback is executed on a + * SSLv2-formatted client hello. + * + * Parsing SSLv2 hellos uses a different code path and need to be tested separately. + */ + { + uint8_t sslv2_client_hello[] = { + SSLv2_CLIENT_HELLO_PREFIX, + SSLv2_CLIENT_HELLO_CIPHER_SUITES, + SSLv2_CLIENT_HELLO_CHALLENGE, + }; + + struct s2n_blob client_hello = { + .data = sslv2_client_hello, + .size = sizeof(sslv2_client_hello), + .allocated = 0, + .growable = 0 + }; + + /* Checks that the handshake gets as far as the client hello callback with a NULL config */ + { + DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER), + s2n_connection_ptr_free); + EXPECT_NOT_NULL(server_conn); + server_conn->config = NULL; + + /* Record version and protocol version are in the header for SSLv2 */ + server_conn->client_hello_version = S2N_SSLv2; + server_conn->client_protocol_version = S2N_TLS12; + + /* S2N_ERR_CONFIG_NULL_BEFORE_CH_CALLBACK is only called in one location, just before the + * client hello callback. Therefore, we can assert that if we hit this error, we + * have gotten as far as the client hello callback without dereferencing the config. + */ + EXPECT_SUCCESS(s2n_stuffer_write(&server_conn->handshake.io, &client_hello)); + EXPECT_FAILURE_WITH_ERRNO(s2n_client_hello_recv(server_conn), S2N_ERR_CONFIG_NULL_BEFORE_CH_CALLBACK); + } + } + END_TEST(); } diff --git a/tls/s2n_client_hello.c b/tls/s2n_client_hello.c index 24e48dfee9b..880d150059a 100644 --- a/tls/s2n_client_hello.c +++ b/tls/s2n_client_hello.c @@ -446,7 +446,7 @@ int s2n_parse_client_hello(struct s2n_connection *conn) } if (conn->client_hello_version == S2N_SSLv2) { - POSIX_GUARD(s2n_sslv2_client_hello_recv(conn)); + POSIX_GUARD(s2n_sslv2_client_hello_parse(conn)); return S2N_SUCCESS; } @@ -594,8 +594,13 @@ int s2n_process_client_hello(struct s2n_connection *conn) POSIX_CHECKED_MEMCPY(previous_cipher_suite_iana, conn->secure->cipher_suite->iana_value, S2N_TLS_CIPHER_SUITE_LEN); /* Now choose the ciphers we have certs for. */ - POSIX_GUARD(s2n_set_cipher_as_tls_server(conn, client_hello->cipher_suites.data, - client_hello->cipher_suites.size / 2)); + if (conn->client_hello_version == S2N_SSLv2) { + POSIX_GUARD(s2n_set_cipher_as_sslv2_server(conn, client_hello->cipher_suites.data, + client_hello->cipher_suites.size / S2N_SSLv2_CIPHER_SUITE_LEN)); + } else { + POSIX_GUARD(s2n_set_cipher_as_tls_server(conn, client_hello->cipher_suites.data, + client_hello->cipher_suites.size / 2)); + } /* Check if this is the second client hello in a hello retry handshake */ if (s2n_is_hello_retry_handshake(conn) && conn->handshake.message_number > 0) { @@ -685,9 +690,7 @@ int s2n_client_hello_recv(struct s2n_connection *conn) } } - if (conn->client_hello_version != S2N_SSLv2) { - POSIX_GUARD(s2n_process_client_hello(conn)); - } + POSIX_GUARD(s2n_process_client_hello(conn)); return 0; } @@ -821,7 +824,7 @@ int s2n_client_hello_send(struct s2n_connection *conn) * Alternatively, the TLS1.0 RFC includes a more modern description of the format: * https://tools.ietf.org/rfc/rfc2246 Appendix E.1 */ -int s2n_sslv2_client_hello_recv(struct s2n_connection *conn) +int s2n_sslv2_client_hello_parse(struct s2n_connection *conn) { struct s2n_client_hello *client_hello = &conn->client_hello; client_hello->sslv2 = true; @@ -831,15 +834,6 @@ int s2n_sslv2_client_hello_recv(struct s2n_connection *conn) POSIX_GUARD(s2n_stuffer_skip_write(&in_stuffer, client_hello->raw_message.size)); struct s2n_stuffer *in = &in_stuffer; - const struct s2n_security_policy *security_policy = NULL; - POSIX_GUARD(s2n_connection_get_security_policy(conn, &security_policy)); - - if (conn->client_protocol_version < security_policy->minimum_protocol_version) { - POSIX_GUARD(s2n_queue_reader_unsupported_protocol_version_alert(conn)); - POSIX_BAIL(S2N_ERR_PROTOCOL_VERSION_UNSUPPORTED); - } - conn->actual_protocol_version = MIN(conn->client_protocol_version, conn->server_protocol_version); - /* We start 5 bytes into the record */ uint16_t cipher_suites_length = 0; POSIX_GUARD(s2n_stuffer_read_uint16(in, &cipher_suites_length)); @@ -858,14 +852,6 @@ int s2n_sslv2_client_hello_recv(struct s2n_connection *conn) client_hello->cipher_suites.data = s2n_stuffer_raw_read(in, cipher_suites_length); POSIX_ENSURE_REF(client_hello->cipher_suites.data); - /* Find potential certificate matches before we choose the cipher. */ - POSIX_GUARD(s2n_conn_find_name_matching_certs(conn)); - - POSIX_GUARD(s2n_set_cipher_as_sslv2_server(conn, client_hello->cipher_suites.data, - client_hello->cipher_suites.size / S2N_SSLv2_CIPHER_SUITE_LEN)); - POSIX_GUARD_RESULT(s2n_signature_algorithm_select(conn)); - POSIX_GUARD(s2n_select_certs_for_server_auth(conn, &conn->handshake_params.our_chain_and_key)); - S2N_ERROR_IF(session_id_length > s2n_stuffer_data_available(in), S2N_ERR_BAD_MESSAGE); POSIX_GUARD(s2n_blob_init(&client_hello->session_id, s2n_stuffer_raw_read(in, session_id_length), session_id_length)); if (session_id_length > 0 && session_id_length <= S2N_TLS_SESSION_ID_MAX_LEN) { diff --git a/tls/s2n_record_read.c b/tls/s2n_record_read.c index 1a078befd3f..d68f6ac5fb4 100644 --- a/tls/s2n_record_read.c +++ b/tls/s2n_record_read.c @@ -85,7 +85,7 @@ int s2n_sslv2_record_header_parse( * * The protocol version read here will likely not be SSLv2, since we only * accept SSLv2 ClientHellos offering higher protocol versions. - * See s2n_sslv2_client_hello_recv. + * See s2n_sslv2_client_hello_parse. */ uint8_t protocol_version[S2N_TLS_PROTOCOL_VERSION_LEN] = { 0 }; POSIX_GUARD(s2n_stuffer_read_bytes(header_in, protocol_version, S2N_TLS_PROTOCOL_VERSION_LEN)); diff --git a/tls/s2n_tls.h b/tls/s2n_tls.h index 3f7b6344ee2..ebabe53fc02 100644 --- a/tls/s2n_tls.h +++ b/tls/s2n_tls.h @@ -27,9 +27,10 @@ int s2n_flush(struct s2n_connection *conn, s2n_blocked_status *more); S2N_RESULT s2n_client_hello_request_validate(struct s2n_connection *conn); S2N_RESULT s2n_client_hello_request_recv(struct s2n_connection *conn); int s2n_client_hello_send(struct s2n_connection *conn); +int s2n_parse_client_hello(struct s2n_connection *conn); int s2n_client_hello_recv(struct s2n_connection *conn); int s2n_establish_session(struct s2n_connection *conn); -int s2n_sslv2_client_hello_recv(struct s2n_connection *conn); +int s2n_sslv2_client_hello_parse(struct s2n_connection *conn); int s2n_server_hello_retry_send(struct s2n_connection *conn); int s2n_server_hello_retry_recv(struct s2n_connection *conn); int s2n_server_hello_write_message(struct s2n_connection *conn);