Skip to content

Commit

Permalink
Account for cipher auth with multiple cert slots
Browse files Browse the repository at this point in the history
  • Loading branch information
samuel40791765 committed Nov 1, 2024
1 parent 97a6706 commit 3f53a7a
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 32 deletions.
2 changes: 1 addition & 1 deletion ssl/extensions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4137,7 +4137,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
Original file line number Diff line number Diff line change
Expand Up @@ -1139,6 +1139,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
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,16 @@ 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 prior
// to TLS 1.2 servers in OpenSSL since TLS 1.3 does not have cipher-based
// authentication configuration.
const uint32_t auth_allowed =
!ssl->server ||
(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;
}

Expand All @@ -397,8 +406,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 +416,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 @@ -457,20 +476,29 @@ 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) {
// 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 prior to TLS 1.2 servers in OpenSSL since TLS
// 1.3 does not have cipher-based authentication configuration.
const uint32_t auth_allowed =
!ssl->server ||
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;
Expand Down
67 changes: 53 additions & 14 deletions ssl/ssl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6474,7 +6474,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 @@ -6490,8 +6490,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 @@ -6516,8 +6516,7 @@ 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;
}
Expand All @@ -6535,8 +6534,7 @@ 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;
}
Expand All @@ -6563,9 +6561,8 @@ 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;
}
Expand Down Expand Up @@ -6598,9 +6595,52 @@ 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.
return;
}

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;
}
Expand All @@ -6627,8 +6667,7 @@ 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;
}
Expand Down
2 changes: 1 addition & 1 deletion ssl/ssl_x509.cc
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 3f53a7a

Please sign in to comment.