bug 1058812 - (1/3) mozilla::pkix: add SignatureAlgorithm::unsupported_algorithm to better handle e.g. roots signed with RSA/MD5 r=briansmith
☠☠ backed out by 7fd338bf0e12 ☠ ☠
authorDavid Keeler <dkeeler@mozilla.com>
Tue, 07 Oct 2014 09:35:42 -0700
changeset 209373 9815045d0c5ac5c9b8a69fdc1b1c974590a0ec19
parent 209372 71e05880cbb000beab99cee987147766bc82b915
child 209374 9692998f547efd8fdfa5707d6424e37e8a06eac7
push id50163
push userdkeeler@mozilla.com
push dateWed, 08 Oct 2014 16:58:47 +0000
treeherdermozilla-inbound@0097b4ffaf33 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbriansmith
bugs1058812
milestone35.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
security/pkix/lib/pkixbuild.cpp
security/pkix/lib/pkixder.cpp
security/pkix/lib/pkixnss.cpp
security/pkix/lib/pkixocsp.cpp
security/pkix/lib/pkixutil.h
--- a/security/pkix/include/pkix/pkixtypes.h
+++ b/security/pkix/include/pkix/pkixtypes.h
@@ -69,16 +69,19 @@ MOZILLA_PKIX_ENUM_CLASS SignatureAlgorit
   // sha-1WithRSAEncryption (OID 1.2.840.113549.1.1.5, RFC 3279 Section 2.2.1)
   rsa_pkcs1_with_sha1 = 16,
 
   // id-dsa-with-sha256 (OID 2.16.840.1.101.3.4.3.2, RFC 5758 Section 3.1)
   dsa_with_sha256 = 17,
 
   // 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
 {
 public:
   Input data;
   SignatureAlgorithm algorithm;
   Input signature;
--- a/security/pkix/lib/pkixbuild.cpp
+++ b/security/pkix/lib/pkixbuild.cpp
@@ -176,18 +176,18 @@ PathBuildingStep::Check(Input potentialI
   // subject public key MUST NOT be used to verify signatures on certificates
   // or CRLs unless the corresponding keyCertSign or cRLSign bit is set."
   rv = BuildForward(trustDomain, potentialIssuer, time, KeyUsage::keyCertSign,
                     requiredEKUIfPresent, requiredPolicy, nullptr, subCACount);
   if (rv != Success) {
     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);
   }
 
   // We avoid doing revocation checking for expired certificates because OCSP
   // responders are allowed to forget about expired certificates, and many OCSP
   // responders return an error when asked for the status of an expired
   // certificate.
--- a/security/pkix/lib/pkixder.cpp
+++ b/security/pkix/lib/pkixder.cpp
@@ -229,18 +229,17 @@ SignatureAlgorithmOIDValue(Reader& algor
   } else if (algorithmID.MatchRest(id_dsa_with_sha1)) {
     algorithm = SignatureAlgorithm::dsa_with_sha1;
   } else if (algorithmID.MatchRest(id_dsa_with_sha256)) {
     algorithm = SignatureAlgorithm::dsa_with_sha256;
   } else if (algorithmID.MatchRest(sha1WithRSASignature)) {
     // 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;
 }
 
 template <typename OidValueParser, typename Algorithm>
 Result
 AlgorithmIdentifier(OidValueParser oidValueParser, Reader& input,
--- a/security/pkix/lib/pkixnss.cpp
+++ b/security/pkix/lib/pkixnss.cpp
@@ -129,16 +129,17 @@ VerifySignedData(const SignedDataWithSig
     case SignatureAlgorithm::dsa_with_sha256:
       pubKeyAlg = SEC_OID_ANSIX9_DSA_SIGNATURE;
       digestAlg = SEC_OID_SHA256;
       break;
     case SignatureAlgorithm::dsa_with_sha1:
       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;
   }
 
   Result rv;
   ScopedSECKeyPublicKey pubKey;
   rv = CheckPublicKeySize(subjectPublicKeyInfo, pubKey);
--- a/security/pkix/lib/pkixocsp.cpp
+++ b/security/pkix/lib/pkixocsp.cpp
@@ -125,18 +125,18 @@ CheckOCSPResponseSignerCert(TrustDomain&
   // XXX(bug 926270) XXX(bug 1008133) XXX(bug 980163): Improve name
   // comparison.
   // TODO: needs test
   if (!InputsAreEqual(potentialSigner.GetIssuer(), issuerSubject)) {
     return Result::ERROR_OCSP_RESPONDER_CERT_INVALID;
   }
 
   // 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
   // need to enforce policy constraints from the issuerChain.
 
   return rv;
 }
 
@@ -202,17 +202,17 @@ MatchResponderID(TrustDomain& trustDomai
   }
 }
 
 static Result
 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;
   }
   return rv;
 }
 
 // RFC 6960 section 4.2.2.2: The OCSP responder must either be the issuer of
 // the cert or it must be a delegated OCSP response signing cert directly
--- a/security/pkix/lib/pkixutil.h
+++ b/security/pkix/lib/pkixutil.h
@@ -188,11 +188,25 @@ DaysBeforeYear(unsigned int year)
 {
   assert(year <= 9999);
   return ((year - 1u) * 365u)
        + ((year - 1u) / 4u)    // leap years are every 4 years,
        - ((year - 1u) / 100u)  // except years divisible by 100,
        + ((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