From e088cb7079a39825f3aeddfab90cd014a0a44340 Mon Sep 17 00:00:00 2001 From: Lindsay Stewart Date: Wed, 2 Aug 2023 00:05:50 -0700 Subject: [PATCH] Clean up receiving supported sig algs --- ...ient_signature_algorithms_extension_test.c | 4 +- tests/unit/s2n_signature_algorithms_test.c | 144 ++++++---- .../s2n_client_signature_algorithms.c | 13 +- tls/extensions/s2n_extension_list.c | 51 +++- tls/extensions/s2n_extension_list.h | 2 + .../s2n_server_signature_algorithms.c | 3 +- tls/s2n_client_cert_verify.c | 6 +- tls/s2n_client_hello.c | 9 +- tls/s2n_handshake.h | 4 - tls/s2n_server_cert_request.c | 5 +- tls/s2n_signature_algorithms.c | 260 ++++++------------ tls/s2n_signature_algorithms.h | 20 +- 12 files changed, 253 insertions(+), 268 deletions(-) diff --git a/tests/unit/s2n_client_signature_algorithms_extension_test.c b/tests/unit/s2n_client_signature_algorithms_extension_test.c index 817e66b3188..d7ff63e5cf1 100644 --- a/tests/unit/s2n_client_signature_algorithms_extension_test.c +++ b/tests/unit/s2n_client_signature_algorithms_extension_test.c @@ -91,7 +91,7 @@ int main(int argc, char **argv) conn->actual_protocol_version = S2N_TLS13; EXPECT_SUCCESS(s2n_client_signature_algorithms_extension.recv(conn, &signature_algorithms_extension)); EXPECT_EQUAL(conn->handshake_params.client_sig_hash_algs.len, sig_hash_algs.len); - EXPECT_FAILURE(s2n_choose_sig_scheme_from_peer_preference_list(conn, &conn->handshake_params.client_sig_hash_algs, + EXPECT_FAILURE(s2n_process_peer_signature_preference_list(conn, &conn->handshake_params.client_sig_hash_algs, &conn->handshake_params.server_cert_sig_scheme)); EXPECT_SUCCESS(s2n_stuffer_free(&signature_algorithms_extension)); @@ -119,7 +119,7 @@ int main(int argc, char **argv) /* If a valid algorithm is offered among unknown algorithms, the valid one should be chosen */ EXPECT_SUCCESS(s2n_client_signature_algorithms_extension.recv(conn, &signature_algorithms_extension)); EXPECT_EQUAL(conn->handshake_params.client_sig_hash_algs.len, sig_hash_algs.len); - EXPECT_SUCCESS(s2n_choose_sig_scheme_from_peer_preference_list(conn, &conn->handshake_params.client_sig_hash_algs, + EXPECT_SUCCESS(s2n_process_peer_signature_preference_list(conn, &conn->handshake_params.client_sig_hash_algs, &conn->handshake_params.server_cert_sig_scheme)); EXPECT_EQUAL(conn->handshake_params.server_cert_sig_scheme->iana_value, TLS_SIGNATURE_SCHEME_RSA_PKCS1_SHA384); diff --git a/tests/unit/s2n_signature_algorithms_test.c b/tests/unit/s2n_signature_algorithms_test.c index 38558a4f93b..2891554afc9 100644 --- a/tests/unit/s2n_signature_algorithms_test.c +++ b/tests/unit/s2n_signature_algorithms_test.c @@ -33,8 +33,11 @@ #define ECDSA_CIPHER_SUITE &s2n_ecdhe_ecdsa_with_aes_128_cbc_sha #define TLS13_CIPHER_SUITE &s2n_tls13_aes_128_gcm_sha256 +#define S2N_IANA_TO_BYTES(iana) (iana) >> 8, (iana) & 0xFF + const struct s2n_signature_scheme *const test_signature_schemes[] = { &s2n_ecdsa_secp384r1_sha384, + &s2n_ecdsa_sha256, &s2n_rsa_pkcs1_sha256, &s2n_rsa_pkcs1_sha224, &s2n_rsa_pkcs1_sha1, @@ -116,28 +119,6 @@ int main(int argc, char **argv) EXPECT_EQUAL(s2n_stuffer_data_available(&result), 0); }; - /* Test: written signatures readable */ - { - DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT), - s2n_connection_ptr_free); - EXPECT_NOT_NULL(conn); - conn->security_policy_override = &test_security_policy; - conn->actual_protocol_version = S2N_TLS13; - - DEFER_CLEANUP(struct s2n_stuffer result = { 0 }, s2n_stuffer_free); - EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&result, 0)); - EXPECT_OK(s2n_signature_algorithms_supported_list_send(conn, &result)); - - struct s2n_sig_scheme_list signatures = { 0 }; - EXPECT_SUCCESS(s2n_recv_supported_sig_scheme_list(&result, &signatures)); - EXPECT_EQUAL(s2n_stuffer_data_available(&result), 0); - - EXPECT_EQUAL(signatures.len, s2n_array_len(test_signature_schemes)); - for (size_t i = 0; i < s2n_array_len(test_signature_schemes); i++) { - EXPECT_EQUAL(signatures.iana_list[i], test_signature_schemes[i]->iana_value); - } - }; - /* Test: do not send TLS1.2 signature schemes if QUIC enabled */ { DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT), @@ -197,7 +178,7 @@ int main(int argc, char **argv) EXPECT_EQUAL(conn->handshake_params.client_cert_sig_scheme, &s2n_rsa_pkcs1_sha256); }; - /* Test: algorithm not included in message */ + /* Test: no algorithm provided */ { struct s2n_stuffer empty = { 0 }; @@ -316,6 +297,72 @@ int main(int argc, char **argv) }; }; + /* s2n_signature_algorithms_supported_list_process */ + { + 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, rsa_cert_chain)); + EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(server_config, ecdsa_cert_chain)); + + DEFER_CLEANUP(struct s2n_config *client_ecdsa_config = s2n_config_new(), s2n_config_ptr_free); + EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(client_ecdsa_config, ecdsa_cert_chain)); + + struct s2n_security_policy test_security_policy = *s2n_fetch_default_config()->security_policy; + test_security_policy.signature_preferences = &test_preferences; + server_config->security_policy = &test_security_policy; + client_ecdsa_config->security_policy = &test_security_policy; + + /* Test: successfully choose server signature scheme */ + { + const struct s2n_signature_scheme *expected = &s2n_ecdsa_sha256; + uint8_t ianas[] = { + S2N_IANA_TO_BYTES(expected->iana_value) + }; + struct s2n_blob peer_ianas = { 0 }; + EXPECT_SUCCESS(s2n_blob_init(&peer_ianas, ianas, sizeof(ianas))); + + DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT), + s2n_connection_ptr_free); + EXPECT_SUCCESS(s2n_connection_set_config(conn, client_ecdsa_config)); + + EXPECT_OK(s2n_signature_algorithms_supported_list_process(conn, &peer_ianas)); + EXPECT_EQUAL(conn->handshake_params.client_cert_sig_scheme, expected); + }; + + /* Test: successfully choose client signature scheme */ + { + const struct s2n_signature_scheme *expected = &s2n_ecdsa_sha256; + uint8_t ianas[] = { + S2N_IANA_TO_BYTES(expected->iana_value) + }; + struct s2n_blob peer_ianas = { 0 }; + EXPECT_SUCCESS(s2n_blob_init(&peer_ianas, ianas, sizeof(ianas))); + + DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_SERVER), + s2n_connection_ptr_free); + conn->actual_protocol_version = S2N_TLS12; + conn->secure->cipher_suite = ECDSA_CIPHER_SUITE; + EXPECT_SUCCESS(s2n_connection_set_config(conn, server_config)); + + EXPECT_OK(s2n_signature_algorithms_supported_list_process(conn, &peer_ianas)); + EXPECT_EQUAL(conn->handshake_params.server_cert_sig_scheme, expected); + }; + + /* Test: choose server most preferred, not client most preferred */ + /* Test: choose legacy default for actual_protocol_version = S2N_TLS13; EXPECT_SUCCESS(s2n_connection_set_config(conn, config)); - struct s2n_sig_scheme_list peer_list = { - .len = 1, - .iana_list = { s2n_rsa_pss_rsae_sha256.iana_value }, + uint8_t ianas[] = { + S2N_IANA_TO_BYTES(s2n_rsa_pss_rsae_sha256.iana_value) }; - - const struct s2n_signature_scheme *result = NULL; + struct s2n_blob peer_list = { 0 }; + EXPECT_SUCCESS(s2n_blob_init(&peer_list, ianas, sizeof(ianas))); if (s2n_is_rsa_pss_signing_supported()) { - EXPECT_SUCCESS(s2n_choose_sig_scheme_from_peer_preference_list(conn, &peer_list, &result)); - EXPECT_EQUAL(result, &s2n_rsa_pss_rsae_sha256); + EXPECT_OK(s2n_signature_algorithms_supported_list_process( + conn, &peer_list)); + EXPECT_EQUAL(conn->handshake_params.client_cert_sig_scheme, + &s2n_rsa_pss_rsae_sha256); } else { - EXPECT_FAILURE_WITH_ERRNO(s2n_choose_sig_scheme_from_peer_preference_list(conn, &peer_list, &result), - S2N_ERR_INVALID_SIGNATURE_SCHEME); + EXPECT_ERROR_WITH_ERRNO(s2n_signature_algorithms_supported_list_process( + conn, &peer_list), S2N_ERR_INVALID_SIGNATURE_SCHEME); } }; }; @@ -863,18 +911,17 @@ int main(int argc, char **argv) /* Test: no shared valid signature schemes, using TLS1.3. Server cant pick preferred */ { - const struct s2n_signature_scheme *result = NULL; conn->secure->cipher_suite = TLS13_CIPHER_SUITE; conn->actual_protocol_version = S2N_TLS13; - struct s2n_sig_scheme_list peer_list = { - .len = 1, - .iana_list = { - s2n_rsa_pkcs1_sha224.iana_value, /* Invalid (wrong protocol version) */ - }, + uint8_t ianas[] = { + /* Invalid (PKCS1 not allowed by TLS1.3) */ + S2N_IANA_TO_BYTES(s2n_rsa_pkcs1_sha224.iana_value) }; + struct s2n_blob peer_list = { 0 }; + EXPECT_SUCCESS(s2n_blob_init(&peer_list, ianas, sizeof(ianas))); - EXPECT_FAILURE_WITH_ERRNO(s2n_choose_sig_scheme_from_peer_preference_list(conn, &peer_list, &result), + EXPECT_ERROR_WITH_ERRNO(s2n_signature_algorithms_supported_list_process(conn, &peer_list), S2N_ERR_INVALID_SIGNATURE_SCHEME); }; @@ -882,20 +929,19 @@ int main(int argc, char **argv) /* Test: no shared valid signature schemes, using TLS1.3. Server picks a preferred */ { - const struct s2n_signature_scheme *result = NULL; conn->secure->cipher_suite = TLS13_CIPHER_SUITE; conn->actual_protocol_version = S2N_TLS13; - struct s2n_sig_scheme_list peer_list = { - .len = 1, - .iana_list = { - s2n_rsa_pkcs1_sha224.iana_value, /* Invalid (wrong protocol version) */ - }, + uint8_t ianas[] = { + /* Invalid (PKCS1 not allowed by TLS1.3) */ + S2N_IANA_TO_BYTES(s2n_rsa_pkcs1_sha224.iana_value) }; + struct s2n_blob peer_list = { 0 }; + EXPECT_SUCCESS(s2n_blob_init(&peer_list, ianas, sizeof(ianas))); - /* behavior is that we fallback to a preferred signature algorithm */ - EXPECT_SUCCESS(s2n_choose_sig_scheme_from_peer_preference_list(conn, &peer_list, &result)); - EXPECT_EQUAL(result, &s2n_rsa_pss_rsae_sha256); + /* behavior is that we fallback to a preferred PSS signature algorithm */ + EXPECT_OK(s2n_signature_algorithms_supported_list_process(conn, &peer_list)); + EXPECT_EQUAL(conn->handshake_params.server_cert_sig_scheme, &s2n_rsa_pss_rsae_sha256); }; s2n_connection_free(conn); @@ -1014,7 +1060,7 @@ int main(int argc, char **argv) server_conn->security_policy_override = &sha384_policy; EXPECT_FAILURE_WITH_ERRNO(s2n_negotiate_test_server_and_client(server_conn, client_conn), - S2N_ERR_INVALID_SIGNATURE_ALGORITHM); + S2N_ERR_INVALID_SIGNATURE_SCHEME); EXPECT_EQUAL(client_conn->actual_protocol_version, S2N_TLS12); EXPECT_EQUAL(server_conn->actual_protocol_version, S2N_TLS12); }; diff --git a/tls/extensions/s2n_client_signature_algorithms.c b/tls/extensions/s2n_client_signature_algorithms.c index c1f06daf2e6..020235a9283 100644 --- a/tls/extensions/s2n_client_signature_algorithms.c +++ b/tls/extensions/s2n_client_signature_algorithms.c @@ -24,6 +24,7 @@ #include "utils/s2n_safety.h" static bool s2n_client_signature_algorithms_should_send(struct s2n_connection *conn); +static int s2n_client_signature_algorithms_if_missing(struct s2n_connection *conn); static int s2n_client_signature_algorithms_send(struct s2n_connection *conn, struct s2n_stuffer *extension); static int s2n_client_signature_algorithms_recv(struct s2n_connection *conn, struct s2n_stuffer *extension); @@ -33,7 +34,7 @@ const s2n_extension_type s2n_client_signature_algorithms_extension = { .send = s2n_client_signature_algorithms_send, .recv = s2n_client_signature_algorithms_recv, .should_send = s2n_client_signature_algorithms_should_send, - .if_missing = s2n_extension_noop_if_missing, + .if_missing = s2n_client_signature_algorithms_if_missing, }; static bool s2n_client_signature_algorithms_should_send(struct s2n_connection *conn) @@ -41,6 +42,13 @@ static bool s2n_client_signature_algorithms_should_send(struct s2n_connection *c return s2n_connection_get_protocol_version(conn) >= S2N_TLS12; } +static int s2n_client_signature_algorithms_if_missing(struct s2n_connection *conn) +{ + struct s2n_blob empty_list = { 0 }; + POSIX_GUARD_RESULT(s2n_signature_algorithms_supported_list_process(conn, &empty_list)); + return S2N_SUCCESS; +} + static int s2n_client_signature_algorithms_send(struct s2n_connection *conn, struct s2n_stuffer *extension) { POSIX_GUARD_RESULT(s2n_signature_algorithms_supported_list_send(conn, extension)); @@ -49,5 +57,6 @@ static int s2n_client_signature_algorithms_send(struct s2n_connection *conn, str static int s2n_client_signature_algorithms_recv(struct s2n_connection *conn, struct s2n_stuffer *extension) { - return s2n_recv_supported_sig_scheme_list(extension, &conn->handshake_params.client_sig_hash_algs); + POSIX_GUARD_RESULT(s2n_signature_algorithms_supported_list_recv(conn, extension)); + return S2N_SUCCESS; } diff --git a/tls/extensions/s2n_extension_list.c b/tls/extensions/s2n_extension_list.c index 71fd5213fdd..3a625730031 100644 --- a/tls/extensions/s2n_extension_list.c +++ b/tls/extensions/s2n_extension_list.c @@ -47,16 +47,38 @@ int s2n_extension_list_recv(s2n_extension_list_id list_type, struct s2n_connecti return S2N_SUCCESS; } +static S2N_RESULT s2n_extension_get_parsed(const s2n_extension_type *extension_type, + s2n_parsed_extensions_list *parsed_extension_list, s2n_parsed_extension **parsed_extension) +{ + RESULT_ENSURE_REF(extension_type); + RESULT_ENSURE_REF(parsed_extension_list); + RESULT_ENSURE_REF(parsed_extension); + + s2n_extension_type_id extension_id = 0; + RESULT_GUARD_POSIX(s2n_extension_supported_iana_value_to_id(extension_type->iana_value, &extension_id)); + + *parsed_extension = &parsed_extension_list->parsed_extensions[extension_id]; + return S2N_RESULT_OK; +} + +S2N_RESULT s2n_extension_skip(const s2n_extension_type *extension_type, + s2n_parsed_extensions_list *parsed_extension_list) +{ + s2n_parsed_extension *parsed_extension = NULL; + RESULT_GUARD(s2n_extension_get_parsed(extension_type, parsed_extension_list, + &parsed_extension)); + RESULT_ENSURE_REF(parsed_extension); + + parsed_extension->processed = true; + return S2N_RESULT_OK; +} + static int s2n_extension_process_impl(const s2n_extension_type *extension_type, struct s2n_connection *conn, s2n_parsed_extension *parsed_extension) { POSIX_ENSURE_REF(extension_type); POSIX_ENSURE_REF(parsed_extension); - if (parsed_extension->processed) { - return S2N_SUCCESS; - } - if (s2n_parsed_extension_is_empty(parsed_extension)) { POSIX_GUARD(s2n_extension_is_missing(extension_type, conn)); return S2N_SUCCESS; @@ -70,7 +92,7 @@ static int s2n_extension_process_impl(const s2n_extension_type *extension_type, POSIX_GUARD(s2n_stuffer_skip_write(&extension_stuffer, parsed_extension->extension.size)); POSIX_GUARD(s2n_extension_recv(extension_type, conn, &extension_stuffer)); - + parsed_extension->processed = true; return S2N_SUCCESS; } @@ -80,12 +102,11 @@ int s2n_extension_process(const s2n_extension_type *extension_type, struct s2n_c POSIX_ENSURE_REF(parsed_extension_list); POSIX_ENSURE_REF(extension_type); - s2n_extension_type_id extension_id; - POSIX_GUARD(s2n_extension_supported_iana_value_to_id(extension_type->iana_value, &extension_id)); + s2n_parsed_extension *parsed_extension = NULL; + POSIX_GUARD_RESULT(s2n_extension_get_parsed(extension_type, parsed_extension_list, + &parsed_extension)); - s2n_parsed_extension *parsed_extension = &parsed_extension_list->parsed_extensions[extension_id]; POSIX_GUARD(s2n_extension_process_impl(extension_type, conn, parsed_extension)); - parsed_extension->processed = true; return S2N_SUCCESS; } @@ -98,8 +119,16 @@ int s2n_extension_list_process(s2n_extension_list_id list_type, struct s2n_conne POSIX_GUARD(s2n_extension_type_list_get(list_type, &extension_type_list)); for (int i = 0; i < extension_type_list->count; i++) { - POSIX_GUARD(s2n_extension_process(extension_type_list->extension_types[i], - conn, parsed_extension_list)); + const s2n_extension_type *extension_type = extension_type_list->extension_types[i]; + + s2n_parsed_extension *parsed_extension = NULL; + POSIX_GUARD_RESULT(s2n_extension_get_parsed(extension_type, parsed_extension_list, + &parsed_extension)); + + if (parsed_extension->processed) { + continue; + } + POSIX_GUARD(s2n_extension_process(extension_type, conn, parsed_extension_list)); } /** diff --git a/tls/extensions/s2n_extension_list.h b/tls/extensions/s2n_extension_list.h index 411c7e4d7cc..fcee0d08445 100644 --- a/tls/extensions/s2n_extension_list.h +++ b/tls/extensions/s2n_extension_list.h @@ -50,6 +50,8 @@ typedef enum { int s2n_extension_list_send(s2n_extension_list_id list_type, struct s2n_connection *conn, struct s2n_stuffer *out); int s2n_extension_list_recv(s2n_extension_list_id list_type, struct s2n_connection *conn, struct s2n_stuffer *in); +S2N_RESULT s2n_extension_skip(const s2n_extension_type *extension_type, + s2n_parsed_extensions_list *parsed_extension_list); int s2n_extension_process(const s2n_extension_type *extension_type, struct s2n_connection *conn, s2n_parsed_extensions_list *parsed_extension_list); int s2n_extension_list_process(s2n_extension_list_id list_type, struct s2n_connection *conn, diff --git a/tls/extensions/s2n_server_signature_algorithms.c b/tls/extensions/s2n_server_signature_algorithms.c index 7599a58a984..2f8edb6411f 100644 --- a/tls/extensions/s2n_server_signature_algorithms.c +++ b/tls/extensions/s2n_server_signature_algorithms.c @@ -44,5 +44,6 @@ static int s2n_signature_algorithms_send(struct s2n_connection *conn, struct s2n static int s2n_signature_algorithms_recv(struct s2n_connection *conn, struct s2n_stuffer *extension) { - return s2n_recv_supported_sig_scheme_list(extension, &conn->handshake_params.server_sig_hash_algs); + POSIX_GUARD_RESULT(s2n_signature_algorithms_supported_list_recv(conn, extension)); + return S2N_SUCCESS; } diff --git a/tls/s2n_client_cert_verify.c b/tls/s2n_client_cert_verify.c index 80c70be08da..3c4a3b2a985 100644 --- a/tls/s2n_client_cert_verify.c +++ b/tls/s2n_client_cert_verify.c @@ -66,13 +66,9 @@ int s2n_client_cert_verify_send(struct s2n_connection *conn) S2N_ASYNC_PKEY_GUARD(conn); struct s2n_stuffer *out = &conn->handshake.io; - if (conn->actual_protocol_version < S2N_TLS12) { - POSIX_GUARD(s2n_choose_default_sig_scheme(conn, &conn->handshake_params.client_cert_sig_scheme, S2N_CLIENT)); - } else { - POSIX_GUARD(s2n_stuffer_write_uint16(out, conn->handshake_params.client_cert_sig_scheme->iana_value)); - } const struct s2n_signature_scheme *chosen_sig_scheme = conn->handshake_params.client_cert_sig_scheme; POSIX_ENSURE_REF(chosen_sig_scheme); + POSIX_GUARD(s2n_stuffer_write_uint16(out, chosen_sig_scheme->iana_value)); /* Use a copy of the hash state since the verify digest computation may modify the running hash state we need later. */ struct s2n_hash_state *hash_state = &hashes->hash_workspace; diff --git a/tls/s2n_client_hello.c b/tls/s2n_client_hello.c index ef147b307e6..de90f4c13c2 100644 --- a/tls/s2n_client_hello.c +++ b/tls/s2n_client_hello.c @@ -28,6 +28,7 @@ #include "stuffer/s2n_stuffer.h" #include "tls/extensions/s2n_client_supported_groups.h" #include "tls/extensions/s2n_extension_list.h" +#include "tls/extensions/s2n_client_signature_algorithms.h" #include "tls/extensions/s2n_server_key_share.h" #include "tls/s2n_alerts.h" #include "tls/s2n_auth_selection.h" @@ -544,6 +545,7 @@ int s2n_process_client_hello(struct s2n_connection *conn) conn->actual_protocol_version = MIN(conn->server_protocol_version, S2N_TLS12); } + POSIX_GUARD_RESULT(s2n_extension_skip(&s2n_client_signature_algorithms_extension, &conn->client_hello.extensions)); POSIX_GUARD(s2n_extension_list_process(S2N_EXTENSION_LIST_CLIENT_HELLO, conn, &conn->client_hello.extensions)); /* After parsing extensions, select a curve and corresponding keyshare to use */ @@ -595,10 +597,8 @@ int s2n_process_client_hello(struct s2n_connection *conn) return S2N_SUCCESS; } - /* And set the signature and hash algorithm used for key exchange signatures */ - POSIX_GUARD(s2n_choose_sig_scheme_from_peer_preference_list(conn, - &conn->handshake_params.client_sig_hash_algs, - &conn->handshake_params.server_cert_sig_scheme)); + POSIX_GUARD(s2n_extension_process(&s2n_client_signature_algorithms_extension, + conn, &conn->client_hello.extensions)); /* And finally, set the certs specified by the final auth + sig_alg combo. */ POSIX_GUARD(s2n_select_certs_for_server_auth(conn, &conn->handshake_params.our_chain_and_key)); @@ -833,7 +833,6 @@ int s2n_sslv2_client_hello_recv(struct s2n_connection *conn) 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(s2n_choose_default_sig_scheme(conn, &conn->handshake_params.server_cert_sig_scheme, S2N_SERVER)); 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); diff --git a/tls/s2n_handshake.h b/tls/s2n_handshake.h index 6ff1664cf48..d474dccc69c 100644 --- a/tls/s2n_handshake.h +++ b/tls/s2n_handshake.h @@ -101,13 +101,9 @@ struct s2n_handshake_parameters { struct s2n_blob client_cert_chain; s2n_pkey_type client_cert_pkey_type; - /* Signature/hash algorithm pairs offered by the client in the signature_algorithms extension */ - struct s2n_sig_scheme_list client_sig_hash_algs; /* Signature scheme chosen by the server */ const struct s2n_signature_scheme *server_cert_sig_scheme; - /* Signature/hash algorithm pairs offered by the server in the certificate request */ - struct s2n_sig_scheme_list server_sig_hash_algs; /* Signature scheme chosen by the client */ const struct s2n_signature_scheme *client_cert_sig_scheme; diff --git a/tls/s2n_server_cert_request.c b/tls/s2n_server_cert_request.c index f838188f2e7..cd501249717 100644 --- a/tls/s2n_server_cert_request.c +++ b/tls/s2n_server_cert_request.c @@ -84,9 +84,6 @@ static int s2n_recv_client_cert_preferences(struct s2n_stuffer *in, s2n_cert_typ static int s2n_set_cert_chain_as_client(struct s2n_connection *conn) { if (s2n_config_get_num_default_certs(conn->config) > 0) { - POSIX_GUARD(s2n_choose_sig_scheme_from_peer_preference_list(conn, &conn->handshake_params.server_sig_hash_algs, - &conn->handshake_params.client_cert_sig_scheme)); - struct s2n_cert_chain_and_key *cert = s2n_config_get_single_default_cert(conn->config); POSIX_ENSURE_REF(cert); conn->handshake_params.our_chain_and_key = cert; @@ -121,7 +118,7 @@ int s2n_cert_req_recv(struct s2n_connection *conn) POSIX_GUARD(s2n_recv_client_cert_preferences(in, &cert_type)); if (conn->actual_protocol_version == S2N_TLS12) { - POSIX_GUARD(s2n_recv_supported_sig_scheme_list(in, &conn->handshake_params.server_sig_hash_algs)); + POSIX_GUARD_RESULT(s2n_signature_algorithms_supported_list_recv(conn, in)); } uint16_t cert_authorities_len = 0; diff --git a/tls/s2n_signature_algorithms.c b/tls/s2n_signature_algorithms.c index fb7668aac0c..62dfe7cc4c0 100644 --- a/tls/s2n_signature_algorithms.c +++ b/tls/s2n_signature_algorithms.c @@ -21,6 +21,7 @@ #include "error/s2n_errno.h" #include "tls/s2n_auth_selection.h" #include "tls/s2n_cipher_suites.h" +#include "tls/s2n_connection.h" #include "tls/s2n_kex.h" #include "tls/s2n_security_policies.h" #include "tls/s2n_signature_scheme.h" @@ -88,86 +89,14 @@ static bool s2n_signature_scheme_is_valid_for_recv(struct s2n_connection *conn, return s2n_result_is_ok(s2n_signature_scheme_validate_for_recv(conn, scheme)); } -static int s2n_is_signature_scheme_usable(struct s2n_connection *conn, const struct s2n_signature_scheme *candidate) -{ - POSIX_ENSURE_REF(conn); - POSIX_ENSURE_REF(candidate); - - POSIX_GUARD_RESULT(s2n_signature_scheme_validate_for_recv(conn, candidate)); - POSIX_GUARD(s2n_is_sig_scheme_valid_for_auth(conn, candidate)); - - return S2N_SUCCESS; -} - -static int s2n_choose_sig_scheme(struct s2n_connection *conn, struct s2n_sig_scheme_list *peer_wire_prefs, - const struct s2n_signature_scheme **chosen_scheme_out) -{ - POSIX_ENSURE_REF(conn); - POSIX_ENSURE_REF(conn->secure); - const struct s2n_signature_preferences *signature_preferences = NULL; - POSIX_GUARD(s2n_connection_get_signature_preferences(conn, &signature_preferences)); - POSIX_ENSURE_REF(signature_preferences); - - struct s2n_cipher_suite *cipher_suite = conn->secure->cipher_suite; - POSIX_ENSURE_REF(cipher_suite); - - for (size_t i = 0; i < signature_preferences->count; i++) { - const struct s2n_signature_scheme *candidate = signature_preferences->signature_schemes[i]; - - if (s2n_is_signature_scheme_usable(conn, candidate) != S2N_SUCCESS) { - continue; - } - - for (size_t j = 0; j < peer_wire_prefs->len; j++) { - uint16_t their_iana_val = peer_wire_prefs->iana_list[j]; - - if (candidate->iana_value == their_iana_val) { - *chosen_scheme_out = candidate; - return S2N_SUCCESS; - } - } - } - - /* do not error even if there's no match */ - return S2N_SUCCESS; -} - -/* similar to s2n_choose_sig_scheme() without matching client's preference */ -int s2n_tls13_default_sig_scheme(struct s2n_connection *conn, - const struct s2n_signature_scheme **chosen_scheme_out) -{ - POSIX_ENSURE_REF(conn); - POSIX_ENSURE_REF(conn->secure); - - const struct s2n_signature_preferences *signature_preferences = NULL; - POSIX_GUARD(s2n_connection_get_signature_preferences(conn, &signature_preferences)); - POSIX_ENSURE_REF(signature_preferences); - - struct s2n_cipher_suite *cipher_suite = conn->secure->cipher_suite; - POSIX_ENSURE_REF(cipher_suite); - - for (size_t i = 0; i < signature_preferences->count; i++) { - const struct s2n_signature_scheme *candidate = signature_preferences->signature_schemes[i]; - - if (s2n_is_signature_scheme_usable(conn, candidate) != S2N_SUCCESS) { - continue; - } - - *chosen_scheme_out = candidate; - return S2N_SUCCESS; - } - - POSIX_BAIL(S2N_ERR_INVALID_SIGNATURE_SCHEME); -} - static S2N_RESULT s2n_signature_algorithms_get_legacy_default(struct s2n_connection *conn, - const struct s2n_signature_scheme **default_sig_scheme) + s2n_mode signer, const struct s2n_signature_scheme **default_sig_scheme) { RESULT_ENSURE_REF(conn); RESULT_ENSURE_REF(default_sig_scheme); s2n_authentication_method auth_method = 0; - if (conn->mode == S2N_SERVER) { + if (signer == S2N_CLIENT) { RESULT_GUARD_POSIX(s2n_get_auth_method_for_cert_type( conn->handshake_params.client_cert_pkey_type, &auth_method)); } else { @@ -189,7 +118,8 @@ S2N_RESULT s2n_signature_algorithm_recv(struct s2n_connection *conn, struct s2n_ RESULT_ENSURE_REF(conn); const struct s2n_signature_scheme **chosen_sig_scheme = NULL; - if (conn->mode == S2N_SERVER) { + s2n_mode peer_mode = S2N_PEER_MODE(conn->mode); + if (peer_mode == S2N_CLIENT) { chosen_sig_scheme = &conn->handshake_params.client_cert_sig_scheme; } else { chosen_sig_scheme = &conn->handshake_params.server_cert_sig_scheme; @@ -197,7 +127,7 @@ S2N_RESULT s2n_signature_algorithm_recv(struct s2n_connection *conn, struct s2n_ /* Before TLS1.2, signature algorithms were fixed instead of negotiated */ if (conn->actual_protocol_version < S2N_TLS12) { - return s2n_signature_algorithms_get_legacy_default(conn, chosen_sig_scheme); + return s2n_signature_algorithms_get_legacy_default(conn, peer_mode, chosen_sig_scheme); } uint16_t iana_value = 0; @@ -226,81 +156,105 @@ S2N_RESULT s2n_signature_algorithm_recv(struct s2n_connection *conn, struct s2n_ RESULT_BAIL(S2N_ERR_INVALID_SIGNATURE_SCHEME); } -int s2n_choose_default_sig_scheme(struct s2n_connection *conn, - const struct s2n_signature_scheme **sig_scheme_out, s2n_mode signer) +static S2N_RESULT s2n_validate_signature_algorithms_contain_iana( + struct s2n_blob *peer_list, uint16_t our_iana) { - POSIX_ENSURE_REF(conn); - POSIX_ENSURE_REF(conn->secure); - POSIX_ENSURE_REF(sig_scheme_out); + struct s2n_stuffer list = { 0 }; + RESULT_GUARD_POSIX(s2n_stuffer_init_written(&list, peer_list)); + while(s2n_stuffer_data_available(&list)) { + uint16_t iana = 0; + RESULT_GUARD_POSIX(s2n_stuffer_read_uint16(&list, &iana)); + if (iana == our_iana) { + return S2N_RESULT_OK; + } + } + RESULT_BAIL(S2N_ERR_INVALID_SIGNATURE_SCHEME); +} - s2n_authentication_method auth_method = 0; - if (signer == S2N_CLIENT) { - POSIX_GUARD(s2n_get_auth_method_for_cert_type(conn->handshake_params.client_cert_pkey_type, &auth_method)); +S2N_RESULT s2n_signature_algorithms_supported_list_process(struct s2n_connection *conn, + struct s2n_blob *peer_list) +{ + RESULT_ENSURE_REF(conn); + RESULT_ENSURE_REF(peer_list); + RESULT_ENSURE_REF(conn->secure); + struct s2n_cipher_suite *cipher_suite = conn->secure->cipher_suite; + RESULT_ENSURE_REF(cipher_suite); + + const struct s2n_signature_scheme **chosen_sig_scheme = NULL; + if (conn->mode == S2N_CLIENT) { + chosen_sig_scheme = &conn->handshake_params.client_cert_sig_scheme; } else { - POSIX_ENSURE_REF(conn->secure->cipher_suite); - auth_method = conn->secure->cipher_suite->auth_method; + chosen_sig_scheme = &conn->handshake_params.server_cert_sig_scheme; } - /* Default our signature digest algorithms. - * For >=TLS 1.2 this default may be overridden by the signature_algorithms extension. - */ - const struct s2n_signature_scheme *default_sig_scheme = &s2n_rsa_pkcs1_md5_sha1; - if (auth_method == S2N_AUTHENTICATION_ECDSA) { - default_sig_scheme = &s2n_ecdsa_sha1; - } else if (conn->actual_protocol_version >= S2N_TLS12) { - default_sig_scheme = &s2n_rsa_pkcs1_sha1; + if (conn->actual_protocol_version < S2N_TLS12) { + RESULT_GUARD(s2n_signature_algorithms_get_legacy_default(conn, conn->mode, chosen_sig_scheme)); + return S2N_RESULT_OK; } - if (conn->actual_protocol_version < S2N_TLS12) { - /* Before TLS1.2, signature algorithms were fixed, not chosen / negotiated. */ - *sig_scheme_out = default_sig_scheme; - return S2N_SUCCESS; - } else { - /* If we attempt to negotiate a default in TLS1.2, we should ensure that - * default is allowed by the local security policy. - */ - const struct s2n_signature_preferences *signature_preferences = NULL; - POSIX_GUARD(s2n_connection_get_signature_preferences(conn, &signature_preferences)); - POSIX_ENSURE_REF(signature_preferences); - for (size_t i = 0; i < signature_preferences->count; i++) { - if (signature_preferences->signature_schemes[i]->iana_value == default_sig_scheme->iana_value) { - *sig_scheme_out = default_sig_scheme; - return S2N_SUCCESS; - } + const struct s2n_signature_preferences *signature_preferences = NULL; + RESULT_GUARD_POSIX(s2n_connection_get_signature_preferences(conn, &signature_preferences)); + RESULT_ENSURE_REF(signature_preferences); + + const struct s2n_signature_scheme *fallback_candidate = NULL; + + for (size_t i = 0; i < signature_preferences->count; i++) { + const struct s2n_signature_scheme *candidate = signature_preferences->signature_schemes[i]; + + if (!s2n_signature_scheme_is_valid_for_recv(conn, candidate)) { + continue; + } + + if (s2n_is_sig_scheme_valid_for_auth(conn, candidate) != S2N_SUCCESS) { + continue; + } + + bool is_peer_supported = s2n_result_is_ok(s2n_validate_signature_algorithms_contain_iana( + peer_list, candidate->iana_value)); + bool is_default = (candidate == &s2n_ecdsa_sha1 || candidate == &s2n_rsa_pkcs1_sha1); + + if (is_peer_supported) { + *chosen_sig_scheme = candidate; + return S2N_RESULT_OK; + } + + if (is_default) { + fallback_candidate = candidate; + } else if (fallback_candidate == NULL) { + fallback_candidate = candidate; } - /* We cannot bail with an error here because existing logic assumes - * that this method should always succeed and calls it even when no default - * is actually necessary. - * If no valid default exists, set an unusable, invalid empty scheme. - */ - *sig_scheme_out = &s2n_null_sig_scheme; - return S2N_SUCCESS; } + + if (fallback_candidate) { + *chosen_sig_scheme = fallback_candidate; + } else { + RESULT_BAIL(S2N_ERR_INVALID_SIGNATURE_SCHEME); + } + return S2N_RESULT_OK; } -int s2n_choose_sig_scheme_from_peer_preference_list(struct s2n_connection *conn, - struct s2n_sig_scheme_list *peer_wire_prefs, - const struct s2n_signature_scheme **sig_scheme_out) +static S2N_RESULT s2n_signature_algorithms_supported_list_parse(struct s2n_connection *conn, + struct s2n_stuffer *in, struct s2n_blob *peer_list) { - POSIX_ENSURE_REF(conn); - POSIX_ENSURE_REF(sig_scheme_out); + uint16_t peer_list_size = 0; + RESULT_GUARD_POSIX(s2n_stuffer_read_uint16(in, &peer_list_size)); - const struct s2n_signature_scheme *chosen_scheme = &s2n_null_sig_scheme; - if (conn->actual_protocol_version < S2N_TLS13) { - POSIX_GUARD(s2n_choose_default_sig_scheme(conn, &chosen_scheme, conn->mode)); - } else { - /* Pick a default signature algorithm in TLS 1.3 https://tools.ietf.org/html/rfc8446#section-4.4.2.2 */ - POSIX_GUARD(s2n_tls13_default_sig_scheme(conn, &chosen_scheme)); - } + uint8_t *peer_list_data = s2n_stuffer_raw_read(in, peer_list_size); + RESULT_ENSURE_REF(peer_list_data); - /* SignatureScheme preference list was first added in TLS 1.2. It will be empty in older TLS versions. */ - if (conn->actual_protocol_version >= S2N_TLS12 && peer_wire_prefs != NULL && peer_wire_prefs->len > 0) { - /* Use a best effort approach to selecting a signature scheme matching client's preferences */ - POSIX_GUARD(s2n_choose_sig_scheme(conn, peer_wire_prefs, &chosen_scheme)); - } + RESULT_GUARD_POSIX(s2n_blob_init(peer_list, peer_list_data, peer_list_size)); + return S2N_RESULT_OK; +} - *sig_scheme_out = chosen_scheme; - return S2N_SUCCESS; +S2N_RESULT s2n_signature_algorithms_supported_list_recv(struct s2n_connection *conn, + struct s2n_stuffer *in) +{ + struct s2n_blob peer_list = { 0 }; + if (s2n_result_is_error(s2n_signature_algorithms_supported_list_parse(conn, in, &peer_list))) { + peer_list = (struct s2n_blob){ 0 }; + } + RESULT_GUARD(s2n_signature_algorithms_supported_list_process(conn, &peer_list)); + return S2N_RESULT_OK; } S2N_RESULT s2n_signature_algorithms_supported_list_send(struct s2n_connection *conn, struct s2n_stuffer *out) @@ -323,37 +277,3 @@ S2N_RESULT s2n_signature_algorithms_supported_list_send(struct s2n_connection *c return S2N_RESULT_OK; } - -int s2n_recv_supported_sig_scheme_list(struct s2n_stuffer *in, struct s2n_sig_scheme_list *sig_hash_algs) -{ - uint16_t length_of_all_pairs; - POSIX_GUARD(s2n_stuffer_read_uint16(in, &length_of_all_pairs)); - if (length_of_all_pairs > s2n_stuffer_data_available(in)) { - /* Malformed length, ignore the extension */ - return 0; - } - - if (length_of_all_pairs % 2) { - /* Pairs occur in two byte lengths. Malformed length, ignore the extension and skip ahead */ - POSIX_GUARD(s2n_stuffer_skip_read(in, length_of_all_pairs)); - return 0; - } - - int pairs_available = length_of_all_pairs / 2; - - if (pairs_available > TLS_SIGNATURE_SCHEME_LIST_MAX_LEN) { - POSIX_BAIL(S2N_ERR_TOO_MANY_SIGNATURE_SCHEMES); - } - - sig_hash_algs->len = 0; - - for (size_t i = 0; i < (size_t) pairs_available; i++) { - uint16_t sig_scheme = 0; - POSIX_GUARD(s2n_stuffer_read_uint16(in, &sig_scheme)); - - sig_hash_algs->iana_list[sig_hash_algs->len] = sig_scheme; - sig_hash_algs->len += 1; - } - - return 0; -} diff --git a/tls/s2n_signature_algorithms.h b/tls/s2n_signature_algorithms.h index 58b93360ab7..9b4a0723b3d 100644 --- a/tls/s2n_signature_algorithms.h +++ b/tls/s2n_signature_algorithms.h @@ -23,22 +23,12 @@ struct s2n_connection; -struct s2n_sig_scheme_list { - uint16_t iana_list[TLS_SIGNATURE_SCHEME_LIST_MAX_LEN]; - uint8_t len; -}; - -int s2n_choose_default_sig_scheme(struct s2n_connection *conn, - const struct s2n_signature_scheme **sig_scheme_out, s2n_mode signer); -int s2n_tls13_default_sig_scheme(struct s2n_connection *conn, - const struct s2n_signature_scheme **sig_scheme_out); - -int s2n_choose_sig_scheme_from_peer_preference_list(struct s2n_connection *conn, - struct s2n_sig_scheme_list *sig_hash_algs, - const struct s2n_signature_scheme **sig_scheme_out); - S2N_RESULT s2n_signature_algorithm_recv(struct s2n_connection *conn, struct s2n_stuffer *in); -int s2n_recv_supported_sig_scheme_list(struct s2n_stuffer *in, struct s2n_sig_scheme_list *sig_hash_algs); +S2N_RESULT s2n_signature_algorithms_supported_list_process(struct s2n_connection *conn, + struct s2n_blob *peer_list); +S2N_RESULT s2n_signature_algorithms_supported_list_recv(struct s2n_connection *conn, + struct s2n_stuffer *in); + S2N_RESULT s2n_signature_algorithms_supported_list_send(struct s2n_connection *conn, struct s2n_stuffer *out);