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

9 files changed, 303 insertions, 78 deletions

diff --git a/gtests/ssl_gtest/ssl_auth_unittest.cc b/gtests/ssl_gtest/ssl_auth_unittest.cc
index f190484b9..c1a810d04 100644
--- a/gtests/ssl_gtest/ssl_auth_unittest.cc
+++ b/gtests/ssl_gtest/ssl_auth_unittest.cc
@@ -783,6 +783,7 @@ TEST_P(TlsConnectTls13, ClientAuthPkcs1SignatureScheme) {
                  1024);
 }
 
+// Client should refuse to connect without a usable signature scheme.
 TEST_P(TlsConnectTls13, ClientAuthPkcs1SignatureSchemeOnly) {
   static const SSLSignatureScheme kSignatureScheme[] = {
       ssl_sig_rsa_pkcs1_sha256};
@@ -790,7 +791,21 @@ TEST_P(TlsConnectTls13, ClientAuthPkcs1SignatureSchemeOnly) {
   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);
@@ -1433,6 +1448,109 @@ TEST_F(TlsAgentStreamTestServer, ConfigureCertRsaPss) {
                                        &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>
diff --git a/gtests/ssl_gtest/ssl_ciphersuite_unittest.cc b/gtests/ssl_gtest/ssl_ciphersuite_unittest.cc
index 175a2d132..7739fe76f 100644
--- a/gtests/ssl_gtest/ssl_ciphersuite_unittest.cc
+++ b/gtests/ssl_gtest/ssl_ciphersuite_unittest.cc
@@ -56,6 +56,9 @@ class TlsCipherSuiteTestBase : public TlsConnectTestBase {
 
     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_);
@@ -69,34 +72,47 @@ class TlsCipherSuiteTestBase : public TlsConnectTestBase {
     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_;
@@ -112,9 +128,11 @@ class TlsCipherSuiteTestBase : public TlsConnectTestBase {
           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);
@@ -192,6 +210,7 @@ class TlsCipherSuiteTestBase : public TlsConnectTestBase {
   SSLAuthType auth_type_;
   SSLKEAType kea_type_;
   SSLNamedGroup group_;
+  SSLNamedGroup cert_group_ = ssl_grp_none;
   SSLSignatureScheme sig_scheme_;
   SSLCipherSuiteInfo csinfo_;
 };
diff --git a/gtests/ssl_gtest/ssl_extension_unittest.cc b/gtests/ssl_gtest/ssl_extension_unittest.cc
index 92632ba87..d7f350c8c 100644
--- a/gtests/ssl_gtest/ssl_extension_unittest.cc
+++ b/gtests/ssl_gtest/ssl_extension_unittest.cc
@@ -652,7 +652,7 @@ TEST_P(TlsExtensionTest12, SignatureAlgorithmDisableDSA) {
       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.
diff --git a/gtests/ssl_gtest/ssl_fuzz_unittest.cc b/gtests/ssl_gtest/ssl_fuzz_unittest.cc
index bb78bc3d1..b222f15cb 100644
--- a/gtests/ssl_gtest/ssl_fuzz_unittest.cc
+++ b/gtests/ssl_gtest/ssl_fuzz_unittest.cc
@@ -252,4 +252,4 @@ INSTANTIATE_TEST_CASE_P(
     FuzzDatagram, TlsFuzzTest,
     ::testing::Combine(TlsConnectTestBase::kTlsVariantsDatagram,
                        TlsConnectTestBase::kTlsV11Plus));
-}
+}  // namespace nss_test
diff --git a/gtests/ssl_gtest/tls_esni_unittest.cc b/gtests/ssl_gtest/tls_esni_unittest.cc
index 84becf3a6..90dccc525 100644
--- a/gtests/ssl_gtest/tls_esni_unittest.cc
+++ b/gtests/ssl_gtest/tls_esni_unittest.cc
@@ -472,4 +472,4 @@ TEST_P(TlsConnectTls13, EsniButTLS12Server) {
   ASSERT_FALSE(SSLInt_ExtensionNegotiated(server_->ssl_fd(),
                                           ssl_tls13_encrypted_sni_xtn));
 }
-}
+}  // namespace nss_test
diff --git a/lib/ssl/ssl3con.c b/lib/ssl/ssl3con.c
index 4646c94d3..f58cff04e 100644
--- a/lib/ssl/ssl3con.c
+++ b/lib/ssl/ssl3con.c
@@ -737,7 +737,7 @@ ssl_KEAEnabled(const sslSocket *ss, SSLKEAType keaType)
 }
 
 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) {
@@ -757,8 +757,13 @@ ssl_HasCert(const sslSocket *ss, SSLAuthType authType)
          * 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)) {
@@ -767,7 +772,114 @@ ssl_HasCert(const sslSocket *ss, SSLAuthType authType)
         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;
 }
