From 05f743449bc9b77aae2c8028221f4efc7459765f Mon Sep 17 00:00:00 2001 From: samuel40791765 Date: Wed, 30 Oct 2024 00:03:54 +0000 Subject: [PATCH] Account for cipher auth with multiple cert slots --- ssl/ssl_privkey.cc | 43 +++++++++++++++++++++++++++++-------------- ssl/ssl_x509.cc | 2 +- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/ssl/ssl_privkey.cc b/ssl/ssl_privkey.cc index 2a6ec0a95ce..1b23524f346 100644 --- a/ssl/ssl_privkey.cc +++ b/ssl/ssl_privkey.cc @@ -386,7 +386,14 @@ static bool ssl_public_key_rsa_pss_check(EVP_PKEY *pubkey, uint16_t sigalg) { 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)) { + // We may have a public key that supports the signature algorithm, but the + // negotiated cipher needs to also allow it. This behavior is only done on + // TLS 1.2 servers in OpenSSL. + const uint32_t auth_allowed = + !SSL_is_server(ssl) || (hs->new_cipher->algorithm_auth & + (ssl_cipher_auth_mask_for_key(hs->local_pubkey.get()) | SSL_aGENERIC)); + if (!auth_allowed || + !pkey_supports_algorithm(ssl, hs->local_pubkey.get(), sigalg)) { return false; } @@ -457,20 +464,28 @@ 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) { + // 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 on TLS 1.2 servers in OpenSSL. + const uint32_t auth_allowed = + !SSL_is_server(ssl) || + hs->new_cipher->algorithm_auth & + (ssl_cipher_auth_mask_for_key(private_key) | SSL_aGENERIC); + if (auth_allowed && 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_x509.cc b/ssl/ssl_x509.cc index 112d0747ed7..b6059d35547 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)) {