From 775855c2f0823bb0356b39bdebde474b2158a2ea Mon Sep 17 00:00:00 2001 From: Lindsay Stewart Date: Tue, 19 Nov 2024 23:34:44 -0800 Subject: [PATCH] fix: allow rsa-pss with TLS1.2 --- tests/integrationv2/common.py | 13 +- tests/integrationv2/utils.py | 8 - tests/unit/s2n_security_policies_test.c | 4 +- tests/unit/s2n_self_talk_certificates_test.c | 159 +++++++++++++++++++ tls/s2n_signature_algorithms.c | 2 - tls/s2n_signature_scheme.c | 6 +- 6 files changed, 175 insertions(+), 17 deletions(-) create mode 100644 tests/unit/s2n_self_talk_certificates_test.c diff --git a/tests/integrationv2/common.py b/tests/integrationv2/common.py index b550d463659..c68bddb2a82 100644 --- a/tests/integrationv2/common.py +++ b/tests/integrationv2/common.py @@ -111,7 +111,16 @@ def __init__(self, name, prefix, location=TEST_CERT_DIRECTORY): self.algorithm = 'RSAPSS' def compatible_with_cipher(self, cipher): - return (self.algorithm == cipher.algorithm) or (cipher.algorithm == 'ANY') + if self.algorithm == cipher.algorithm: + return True + if cipher.algorithm == 'ANY': + return True + if self.algorithm == 'RSAPSS': + if cipher.algorithm != 'RSA': + return False + if 'ECDHE' in cipher.name: + return True + return False def compatible_with_curve(self, curve): if self.algorithm != 'EC': @@ -442,7 +451,7 @@ class Signatures(object): RSA_PSS_PSS_SHA256 = Signature( 'rsa_pss_pss_sha256', - min_protocol=Protocols.TLS13, + min_protocol=Protocols.TLS12, sig_type='RSA-PSS-PSS', sig_digest='SHA256') diff --git a/tests/integrationv2/utils.py b/tests/integrationv2/utils.py index 3d7c2022120..f7a67d307b7 100644 --- a/tests/integrationv2/utils.py +++ b/tests/integrationv2/utils.py @@ -72,14 +72,6 @@ def invalid_test_parameters(*args, **kwargs): # Always consider S2N providers.append(S2N) - # Only TLS1.3 supports RSA-PSS-PSS certificates - # (Earlier versions support RSA-PSS signatures, just via RSA-PSS-RSAE) - if protocol and protocol is not Protocols.TLS13: - if client_certificate and client_certificate.algorithm == 'RSAPSS': - return True - if certificate and certificate.algorithm == 'RSAPSS': - return True - for provider_ in providers: if not provider_.supports_protocol(protocol): return True diff --git a/tests/unit/s2n_security_policies_test.c b/tests/unit/s2n_security_policies_test.c index efaf6901642..6453c2652fb 100644 --- a/tests/unit/s2n_security_policies_test.c +++ b/tests/unit/s2n_security_policies_test.c @@ -153,7 +153,7 @@ int main(int argc, char **argv) int max = security_policy->signature_preferences->signature_schemes[i]->maximum_protocol_version; s2n_signature_algorithm sig_alg = security_policy->signature_preferences->signature_schemes[i]->sig_alg; - if (min == S2N_TLS13 || max >= S2N_TLS13) { + if (min <= S2N_TLS13 || max >= S2N_TLS13) { has_tls_13_sig_alg = true; } @@ -826,7 +826,7 @@ int main(int argc, char **argv) } /* If scheme will be used for pre-tls1.3 */ - if (min_version < S2N_TLS13) { + if (min_version < S2N_TLS12) { EXPECT_NOT_EQUAL(scheme->sig_alg, S2N_SIGNATURE_RSA_PSS_PSS); } } diff --git a/tests/unit/s2n_self_talk_certificates_test.c b/tests/unit/s2n_self_talk_certificates_test.c new file mode 100644 index 00000000000..295065a7b4a --- /dev/null +++ b/tests/unit/s2n_self_talk_certificates_test.c @@ -0,0 +1,159 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +#include "crypto/s2n_rsa_pss.h" +#include "s2n_test.h" +#include "testlib/s2n_testlib.h" +#include "tls/s2n_tls.h" + +static uint8_t s2n_noop_verify_host_fn(const char *name, size_t len, void *data) +{ + return 1; +} + +static S2N_RESULT s2n_test_load_certificate(struct s2n_cert_chain_and_key **chain_out, + char *chain_pem_out, const char *chain_pem_path, const char *key_pem_path) +{ + RESULT_ENSURE_REF(chain_out); + + char key_pem[S2N_MAX_TEST_PEM_SIZE] = { 0 }; + RESULT_GUARD_POSIX(s2n_read_test_pem(chain_pem_path, chain_pem_out, S2N_MAX_TEST_PEM_SIZE)); + RESULT_GUARD_POSIX(s2n_read_test_pem(key_pem_path, key_pem, S2N_MAX_TEST_PEM_SIZE)); + + *chain_out = s2n_cert_chain_and_key_new(); + RESULT_ENSURE_REF(*chain_out); + RESULT_GUARD_POSIX(s2n_cert_chain_and_key_load_pem(*chain_out, chain_pem_out, key_pem)); + + return S2N_RESULT_OK; +} + +static bool s2n_test_is_valid(struct s2n_cert_chain_and_key *chain, uint8_t version) +{ + const s2n_pkey_type cert_type = s2n_cert_chain_and_key_get_pkey_type(chain); + if (cert_type == S2N_PKEY_TYPE_RSA_PSS) { + if (!s2n_is_rsa_pss_certs_supported()) { + return false; + } + /* RSA-PSS certificates were introduced in TLS1.3, but must also be + * supported in TLS1.2. They are not supported for earlier versions. + * + *= https://www.rfc-editor.org/rfc/rfc8446#section-4.2.3 + *= type=test + *# - Implementations that advertise support for RSASSA-PSS (which is + *# mandatory in TLS 1.3) MUST be prepared to accept a signature using + *# that scheme even when TLS 1.2 is negotiated. In TLS 1.2, + *# RSASSA-PSS is used with RSA cipher suites. + */ + if (version < S2N_TLS12) { + return false; + } + } + return true; +} + +static void s2n_reset_highest_version(const uint8_t *saved_version) +{ + s2n_highest_protocol_version = *saved_version; +} + +int main(int argc, char **argv) +{ + BEGIN_TEST(); + + const uint8_t test_versions[] = { S2N_TLS13, S2N_TLS12, S2N_TLS11, S2N_TLS10 }; + const char *test_certs[][2] = { + { S2N_RSA_2048_PKCS1_SHA256_CERT_CHAIN, S2N_RSA_2048_PKCS1_SHA256_CERT_KEY }, + { S2N_ECDSA_P384_PKCS1_CERT_CHAIN, S2N_ECDSA_P384_PKCS1_KEY }, + { S2N_ECDSA_P512_CERT_CHAIN, S2N_ECDSA_P512_KEY }, + { S2N_RSA_PSS_2048_SHA256_LEAF_CERT, S2N_RSA_PSS_2048_SHA256_LEAF_KEY }, + }; + + for (size_t cert_i = 0; cert_i < s2n_array_len(test_certs); cert_i++) { + DEFER_CLEANUP(struct s2n_cert_chain_and_key *chain = NULL, + s2n_cert_chain_and_key_ptr_free); + char pem[S2N_MAX_TEST_PEM_SIZE] = { 0 }; + EXPECT_OK(s2n_test_load_certificate(&chain, pem, + test_certs[cert_i][0], test_certs[cert_i][1])); + + for (size_t version_i = 0; version_i < s2n_array_len(test_versions); version_i++) { + uint8_t version = test_versions[version_i]; + if (!s2n_is_tls13_fully_supported()) { + version = MIN(S2N_TLS12, version); + } + + /* Make sure that we undo any overrides, even if the test is + * later changed to somehow bail early or continue. + */ + /* cppcheck-suppress unreadVariable */ + DEFER_CLEANUP(const uint8_t highest_version = s2n_highest_protocol_version, + s2n_reset_highest_version); + + /* We intentionally use the default policies. + * The default policies should support all certificate types. + */ + const char *security_policy = "default"; + if (version >= S2N_TLS13) { + security_policy = "default_tls13"; + } else if (version < S2N_TLS12) { + /* Rather than try to modify the security policies for older + * versions, force negotiation of the older versions. + */ + s2n_highest_protocol_version = version; + security_policy = "test_all"; + } + + DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free); + EXPECT_NOT_NULL(config); + EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(config, chain)); + EXPECT_SUCCESS(s2n_config_add_pem_to_trust_store(config, pem)); + EXPECT_SUCCESS(s2n_config_set_verify_host_callback(config, s2n_noop_verify_host_fn, NULL)); + EXPECT_SUCCESS(s2n_config_set_max_blinding_delay(config, 0)); + EXPECT_SUCCESS(s2n_config_set_cipher_preferences(config, security_policy)); + + 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)); + + 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_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)); + + bool handshake_success = + (s2n_negotiate_test_server_and_client(server, client) == S2N_SUCCESS); + if (handshake_success) { + EXPECT_EQUAL(server->actual_protocol_version, version); + EXPECT_EQUAL(client->actual_protocol_version, version); + } + + const char *error_message = "Handshake failed"; + if (!s2n_test_is_valid(chain, version)) { + error_message = "Handshake unexpectedly succeeded"; + } + if (handshake_success != s2n_test_is_valid(chain, version)) { + fprintf(stderr, "%s version=%i cert=%s\n", + error_message, version, test_certs[cert_i][0]); + FAIL_MSG(error_message); + } + } + } + + END_TEST(); +} diff --git a/tls/s2n_signature_algorithms.c b/tls/s2n_signature_algorithms.c index bfb5b6b7d22..2710bed1a09 100644 --- a/tls/s2n_signature_algorithms.c +++ b/tls/s2n_signature_algorithms.c @@ -76,8 +76,6 @@ static S2N_RESULT s2n_signature_scheme_validate_for_recv(struct s2n_connection * if (conn->actual_protocol_version >= S2N_TLS13) { RESULT_ENSURE_NE(scheme->hash_alg, S2N_HASH_SHA1); RESULT_ENSURE_NE(scheme->sig_alg, S2N_SIGNATURE_RSA); - } else { - RESULT_ENSURE_NE(scheme->sig_alg, S2N_SIGNATURE_RSA_PSS_PSS); } return S2N_RESULT_OK; diff --git a/tls/s2n_signature_scheme.c b/tls/s2n_signature_scheme.c index 498085e602b..93ca30fb602 100644 --- a/tls/s2n_signature_scheme.c +++ b/tls/s2n_signature_scheme.c @@ -181,7 +181,7 @@ const struct s2n_signature_scheme s2n_rsa_pss_pss_sha256 = { .sig_alg = S2N_SIGNATURE_RSA_PSS_PSS, .libcrypto_nid = NID_rsassaPss, .signature_curve = NULL, /* Elliptic Curve not needed for RSA */ - .minimum_protocol_version = S2N_TLS13, + .minimum_protocol_version = S2N_TLS12, }; const struct s2n_signature_scheme s2n_rsa_pss_pss_sha384 = { @@ -191,7 +191,7 @@ const struct s2n_signature_scheme s2n_rsa_pss_pss_sha384 = { .sig_alg = S2N_SIGNATURE_RSA_PSS_PSS, .libcrypto_nid = NID_rsassaPss, .signature_curve = NULL, /* Elliptic Curve not needed for RSA */ - .minimum_protocol_version = S2N_TLS13, + .minimum_protocol_version = S2N_TLS12, }; const struct s2n_signature_scheme s2n_rsa_pss_pss_sha512 = { @@ -201,7 +201,7 @@ const struct s2n_signature_scheme s2n_rsa_pss_pss_sha512 = { .sig_alg = S2N_SIGNATURE_RSA_PSS_PSS, .libcrypto_nid = NID_rsassaPss, .signature_curve = NULL, /* Elliptic Curve not needed for RSA */ - .minimum_protocol_version = S2N_TLS13, + .minimum_protocol_version = S2N_TLS12, }; /* Chosen based on AWS server recommendations as of 05/24.