@@ -798,6 +910,9 @@ ssl3_config_match_init(sslSocket *ss)
     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++) {
@@ -820,10 +935,11 @@ ssl3_config_match_init(sslSocket *ss)
 
             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;
                 }
             }
@@ -887,7 +1003,7 @@ ssl3_config_match(const ssl3CipherSuiteCfg *suite, PRUint8 policy,
         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;
     }
 
@@ -4156,6 +4272,9 @@ ssl_SignatureSchemeValid(SSLSignatureScheme scheme, SECOidTag spkiOid,
         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;
@@ -4270,6 +4389,7 @@ ssl_SignatureSchemeFromSpki(const CERTSubjectPublicKeyInfo *spki,
     return SECSuccess;
 }
 
+/* Check that a signature scheme is enabled by configuration. */
 PRBool
 ssl_SignatureSchemeEnabled(const sslSocket *ss, SSLSignatureScheme scheme)
 {
@@ -8052,6 +8172,7 @@ ssl3_NegotiateCipherSuiteInner(sslSocket *ss, const SECItem *suites,
             }
         }
     }
+    PORT_SetError(SSL_ERROR_NO_CYPHER_OVERLAP);
     return SECFailure;
 }
 
@@ -8076,6 +8197,14 @@ ssl3_NegotiateCipherSuite(sslSocket *ss, const SECItem *suites,
     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;
@@ -8205,13 +8334,6 @@ ssl3_ServerCallSNICallback(sslSocket *ss)
                     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.
@@ -8845,9 +8967,7 @@ ssl3_HandleClientHelloPart2(sslSocket *ss,
     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. */
@@ -8858,18 +8978,18 @@ ssl3_HandleClientHelloPart2(sslSocket *ss,
             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) {
@@ -8887,21 +9007,12 @@ ssl3_HandleClientHelloPart2(sslSocket *ss,
                 }
             }
         } 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;
     }
 
@@ -9669,10 +9780,9 @@ ssl3_SendServerKeyExchange(sslSocket *ss)
 }
 
 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;
 
@@ -9681,33 +9791,13 @@ ssl3_EncodeSigAlgs(const sslSocket *ss, sslBuffer *buf)
         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;
         }
     }
@@ -9753,7 +9843,7 @@ ssl3_SendCertificateRequest(sslSocket *ss)
 
     length = 1 + certTypesLength + 2 + calen;
     if (isTLS12) {
-        rv = ssl3_EncodeSigAlgs(ss, &sigAlgsBuf);
+        rv = ssl3_EncodeSigAlgs(ss, ss->version, &sigAlgsBuf);
         if (rv != SECSuccess) {
             return rv;
         }
diff --git a/lib/ssl/ssl3exthandle.c b/lib/ssl/ssl3exthandle.c
index f4b50f8a7..206cb00e4 100644
--- a/lib/ssl/ssl3exthandle.c
+++ b/lib/ssl/ssl3exthandle.c
@@ -1636,13 +1636,18 @@ 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;
     }
diff --git a/lib/ssl/sslimpl.h b/lib/ssl/sslimpl.h
index e12b20112..a9ea7679e 100644
--- a/lib/ssl/sslimpl.h
+++ b/lib/ssl/sslimpl.h
@@ -1681,7 +1681,8 @@ 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,
diff --git a/lib/ssl/tls13con.c b/lib/ssl/tls13con.c
index ec7af24b2..a587e3890 100644
--- a/lib/ssl/tls13con.c
+++ b/lib/ssl/tls13con.c
@@ -1731,18 +1731,10 @@ tls13_HandleClientHelloPart2(sslSocket *ss,
         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;
     }