Bug 1549225 - Up front Signature Scheme validation, r=ueno
authorMartin Thomson <mt@lowentropy.net>
Fri, 06 Sep 2019 19:59:11 +1000
changeset 15329 9b418f0a4912e0a7c928d0b0774e1815238984ee
parent 15328 c319019aee75914fcbc8df0a530de7114e81229b
child 15330 efb895a43899642a3f3a6bfe2957870dbaccb91b
push id3528
push usermartin.thomson@gmail.com
push dateThu, 10 Oct 2019 06:46:49 +0000
reviewersueno
bugs1549225
Bug 1549225 - Up front Signature Scheme validation, r=ueno Summary: This patch started as an attempt to ensure that a DSA signature scheme would not be advertised if we weren't willing to negotiate versions less than TLS 1.3. Then I realized that we didn't do the same for PKCS#1 RSA. Then I realized that we were still willing to try to establish connections when we had a certificate that we couldn't use. Then I realized that ssl3_config_match_init() wasn't being run consistently. On resumption, we only ran it when we were PARANOID. That's silly because we weren't checking policies. Then I realized that we were allowing ECDSA certificates to be used when the named group in the certificate was disabled. We weren't enforcing that consistently either. However, I also discovered that the check we have wouldn't work without a tweak because in TLS 1.3 the named group is part of the signature scheme; the configured named groups are only used prior to TLS 1.3 when selecting ECDSA/ECDH certificates. So that sounds like a lot of changes but what it boils down to is more robust checking of the configuration prior to starting a connection. As a result, we should be offering fewer options that we're unwilling or unable to follow through on. A good number of tests needed tweaking as a result because we were relying on getting past the checks in those tests. No real problems were found as a result; this just moves failures that might arise from misconfiguration a little earlier in the process. Differential Revision: https://phabricator.services.mozilla.com/D45966
gtests/ssl_gtest/ssl_auth_unittest.cc
gtests/ssl_gtest/ssl_ciphersuite_unittest.cc
gtests/ssl_gtest/ssl_extension_unittest.cc
gtests/ssl_gtest/ssl_fuzz_unittest.cc
gtests/ssl_gtest/tls_esni_unittest.cc
lib/ssl/ssl3con.c
lib/ssl/ssl3exthandle.c
lib/ssl/sslimpl.h
lib/ssl/tls13con.c
--- a/gtests/ssl_gtest/ssl_auth_unittest.cc
+++ b/gtests/ssl_gtest/ssl_auth_unittest.cc
@@ -778,24 +778,39 @@ TEST_P(TlsConnectTls13, ClientAuthPkcs1S
       client_, kTlsHandshakeCertificateVerify);
   capture_cert_verify->EnableDecryption();
 
   Connect();
   CheckSigScheme(capture_cert_verify, 0, server_, ssl_sig_rsa_pss_rsae_sha256,
                  1024);
 }
 
+// Client should refuse to connect without a usable signature scheme.
 TEST_P(TlsConnectTls13, ClientAuthPkcs1SignatureSchemeOnly) {
   static const SSLSignatureScheme kSignatureScheme[] = {
       ssl_sig_rsa_pkcs1_sha256};
 
   Reset(TlsAgent::kServerRsa, "rsa");
   client_->SetSignatureSchemes(kSignatureScheme,
                                PR_ARRAY_SIZE(kSignatureScheme));
-  server_->SetSignatureSchemes(kSignatureScheme,
+  client_->SetupClientAuth();
+  client_->StartConnect();
+  client_->Handshake();
+  EXPECT_EQ(TlsAgent::STATE_ERROR, client_->state());
+  client_->CheckErrorCode(SSL_ERROR_NO_SUPPORTED_SIGNATURE_ALGORITHM);
+}
+
+// Though the client has a usable signature scheme, when a certificate is
+// requested, it can't produce one.
+TEST_P(TlsConnectTls13, ClientAuthPkcs1AndEcdsaScheme) {
+  static const SSLSignatureScheme kSignatureScheme[] = {
+      ssl_sig_rsa_pkcs1_sha256, ssl_sig_ecdsa_secp256r1_sha256};
+
+  Reset(TlsAgent::kServerRsa, "rsa");
+  client_->SetSignatureSchemes(kSignatureScheme,
                                PR_ARRAY_SIZE(kSignatureScheme));
   client_->SetupClientAuth();
   server_->RequestClientAuth(true);
 
   ConnectExpectAlert(server_, kTlsAlertHandshakeFailure);
   server_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_SIGNATURE_ALGORITHM);
   client_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
 }
@@ -1428,16 +1443,119 @@ TEST_F(TlsAgentStreamTestServer, Configu
   EXPECT_FALSE(agent_->ConfigServerCert(TlsAgent::kServerRsaPss, false,
                                         &ServerCertDataRsaPkcs1Decrypt));
 
   // Configuring for only rsa_pss should work.
   EXPECT_TRUE(agent_->ConfigServerCert(TlsAgent::kServerRsaPss, false,
                                        &ServerCertDataRsaPss));
 }
 
