From a56e9bfdda830e712f4674055af85cc573a38efe Mon Sep 17 00:00:00 2001 From: James Mayclin Date: Thu, 22 Feb 2024 23:39:16 -0800 Subject: [PATCH] feat: add cert key preferences (#4434) --- crypto/s2n_certificate.c | 8 - crypto/s2n_certificate.h | 6 +- crypto/s2n_openssl_x509.c | 20 ++ tests/unit/s2n_config_test.c | 2 +- tests/unit/s2n_openssl_x509_test.c | 57 +++-- ...2n_security_policy_cert_preferences_test.c | 194 +++++++++++++----- tests/unit/s2n_x509_validator_test.c | 95 +++++++++ tls/s2n_auth_selection.c | 2 +- tls/s2n_certificate_keys.c | 73 +++++++ tls/s2n_certificate_keys.h | 44 ++++ tls/s2n_security_policies.c | 30 ++- tls/s2n_security_policies.h | 10 +- tls/s2n_x509_validator.c | 55 +++-- 13 files changed, 502 insertions(+), 94 deletions(-) create mode 100644 tls/s2n_certificate_keys.c create mode 100644 tls/s2n_certificate_keys.h diff --git a/crypto/s2n_certificate.c b/crypto/s2n_certificate.c index b8b4c702d27..77629baf9b4 100644 --- a/crypto/s2n_certificate.c +++ b/crypto/s2n_certificate.c @@ -376,14 +376,6 @@ int s2n_cert_chain_and_key_load(struct s2n_cert_chain_and_key *chain_and_key) /* Populate name information from the SAN/CN for the leaf certificate */ POSIX_GUARD(s2n_cert_chain_and_key_set_names(chain_and_key, leaf_cert)); - /* Populate ec curve libcrypto nid */ - if (pkey_type == S2N_PKEY_TYPE_ECDSA) { - int nid = EC_GROUP_get_curve_name(EC_KEY_get0_group(public_key.key.ecdsa_key.ec_key)); - POSIX_ENSURE(nid > 0, S2N_ERR_CERT_TYPE_UNSUPPORTED); - POSIX_ENSURE(nid < UINT16_MAX, S2N_ERR_CERT_TYPE_UNSUPPORTED); - head->ec_curve_nid = nid; - } - /* populate libcrypto nid's required for cert restrictions */ struct s2n_cert *current = head->next; while (current != NULL) { diff --git a/crypto/s2n_certificate.h b/crypto/s2n_certificate.h index 916fc5c5d05..87217640f46 100644 --- a/crypto/s2n_certificate.h +++ b/crypto/s2n_certificate.h @@ -28,12 +28,16 @@ struct s2n_cert_info { int signature_nid; /* This field is not populated for RSA_PSS signatures */ int signature_digest_nid; + /* For EC certs this field is the curve (e.g. NID_secp521r1) and not the generic + * EC key NID (NID_X9_62_id_ecPublicKey) + */ + int public_key_nid; + int public_key_bits; bool self_signed; }; struct s2n_cert { s2n_pkey_type pkey_type; - uint16_t ec_curve_nid; s2n_cert_public_key public_key; struct s2n_cert_info info; struct s2n_blob raw; diff --git a/crypto/s2n_openssl_x509.c b/crypto/s2n_openssl_x509.c index 92d1266cf7b..bcc3bbe02d1 100644 --- a/crypto/s2n_openssl_x509.c +++ b/crypto/s2n_openssl_x509.c @@ -17,6 +17,9 @@ #include "api/s2n.h" +DEFINE_POINTER_CLEANUP_FUNC(EVP_PKEY *, EVP_PKEY_free); +DEFINE_POINTER_CLEANUP_FUNC(EC_KEY *, EC_KEY_free); + S2N_CLEANUP_RESULT s2n_openssl_x509_stack_pop_free(STACK_OF(X509) **cert_chain) { RESULT_ENSURE_REF(*cert_chain); @@ -114,5 +117,22 @@ S2N_RESULT s2n_openssl_x509_get_cert_info(X509 *cert, struct s2n_cert_info *info RESULT_GUARD_OSSL(OBJ_find_sigid_algs(info->signature_nid, &info->signature_digest_nid, NULL), S2N_ERR_CERT_TYPE_UNSUPPORTED); + DEFER_CLEANUP(EVP_PKEY *pubkey = X509_get_pubkey(cert), EVP_PKEY_free_pointer); + RESULT_ENSURE(pubkey != NULL, S2N_ERR_DECODE_CERTIFICATE); + + info->public_key_bits = EVP_PKEY_bits(pubkey); + RESULT_ENSURE(info->public_key_bits > 0, S2N_ERR_CERT_TYPE_UNSUPPORTED); + + if (EVP_PKEY_base_id(pubkey) == EVP_PKEY_EC) { + DEFER_CLEANUP(EC_KEY *ec_key = EVP_PKEY_get1_EC_KEY(pubkey), EC_KEY_free_pointer); + RESULT_ENSURE_REF(ec_key); + const EC_GROUP *ec_group = EC_KEY_get0_group(ec_key); + RESULT_ENSURE_REF(ec_group); + info->public_key_nid = EC_GROUP_get_curve_name(ec_group); + } else { + info->public_key_nid = EVP_PKEY_id(pubkey); + } + RESULT_ENSURE(info->public_key_nid != NID_undef, S2N_ERR_CERT_TYPE_UNSUPPORTED); + return S2N_RESULT_OK; } diff --git a/tests/unit/s2n_config_test.c b/tests/unit/s2n_config_test.c index b26ffbf68b4..fda61afb0fc 100644 --- a/tests/unit/s2n_config_test.c +++ b/tests/unit/s2n_config_test.c @@ -1124,7 +1124,7 @@ int main(int argc, char **argv) { DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free); EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(config, valid_cert)); - EXPECT_OK(s2n_config_validate_loaded_certificates(config, &security_policy_rfc9151)); + EXPECT_OK(s2n_config_validate_loaded_certificates(config, &rfc9151_applied_locally)); }; /* when cert preferences don't apply locally, invalid certs are accepted */ diff --git a/tests/unit/s2n_openssl_x509_test.c b/tests/unit/s2n_openssl_x509_test.c index c4c8d8b4ab6..85fa4500fd8 100644 --- a/tests/unit/s2n_openssl_x509_test.c +++ b/tests/unit/s2n_openssl_x509_test.c @@ -17,12 +17,24 @@ #include #include +#include "crypto/s2n_rsa_pss.h" #include "s2n_test.h" #include "testlib/s2n_testlib.h" S2N_RESULT s2n_x509_validator_read_asn1_cert(struct s2n_stuffer *cert_chain_in_stuffer, struct s2n_blob *asn1_cert); +static S2N_RESULT s2n_test_assert_s2n_cert_info_equality(const struct s2n_cert_info *info_a, + const struct s2n_cert_info *info_b) +{ + RESULT_ENSURE_EQ(info_a->public_key_bits, info_b->public_key_bits); + RESULT_ENSURE_EQ(info_a->public_key_nid, info_b->public_key_nid); + RESULT_ENSURE_EQ(info_a->signature_nid, info_b->signature_nid); + RESULT_ENSURE_EQ(info_a->signature_digest_nid, info_b->signature_digest_nid); + RESULT_ENSURE_EQ(info_a->self_signed, info_b->self_signed); + return S2N_RESULT_OK; +} + int main(int argc, char **argv) { BEGIN_TEST(); @@ -87,6 +99,8 @@ int main(int argc, char **argv) const char *digest; int expected_signature_nid; int expected_digest_nid; + int expected_public_key_nid; + int expected_public_key_bits; } test_cases[] = { { .key_type = "ec", @@ -95,6 +109,8 @@ int main(int argc, char **argv) .digest = "sha256", .expected_signature_nid = NID_ecdsa_with_SHA256, .expected_digest_nid = NID_sha256, + .expected_public_key_nid = NID_secp384r1, + .expected_public_key_bits = 384, }, { .key_type = "ec", @@ -103,6 +119,8 @@ int main(int argc, char **argv) .digest = "sha384", .expected_signature_nid = NID_ecdsa_with_SHA384, .expected_digest_nid = NID_sha384, + .expected_public_key_nid = NID_X9_62_prime256v1, + .expected_public_key_bits = 256, }, { .key_type = "ec", @@ -111,6 +129,8 @@ int main(int argc, char **argv) .digest = "sha512", .expected_signature_nid = NID_ecdsa_with_SHA512, .expected_digest_nid = NID_sha512, + .expected_public_key_nid = NID_secp521r1, + .expected_public_key_bits = 521, }, { .key_type = "rsae", @@ -119,6 +139,8 @@ int main(int argc, char **argv) .digest = "sha1", .expected_signature_nid = NID_sha1WithRSAEncryption, .expected_digest_nid = NID_sha1, + .expected_public_key_nid = NID_rsaEncryption, + .expected_public_key_bits = 2048, }, { .key_type = "rsae", @@ -127,6 +149,8 @@ int main(int argc, char **argv) .digest = "sha224", .expected_signature_nid = NID_sha224WithRSAEncryption, .expected_digest_nid = NID_sha224, + .expected_public_key_nid = NID_rsaEncryption, + .expected_public_key_bits = 2048, }, { .key_type = "rsae", @@ -135,9 +159,10 @@ int main(int argc, char **argv) .digest = "sha384", .expected_signature_nid = NID_sha384WithRSAEncryption, .expected_digest_nid = NID_sha384, + .expected_public_key_nid = NID_rsaEncryption, + .expected_public_key_bits = 3072, }, -/* openssl 1.0.* does not support rsapss signatures */ -#if S2N_OPENSSL_VERSION_AT_LEAST(1, 1, 0) +#if RSA_PSS_CERTS_SUPPORTED { .key_type = "rsae", .signature = "pss", @@ -145,6 +170,8 @@ int main(int argc, char **argv) .digest = "sha384", .expected_signature_nid = NID_rsassaPss, .expected_digest_nid = NID_undef, + .expected_public_key_nid = NID_rsaEncryption, + .expected_public_key_bits = 4096, }, { .key_type = "rsapss", @@ -153,6 +180,8 @@ int main(int argc, char **argv) .digest = "sha256", .expected_signature_nid = NID_rsassaPss, .expected_digest_nid = NID_undef, + .expected_public_key_nid = NID_rsassaPss, + .expected_public_key_bits = 2048, }, #endif }; @@ -189,18 +218,20 @@ int main(int argc, char **argv) EXPECT_OK(s2n_openssl_x509_get_cert_info(intermediate, &intermediate_info)); EXPECT_OK(s2n_openssl_x509_get_cert_info(root, &root_info)); - /* assert that cert info matches expected values */ - EXPECT_EQUAL(leaf_info.signature_nid, test_cases[i].expected_signature_nid); - EXPECT_EQUAL(leaf_info.signature_digest_nid, test_cases[i].expected_digest_nid); - EXPECT_EQUAL(leaf_info.self_signed, false); - - /* leaf and intermediate should have the same infos */ - EXPECT_EQUAL(memcmp(&leaf_info, &intermediate_info, sizeof(struct s2n_cert_info)), 0); + struct s2n_cert_info expected_info = { + .signature_nid = test_cases[i].expected_signature_nid, + .signature_digest_nid = test_cases[i].expected_digest_nid, + .public_key_nid = test_cases[i].expected_public_key_nid, + .public_key_bits = test_cases[i].expected_public_key_bits, + .self_signed = false, + }; - /* root should be self-signed */ - EXPECT_EQUAL(root_info.signature_nid, test_cases[i].expected_signature_nid); - EXPECT_EQUAL(root_info.signature_digest_nid, test_cases[i].expected_digest_nid); - EXPECT_EQUAL(root_info.self_signed, true); + /* assert that cert info matches expected values */ + EXPECT_OK(s2n_test_assert_s2n_cert_info_equality(&leaf_info, &expected_info)); + EXPECT_OK(s2n_test_assert_s2n_cert_info_equality(&intermediate_info, &expected_info)); + /* root should be self-signed, but otherwise equal */ + expected_info.self_signed = true; + EXPECT_OK(s2n_test_assert_s2n_cert_info_equality(&root_info, &expected_info)); } END_TEST(); diff --git a/tests/unit/s2n_security_policy_cert_preferences_test.c b/tests/unit/s2n_security_policy_cert_preferences_test.c index f49f0d0c04b..28222d0c5c8 100644 --- a/tests/unit/s2n_security_policy_cert_preferences_test.c +++ b/tests/unit/s2n_security_policy_cert_preferences_test.c @@ -15,25 +15,85 @@ #include "s2n_test.h" #include "testlib/s2n_testlib.h" +#include "tls/s2n_certificate_keys.h" #include "tls/s2n_security_policies.h" #include "tls/s2n_signature_scheme.h" +#define CHAIN_LENGTH 3 + +static S2N_RESULT s2n_test_construct_cert_chain( + struct s2n_cert *certs, + size_t certs_length, + struct s2n_cert_chain *cert_chain, + struct s2n_cert_chain_and_key *chain, + const struct s2n_cert_info *valid_info) +{ + RESULT_ENSURE_REF(certs); + RESULT_ENSURE_REF(cert_chain); + RESULT_ENSURE_REF(chain); + RESULT_ENSURE_REF(valid_info); + + for (size_t i = 0; i < certs_length; i++) { + certs[i].info = *valid_info; + if (i != certs_length - 1) { + certs[i].next = &certs[i + 1]; + } + } + + /* root cert */ + certs[certs_length - 1].info.self_signed = true; + + cert_chain->head = &certs[0]; + chain->cert_chain = cert_chain; + + return S2N_RESULT_OK; +} + int main(int argc, char **argv) { BEGIN_TEST(); + const int valid_sig_nid = NID_ecdsa_with_SHA256; + const int valid_hash_nid = NID_sha256; + const int valid_key_size = 384; + const int valid_key_nid = NID_secp384r1; + + const int invalid_sig_nid = NID_sha256WithRSAEncryption; + const int invalid_hash_nid = NID_sha1; + const int invalid_key_size = 256; + const int invalid_key_nid = NID_X9_62_prime256v1; + + const struct s2n_cert_info valid_info = { + .self_signed = false, + .signature_nid = valid_sig_nid, + .signature_digest_nid = valid_hash_nid, + .public_key_bits = valid_key_size, + .public_key_nid = valid_key_nid, + }; + const struct s2n_signature_scheme *const test_sig_scheme_list[] = { &s2n_ecdsa_sha256, &s2n_rsa_pkcs1_sha1, }; + const struct s2n_certificate_key *const test_cert_key_list[] = { + &s2n_ec_p384, + &s2n_rsa_rsae_3072, + }; + const struct s2n_signature_preferences test_certificate_signature_preferences = { .count = s2n_array_len(test_sig_scheme_list), .signature_schemes = test_sig_scheme_list, }; + const struct s2n_certificate_key_preferences test_cert_key_preferences = { + .count = s2n_array_len(test_cert_key_list), + .certificate_keys = test_cert_key_list, + }; + const struct s2n_security_policy test_sp = { .certificate_signature_preferences = &test_certificate_signature_preferences, + .certificate_key_preferences = &test_cert_key_preferences, .certificate_preferences_apply_locally = true, }; @@ -65,7 +125,8 @@ int main(int argc, char **argv) .signature_nid = NID_ecdsa_with_SHA256, }; - EXPECT_OK(s2n_security_policy_validate_cert_signature(&test_sp, &info)); + EXPECT_OK(s2n_security_policy_validate_cert_signature(&test_sp, &info, + S2N_ERR_SECURITY_POLICY_INCOMPATIBLE_CERT)); }; /* Certificate signature algorithm is not in test certificate signature preferences list */ @@ -76,7 +137,8 @@ int main(int argc, char **argv) .signature_nid = NID_rsassaPss, }; - EXPECT_ERROR_WITH_ERRNO(s2n_security_policy_validate_cert_signature(&test_sp, &info), + EXPECT_ERROR_WITH_ERRNO(s2n_security_policy_validate_cert_signature(&test_sp, &info, + S2N_ERR_SECURITY_POLICY_INCOMPATIBLE_CERT), S2N_ERR_SECURITY_POLICY_INCOMPATIBLE_CERT); }; @@ -88,71 +150,99 @@ int main(int argc, char **argv) .signature_nid = NID_rsassaPss, }; - EXPECT_OK(s2n_security_policy_validate_cert_signature(&test_pss_sp, &info)); + EXPECT_OK(s2n_security_policy_validate_cert_signature(&test_pss_sp, &info, + S2N_ERR_SECURITY_POLICY_INCOMPATIBLE_CERT)); }; }; - /* s2n_security_policy_validate_certificate_chain */ + /* s2n_security_policy_validate_cert_key() */ { - int valid_sig_nid = NID_ecdsa_with_SHA256; - int valid_hash_nid = NID_sha256; - - int invalid_sig_nid = NID_sha256WithRSAEncryption; - int invalid_hash_nide = NID_sha256; + /* Certificate key is in test certificate key list */ + { + struct s2n_cert_info info = { + .public_key_nid = valid_key_nid, + .public_key_bits = valid_key_size, + }; - struct s2n_cert_info valid = { - .self_signed = false, - .signature_nid = valid_sig_nid, - .signature_digest_nid = valid_hash_nid, + EXPECT_OK(s2n_security_policy_validate_cert_key(&test_sp, &info, + S2N_ERR_SECURITY_POLICY_INCOMPATIBLE_CERT)); }; - struct s2n_cert root = { 0 }; - root.info = valid; - root.info.self_signed = true; - - struct s2n_cert intermediate = { 0 }; - intermediate.info = valid; - intermediate.next = &root; - - struct s2n_cert leaf = { 0 }; - leaf.info = valid; - leaf.next = &intermediate; - struct s2n_cert_chain cert_chain = { 0 }; - cert_chain.head = &leaf; - - struct s2n_cert_chain_and_key chain = { 0 }; - chain.cert_chain = &cert_chain; - - /* valid chain */ + /* Certificate key is not in test certificate key list */ { - EXPECT_OK(s2n_security_policy_validate_certificate_chain(&test_sp, &chain)); - }; + struct s2n_cert_info info = { + .public_key_nid = invalid_key_nid, + .signature_nid = invalid_key_size, + }; - /* an invalid root signature causes a failure */ - { - struct s2n_cert_info valid_root = root.info; - root.info.signature_nid = invalid_sig_nid; - root.info.signature_digest_nid = invalid_hash_nide; - EXPECT_ERROR_WITH_ERRNO(s2n_security_policy_validate_certificate_chain(&test_sp, &chain), + EXPECT_ERROR_WITH_ERRNO(s2n_security_policy_validate_cert_key(&test_sp, &info, + S2N_ERR_SECURITY_POLICY_INCOMPATIBLE_CERT), S2N_ERR_SECURITY_POLICY_INCOMPATIBLE_CERT); - /* restore root values */ - root.info = valid_root; }; + }; - /* an invalid intermediate causes a failure */ + /* s2n_security_policy_validate_certificate_chain() */ + { + /* a valid certificate chain passes validation */ { - intermediate.info.signature_nid = invalid_sig_nid; - intermediate.info.signature_digest_nid = invalid_hash_nide; - EXPECT_ERROR_WITH_ERRNO(s2n_security_policy_validate_certificate_chain(&test_sp, &chain), - S2N_ERR_SECURITY_POLICY_INCOMPATIBLE_CERT); + struct s2n_cert certs[CHAIN_LENGTH] = { 0 }; + struct s2n_cert_chain cert_chain = { 0 }; + struct s2n_cert_chain_and_key chain = { 0 }; + EXPECT_OK(s2n_test_construct_cert_chain(certs, CHAIN_LENGTH, &cert_chain, &chain, &valid_info)); + EXPECT_OK(s2n_security_policy_validate_certificate_chain(&test_sp, &chain)); }; - /* when certificate_preferences_apply_locally is false then validation succeeds */ - { - struct s2n_security_policy test_sp_no_local = test_sp; - test_sp_no_local.certificate_preferences_apply_locally = false; - EXPECT_OK(s2n_security_policy_validate_certificate_chain(&test_sp_no_local, &chain)); - }; + /* test that failures can be detected for any cert in the chain */ + for (size_t i = 0; i < CHAIN_LENGTH; i++) { + /* an invalid signature causes a failure */ + { + struct s2n_cert certs[CHAIN_LENGTH] = { 0 }; + struct s2n_cert_chain cert_chain = { 0 }; + struct s2n_cert_chain_and_key chain = { 0 }; + EXPECT_OK(s2n_test_construct_cert_chain(certs, CHAIN_LENGTH, &cert_chain, &chain, &valid_info)); + certs[i].info.signature_nid = invalid_sig_nid; + certs[i].info.signature_digest_nid = invalid_hash_nid; + EXPECT_ERROR_WITH_ERRNO(s2n_security_policy_validate_certificate_chain(&test_sp, &chain), + S2N_ERR_SECURITY_POLICY_INCOMPATIBLE_CERT); + }; + + /* an invalid key nid causes a failure */ + { + struct s2n_cert certs[CHAIN_LENGTH] = { 0 }; + struct s2n_cert_chain cert_chain = { 0 }; + struct s2n_cert_chain_and_key chain = { 0 }; + EXPECT_OK(s2n_test_construct_cert_chain(certs, CHAIN_LENGTH, &cert_chain, &chain, &valid_info)); + certs[i].info.public_key_nid = invalid_key_nid; + EXPECT_ERROR_WITH_ERRNO(s2n_security_policy_validate_certificate_chain(&test_sp, &chain), + S2N_ERR_SECURITY_POLICY_INCOMPATIBLE_CERT); + }; + + /* an invalid key size causes a failure */ + { + struct s2n_cert certs[CHAIN_LENGTH] = { 0 }; + struct s2n_cert_chain cert_chain = { 0 }; + struct s2n_cert_chain_and_key chain = { 0 }; + EXPECT_OK(s2n_test_construct_cert_chain(certs, CHAIN_LENGTH, &cert_chain, &chain, &valid_info)); + certs[i].info.public_key_bits = invalid_key_size; + EXPECT_ERROR_WITH_ERRNO(s2n_security_policy_validate_certificate_chain(&test_sp, &chain), + S2N_ERR_SECURITY_POLICY_INCOMPATIBLE_CERT); + }; + + /* when certificate_preferences_apply_locally is false then validation succeeds */ + { + struct s2n_cert certs[CHAIN_LENGTH] = { 0 }; + struct s2n_cert_chain cert_chain = { 0 }; + struct s2n_cert_chain_and_key chain = { 0 }; + EXPECT_OK(s2n_test_construct_cert_chain(certs, CHAIN_LENGTH, &cert_chain, &chain, &valid_info)); + + struct s2n_security_policy test_sp_no_local = test_sp; + test_sp_no_local.certificate_preferences_apply_locally = false; + + certs[i].info.signature_nid = invalid_sig_nid; + certs[i].info.public_key_nid = invalid_key_nid; + EXPECT_OK(s2n_security_policy_validate_certificate_chain(&test_sp_no_local, &chain)); + }; + } }; /* s2n_connection_set_cipher_preferences */ diff --git a/tests/unit/s2n_x509_validator_test.c b/tests/unit/s2n_x509_validator_test.c index 45b1f9b88fd..2b7c4e60216 100644 --- a/tests/unit/s2n_x509_validator_test.c +++ b/tests/unit/s2n_x509_validator_test.c @@ -16,6 +16,7 @@ #include "crypto/s2n_openssl_x509.h" #include "s2n_test.h" #include "testlib/s2n_testlib.h" +#include "tls/s2n_certificate_keys.h" static int mock_time(void *data, uint64_t *timestamp) { @@ -2188,5 +2189,99 @@ int main(int argc, char **argv) } } + /* Test that CAs must comply with cert preferences */ + { + uint8_t invalid_root_pem[S2N_MAX_TEST_PEM_SIZE] = { 0 }; + uint32_t root_pem_len = 0; + EXPECT_SUCCESS(s2n_read_test_pem_and_len(S2N_MIXED_CHAIN_CA, &invalid_root_pem[0], &root_pem_len, + S2N_MAX_TEST_PEM_SIZE)); + + uint8_t chain_pem[S2N_MAX_TEST_PEM_SIZE] = { 0 }; + uint32_t chain_pem_len = 0; + EXPECT_SUCCESS(s2n_read_test_pem_and_len(S2N_MIXED_CHAIN_CERTS, &chain_pem[0], &chain_pem_len, + S2N_MAX_TEST_PEM_SIZE)); + + const struct s2n_certificate_key *const s2n_certificate_key_preferences_list_rfc9151[] = { + &s2n_ec_p384, + &s2n_rsa_rsae_3072, + &s2n_rsa_rsae_4096, + }; + + const struct s2n_certificate_key_preferences s2n_certificate_key_preferences_rfc9151 = { + .count = s2n_array_len(s2n_certificate_key_preferences_list_rfc9151), + .certificate_keys = s2n_certificate_key_preferences_list_rfc9151, + }; + + struct s2n_security_policy security_policy_not_local = security_policy_rfc9151; + security_policy_not_local.certificate_preferences_apply_locally = false; + security_policy_not_local.certificate_key_preferences = &s2n_certificate_key_preferences_rfc9151; + + /* when the peer sends the full chain with a non-compliant CA, verification fails when reading in the certs */ + { + DEFER_CLEANUP(struct s2n_config *config = s2n_config_new_minimal(), s2n_config_ptr_free); + EXPECT_SUCCESS(s2n_config_add_pem_to_trust_store(config, (char *) &invalid_root_pem[0])); + + DEFER_CLEANUP(struct s2n_x509_validator validator = { 0 }, s2n_x509_validator_wipe); + EXPECT_SUCCESS(s2n_x509_validator_init(&validator, &config->trust_store, 0)); + + DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT), s2n_connection_ptr_free); + EXPECT_NOT_NULL(conn); + conn->security_policy_override = &security_policy_not_local; + EXPECT_SUCCESS(s2n_set_server_name(conn, "localhost")); + + DEFER_CLEANUP(struct s2n_stuffer cert_chain_stuffer = { 0 }, s2n_stuffer_free); + + EXPECT_OK(s2n_test_cert_chain_data_from_pem_data(conn, &chain_pem[0], chain_pem_len, &cert_chain_stuffer)); + uint32_t chain_len = s2n_stuffer_data_available(&cert_chain_stuffer); + uint8_t *chain_data = s2n_stuffer_raw_read(&cert_chain_stuffer, chain_len); + EXPECT_NOT_NULL(chain_data); + + DEFER_CLEANUP(struct s2n_pkey public_key_out = { 0 }, s2n_pkey_free); + EXPECT_SUCCESS(s2n_pkey_zero_init(&public_key_out)); + s2n_pkey_type pkey_type = S2N_PKEY_TYPE_UNKNOWN; + EXPECT_ERROR_WITH_ERRNO(s2n_x509_validator_validate_cert_chain(&validator, conn, chain_data, chain_len, + &pkey_type, &public_key_out), + S2N_ERR_CERT_UNTRUSTED); + /* Failed while processing/reading in the cert chain */ + EXPECT_TRUE(validator.state == INIT); + }; + + /* when the peer sends only compliant certs from a non-compliant CA, + * validation fails on the local trust store + */ + { + DEFER_CLEANUP(struct s2n_config *config = s2n_config_new_minimal(), s2n_config_ptr_free); + EXPECT_SUCCESS(s2n_config_add_pem_to_trust_store(config, (char *) &invalid_root_pem[0])); + + DEFER_CLEANUP(struct s2n_x509_validator validator = { 0 }, s2n_x509_validator_wipe); + EXPECT_SUCCESS(s2n_x509_validator_init(&validator, &config->trust_store, 0)); + + DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT), s2n_connection_ptr_free); + EXPECT_NOT_NULL(conn); + conn->security_policy_override = &security_policy_not_local; + EXPECT_SUCCESS(s2n_set_server_name(conn, "localhost")); + + DEFER_CLEANUP(struct s2n_stuffer cert_chain_stuffer = { 0 }, s2n_stuffer_free); + + /* `chain_pem_len - root_pem_len`: only use the first two certs in the chain (leaf and intermediate) to + * ensure we are correctly failing on the local trust store and not the chain that the peer sent. + */ + EXPECT_OK(s2n_test_cert_chain_data_from_pem_data(conn, &chain_pem[0], chain_pem_len - root_pem_len, + &cert_chain_stuffer)); + uint32_t chain_len = s2n_stuffer_data_available(&cert_chain_stuffer); + uint8_t *chain_data = s2n_stuffer_raw_read(&cert_chain_stuffer, chain_len); + EXPECT_NOT_NULL(chain_data); + + DEFER_CLEANUP(struct s2n_pkey public_key_out = { 0 }, s2n_pkey_free); + EXPECT_SUCCESS(s2n_pkey_zero_init(&public_key_out)); + s2n_pkey_type pkey_type = S2N_PKEY_TYPE_UNKNOWN; + EXPECT_ERROR_WITH_ERRNO(s2n_x509_validator_validate_cert_chain(&validator, conn, chain_data, chain_len, + &pkey_type, &public_key_out), + S2N_ERR_SECURITY_POLICY_INCOMPATIBLE_CERT); + /* X509_verify_cert finished successfully */ + EXPECT_TRUE(validator.state == VALIDATED); + }; + }; + END_TEST(); } diff --git a/tls/s2n_auth_selection.c b/tls/s2n_auth_selection.c index 564b8488027..3d56a022104 100644 --- a/tls/s2n_auth_selection.c +++ b/tls/s2n_auth_selection.c @@ -123,7 +123,7 @@ static int s2n_certs_exist_for_sig_scheme(struct s2n_connection *conn, const str POSIX_ENSURE_REF(cert->cert_chain); POSIX_ENSURE_REF(cert->cert_chain->head); POSIX_ENSURE_EQ(cert->cert_chain->head->pkey_type, S2N_PKEY_TYPE_ECDSA); - POSIX_ENSURE_EQ(cert->cert_chain->head->ec_curve_nid, sig_scheme->signature_curve->libcrypto_nid); + POSIX_ENSURE_EQ(cert->cert_chain->head->info.public_key_nid, sig_scheme->signature_curve->libcrypto_nid); } return S2N_SUCCESS; diff --git a/tls/s2n_certificate_keys.c b/tls/s2n_certificate_keys.c new file mode 100644 index 00000000000..276c47935ef --- /dev/null +++ b/tls/s2n_certificate_keys.c @@ -0,0 +1,73 @@ +/* + * 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 "tls/s2n_certificate_keys.h" + +#include + +const struct s2n_certificate_key s2n_rsa_rsae_1024 = { + .public_key_libcrypto_nid = NID_rsaEncryption, + .bits = 1024, +}; + +const struct s2n_certificate_key s2n_rsa_rsae_2048 = { + .public_key_libcrypto_nid = NID_rsaEncryption, + .bits = 2048, +}; + +const struct s2n_certificate_key s2n_rsa_rsae_3072 = { + .public_key_libcrypto_nid = NID_rsaEncryption, + .bits = 3072, +}; + +const struct s2n_certificate_key s2n_rsa_rsae_4096 = { + .public_key_libcrypto_nid = NID_rsaEncryption, + .bits = 4096, +}; + +const struct s2n_certificate_key s2n_rsa_pss_1024 = { + .public_key_libcrypto_nid = NID_rsassaPss, + .bits = 1024, +}; + +const struct s2n_certificate_key s2n_rsa_pss_2048 = { + .public_key_libcrypto_nid = NID_rsassaPss, + .bits = 2048, +}; + +const struct s2n_certificate_key s2n_rsa_pss_3072 = { + .public_key_libcrypto_nid = NID_rsassaPss, + .bits = 3072, +}; + +const struct s2n_certificate_key s2n_rsa_pss_4096 = { + .public_key_libcrypto_nid = NID_rsassaPss, + .bits = 4096, +}; + +const struct s2n_certificate_key s2n_ec_p256 = { + .public_key_libcrypto_nid = NID_X9_62_prime256v1, + .bits = 256, +}; + +const struct s2n_certificate_key s2n_ec_p384 = { + .public_key_libcrypto_nid = NID_secp384r1, + .bits = 384, +}; + +const struct s2n_certificate_key s2n_ec_p521 = { + .public_key_libcrypto_nid = NID_secp521r1, + .bits = 521, +}; diff --git a/tls/s2n_certificate_keys.h b/tls/s2n_certificate_keys.h new file mode 100644 index 00000000000..bcae037374a --- /dev/null +++ b/tls/s2n_certificate_keys.h @@ -0,0 +1,44 @@ +/* + * 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. + */ + +#pragma once + +#include + +struct s2n_certificate_key { + uint16_t public_key_libcrypto_nid; + + /* modulus for RSA key, size for EC key */ + uint16_t bits; +}; + +struct s2n_certificate_key_preferences { + uint8_t count; + const struct s2n_certificate_key *const *certificate_keys; +}; + +extern const struct s2n_certificate_key s2n_rsa_rsae_1024; +extern const struct s2n_certificate_key s2n_rsa_rsae_2048; +extern const struct s2n_certificate_key s2n_rsa_rsae_3072; +extern const struct s2n_certificate_key s2n_rsa_rsae_4096; + +extern const struct s2n_certificate_key s2n_rsa_pss_1024; +extern const struct s2n_certificate_key s2n_rsa_pss_2048; +extern const struct s2n_certificate_key s2n_rsa_pss_3072; +extern const struct s2n_certificate_key s2n_rsa_pss_4096; + +extern const struct s2n_certificate_key s2n_ec_p256; +extern const struct s2n_certificate_key s2n_ec_p384; +extern const struct s2n_certificate_key s2n_ec_p521; diff --git a/tls/s2n_security_policies.c b/tls/s2n_security_policies.c index 66e05112d2b..4bdec0f1414 100644 --- a/tls/s2n_security_policies.c +++ b/tls/s2n_security_policies.c @@ -16,6 +16,7 @@ #include "tls/s2n_security_policies.h" #include "api/s2n.h" +#include "tls/s2n_certificate_keys.h" #include "tls/s2n_connection.h" #include "utils/s2n_safety.h" @@ -1465,8 +1466,8 @@ S2N_RESULT s2n_security_policy_get_version(const struct s2n_security_policy *sec RESULT_BAIL(S2N_ERR_INVALID_SECURITY_POLICY); } -S2N_RESULT s2n_security_policy_validate_cert_signature( - const struct s2n_security_policy *security_policy, const struct s2n_cert_info *info) +S2N_RESULT s2n_security_policy_validate_cert_signature(const struct s2n_security_policy *security_policy, + const struct s2n_cert_info *info, s2n_error error) { RESULT_ENSURE_REF(info); RESULT_ENSURE_REF(security_policy); @@ -1479,9 +1480,27 @@ S2N_RESULT s2n_security_policy_validate_cert_signature( } } - RESULT_BAIL(S2N_ERR_SECURITY_POLICY_INCOMPATIBLE_CERT); + RESULT_BAIL(error); } + return S2N_RESULT_OK; +} + +S2N_RESULT s2n_security_policy_validate_cert_key(const struct s2n_security_policy *security_policy, + const struct s2n_cert_info *info, s2n_error error) +{ + RESULT_ENSURE_REF(info); + RESULT_ENSURE_REF(security_policy); + const struct s2n_certificate_key_preferences *key_preferences = security_policy->certificate_key_preferences; + if (key_preferences != NULL) { + for (size_t i = 0; i < key_preferences->count; i++) { + if (key_preferences->certificate_keys[i]->public_key_libcrypto_nid == info->public_key_nid + && key_preferences->certificate_keys[i]->bits == info->public_key_bits) { + return S2N_RESULT_OK; + } + } + RESULT_BAIL(error); + } return S2N_RESULT_OK; } @@ -1499,7 +1518,10 @@ S2N_RESULT s2n_security_policy_validate_certificate_chain( struct s2n_cert *current = cert_key_pair->cert_chain->head; while (current != NULL) { - RESULT_GUARD(s2n_security_policy_validate_cert_signature(security_policy, ¤t->info)); + RESULT_GUARD(s2n_security_policy_validate_cert_key(security_policy, ¤t->info, + S2N_ERR_SECURITY_POLICY_INCOMPATIBLE_CERT)); + RESULT_GUARD(s2n_security_policy_validate_cert_signature(security_policy, ¤t->info, + S2N_ERR_SECURITY_POLICY_INCOMPATIBLE_CERT)); current = current->next; } return S2N_RESULT_OK; diff --git a/tls/s2n_security_policies.h b/tls/s2n_security_policies.h index d593253eb3e..90bfda1296b 100644 --- a/tls/s2n_security_policies.h +++ b/tls/s2n_security_policies.h @@ -70,6 +70,12 @@ struct s2n_security_policy { * https://www.rfc-editor.org/rfc/rfc8446#section-4.2.7 */ const struct s2n_ecc_preferences *ecc_preferences; + /* This field determines what public keys are allowed for use. It restricts + * both the type of the key (Elliptic Curve, RSA w/ Encryption, RSA PSS) and + * the size of the key. Note that this field structure is likely to change + * until https://github.com/aws/s2n-tls/issues/4435 is closed. + */ + const struct s2n_certificate_key_preferences *certificate_key_preferences; /* This field controls whether the certificate_signature_preferences apply * to local certs loaded on configs. */ @@ -203,4 +209,6 @@ S2N_RESULT s2n_security_policy_get_version(const struct s2n_security_policy *sec S2N_RESULT s2n_security_policy_validate_certificate_chain(const struct s2n_security_policy *security_policy, const struct s2n_cert_chain_and_key *cert_key_pair); S2N_RESULT s2n_security_policy_validate_cert_signature( - const struct s2n_security_policy *security_policy, const struct s2n_cert_info *info); + const struct s2n_security_policy *security_policy, const struct s2n_cert_info *info, s2n_error error); +S2N_RESULT s2n_security_policy_validate_cert_key( + const struct s2n_security_policy *security_policy, const struct s2n_cert_info *info, s2n_error error); diff --git a/tls/s2n_x509_validator.c b/tls/s2n_x509_validator.c index 02efad886f4..106620df620 100644 --- a/tls/s2n_x509_validator.c +++ b/tls/s2n_x509_validator.c @@ -436,26 +436,54 @@ S2N_RESULT s2n_x509_validator_check_cert_preferences(struct s2n_connection *conn *# "signature_algorithms" extension also applies to signatures appearing in *# certificates. */ - if (security_policy->certificate_signature_preferences == NULL) { - return S2N_RESULT_OK; - } - struct s2n_cert_info info = { 0 }; RESULT_GUARD(s2n_openssl_x509_get_cert_info(cert, &info)); - if (info.self_signed) { - return S2N_RESULT_OK; + bool certificate_preferences_defined = security_policy->certificate_signature_preferences != NULL + || security_policy->certificate_key_preferences != NULL; + if (certificate_preferences_defined && !info.self_signed && conn->actual_protocol_version == S2N_TLS13) { + /* Ensure that the certificate signature does not use SHA-1. While this check + * would ideally apply to all connections, we only enforce it when certificate + * preferences exist to stay backwards compatible. + */ + RESULT_ENSURE(info.signature_digest_nid != NID_sha1, S2N_ERR_CERT_UNTRUSTED); } - /* Ensure that the certificate signature does not use SHA-1. While this check - * would ideally apply to all connections, we only enforce it when certificate - * preferences exist to stay backwards compatible. - */ - if (conn->actual_protocol_version == S2N_TLS13) { - RESULT_ENSURE(info.signature_digest_nid != NID_sha1, S2N_ERR_CERT_UNTRUSTED); + if (!info.self_signed) { + RESULT_GUARD(s2n_security_policy_validate_cert_signature(security_policy, &info, S2N_ERR_CERT_UNTRUSTED)); } + RESULT_GUARD(s2n_security_policy_validate_cert_key(security_policy, &info, S2N_ERR_CERT_UNTRUSTED)); + + return S2N_RESULT_OK; +} + +/* Validates that the root certificate uses a key allowed by the security policy + * certificate preferences. + */ +static S2N_RESULT s2n_x509_validator_check_root_cert(struct s2n_x509_validator *validator, struct s2n_connection *conn) +{ + RESULT_ENSURE_REF(validator); + RESULT_ENSURE_REF(conn); + + const struct s2n_security_policy *security_policy = NULL; + RESULT_GUARD_POSIX(s2n_connection_get_security_policy(conn, &security_policy)); + RESULT_ENSURE_REF(security_policy); + + RESULT_ENSURE_REF(validator->store_ctx); + DEFER_CLEANUP(STACK_OF(X509) *cert_chain = X509_STORE_CTX_get1_chain(validator->store_ctx), + s2n_openssl_x509_stack_pop_free); + RESULT_ENSURE_REF(cert_chain); + + const int certs_in_chain = sk_X509_num(cert_chain); + RESULT_ENSURE(certs_in_chain > 0, S2N_ERR_CERT_UNTRUSTED); + X509 *root = sk_X509_value(cert_chain, certs_in_chain - 1); + RESULT_ENSURE_REF(root); + + struct s2n_cert_info info = { 0 }; + RESULT_GUARD(s2n_openssl_x509_get_cert_info(root, &info)); - RESULT_ENSURE_OK(s2n_security_policy_validate_cert_signature(security_policy, &info), S2N_ERR_CERT_UNTRUSTED); + RESULT_GUARD(s2n_security_policy_validate_cert_key(security_policy, &info, + S2N_ERR_SECURITY_POLICY_INCOMPATIBLE_CERT)); return S2N_RESULT_OK; } @@ -745,6 +773,7 @@ S2N_RESULT s2n_x509_validator_validate_cert_chain(struct s2n_x509_validator *val if (validator->state == READY_TO_VERIFY) { RESULT_GUARD(s2n_x509_validator_verify_cert_chain(validator, conn)); + RESULT_GUARD(s2n_x509_validator_check_root_cert(validator, conn)); } if (conn->actual_protocol_version >= S2N_TLS13) {