diff --git a/ssl/extensions.cc b/ssl/extensions.cc index 45b9e8d3c9..a44bb87095 100644 --- a/ssl/extensions.cc +++ b/ssl/extensions.cc @@ -4139,7 +4139,7 @@ bool tls1_choose_signature_algorithm(SSL_HANDSHAKE *hs, uint16_t *out) { // Before TLS 1.2, the signature algorithm isn't negotiated as part of the // handshake. if (ssl_protocol_version(ssl) < TLS1_2_VERSION) { - if (tls1_get_legacy_signature_algorithm(out, hs->local_pubkey.get()) || + if (ssl_public_key_supports_legacy_signature_algorithm(out, hs) || ssl_cert_private_keys_supports_legacy_signature_algorithm(out, hs)) { return true; } diff --git a/ssl/internal.h b/ssl/internal.h index 494a466801..bd4a51087b 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -1140,6 +1140,11 @@ enum ssl_private_key_result_t ssl_private_key_decrypt(SSL_HANDSHAKE *hs, bool ssl_public_key_supports_signature_algorithm(SSL_HANDSHAKE *hs, uint16_t sigalg); +// ssl_public_key_supports_legacy_signature_algorithm is the tls1.0/1.1 +// version of |ssl_public_key_supports_signature_algorithm|. +bool ssl_public_key_supports_legacy_signature_algorithm(uint16_t *out, + SSL_HANDSHAKE *hs); + // ssl_cert_private_keys_supports_signature_algorithm returns whether any of // |hs|'s available private keys supports |sigalg|. If one does, we switch to // using that private key and the corresponding certificate for the rest of the diff --git a/ssl/ssl_privkey.cc b/ssl/ssl_privkey.cc index 2a6ec0a95c..6806e54a3c 100644 --- a/ssl/ssl_privkey.cc +++ b/ssl/ssl_privkey.cc @@ -383,10 +383,25 @@ static bool ssl_public_key_rsa_pss_check(EVP_PKEY *pubkey, uint16_t sigalg) { return true; } +static bool tls12_pkey_supports_cipher_auth(SSL_HANDSHAKE *hs, + const EVP_PKEY *key) { + SSL *const ssl = hs->ssl; + // We may have a private key that supports the signature algorithm, but we + // need to verify that the negotiated cipher allows it. This behavior is only + // done in OpenSSL servers with TLS version 1.2 and below since TLS 1.3 does + // not have cipher-based authentication configuration. Since authentication is + // configured outside the ciphersuite in TLS 1.3, we use the |SSL_aGENERIC| + // flag defined for all TLS 1.3 ciphers to indicate support. + return !ssl->server || (hs->new_cipher->algorithm_auth & + (ssl_cipher_auth_mask_for_key(key) | SSL_aGENERIC)); +} + bool ssl_public_key_supports_signature_algorithm(SSL_HANDSHAKE *hs, uint16_t sigalg) { SSL *const ssl = hs->ssl; - if (!pkey_supports_algorithm(ssl, hs->local_pubkey.get(), sigalg)) { + assert(ssl_protocol_version(ssl) >= TLS1_2_VERSION); + if (!tls12_pkey_supports_cipher_auth(hs, hs->local_pubkey.get()) || + !pkey_supports_algorithm(ssl, hs->local_pubkey.get(), sigalg)) { return false; } @@ -397,8 +412,7 @@ bool ssl_public_key_supports_signature_algorithm(SSL_HANDSHAKE *hs, return true; } -UniquePtr ssl_cert_parse_leaf_pubkey( - STACK_OF(CRYPTO_BUFFER) *chain) { +UniquePtr ssl_cert_parse_leaf_pubkey(STACK_OF(CRYPTO_BUFFER) *chain) { const CRYPTO_BUFFER *buf = sk_CRYPTO_BUFFER_value(chain, 0); if (buf == nullptr) { return nullptr; @@ -408,6 +422,17 @@ UniquePtr ssl_cert_parse_leaf_pubkey( return ssl_cert_parse_pubkey(&leaf); } +bool ssl_public_key_supports_legacy_signature_algorithm(uint16_t *out, + SSL_HANDSHAKE *hs) { + SSL *const ssl = hs->ssl; + assert(ssl_protocol_version(ssl) < TLS1_2_VERSION); + const uint32_t auth_allowed = + !ssl->server || (hs->new_cipher->algorithm_auth & + ssl_cipher_auth_mask_for_key(hs->local_pubkey.get())); + return tls1_get_legacy_signature_algorithm(out, hs->local_pubkey.get()) && + auth_allowed; +} + bool ssl_cert_private_keys_supports_legacy_signature_algorithm( uint16_t *out, SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; @@ -447,6 +472,7 @@ bool ssl_cert_private_keys_supports_legacy_signature_algorithm( bool ssl_cert_private_keys_supports_signature_algorithm(SSL_HANDSHAKE *hs, uint16_t sigalg) { SSL *const ssl = hs->ssl; + assert(ssl_protocol_version(ssl) >= TLS1_2_VERSION); CERT *cert = hs->config->cert.get(); // Only the server without delegated credentials has support for multiple // certificate slots. @@ -457,20 +483,22 @@ bool ssl_cert_private_keys_supports_signature_algorithm(SSL_HANDSHAKE *hs, for (size_t i = 0; i < cert->cert_private_keys.size(); i++) { EVP_PKEY *private_key = cert->cert_private_keys[i].privatekey.get(); UniquePtr public_key = - ssl_cert_parse_leaf_pubkey(cert->cert_private_keys[i].chain.get()); - if (private_key != nullptr && public_key != nullptr && - pkey_supports_algorithm(ssl, private_key, sigalg)) { - if (!ssl_public_key_rsa_pss_check(public_key.get(), sigalg)) { - return false; - } + ssl_cert_parse_leaf_pubkey(cert->cert_private_keys[i].chain.get()); + if (private_key != nullptr && public_key != nullptr) { + if (tls12_pkey_supports_cipher_auth(hs, private_key) && + pkey_supports_algorithm(ssl, private_key, sigalg)) { + if (!ssl_public_key_rsa_pss_check(public_key.get(), sigalg)) { + return false; + } - // Update certificate slot index if all checks have passed. - // - // If the server has a valid private key available to use, we switch to - // using that certificate for the rest of the connection. - cert->cert_private_key_idx = (int)i; - hs->local_pubkey = std::move(public_key); - return true; + // Update certificate slot index if all checks have passed. + // + // If the server has a valid private key available to use, we switch to + // using that certificate for the rest of the connection. + cert->cert_private_key_idx = (int)i; + hs->local_pubkey = std::move(public_key); + return true; + } } } return false; diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index 12808f1dea..f85ecc0654 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -6484,7 +6484,7 @@ class MultipleCertificateSlotTest SSL_CTX *server_ctx, std::vector sigalgs, int last_cert_type_set, - int should_fail) { + int should_connect) { EXPECT_TRUE(SSL_CTX_set_signing_algorithm_prefs(client_ctx, sigalgs.data(), sigalgs.size())); EXPECT_TRUE(SSL_CTX_set_verify_algorithm_prefs(client_ctx, sigalgs.data(), @@ -6500,8 +6500,8 @@ class MultipleCertificateSlotTest EXPECT_EQ(ConnectClientAndServer(&client, &server, client_ctx, server_ctx, config, false), - should_fail); - if (!should_fail) { + should_connect); + if (!should_connect) { return; } @@ -6526,10 +6526,9 @@ INSTANTIATE_TEST_SUITE_P( // Sets up the |SSL_CTX| with |SSL_CTX_use_certificate| & |SSL_use_PrivateKey|. TEST_P(MultipleCertificateSlotTest, CertificateSlotIndex) { - if ((version == TLS1_1_VERSION || version == TLS1_VERSION) && - slot_index == SSL_PKEY_ED25519) { + if (version < TLS1_2_VERSION && slot_index == SSL_PKEY_ED25519) { // ED25519 is not supported in versions prior to TLS1.2. - return; + GTEST_SKIP(); } bssl::UniquePtr client_ctx(SSL_CTX_new(TLS_method())); bssl::UniquePtr server_ctx(CreateContextWithCertificate( @@ -6545,10 +6544,9 @@ TEST_P(MultipleCertificateSlotTest, CertificateSlotIndex) { // Sets up the |SSL_CTX| with |SSL_CTX_set_chain_and_key|. TEST_P(MultipleCertificateSlotTest, SetChainAndKeyIndex) { - if ((version == TLS1_1_VERSION || version == TLS1_VERSION) && - slot_index == SSL_PKEY_ED25519) { + if (version < TLS1_2_VERSION && slot_index == SSL_PKEY_ED25519) { // ED25519 is not supported in versions prior to TLS1.2. - return; + GTEST_SKIP(); } bssl::UniquePtr client_ctx(SSL_CTX_new(TLS_method())); bssl::UniquePtr server_ctx(SSL_CTX_new(TLS_method())); @@ -6573,11 +6571,10 @@ TEST_P(MultipleCertificateSlotTest, SetChainAndKeyIndex) { slot_index, true); } -TEST_P(MultipleCertificateSlotTest, AutomaticSelection) { - if ((version == TLS1_1_VERSION || version == TLS1_VERSION) && - slot_index == SSL_PKEY_ED25519) { +TEST_P(MultipleCertificateSlotTest, AutomaticSelectionSigAlgs) { + if (version < TLS1_2_VERSION && slot_index == SSL_PKEY_ED25519) { // ED25519 is not supported in versions prior to TLS1.2. - return; + GTEST_SKIP(); } bssl::UniquePtr client_ctx(SSL_CTX_new(TLS_method())); @@ -6608,11 +6605,54 @@ TEST_P(MultipleCertificateSlotTest, AutomaticSelection) { {certificate_key_param().corresponding_sigalg}, SSL_PKEY_ED25519, true); } +TEST_P(MultipleCertificateSlotTest, AutomaticSelectionCipherAuth) { + if ((version < TLS1_2_VERSION && slot_index == SSL_PKEY_ED25519) || + version >= TLS1_3_VERSION) { + // ED25519 is not supported in versions prior to TLS1.2. + // TLS 1.3 not have cipher-based authentication configuration. + GTEST_SKIP(); + } + + bssl::UniquePtr client_ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr server_ctx(SSL_CTX_new(TLS_method())); + + ASSERT_TRUE( + SSL_CTX_use_certificate(server_ctx.get(), GetTestCertificate().get())); + ASSERT_TRUE(SSL_CTX_use_PrivateKey(server_ctx.get(), GetTestKey().get())); + ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx.get(), + GetECDSATestCertificate().get())); + ASSERT_TRUE( + SSL_CTX_use_PrivateKey(server_ctx.get(), GetECDSATestKey().get())); + ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx.get(), + GetED25519TestCertificate().get())); + ASSERT_TRUE( + SSL_CTX_use_PrivateKey(server_ctx.get(), GetED25519TestKey().get())); + + + // Versions prior to TLS1.3 need a valid authentication cipher suite to pair + // with the certificate. + if (version < TLS1_3_VERSION) { + ASSERT_TRUE(SSL_CTX_set_cipher_list(client_ctx.get(), + certificate_key_param().suite)); + } + + // We allow all possible sigalgs in this test, but either the ECDSA or ED25519 + // certificate could be chosen when using an |SSL_aECDSA| ciphersuite. + std::vector sigalgs = {SSL_SIGN_RSA_PSS_RSAE_SHA256}; + if (slot_index == SSL_PKEY_ED25519) { + sigalgs.push_back(SSL_SIGN_ED25519); + } else { + sigalgs.push_back(SSL_SIGN_ECDSA_SECP256R1_SHA256); + } + + StandardCertificateSlotIndexTests(client_ctx.get(), server_ctx.get(), sigalgs, + SSL_PKEY_ED25519, true); +} + TEST_P(MultipleCertificateSlotTest, MissingCertificate) { - if ((version == TLS1_1_VERSION || version == TLS1_VERSION) && - slot_index == SSL_PKEY_ED25519) { + if (version < TLS1_2_VERSION && slot_index == SSL_PKEY_ED25519) { // ED25519 is not supported in versions prior to TLS1.2. - return; + GTEST_SKIP(); } bssl::UniquePtr client_ctx(SSL_CTX_new(TLS_method())); @@ -6637,10 +6677,9 @@ TEST_P(MultipleCertificateSlotTest, MissingCertificate) { } TEST_P(MultipleCertificateSlotTest, MissingPrivateKey) { - if ((version == TLS1_1_VERSION || version == TLS1_VERSION) && - slot_index == SSL_PKEY_ED25519) { + if (version < TLS1_2_VERSION && slot_index == SSL_PKEY_ED25519) { // ED25519 is not supported in versions prior to TLS1.2. - return; + GTEST_SKIP(); } bssl::UniquePtr client_ctx(SSL_CTX_new(TLS_method())); diff --git a/ssl/ssl_x509.cc b/ssl/ssl_x509.cc index 112d0747ed..b6059d3554 100644 --- a/ssl/ssl_x509.cc +++ b/ssl/ssl_x509.cc @@ -195,7 +195,7 @@ static UniquePtr new_leafless_chain(void) { // ssl_cert_set_chain sets elements 1.. of |cert->chain| to the serialised // forms of elements of |chain|. It returns one on success or zero on error, in -// which case no change to |cert->chain| is made. It preverses the existing +// which case no change to |cert->chain| is made. It preserves the existing // leaf from |cert->chain|, if any. static bool ssl_cert_set_chain(CERT *cert, STACK_OF(X509) *chain) { if (!ssl_cert_check_cert_private_keys_usage(cert)) {