+// A server should refuse to even start a handshake with
+// misconfigured certificate and signature scheme.
+TEST_P(TlsConnectTls12Plus, MisconfiguredCertScheme) {
+  Reset(TlsAgent::kServerDsa);
+  static const SSLSignatureScheme kScheme[] = {ssl_sig_ecdsa_secp256r1_sha256};
+  server_->SetSignatureSchemes(kScheme, PR_ARRAY_SIZE(kScheme));
+  ConnectExpectAlert(server_, kTlsAlertHandshakeFailure);
+  if (version_ < SSL_LIBRARY_VERSION_TLS_1_3) {
+    // TLS 1.2 disables cipher suites, which leads to a different error.
+    server_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
+  } else {
+    server_->CheckErrorCode(SSL_ERROR_NO_SUPPORTED_SIGNATURE_ALGORITHM);
+  }
+  client_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
+}
+
+// In TLS 1.2, disabling an EC group causes ECDSA to be invalid.
+TEST_P(TlsConnectTls12, Tls12CertDisabledGroup) {
+  Reset(TlsAgent::kServerEcdsa256);
+  static const std::vector<SSLNamedGroup> k25519 = {ssl_grp_ec_curve25519};
+  server_->ConfigNamedGroups(k25519);
+  ConnectExpectAlert(server_, kTlsAlertHandshakeFailure);
+  server_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
+  client_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
+}
+
+// In TLS 1.3, ECDSA configuration only depends on the signature scheme.
+TEST_P(TlsConnectTls13, Tls13CertDisabledGroup) {
+  Reset(TlsAgent::kServerEcdsa256);
+  static const std::vector<SSLNamedGroup> k25519 = {ssl_grp_ec_curve25519};
+  server_->ConfigNamedGroups(k25519);
+  Connect();
+}
+
+// A client should refuse to even start a handshake with only DSA.
+TEST_P(TlsConnectTls13, Tls13DsaOnlyClient) {
+  static const SSLSignatureScheme kDsa[] = {ssl_sig_dsa_sha256};
+  client_->SetSignatureSchemes(kDsa, PR_ARRAY_SIZE(kDsa));
+  client_->StartConnect();
+  client_->Handshake();
+  EXPECT_EQ(TlsAgent::STATE_ERROR, client_->state());
+  client_->CheckErrorCode(SSL_ERROR_NO_SUPPORTED_SIGNATURE_ALGORITHM);
+}
+
+TEST_P(TlsConnectTls13, Tls13DsaOnlyServer) {
+  Reset(TlsAgent::kServerDsa);
+  static const SSLSignatureScheme kDsa[] = {ssl_sig_dsa_sha256};
+  server_->SetSignatureSchemes(kDsa, PR_ARRAY_SIZE(kDsa));
+  ConnectExpectAlert(server_, kTlsAlertHandshakeFailure);
+  server_->CheckErrorCode(SSL_ERROR_NO_SUPPORTED_SIGNATURE_ALGORITHM);
+  client_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
+}
+
+TEST_P(TlsConnectTls13, Tls13Pkcs1OnlyClient) {
+  static const SSLSignatureScheme kPkcs1[] = {ssl_sig_rsa_pkcs1_sha256};
+  client_->SetSignatureSchemes(kPkcs1, PR_ARRAY_SIZE(kPkcs1));
+  client_->StartConnect();
+  client_->Handshake();
+  EXPECT_EQ(TlsAgent::STATE_ERROR, client_->state());
+  client_->CheckErrorCode(SSL_ERROR_NO_SUPPORTED_SIGNATURE_ALGORITHM);
+}
+
+TEST_P(TlsConnectTls13, Tls13Pkcs1OnlyServer) {
+  static const SSLSignatureScheme kPkcs1[] = {ssl_sig_rsa_pkcs1_sha256};
+  server_->SetSignatureSchemes(kPkcs1, PR_ARRAY_SIZE(kPkcs1));
+  ConnectExpectAlert(server_, kTlsAlertHandshakeFailure);
+  server_->CheckErrorCode(SSL_ERROR_NO_SUPPORTED_SIGNATURE_ALGORITHM);
+  client_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
+}
+
+TEST_P(TlsConnectTls13, Tls13DsaIsNotAdvertisedClient) {
+  EnsureTlsSetup();
+  static const SSLSignatureScheme kSchemes[] = {ssl_sig_dsa_sha256,
+                                                ssl_sig_rsa_pss_rsae_sha256};
+  client_->SetSignatureSchemes(kSchemes, PR_ARRAY_SIZE(kSchemes));
+  auto capture =
+      MakeTlsFilter<TlsExtensionCapture>(client_, ssl_signature_algorithms_xtn);
+  Connect();
+  // We should only have the one signature algorithm advertised.
+  static const uint8_t kExpectedExt[] = {0, 2, ssl_sig_rsa_pss_rsae_sha256 >> 8,
+                                         ssl_sig_rsa_pss_rsae_sha256 & 0xff};
+  ASSERT_EQ(DataBuffer(kExpectedExt, sizeof(kExpectedExt)),
+            capture->extension());
+}
+
+TEST_P(TlsConnectTls13, Tls13DsaIsNotAdvertisedServer) {
+  EnsureTlsSetup();
+  static const SSLSignatureScheme kSchemes[] = {ssl_sig_dsa_sha256,
+                                                ssl_sig_rsa_pss_rsae_sha256};
+  server_->SetSignatureSchemes(kSchemes, PR_ARRAY_SIZE(kSchemes));
+  auto capture = MakeTlsFilter<TlsExtensionCapture>(
+      server_, ssl_signature_algorithms_xtn, true);
+  capture->SetHandshakeTypes({kTlsHandshakeCertificateRequest});
+  capture->EnableDecryption();
+  server_->RequestClientAuth(false);  // So we get a CertificateRequest.
+  Connect();
+  // We should only have the one signature algorithm advertised.
+  static const uint8_t kExpectedExt[] = {0, 2, ssl_sig_rsa_pss_rsae_sha256 >> 8,
+                                         ssl_sig_rsa_pss_rsae_sha256 & 0xff};
+  ASSERT_EQ(DataBuffer(kExpectedExt, sizeof(kExpectedExt)),
+            capture->extension());
+}
+
 // variant, version, certificate, auth type, signature scheme
 typedef std::tuple<SSLProtocolVariant, uint16_t, std::string, SSLAuthType,
                    SSLSignatureScheme>
     SignatureSchemeProfile;
 
 class TlsSignatureSchemeConfiguration
     : public TlsConnectTestBase,
       public ::testing::WithParamInterface<SignatureSchemeProfile> {
--- a/gtests/ssl_gtest/ssl_ciphersuite_unittest.cc
+++ b/gtests/ssl_gtest/ssl_ciphersuite_unittest.cc
@@ -51,75 +51,93 @@ class TlsCipherSuiteTestBase : public Tl
   void EnableSingleCipher() {
     EnsureTlsSetup();
     // It doesn't matter which does this, but the test is better if both do it.
     client_->EnableSingleCipher(cipher_suite_);
     server_->EnableSingleCipher(cipher_suite_);
 
     if (version_ >= SSL_LIBRARY_VERSION_TLS_1_3) {
       std::vector<SSLNamedGroup> groups = {group_};
+      if (cert_group_ != ssl_grp_none) {
+        groups.push_back(cert_group_);
+      }
       client_->ConfigNamedGroups(groups);
       server_->ConfigNamedGroups(groups);
       kea_type_ = SSLInt_GetKEAType(group_);
 
       client_->SetSignatureSchemes(&sig_scheme_, 1);
       server_->SetSignatureSchemes(&sig_scheme_, 1);
     }
   }
 
   virtual void SetupCertificate() {
     if (version_ >= SSL_LIBRARY_VERSION_TLS_1_3) {
       switch (sig_scheme_) {
         case ssl_sig_rsa_pss_rsae_sha256:
+          std::cerr << "Signature scheme: rsa_pss_rsae_sha256" << std::endl;
+          Reset(TlsAgent::kServerRsaSign);
+          auth_type_ = ssl_auth_rsa_sign;
+          break;
         case ssl_sig_rsa_pss_rsae_sha384:
+          std::cerr << "Signature scheme: rsa_pss_rsae_sha384" << std::endl;
           Reset(TlsAgent::kServerRsaSign);
           auth_type_ = ssl_auth_rsa_sign;
           break;
         case ssl_sig_rsa_pss_rsae_sha512:
           // You can't fit SHA-512 PSS in a 1024-bit key.
+          std::cerr << "Signature scheme: rsa_pss_rsae_sha512" << std::endl;
           Reset(TlsAgent::kRsa2048);
           auth_type_ = ssl_auth_rsa_sign;
           break;
         case ssl_sig_rsa_pss_pss_sha256:
+          std::cerr << "Signature scheme: rsa_pss_pss_sha256" << std::endl;
           Reset(TlsAgent::kServerRsaPss);
           auth_type_ = ssl_auth_rsa_pss;
           break;
         case ssl_sig_rsa_pss_pss_sha384:
+          std::cerr << "Signature scheme: rsa_pss_pss_sha384" << std::endl;
           Reset("rsa_pss384");
           auth_type_ = ssl_auth_rsa_pss;
           break;
         case ssl_sig_rsa_pss_pss_sha512:
+          std::cerr << "Signature scheme: rsa_pss_pss_sha512" << std::endl;
           Reset("rsa_pss512");
           auth_type_ = ssl_auth_rsa_pss;
           break;
         case ssl_sig_ecdsa_secp256r1_sha256:
+          std::cerr << "Signature scheme: ecdsa_secp256r1_sha256" << std::endl;
           Reset(TlsAgent::kServerEcdsa256);
           auth_type_ = ssl_auth_ecdsa;
+          cert_group_ = ssl_grp_ec_secp256r1;
           break;
         case ssl_sig_ecdsa_secp384r1_sha384:
+          std::cerr << "Signature scheme: ecdsa_secp384r1_sha384" << std::endl;
           Reset(TlsAgent::kServerEcdsa384);
           auth_type_ = ssl_auth_ecdsa;
+          cert_group_ = ssl_grp_ec_secp384r1;
           break;
         default:
           ADD_FAILURE() << "Unsupported signature scheme: " << sig_scheme_;
           break;
       }
     } else {
       switch (csinfo_.authType) {
         case ssl_auth_rsa_sign:
           Reset(TlsAgent::kServerRsaSign);
           break;
         case ssl_auth_rsa_decrypt:
           Reset(TlsAgent::kServerRsaDecrypt);
           break;
         case ssl_auth_ecdsa:
           Reset(TlsAgent::kServerEcdsa256);
+          cert_group_ = ssl_grp_ec_secp256r1;
           break;
         case ssl_auth_ecdh_ecdsa:
           Reset(TlsAgent::kServerEcdhEcdsa);
+          cert_group_ = ssl_grp_ec_secp256r1;
           break;
         case ssl_auth_ecdh_rsa:
           Reset(TlsAgent::kServerEcdhRsa);
           break;
         case ssl_auth_dsa:
           Reset(TlsAgent::kServerDsa);
           break;
         default:
@@ -187,16 +205,17 @@ class TlsCipherSuiteTestBase : public Tl
     return limit;
   }
 
  protected:
   uint16_t cipher_suite_;
   SSLAuthType auth_type_;
   SSLKEAType kea_type_;
   SSLNamedGroup group_;
+  SSLNamedGroup cert_group_ = ssl_grp_none;
   SSLSignatureScheme sig_scheme_;
   SSLCipherSuiteInfo csinfo_;
 };
 
 class TlsCipherSuiteTest
     : public TlsCipherSuiteTestBase,
       public ::testing::WithParamInterface<CipherSuiteProfile> {
  public:
--- a/gtests/ssl_gtest/ssl_extension_unittest.cc
+++ b/gtests/ssl_gtest/ssl_extension_unittest.cc
@@ -647,17 +647,17 @@ TEST_P(TlsExtensionTest12, SignatureAlgo
                               0);
   ASSERT_EQ(SECSuccess, rv);
 
   Reset(TlsAgent::kServerDsa);
   auto capture2 =
       MakeTlsFilter<TlsExtensionCapture>(client_, ssl_signature_algorithms_xtn);
   client_->SetSignatureSchemes(schemes.data(), schemes.size());
   ConnectExpectAlert(server_, kTlsAlertHandshakeFailure);
-  server_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_SIGNATURE_ALGORITHM);
+  server_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
   client_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
 
   // Check if no DSA algorithms are advertised.
   EXPECT_TRUE(capture2->captured());
   const DataBuffer& ext2 = capture2->extension();
   EXPECT_EQ(2U + 2U, ext2.len());
   uint32_t v = 0;
   EXPECT_TRUE(ext2.Read(2, 2, &v));
--- a/gtests/ssl_gtest/ssl_fuzz_unittest.cc
+++ b/gtests/ssl_gtest/ssl_fuzz_unittest.cc
@@ -247,9 +247,9 @@ FUZZ_P(TlsFuzzTest, UnencryptedSessionTi
 INSTANTIATE_TEST_CASE_P(
     FuzzStream, TlsFuzzTest,
     ::testing::Combine(TlsConnectTestBase::kTlsVariantsStream,
                        TlsConnectTestBase::kTlsVAll));
 INSTANTIATE_TEST_CASE_P(
     FuzzDatagram, TlsFuzzTest,
     ::testing::Combine(TlsConnectTestBase::kTlsVariantsDatagram,
                        TlsConnectTestBase::kTlsV11Plus));
-}
+}  // namespace nss_test
--- a/gtests/ssl_gtest/tls_esni_unittest.cc
+++ b/gtests/ssl_gtest/tls_esni_unittest.cc
@@ -467,9 +467,9 @@ TEST_P(TlsConnectTls13, EsniButTLS12Serv
   server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
                            SSL_LIBRARY_VERSION_TLS_1_2);
   ConnectExpectAlert(client_, kTlsAlertProtocolVersion);
   client_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_VERSION);
   server_->CheckErrorCode(SSL_ERROR_PROTOCOL_VERSION_ALERT);
   ASSERT_FALSE(SSLInt_ExtensionNegotiated(server_->ssl_fd(),
                                           ssl_tls13_encrypted_sni_xtn));
 }
