From 2482844e13171b9ed3e224f4edb462fb0a9f11ce Mon Sep 17 00:00:00 2001 From: Lindsay Stewart Date: Mon, 20 Nov 2023 13:49:17 -0800 Subject: [PATCH] Remove NULLs in s2n_kex (#4293) --- tls/s2n_auth_selection.c | 4 ++- tls/s2n_cipher_suites.c | 28 +++++++--------- tls/s2n_kex.c | 71 +++++++++++++++++++++++++++++++++------- tls/s2n_kex.h | 1 + 4 files changed, 76 insertions(+), 28 deletions(-) diff --git a/tls/s2n_auth_selection.c b/tls/s2n_auth_selection.c index 31ed894e1b7..564b8488027 100644 --- a/tls/s2n_auth_selection.c +++ b/tls/s2n_auth_selection.c @@ -88,7 +88,9 @@ static int s2n_is_sig_alg_valid_for_cipher_suite(s2n_signature_algorithm sig_alg * Therefore, if a cipher suite uses a non-ephemeral kex, then any signature * algorithm that requires RSA-PSS certificates is not valid. */ - if (cipher_suite->key_exchange_alg != NULL && !cipher_suite->key_exchange_alg->is_ephemeral) { + const struct s2n_kex *kex = cipher_suite->key_exchange_alg; + POSIX_ENSURE_REF(kex); + if (!kex->is_ephemeral) { POSIX_ENSURE_NE(cert_type_for_sig_alg, S2N_PKEY_TYPE_RSA_PSS); } diff --git a/tls/s2n_cipher_suites.c b/tls/s2n_cipher_suites.c index 2ea36324938..c00e7852217 100644 --- a/tls/s2n_cipher_suites.c +++ b/tls/s2n_cipher_suites.c @@ -698,7 +698,7 @@ struct s2n_cipher_suite s2n_tls13_aes_128_gcm_sha256 = { .available = 0, .name = "TLS_AES_128_GCM_SHA256", .iana_value = { TLS_AES_128_GCM_SHA256 }, - .key_exchange_alg = NULL, + .key_exchange_alg = &s2n_tls13_kex, .auth_method = S2N_AUTHENTICATION_METHOD_TLS13, .record_alg = NULL, .all_record_algs = { &s2n_tls13_record_alg_aes128_gcm }, @@ -712,7 +712,7 @@ struct s2n_cipher_suite s2n_tls13_aes_256_gcm_sha384 = { .available = 0, .name = "TLS_AES_256_GCM_SHA384", .iana_value = { TLS_AES_256_GCM_SHA384 }, - .key_exchange_alg = NULL, + .key_exchange_alg = &s2n_tls13_kex, .auth_method = S2N_AUTHENTICATION_METHOD_TLS13, .record_alg = NULL, .all_record_algs = { &s2n_tls13_record_alg_aes256_gcm }, @@ -726,7 +726,7 @@ struct s2n_cipher_suite s2n_tls13_chacha20_poly1305_sha256 = { .available = 0, .name = "TLS_CHACHA20_POLY1305_SHA256", .iana_value = { TLS_CHACHA20_POLY1305_SHA256 }, - .key_exchange_alg = NULL, + .key_exchange_alg = &s2n_tls13_kex, .auth_method = S2N_AUTHENTICATION_METHOD_TLS13, .record_alg = NULL, .all_record_algs = { &s2n_tls13_record_alg_chacha20_poly1305 }, @@ -1276,19 +1276,15 @@ static int s2n_set_cipher_as_server(struct s2n_connection *conn, uint8_t *wire, continue; } - /* TLS 1.3 does not include key exchange in cipher suites */ - if (match->minimum_required_tls_version < S2N_TLS13) { - /* If the kex is not supported continue to the next candidate */ - bool kex_supported = false; - POSIX_GUARD_RESULT(s2n_kex_supported(match, conn, &kex_supported)); - if (!kex_supported) { - continue; - } - - /* If the kex is not configured correctly continue to the next candidate */ - if (s2n_result_is_error(s2n_configure_kex(match, conn))) { - continue; - } + /* If the kex is not supported continue to the next candidate */ + bool kex_supported = false; + POSIX_GUARD_RESULT(s2n_kex_supported(match, conn, &kex_supported)); + if (!kex_supported) { + continue; + } + /* If the kex is not configured correctly continue to the next candidate */ + if (s2n_result_is_error(s2n_configure_kex(match, conn))) { + continue; } /** diff --git a/tls/s2n_kex.c b/tls/s2n_kex.c index 0e5a80d8bf4..ac7529c5371 100644 --- a/tls/s2n_kex.c +++ b/tls/s2n_kex.c @@ -25,6 +25,14 @@ #include "tls/s2n_tls.h" #include "utils/s2n_safety.h" +static S2N_RESULT s2n_check_tls13(const struct s2n_cipher_suite *cipher_suite, + struct s2n_connection *conn, bool *is_supported) +{ + RESULT_ENSURE_REF(is_supported); + *is_supported = (s2n_connection_get_protocol_version(conn) >= S2N_TLS13); + return S2N_RESULT_OK; +} + static S2N_RESULT s2n_check_rsa_key(const struct s2n_cipher_suite *cipher_suite, struct s2n_connection *conn, bool *is_supported) { RESULT_ENSURE_REF(cipher_suite); @@ -135,11 +143,6 @@ static S2N_RESULT s2n_configure_kem(const struct s2n_cipher_suite *cipher_suite, return S2N_RESULT_OK; } -static S2N_RESULT s2n_no_op_configure(const struct s2n_cipher_suite *cipher_suite, struct s2n_connection *conn) -{ - return S2N_RESULT_OK; -} - static S2N_RESULT s2n_check_hybrid_ecdhe_kem(const struct s2n_cipher_suite *cipher_suite, struct s2n_connection *conn, bool *is_supported) { RESULT_ENSURE_REF(cipher_suite); @@ -156,6 +159,34 @@ static S2N_RESULT s2n_check_hybrid_ecdhe_kem(const struct s2n_cipher_suite *ciph return S2N_RESULT_OK; } +static S2N_RESULT s2n_kex_configure_noop(const struct s2n_cipher_suite *cipher_suite, + struct s2n_connection *conn) +{ + return S2N_RESULT_OK; +} + +static int s2n_kex_server_key_recv_read_data_unimplemented(struct s2n_connection *conn, + struct s2n_blob *data_to_verify, struct s2n_kex_raw_server_data *kex_data) +{ + POSIX_BAIL(S2N_ERR_UNIMPLEMENTED); +} + +static int s2n_kex_server_key_recv_parse_data_unimplemented(struct s2n_connection *conn, + struct s2n_kex_raw_server_data *kex_data) +{ + POSIX_BAIL(S2N_ERR_UNIMPLEMENTED); +} + +static int s2n_kex_io_unimplemented(struct s2n_connection *conn, struct s2n_blob *data_to_sign) +{ + POSIX_BAIL(S2N_ERR_UNIMPLEMENTED); +} + +static int s2n_kex_prf_unimplemented(struct s2n_connection *conn, struct s2n_blob *premaster_secret) +{ + POSIX_BAIL(S2N_ERR_UNIMPLEMENTED); +} + const struct s2n_kex s2n_kem = { .is_ephemeral = true, .connection_supported = &s2n_check_kem, @@ -165,15 +196,16 @@ const struct s2n_kex s2n_kem = { .server_key_send = &s2n_kem_server_key_send, .client_key_recv = &s2n_kem_client_key_recv, .client_key_send = &s2n_kem_client_key_send, + .prf = &s2n_kex_prf_unimplemented, }; const struct s2n_kex s2n_rsa = { .is_ephemeral = false, .connection_supported = &s2n_check_rsa_key, - .configure_connection = &s2n_no_op_configure, - .server_key_recv_read_data = NULL, - .server_key_recv_parse_data = NULL, - .server_key_send = NULL, + .configure_connection = &s2n_kex_configure_noop, + .server_key_recv_read_data = &s2n_kex_server_key_recv_read_data_unimplemented, + .server_key_recv_parse_data = &s2n_kex_server_key_recv_parse_data_unimplemented, + .server_key_send = &s2n_kex_io_unimplemented, .client_key_recv = &s2n_rsa_client_key_recv, .client_key_send = &s2n_rsa_client_key_send, .prf = &s2n_prf_calculate_master_secret, @@ -182,7 +214,7 @@ const struct s2n_kex s2n_rsa = { const struct s2n_kex s2n_dhe = { .is_ephemeral = true, .connection_supported = &s2n_check_dhe, - .configure_connection = &s2n_no_op_configure, + .configure_connection = &s2n_kex_configure_noop, .server_key_recv_read_data = &s2n_dhe_server_key_recv_read_data, .server_key_recv_parse_data = &s2n_dhe_server_key_recv_parse_data, .server_key_send = &s2n_dhe_server_key_send, @@ -194,7 +226,7 @@ const struct s2n_kex s2n_dhe = { const struct s2n_kex s2n_ecdhe = { .is_ephemeral = true, .connection_supported = &s2n_check_ecdhe, - .configure_connection = &s2n_no_op_configure, + .configure_connection = &s2n_kex_configure_noop, .server_key_recv_read_data = &s2n_ecdhe_server_key_recv_read_data, .server_key_recv_parse_data = &s2n_ecdhe_server_key_recv_parse_data, .server_key_send = &s2n_ecdhe_server_key_send, @@ -216,6 +248,23 @@ const struct s2n_kex s2n_hybrid_ecdhe_kem = { .prf = &s2n_hybrid_prf_master_secret, }; +/* TLS1.3 key exchange is implemented differently from previous versions and does + * not currently require most of the functionality offered by s2n_kex. + * This structure primarily acts as a placeholder, so its methods are either + * noops or unimplemented. + */ +const struct s2n_kex s2n_tls13_kex = { + .is_ephemeral = true, + .connection_supported = &s2n_check_tls13, + .configure_connection = &s2n_kex_configure_noop, + .server_key_recv_read_data = &s2n_kex_server_key_recv_read_data_unimplemented, + .server_key_recv_parse_data = &s2n_kex_server_key_recv_parse_data_unimplemented, + .server_key_send = &s2n_kex_io_unimplemented, + .client_key_recv = &s2n_kex_io_unimplemented, + .client_key_send = &s2n_kex_io_unimplemented, + .prf = &s2n_kex_prf_unimplemented, +}; + S2N_RESULT s2n_kex_supported(const struct s2n_cipher_suite *cipher_suite, struct s2n_connection *conn, bool *is_supported) { RESULT_ENSURE_REF(cipher_suite); diff --git a/tls/s2n_kex.h b/tls/s2n_kex.h index 9f31b0bd2a5..f229914e2ad 100644 --- a/tls/s2n_kex.h +++ b/tls/s2n_kex.h @@ -40,6 +40,7 @@ extern const struct s2n_kex s2n_rsa; extern const struct s2n_kex s2n_dhe; extern const struct s2n_kex s2n_ecdhe; extern const struct s2n_kex s2n_hybrid_ecdhe_kem; +extern const struct s2n_kex s2n_tls13_kex; S2N_RESULT s2n_kex_supported(const struct s2n_cipher_suite *cipher_suite, struct s2n_connection *conn, bool *is_supported); S2N_RESULT s2n_configure_kex(const struct s2n_cipher_suite *cipher_suite, struct s2n_connection *conn);