bug 1058812 - (1/3) mozilla::pkix: add SignatureAlgorithm::unsupported_algorithm to better handle e.g. roots signed with RSA/MD5 r=briansmith
authorDavid Keeler <dkeeler@mozilla.com>
Tue, 07 Oct 2014 09:35:42 -0700
changeset 233997 18d10c64257d137c33282b7aaed5932c17692843
parent 233996 e36449d3325c34e20fd5cab71a08b3cced953bb7
child 233998 a4356f6fb7e79422f862bd6066dcc938cc588164
push id611
push userraliiev@mozilla.com
push dateMon, 05 Jan 2015 23:23:16 +0000
treeherdermozilla-release@345cd3b9c445 [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