-}
+}  // namespace nss_test
--- a/lib/ssl/ssl3con.c
+++ b/lib/ssl/ssl3con.c
@@ -732,17 +732,17 @@ ssl_KEAEnabled(const sslSocket *ss, SSLK
         case ssl_kea_fortezza:
         default:
             PORT_Assert(0);
     }
     return PR_FALSE;
 }
 
 static PRBool
-ssl_HasCert(const sslSocket *ss, SSLAuthType authType)
+ssl_HasCert(const sslSocket *ss, PRUint16 maxVersion, SSLAuthType authType)
 {
     PRCList *cursor;
     if (authType == ssl_auth_null || authType == ssl_auth_psk || authType == ssl_auth_tls13_any) {
         return PR_TRUE;
     }
     for (cursor = PR_NEXT_LINK(&ss->serverCerts);
          cursor != &ss->serverCerts;
          cursor = PR_NEXT_LINK(cursor)) {
@@ -752,27 +752,139 @@ ssl_HasCert(const sslSocket *ss, SSLAuth
             !cert->serverCertChain ||
             !SSL_CERT_IS(cert, authType)) {
             continue;
         }
         /* When called from ssl3_config_match_init(), all the EC curves will be
          * enabled, so this will essentially do nothing (unless we implement
          * curve configuration).  However, once we have seen the
          * supported_groups extension and this is called from config_match(),
-         * this will filter out certificates with an unsupported curve. */
-        if ((authType == ssl_auth_ecdsa ||
+         * this will filter out certificates with an unsupported curve.
+         *
+         * If we might negotiate TLS 1.3, skip this test as group configuration
+         * doesn't affect choices in TLS 1.3.
+         */
+        if (maxVersion < SSL_LIBRARY_VERSION_TLS_1_3 &&
+            (authType == ssl_auth_ecdsa ||
              authType == ssl_auth_ecdh_ecdsa ||
              authType == ssl_auth_ecdh_rsa) &&
             !ssl_NamedGroupEnabled(ss, cert->namedCurve)) {
             continue;
         }
         return PR_TRUE;
     }
     if (authType == ssl_auth_rsa_sign) {
-        return ssl_HasCert(ss, ssl_auth_rsa_pss);
+        return ssl_HasCert(ss, maxVersion, ssl_auth_rsa_pss);
+    }
+    return PR_FALSE;
+}
+
+/* Check that a signature scheme is accepted.
+ * Both by policy and by having a token that supports it. */
+static PRBool
+ssl_SignatureSchemeAccepted(PRUint16 minVersion,
+                            SSLSignatureScheme scheme)
+{
+    /* Disable RSA-PSS schemes if there are no tokens to verify them. */
+    if (ssl_IsRsaPssSignatureScheme(scheme)) {
+        if (!PK11_TokenExists(auth_alg_defs[ssl_auth_rsa_pss])) {
+            return PR_FALSE;
+        }
+    } else if (ssl_IsRsaPkcs1SignatureScheme(scheme)) {
+        /* Disable PKCS#1 signatures if we are limited to TLS 1.3. */
+        if (minVersion >= SSL_LIBRARY_VERSION_TLS_1_3) {
+            return PR_FALSE;
+        }
+    } else if (ssl_IsDsaSignatureScheme(scheme)) {
+        /* DSA: not in TLS 1.3, and check policy. */
+        if (minVersion >= SSL_LIBRARY_VERSION_TLS_1_3) {
+            return PR_FALSE;
+        }
+        PRUint32 dsaPolicy;
+        SECStatus rv = NSS_GetAlgorithmPolicy(SEC_OID_ANSIX9_DSA_SIGNATURE,
+                                              &dsaPolicy);
+        if (rv == SECSuccess && (dsaPolicy & NSS_USE_ALG_IN_SSL_KX) == 0) {
+            return PR_FALSE;
+        }
+    }
+
+    /* Hash policy. */
+    PRUint32 hashPolicy;
+    SSLHashType hashType = ssl_SignatureSchemeToHashType(scheme);
+    SECOidTag hashOID = ssl3_HashTypeToOID(hashType);
+    SECStatus rv = NSS_GetAlgorithmPolicy(hashOID, &hashPolicy);
+    if (rv == SECSuccess && (hashPolicy & NSS_USE_ALG_IN_SSL_KX) == 0) {
+        return PR_FALSE;
+    }
+    return PR_TRUE;
+}
+
+static SECStatus
+ssl_CheckSignatureSchemes(sslSocket *ss)
+{
+    if (ss->vrange.max < SSL_LIBRARY_VERSION_TLS_1_2) {
+        return SECSuccess;
+    }
+
+    /* If this is a server using TLS 1.3, we just need to have one signature
+     * scheme for which we have a usable certificate.
+     *
+     * Note: Certificates for earlier TLS versions are checked along with the
+     * cipher suite in ssl3_config_match_init. */
+    if (ss->sec.isServer && ss->vrange.max >= SSL_LIBRARY_VERSION_TLS_1_3) {
+        PRBool foundCert = PR_FALSE;
+        for (unsigned int i = 0; i < ss->ssl3.signatureSchemeCount; ++i) {
+            SSLAuthType authType =
+                ssl_SignatureSchemeToAuthType(ss->ssl3.signatureSchemes[i]);
+            if (ssl_HasCert(ss, ss->vrange.max, authType)) {
+                foundCert = PR_TRUE;
+                break;
+            }
+        }
+        if (!foundCert) {
+            PORT_SetError(SSL_ERROR_NO_SUPPORTED_SIGNATURE_ALGORITHM);
+            return SECFailure;
+        }
+    }
+
+    /* Ensure that there is a signature scheme that can be accepted.*/
+    for (unsigned int i = 0; i < ss->ssl3.signatureSchemeCount; ++i) {
+        if (ssl_SignatureSchemeAccepted(ss->vrange.min,
+                                        ss->ssl3.signatureSchemes[i])) {
+            return SECSuccess;
+        }
+    }
+    PORT_SetError(SSL_ERROR_NO_SUPPORTED_SIGNATURE_ALGORITHM);
+    return SECFailure;
+}
+
+/* For a server, check that a signature scheme that can be used with the
+ * provided authType is both enabled and usable. */
+static PRBool
+ssl_HasSignatureScheme(const sslSocket *ss, SSLAuthType authType)
+{
+    PORT_Assert(ss->sec.isServer);
+    PORT_Assert(ss->ssl3.hs.preliminaryInfo & ssl_preinfo_version);
+    PORT_Assert(authType != ssl_auth_null);
+    PORT_Assert(authType != ssl_auth_tls13_any);
+    if (ss->version < SSL_LIBRARY_VERSION_TLS_1_2 ||
+        authType == ssl_auth_rsa_decrypt ||
+        authType == ssl_auth_ecdh_rsa ||
+        authType == ssl_auth_ecdh_ecdsa) {
+        return PR_TRUE;
+    }
+    for (unsigned int i = 0; i < ss->ssl3.signatureSchemeCount; ++i) {
+        SSLSignatureScheme scheme = ss->ssl3.signatureSchemes[i];
+        SSLAuthType schemeAuthType = ssl_SignatureSchemeToAuthType(scheme);
+        PRBool acceptable = authType == schemeAuthType ||
+                            (schemeAuthType == ssl_auth_rsa_pss &&
+                             authType == ssl_auth_rsa_sign);
+        if (acceptable && ssl_SignatureSchemeAccepted(ss->version, scheme)) {
+            return PR_TRUE;
+        }
     }
     return PR_FALSE;
 }
 
 /* Initialize the suite->isPresent value for config_match
  * Returns count of enabled ciphers supported by extant tokens,
  * regardless of policy or user preference.
  * If this returns zero, the user cannot do SSL v3.
@@ -793,16 +905,19 @@ ssl3_config_match_init(sslSocket *ss)
     PORT_Assert(ss);
     if (!ss) {
         PORT_SetError(SEC_ERROR_INVALID_ARGS);
         return 0;
     }
     if (SSL_ALL_VERSIONS_DISABLED(&ss->vrange)) {
         return 0;
     }
+    if (ssl_CheckSignatureSchemes(ss) != SECSuccess) {
+        return 0; /* Code already set. */
+    }
 
     ssl_FilterSupportedGroups(ss);
     for (i = 0; i < ssl_V3_SUITES_IMPLEMENTED; i++) {
         suite = &ss->cipherSuites[i];
         if (suite->enabled) {
             ++numEnabled;
             /* We need the cipher defs to see if we have a token that can handle
              * this cipher.  It isn't part of the static definition.
@@ -815,20 +930,21 @@ ssl3_config_match_init(sslSocket *ss)
             cipher_alg = ssl_GetBulkCipherDef(cipher_def)->calg;
             cipher_mech = ssl3_Alg2Mech(cipher_alg);
 
             /* Mark the suites that are backed by real tokens, certs and keys */
             suite->isPresent = PR_TRUE;
 
             authType = kea_defs[cipher_def->key_exchange_alg].authKeyType;
             if (authType != ssl_auth_null && authType != ssl_auth_tls13_any) {
-                if (ss->sec.isServer && !ssl_HasCert(ss, authType)) {
+                if (ss->sec.isServer &&
+                    !(ssl_HasCert(ss, ss->vrange.max, authType) &&
+                      ssl_HasSignatureScheme(ss, authType))) {
                     suite->isPresent = PR_FALSE;
-                }
-                if (!PK11_TokenExists(auth_alg_defs[authType])) {
+                } else if (!PK11_TokenExists(auth_alg_defs[authType])) {
                     suite->isPresent = PR_FALSE;
                 }
             }
 
             keaType = kea_defs[cipher_def->key_exchange_alg].exchKeyType;
             if (keaType != ssl_kea_null &&
                 keaType != ssl_kea_tls13_any &&
                 !PK11_TokenExists(kea_alg_defs[keaType])) {
@@ -882,17 +998,17 @@ ssl3_config_match(const ssl3CipherSuiteC
     cipher_def = ssl_LookupCipherSuiteDef(suite->cipher_suite);
     PORT_Assert(cipher_def != NULL);
     kea_def = &kea_defs[cipher_def->key_exchange_alg];
     PORT_Assert(kea_def != NULL);
     if (!ssl_KEAEnabled(ss, kea_def->exchKeyType)) {
         return PR_FALSE;
     }
 
-    if (ss->sec.isServer && !ssl_HasCert(ss, kea_def->authKeyType)) {
+    if (ss->sec.isServer && !ssl_HasCert(ss, vrange->max, kea_def->authKeyType)) {
         return PR_FALSE;
     }
 
     return ssl3_CipherSuiteAllowedForVersionRange(suite->cipher_suite, vrange);
 }
 
 /* Return the number of cipher suites that are usable. */
 /* called from ssl3_SendClientHello */
@@ -4151,16 +4267,19 @@ ssl_SignatureSchemeValid(SSLSignatureSch
     }
     if (isTls13) {
         if (ssl_SignatureSchemeToHashType(scheme) == ssl_hash_sha1) {
             return PR_FALSE;
         }
         if (ssl_IsRsaPkcs1SignatureScheme(scheme)) {
             return PR_FALSE;
         }
+        if (ssl_IsDsaSignatureScheme(scheme)) {
+            return PR_FALSE;
+        }
         /* With TLS 1.3, EC keys should have been selected based on calling
          * ssl_SignatureSchemeFromSpki(), reject them otherwise. */
         return spkiOid != SEC_OID_ANSIX962_EC_PUBLIC_KEY;
     }
     return PR_TRUE;
 }
 
 static SECStatus
@@ -4265,16 +4384,17 @@ ssl_SignatureSchemeFromSpki(const CERTSu
     if (isTls13 && spkiOid == SEC_OID_ANSIX962_EC_PUBLIC_KEY) {
         return ssl_SignatureSchemeFromEcSpki(spki, scheme);
     }
 
     *scheme = ssl_sig_none;
     return SECSuccess;
 }
 
+/* Check that a signature scheme is enabled by configuration. */
 PRBool
 ssl_SignatureSchemeEnabled(const sslSocket *ss, SSLSignatureScheme scheme)
 {
     unsigned int i;
     for (i = 0; i < ss->ssl3.signatureSchemeCount; ++i) {
         if (scheme == ss->ssl3.signatureSchemes[i]) {
             return PR_TRUE;
         }
@@ -8047,16 +8167,17 @@ ssl3_NegotiateCipherSuiteInner(sslSocket
         for (i = 0; i + 1 < suites->len; i += 2) {
             PRUint16 suite_i = (suites->data[i] << 8) | suites->data[i + 1];
             if (suite_i == suite->cipher_suite) {
                 *suitep = suite_i;
                 return SECSuccess;
             }
         }
     }
+    PORT_SetError(SSL_ERROR_NO_CYPHER_OVERLAP);
     return SECFailure;
 }
 
 /* Select a cipher suite.
 **
 ** NOTE: This suite selection algorithm should be the same as the one in
 ** ssl3_HandleV2ClientHello().
 **
@@ -8071,16 +8192,24 @@ ssl3_NegotiateCipherSuiteInner(sslSocket
 */
 SECStatus
 ssl3_NegotiateCipherSuite(sslSocket *ss, const SECItem *suites,
                           PRBool initHashes)
 {
     PRUint16 selected;
     SECStatus rv;
 
+    /* Ensure that only valid cipher suites are enabled. */
+    if (ssl3_config_match_init(ss) == 0) {
+        /* No configured cipher is both supported by PK11 and allowed.
+         * This is a configuration error, so report handshake failure.*/
+        FATAL_ERROR(ss, PORT_GetError(), handshake_failure);
+        return SECFailure;
+    }
+
     rv = ssl3_NegotiateCipherSuiteInner(ss, suites, ss->version, &selected);
     if (rv != SECSuccess) {
         return SECFailure;
     }
 
     ss->ssl3.hs.cipher_suite = selected;
     return ssl3_SetupCipherSuite(ss, initHashes);
 }
@@ -8200,23 +8329,16 @@ ssl3_ServerCallSNICallback(sslSocket *ss
                 rv = SECITEM_CopyItem(NULL, pwsName, name);
                 ssl_ReleaseSpecWriteLock(ss); /***************************/
                 if (rv != SECSuccess) {
                     errCode = SSL_ERROR_INTERNAL_ERROR_ALERT;
                     desc = internal_error;
                     ret = SSL_SNI_SEND_ALERT;
                     break;
                 }
-                if (ssl3_config_match_init(ss) == 0) {
-                    /* no ciphers are working/supported */
-                    errCode = PORT_GetError();
-                    desc = handshake_failure;
-                    ret = SSL_SNI_SEND_ALERT;
-                    break;
-                }
                 /* Need to tell the client that application has picked
                  * the name from the offered list and reconfigured the socket.
                  * Don't do this if we negotiated ESNI.
                  */
                 if (!ssl3_ExtensionNegotiated(ss, ssl_tls13_encrypted_sni_xtn)) {
                     ssl3_RegisterExtensionSender(ss, &ss->xtnData, ssl_server_name_xtn,
                                                  ssl_SendEmptyExtension);
                 }
@@ -8840,41 +8962,39 @@ ssl3_HandleClientHelloPart2(sslSocket *s
     }
 
     /* If we already have a session for this client, be sure to pick the same
     ** cipher suite we picked before.  This is not a loop, despite appearances.
     */
     if (sid)
         do {
             ssl3CipherSuiteCfg *suite;
-#ifdef PARANOID
             SSLVersionRange vrange = { ss->version, ss->version };
-#endif
 
             suite = ss->cipherSuites;
             /* Find the entry for the cipher suite used in the cached session. */
             for (j = ssl_V3_SUITES_IMPLEMENTED; j > 0; --j, ++suite) {
                 if (suite->cipher_suite == sid->u.ssl3.cipherSuite)
                     break;
             }
             PORT_Assert(j > 0);
             if (j == 0)
                 break;
-#ifdef PARANOID
+
             /* Double check that the cached cipher suite is still enabled,
              * implemented, and allowed by policy.  Might have been disabled.
-             * The product policy won't change during the process lifetime.
-             * Implemented ("isPresent") shouldn't change for servers.
              */
+            if (ssl3_config_match_init(ss) == 0) {
+                desc = handshake_failure;
+                errCode = PORT_GetError();
+                goto alert_loser;
+            }
             if (!ssl3_config_match(suite, ss->ssl3.policy, &vrange, ss))
                 break;
-#else
-            if (!suite->enabled)
-                break;
-#endif
+
             /* Double check that the cached cipher suite is in the client's
              * list.  If it isn't, fall through and start a new session. */
             for (i = 0; i + 1 < suites->len; i += 2) {
                 PRUint16 suite_i = (suites->data[i] << 8) | suites->data[i + 1];
                 if (suite_i == suite->cipher_suite) {
                     ss->ssl3.hs.cipher_suite = suite_i;
                     rv = ssl3_SetupCipherSuite(ss, PR_TRUE);
                     if (rv != SECSuccess) {
@@ -8882,31 +9002,22 @@ ssl3_HandleClientHelloPart2(sslSocket *s
                         errCode = PORT_GetError();
                         goto alert_loser;
                     }
 
                     goto cipher_found;
                 }
             }
         } while (0);
-/* START A NEW SESSION */
-
-#ifndef PARANOID
-    /* Look for a matching cipher suite. */
-    if (ssl3_config_match_init(ss) == 0) {
-        desc = internal_error;
-        errCode = PORT_GetError(); /* error code is already set. */
-        goto alert_loser;
-    }
-#endif
+    /* START A NEW SESSION */
 
     rv = ssl3_NegotiateCipherSuite(ss, suites, PR_TRUE);
     if (rv != SECSuccess) {
         desc = handshake_failure;
-        errCode = SSL_ERROR_NO_CYPHER_OVERLAP;
+        errCode = PORT_GetError();
         goto alert_loser;
     }
 
 cipher_found:
     suites->data = NULL;
 
     /* If there are any failures while processing the old sid,
      * we don't consider them to be errors.  Instead, We just behave
@@ -9664,55 +9775,34 @@ ssl3_SendServerKeyExchange(sslSocket *ss
             PORT_SetError(SEC_ERROR_LIBRARY_FAILURE);
             break;
     }
 
     return SECFailure;
 }
 
 SECStatus
-ssl3_EncodeSigAlgs(const sslSocket *ss, sslBuffer *buf)
+ssl3_EncodeSigAlgs(const sslSocket *ss, PRUint16 minVersion, sslBuffer *buf)
 {
     unsigned int lengthOffset;
-    unsigned int i;
     PRBool found = PR_FALSE;
     SECStatus rv;
 
     rv = sslBuffer_Skip(buf, 2, &lengthOffset);
     if (rv != SECSuccess) {
         return SECFailure;
     }
 
-    for (i = 0; i < ss->ssl3.signatureSchemeCount; ++i) {
-        PRUint32 policy = 0;
-        SSLHashType hashType = ssl_SignatureSchemeToHashType(
-            ss->ssl3.signatureSchemes[i]);
-        SECOidTag hashOID = ssl3_HashTypeToOID(hashType);
-
-        /* Skip RSA-PSS schemes if there are no tokens to verify them. */
-        if (ssl_IsRsaPssSignatureScheme(ss->ssl3.signatureSchemes[i]) &&
-            !PK11_TokenExists(auth_alg_defs[ssl_auth_rsa_pss])) {
-            continue;
-        }
-
-        /* Skip DSA scheme if it is disabled by policy. */
-        if (ssl_IsDsaSignatureScheme(ss->ssl3.signatureSchemes[i]) &&
-            (NSS_GetAlgorithmPolicy(SEC_OID_ANSIX9_DSA_SIGNATURE, &policy) ==
-             SECSuccess) &&
-            !(policy & NSS_USE_ALG_IN_SSL_KX)) {
-            continue;
-        }
-
-        if ((NSS_GetAlgorithmPolicy(hashOID, &policy) != SECSuccess) ||
-            (policy & NSS_USE_ALG_IN_SSL_KX)) {
+    for (unsigned int i = 0; i < ss->ssl3.signatureSchemeCount; ++i) {
+        if (ssl_SignatureSchemeAccepted(minVersion,
+                                        ss->ssl3.signatureSchemes[i])) {
             rv = sslBuffer_AppendNumber(buf, ss->ssl3.signatureSchemes[i], 2);
             if (rv != SECSuccess) {
                 return SECFailure;
             }
-
             found = PR_TRUE;
         }
     }
 
     if (!found) {
         PORT_SetError(SSL_ERROR_NO_SUPPORTED_SIGNATURE_ALGORITHM);
         return SECFailure;
     }
@@ -9748,17 +9838,17 @@ ssl3_SendCertificateRequest(sslSocket *s
     if (rv != SECSuccess) {
         return rv;
     }
     certTypes = certificate_types;
     certTypesLength = sizeof certificate_types;
 
     length = 1 + certTypesLength + 2 + calen;
     if (isTLS12) {
-        rv = ssl3_EncodeSigAlgs(ss, &sigAlgsBuf);
+        rv = ssl3_EncodeSigAlgs(ss, ss->version, &sigAlgsBuf);
         if (rv != SECSuccess) {
             return rv;
         }
         length += SSL_BUFFER_LEN(&sigAlgsBuf);
     }
 
     rv = ssl3_AppendHandshakeHeader(ss, ssl_hs_certificate_request, length);
     if (rv != SECSuccess) {
--- a/lib/ssl/ssl3exthandle.c
+++ b/lib/ssl/ssl3exthandle.c
@@ -1631,23 +1631,28 @@ ssl3_HandleSigAlgsXtn(const sslSocket *s
 }
 
 /* ssl3_ClientSendSigAlgsXtn sends the signature_algorithm extension for TLS
  * 1.2 ClientHellos. */
 SECStatus
 ssl3_SendSigAlgsXtn(const sslSocket *ss, TLSExtensionData *xtnData,
                     sslBuffer *buf, PRBool *added)
 {
-    SECStatus rv;
-
     if (ss->vrange.max < SSL_LIBRARY_VERSION_TLS_1_2) {
         return SECSuccess;
     }
 
-    rv = ssl3_EncodeSigAlgs(ss, buf);
+    PRUint16 minVersion;
+    if (ss->sec.isServer) {
+        minVersion = ss->version; /* CertificateRequest */
+    } else {
+        minVersion = ss->vrange.min; /* ClientHello */
+    }
+
+    SECStatus rv = ssl3_EncodeSigAlgs(ss, minVersion, buf);
     if (rv != SECSuccess) {
         return SECFailure;
     }
 
     *added = PR_TRUE;
     return SECSuccess;
 }
 
--- a/lib/ssl/sslimpl.h
+++ b/lib/ssl/sslimpl.h
@@ -1676,17 +1676,18 @@ void ssl3_SendAlertForCertError(sslSocke
 SECStatus ssl3_HandleNoCertificate(sslSocket *ss);
 SECStatus ssl3_SendEmptyCertificate(sslSocket *ss);
 void ssl3_CleanupPeerCerts(sslSocket *ss);
 SECStatus ssl3_SendCertificateStatus(sslSocket *ss);
 SECStatus ssl_SetAuthKeyBits(sslSocket *ss, const SECKEYPublicKey *pubKey);
 SECStatus ssl3_AuthCertificate(sslSocket *ss);
 SECStatus ssl_ReadCertificateStatus(sslSocket *ss, PRUint8 *b,
                                     PRUint32 length);
-SECStatus ssl3_EncodeSigAlgs(const sslSocket *ss, sslBuffer *buf);
+SECStatus ssl3_EncodeSigAlgs(const sslSocket *ss, PRUint16 minVersion,
+                             sslBuffer *buf);
 SECStatus ssl_GetCertificateRequestCAs(const sslSocket *ss,
                                        unsigned int *calenp,
                                        const SECItem **namesp,
                                        unsigned int *nnamesp);
 SECStatus ssl3_ParseCertificateRequestCAs(sslSocket *ss, PRUint8 **b,
                                           PRUint32 *length, CERTDistNames *ca_list);
 SECStatus ssl3_CompleteHandleCertificateRequest(
     sslSocket *ss, const SSLSignatureScheme *signatureSchemes,
--- a/lib/ssl/tls13con.c
+++ b/lib/ssl/tls13con.c
@@ -1726,28 +1726,20 @@ tls13_HandleClientHelloPart2(sslSocket *
     }
 
     ss->ssl3.hs.endOfFlight = PR_TRUE;
 
     if (ssl3_ExtensionNegotiated(ss, ssl_tls13_early_data_xtn)) {
         ss->ssl3.hs.zeroRttState = ssl_0rtt_sent;
     }
 
-#ifndef PARANOID
-    /* Look for a matching cipher suite. */
-    if (ssl3_config_match_init(ss) == 0) { /* no ciphers are working/supported by PK11 */
-        FATAL_ERROR(ss, PORT_GetError(), internal_error);
-        goto loser;
-    }
-#endif
-
     /* Negotiate cipher suite. */
     rv = ssl3_NegotiateCipherSuite(ss, suites, PR_FALSE);
     if (rv != SECSuccess) {
-        FATAL_ERROR(ss, SSL_ERROR_NO_CYPHER_OVERLAP, handshake_failure);
+        FATAL_ERROR(ss, PORT_GetError(), handshake_failure);
         goto loser;
     }
 
     /* If we are going around again, then we should make sure that the cipher
      * suite selection doesn't change. That's a sign of client shennanigans. */
     if (ss->ssl3.hs.helloRetry) {
 
         /* Update sequence numbers before checking the cookie so that any alerts