Account for cipher auth with multiple cert slots

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;
     }

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

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<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;
@@ -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;
@@ -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<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;

ssl/ssl_test.cc

@@ -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(),
@@ -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<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
   bssl::UniquePtr<SSL_CTX> 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<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
   bssl::UniquePtr<SSL_CTX> 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<SSL_CTX> 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<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()));
@@ -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()));

ssl/ssl_x509.cc

@@ -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)) {