Blob Blame History Raw
diff -up ./lib/ssl/ssl3con.c.alert-fix ./lib/ssl/ssl3con.c
--- ./lib/ssl/ssl3con.c.alert-fix	2021-06-10 05:33:12.000000000 -0700
+++ ./lib/ssl/ssl3con.c	2021-07-06 17:08:25.894018521 -0700
@@ -4319,7 +4319,11 @@ ssl_SignatureSchemeValid(SSLSignatureSch
     if (!ssl_IsSupportedSignatureScheme(scheme)) {
         return PR_FALSE;
     }
-    if (!ssl_SignatureSchemeMatchesSpkiOid(scheme, spkiOid)) {
+    /* if we are purposefully passed SEC_OID_UNKOWN, it means
+     * we not checking the scheme against a potential key, so skip
+     * the call */
+    if ((spkiOid != SEC_OID_UNKNOWN) &&
+        !ssl_SignatureSchemeMatchesSpkiOid(scheme, spkiOid)) {
         return PR_FALSE;
     }
     if (isTls13) {
@@ -4517,7 +4521,8 @@ ssl_CheckSignatureSchemeConsistency(sslS
     }
 
     /* Verify that the signature scheme matches the signing key. */
-    if (!ssl_SignatureSchemeValid(scheme, spkiOid, isTLS13)) {
+    if ((spkiOid == SEC_OID_UNKNOWN) || 
+         !ssl_SignatureSchemeValid(scheme, spkiOid, isTLS13)) {
         PORT_SetError(SSL_ERROR_INCORRECT_SIGNATURE_ALGORITHM);
         return SECFailure;
     }
@@ -4533,6 +4538,7 @@ ssl_CheckSignatureSchemeConsistency(sslS
 PRBool
 ssl_IsSupportedSignatureScheme(SSLSignatureScheme scheme)
 {
+    PRBool isSupported = PR_FALSE;
     switch (scheme) {
         case ssl_sig_rsa_pkcs1_sha1:
         case ssl_sig_rsa_pkcs1_sha256:
@@ -4552,7 +4558,8 @@ ssl_IsSupportedSignatureScheme(SSLSignat
         case ssl_sig_dsa_sha384:
         case ssl_sig_dsa_sha512:
         case ssl_sig_ecdsa_sha1:
-            return PR_TRUE;
+            isSupported = PR_TRUE;
+            break;
 
         case ssl_sig_rsa_pkcs1_sha1md5:
         case ssl_sig_none:
@@ -4560,7 +4567,19 @@ ssl_IsSupportedSignatureScheme(SSLSignat
         case ssl_sig_ed448:
             return PR_FALSE;
     }
-    return PR_FALSE;
+    if (isSupported) {
+        SECOidTag hashOID = ssl3_HashTypeToOID(ssl_SignatureSchemeToHashType(scheme));
+        PRUint32 policy;
+        const PRUint32 sigSchemePolicy=
+                NSS_USE_ALG_IN_SSL_KX|NSS_USE_ALG_IN_SIGNATURE;
+        /* check hash policy */
+        if ((NSS_GetAlgorithmPolicy(hashOID, &policy) == SECSuccess) &&
+            ((policy & sigSchemePolicy) != sigSchemePolicy)) {
+            return PR_FALSE;
+        }
+        /* check algorithm policy */
+    }
+    return isSupported;
 }
 
 PRBool
@@ -6533,6 +6552,9 @@ ssl_PickSignatureScheme(sslSocket *ss,
     }
 
     spkiOid = SECOID_GetAlgorithmTag(&cert->subjectPublicKeyInfo.algorithm);
+    if (spkiOid == SEC_OID_UNKNOWN) {
+        goto loser;
+    }
 
     /* Now we have to search based on the key type. Go through our preferred
      * schemes in order and find the first that can be used. */
@@ -6547,6 +6569,7 @@ ssl_PickSignatureScheme(sslSocket *ss,
         }
     }
 
+loser:
     PORT_SetError(SSL_ERROR_UNSUPPORTED_SIGNATURE_ALGORITHM);
     return SECFailure;
 }
@@ -7700,7 +7723,8 @@ ssl_ParseSignatureSchemes(const sslSocke
             PORT_SetError(SEC_ERROR_LIBRARY_FAILURE);
             return SECFailure;
         }
-        if (ssl_IsSupportedSignatureScheme((SSLSignatureScheme)tmp)) {
+        if (ssl_SignatureSchemeValid((SSLSignatureScheme)tmp, SEC_OID_UNKNOWN,
+            (PRBool)ss->version >= SSL_LIBRARY_VERSION_TLS_1_3)) {;
             schemes[numSupported++] = (SSLSignatureScheme)tmp;
         }
     }
@@ -10286,7 +10310,12 @@ ssl3_HandleCertificateVerify(sslSocket *
         PORT_Assert(ss->ssl3.hs.hashType == handshake_hash_record);
         rv = ssl_ConsumeSignatureScheme(ss, &b, &length, &sigScheme);
         if (rv != SECSuccess) {
-            goto loser; /* malformed or unsupported. */
+            errCode = PORT_GetError();
+            /* unsupported == illegal_parameter, others == handshake_failure. */
+            if (errCode  == SSL_ERROR_UNSUPPORTED_SIGNATURE_ALGORITHM) {
+                desc = illegal_parameter;
+            }
+            goto alert_loser;
         }
         rv = ssl_CheckSignatureSchemeConsistency(
             ss, sigScheme, &ss->sec.peerCert->subjectPublicKeyInfo);
diff -up ./gtests/ssl_gtest/ssl_extension_unittest.cc.alert-fix ./gtests/ssl_gtest/ssl_extension_unittest.cc
--- ./gtests/ssl_gtest/ssl_extension_unittest.cc.alert-fix	2021-07-07 11:32:11.634376932 -0700
+++ ./gtests/ssl_gtest/ssl_extension_unittest.cc	2021-07-07 11:33:30.595841110 -0700
@@ -428,7 +428,10 @@ TEST_P(TlsExtensionTest12Plus, Signature
 }
 
 TEST_P(TlsExtensionTest12Plus, SignatureAlgorithmsTrailingData) {
-  const uint8_t val[] = {0x00, 0x02, 0x04, 0x01, 0x00};  // sha-256, rsa
+  // make sure the test uses an algorithm that is legal for
+  // tls 1.3 (or tls 1.3 will through and illegalParameter
+  // instead of a decode error)
+  const uint8_t val[] = {0x00, 0x02, 0x08, 0x09, 0x00};  // sha-256, rsa-pss-pss
   DataBuffer extension(val, sizeof(val));
   ClientHelloErrorTest(std::make_shared<TlsExtensionReplacer>(
       client_, ssl_signature_algorithms_xtn, extension));