bug 1058812 - (1/3) mozilla::pkix: add SignatureAlgorithm::unsupported_algorithm to better handle e.g. roots signed with RSA/MD5 r=briansmith

security/pkix/include/pkix/pkixtypes.h

@@ -74,6 +74,9 @@ MOZILLA_PKIX_ENUM_CLASS SignatureAlgorithm
 
   // id-dsa-with-sha1 (OID 1.2.840.10040.4.3, RFC 3279 Section 2.2.2)
   dsa_with_sha1 = 18,
+
+  // Used to indicate any unsupported algorithm.
+  unsupported_algorithm = 19,
 };
 
 struct SignedDataWithSignature

security/pkix/lib/pkixbuild.cpp

@@ -181,8 +181,8 @@ PathBuildingStep::Check(Input potentialIssuerDER,
     return RecordResult(rv, keepGoing);
   }
 
-  rv = trustDomain.VerifySignedData(subject.GetSignedData(),
-                                    potentialIssuer.GetSubjectPublicKeyInfo());
+  rv = WrappedVerifySignedData(trustDomain, subject.GetSignedData(),
+                               potentialIssuer.GetSubjectPublicKeyInfo());
   if (rv != Success) {
     return RecordResult(rv, keepGoing);
   }

security/pkix/lib/pkixder.cpp

@@ -234,8 +234,7 @@ SignatureAlgorithmOIDValue(Reader& algorithmID,
     // XXX(bug 1042479): recognize this old OID for compatibility.
     algorithm = SignatureAlgorithm::rsa_pkcs1_with_sha1;
   } else {
-    // Any MD5-based signature algorithm, or any unknown signature algorithm.
-    return Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED;
+    algorithm = SignatureAlgorithm::unsupported_algorithm;
   }
 
   return Success;

security/pkix/lib/pkixnss.cpp

@@ -134,6 +134,7 @@ VerifySignedData(const SignedDataWithSignature& sd,
       pubKeyAlg = SEC_OID_ANSIX9_DSA_SIGNATURE;
       digestAlg = SEC_OID_SHA1;
       break;
+    case SignatureAlgorithm::unsupported_algorithm:
     default:
       PR_NOT_REACHED("unknown signature algorithm");
       return Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED;

security/pkix/lib/pkixocsp.cpp

@@ -130,8 +130,8 @@ CheckOCSPResponseSignerCert(TrustDomain& trustDomain,
   }
 
   // TODO(bug 926260): check name constraints
-  rv = trustDomain.VerifySignedData(potentialSigner.GetSignedData(),
-                                    issuerSubjectPublicKeyInfo);
+  rv = WrappedVerifySignedData(trustDomain, potentialSigner.GetSignedData(),
+                               issuerSubjectPublicKeyInfo);
 
   // TODO: check for revocation of the OCSP responder certificate unless no-check
   // or the caller forcing no-check. To properly support the no-check policy, we'd
@@ -207,7 +207,7 @@ VerifyOCSPSignedData(TrustDomain& trustDomain,
                      const SignedDataWithSignature& signedResponseData,
                      Input spki)
 {
-  Result rv = trustDomain.VerifySignedData(signedResponseData, spki);
+  Result rv = WrappedVerifySignedData(trustDomain, signedResponseData, spki);
   if (rv == Result::ERROR_BAD_SIGNATURE) {
     rv = Result::ERROR_OCSP_BAD_SIGNATURE;
   }

security/pkix/lib/pkixutil.h

@@ -193,6 +193,20 @@ DaysBeforeYear(unsigned int year)
        + ((year - 1u) / 400u); // except years divisible by 400.
 }
 
+// Ensures that we do not call the TrustDomain's VerifySignedData function if
+// the algorithm is unsupported.
+inline Result
+WrappedVerifySignedData(TrustDomain& trustDomain,
+                        const SignedDataWithSignature& signedData,
+                        Input subjectPublicKeyInfo)
+{
+  if (signedData.algorithm == SignatureAlgorithm::unsupported_algorithm) {
+    return Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED;
+  }
+
+  return trustDomain.VerifySignedData(signedData, subjectPublicKeyInfo);
+}
+
 } } // namespace mozilla::pkix
 
 #endif // mozilla_pkix__pkixutil_h