Skip to content

Commit

Permalink
Account for cipher auth with multiple cert slots (#1956)
Browse files Browse the repository at this point in the history 
Ruby has a dependency on the multiple certificate slot mechanisms that
OpenSSL allows for. We've already done the work to support this, but
another Ruby 3.1 test exposed a gap in our support. We were only
looking at the negotiated signature algorithms to retrieve the right
certificate for the server to send back, but the cipher authentication
scheme was also checked in OpenSSL as well. Ruby's tests happen to
only depend on configuring the authentication scheme which brought
this to light.
We already happen to do this when checking private keys for TLS 1.0/1.1,
this also fixes it to check the cipher against the initial public key.
This change also introduces all of the mentioned cipher authenticated
check behavior for TLS 1.2.

### Testing:
New unit test that allows all possible sigalgs, with the cipher
authentication suite being the only restriction.

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.
  • Loading branch information
@samuel40791765
samuel40791765 authored Nov 13, 2024
1 parent f9757c4 commit 3aa32cc
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 37 deletions.
2 changes: 1 addition & 1 deletion ssl/extensions.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
5 changes: 5 additions & 0 deletions ssl/internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
60 changes: 44 additions & 16 deletions ssl/ssl_privkey.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -397,8 +412,7 @@ bool ssl_public_key_supports_signature_algorithm(SSL_HANDSHAKE *hs,
return true;
}

UniquePtr<EVP_PKEY> ssl_cert_parse_leaf_pubkey(
STACK_OF(CRYPTO_BUFFER) *chain) {
UniquePtr<EVP_PKEY> 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;
Expand All @@ -408,6 +422,17 @@ UniquePtr<EVP_PKEY> 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;
Expand Down Expand Up @@ -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.
Expand All @@ -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<EVP_PKEY> 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;
Expand Down
77 changes: 58 additions & 19 deletions ssl/ssl_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6484,7 +6484,7 @@ class MultipleCertificateSlotTest
SSL_CTX *server_ctx,
std::vector<uint16_t> 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(),
Expand All @@ -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;
}

Expand All @@ -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<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
bssl::UniquePtr<SSL_CTX> server_ctx(CreateContextWithCertificate(
Expand All @@ -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<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_method()));
Expand All @@ -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<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
Expand Down Expand Up @@ -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<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
bssl::UniquePtr<SSL_CTX> 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<uint16_t> 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<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
Expand All @@ -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<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
Expand Down
2 changes: 1 addition & 1 deletion ssl/ssl_x509.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ static UniquePtr<STACK_OF(CRYPTO_BUFFER)> 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)) {
Expand Down

0 comments on commit 3aa32cc

Please sign in to comment.