Bug 1019814: Remove CERTCertificate dependency from TrustDomain::GetCertTrust, r=keeler
authorBrian Smith <brian@briansmith.org>
Tue, 03 Jun 2014 10:47:25 -0700
changeset 206747 44be87ea2e1be101218c3cb66032f599c465610a
parent 206746 7b366010fbbcef0bc382fa6c78f5117734e012bc
child 206748 efbad7dfa8706b46226f5d05f23680aebbba5961
push id494
push userraliiev@mozilla.com
push dateMon, 25 Aug 2014 18:42:16 +0000
treeherdermozilla-release@a3cc3e46b571 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1019814
milestone32.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 1019814: Remove CERTCertificate dependency from TrustDomain::GetCertTrust, r=keeler
security/apps/AppTrustDomain.cpp
security/apps/AppTrustDomain.h
security/certverifier/NSSCertDBTrustDomain.cpp
security/certverifier/NSSCertDBTrustDomain.h
security/pkix/include/pkix/pkixtypes.h
security/pkix/lib/pkixcheck.cpp
security/pkix/lib/pkixutil.h
security/pkix/test/gtest/pkix_cert_chain_length_tests.cpp
--- a/security/apps/AppTrustDomain.cpp
+++ b/security/apps/AppTrustDomain.cpp
@@ -99,35 +99,46 @@ AppTrustDomain::FindPotentialIssuers(con
   results = CERT_CreateSubjectCertList(nullptr, CERT_GetDefaultCertDB(),
                                        encodedIssuerName, time, true);
   return SECSuccess;
 }
 
 SECStatus
 AppTrustDomain::GetCertTrust(EndEntityOrCA endEntityOrCA,
                              const CertPolicyId& policy,
-                             const CERTCertificate* candidateCert,
+                             const SECItem& candidateCertDER,
                      /*out*/ TrustLevel* trustLevel)
 {
   MOZ_ASSERT(policy.IsAnyPolicy());
-  MOZ_ASSERT(candidateCert);
   MOZ_ASSERT(trustLevel);
   MOZ_ASSERT(mTrustedRoot);
-  if (!candidateCert || !trustLevel || !policy.IsAnyPolicy()) {
+  if (!trustLevel || !policy.IsAnyPolicy()) {
     PR_SetError(SEC_ERROR_INVALID_ARGS, 0);
     return SECFailure;
   }
   if (!mTrustedRoot) {
     PR_SetError(PR_INVALID_STATE_ERROR, 0);
     return SECFailure;
   }
 
   // Handle active distrust of the certificate.
+
+  // XXX: This would be cleaner and more efficient if we could get the trust
+  // information without constructing a CERTCertificate here, but NSS doesn't
+  // expose it in any other easy-to-use fashion.
+  ScopedCERTCertificate candidateCert(
+    CERT_NewTempCertificate(CERT_GetDefaultCertDB(),
+                            const_cast<SECItem*>(&candidateCertDER), nullptr,
+                            false, true));
+  if (!candidateCert) {
+    return SECFailure;
+  }
+
   CERTCertTrust trust;
-  if (CERT_GetCertTrust(candidateCert, &trust) == SECSuccess) {
+  if (CERT_GetCertTrust(candidateCert.get(), &trust) == SECSuccess) {
     PRUint32 flags = SEC_GET_TRUST_FLAGS(&trust, trustObjectSigning);
 
     // For DISTRUST, we use the CERTDB_TRUSTED or CERTDB_TRUSTED_CA bit,
     // because we can have active distrust for either type of cert. Note that
     // CERTDB_TERMINAL_RECORD means "stop trying to inherit trust" so if the
     // relevant trust bit isn't set then that means the cert must be considered
     // distrusted.
     PRUint32 relevantTrustBit = endEntityOrCA == EndEntityOrCA::MustBeCA
@@ -136,17 +147,17 @@ AppTrustDomain::GetCertTrust(EndEntityOr
     if (((flags & (relevantTrustBit | CERTDB_TERMINAL_RECORD)))
             == CERTDB_TERMINAL_RECORD) {
       *trustLevel = TrustLevel::ActivelyDistrusted;
       return SECSuccess;
     }
   }
 
   // mTrustedRoot is the only trust anchor for this validation.
-  if (CERT_CompareCerts(mTrustedRoot.get(), candidateCert)) {
+  if (CERT_CompareCerts(mTrustedRoot.get(), candidateCert.get())) {
     *trustLevel = TrustLevel::TrustAnchor;
     return SECSuccess;
   }
 
   *trustLevel = TrustLevel::InheritsTrust;
   return SECSuccess;
 }
 
--- a/security/apps/AppTrustDomain.h
+++ b/security/apps/AppTrustDomain.h
@@ -17,17 +17,17 @@ class AppTrustDomain MOZ_FINAL : public 
 {
 public:
   AppTrustDomain(void* pinArg);
 
   SECStatus SetTrustedRoot(AppTrustedRoot trustedRoot);
 
   SECStatus GetCertTrust(mozilla::pkix::EndEntityOrCA endEntityOrCA,
                          const mozilla::pkix::CertPolicyId& policy,
-                         const CERTCertificate* candidateCert,
+                         const SECItem& candidateCertDER,
                  /*out*/ mozilla::pkix::TrustLevel* trustLevel) MOZ_OVERRIDE;
   SECStatus FindPotentialIssuers(const SECItem* encodedIssuerName,
                                  PRTime time,
                          /*out*/ mozilla::pkix::ScopedCERTCertList& results)
                                  MOZ_OVERRIDE;
   SECStatus VerifySignedData(const CERTSignedData* signedData,
                              const CERTCertificate* cert) MOZ_OVERRIDE;
   SECStatus CheckRevocation(mozilla::pkix::EndEntityOrCA endEntityOrCA,
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -67,41 +67,54 @@ NSSCertDBTrustDomain::FindPotentialIssue
   results = CERT_CreateSubjectCertList(nullptr, CERT_GetDefaultCertDB(),
                                        encodedIssuerName, time, true);
   return SECSuccess;
 }
 
 SECStatus
 NSSCertDBTrustDomain::GetCertTrust(EndEntityOrCA endEntityOrCA,
                                    const CertPolicyId& policy,
-                                   const CERTCertificate* candidateCert,
+                                   const SECItem& candidateCertDER,
                                    /*out*/ TrustLevel* trustLevel)
 {
-  PR_ASSERT(candidateCert);
   PR_ASSERT(trustLevel);
 
-  if (!candidateCert || !trustLevel) {
+  if (!trustLevel) {
     PR_SetError(SEC_ERROR_INVALID_ARGS, 0);
     return SECFailure;
   }
 
 #ifdef MOZ_NO_EV_CERTS
   if (!policy.IsAnyPolicy()) {
     PR_SetError(SEC_ERROR_POLICY_VALIDATION_FAILED, 0);
     return SECFailure;
   }
 #endif
 
+  // XXX: This would be cleaner and more efficient if we could get the trust
+  // information without constructing a CERTCertificate here, but NSS doesn't
+  // expose it in any other easy-to-use fashion. The use of
+  // CERT_NewTempCertificate to get a CERTCertificate shouldn't be a
+  // performance problem because NSS will just find the existing
+  // CERTCertificate in its in-memory cache and return it.
+  ScopedCERTCertificate candidateCert(
+    CERT_NewTempCertificate(CERT_GetDefaultCertDB(),
+                            const_cast<SECItem*>(&candidateCertDER), nullptr,
+                            false, true));
+  if (!candidateCert) {
+    return SECFailure;
+  }
+
   // XXX: CERT_GetCertTrust seems to be abusing SECStatus as a boolean, where
   // SECSuccess means that there is a trust record and SECFailure means there
   // is not a trust record. I looked at NSS's internal uses of
   // CERT_GetCertTrust, and all that code uses the result as a boolean meaning
   // "We have a trust record."
   CERTCertTrust trust;
-  if (CERT_GetCertTrust(candidateCert, &trust) == SECSuccess) {
+  if (CERT_GetCertTrust(candidateCert.get(), &trust) == SECSuccess) {
     PRUint32 flags = SEC_GET_TRUST_FLAGS(&trust, mCertDBTrustType);
 
     // For DISTRUST, we use the CERTDB_TRUSTED or CERTDB_TRUSTED_CA bit,
     // because we can have active distrust for either type of cert. Note that
     // CERTDB_TERMINAL_RECORD means "stop trying to inherit trust" so if the
     // relevant trust bit isn't set then that means the cert must be considered
     // distrusted.
     PRUint32 relevantTrustBit =
@@ -117,17 +130,17 @@ NSSCertDBTrustDomain::GetCertTrust(EndEn
     // needed to consider end-entity certs to be their own trust anchors since
     // Gecko implemented nsICertOverrideService.
     if (flags & CERTDB_TRUSTED_CA) {
       if (policy.IsAnyPolicy()) {
         *trustLevel = TrustLevel::TrustAnchor;
         return SECSuccess;
       }
 #ifndef MOZ_NO_EV_CERTS
-      if (CertIsAuthoritativeForEVPolicy(candidateCert, policy)) {
+      if (CertIsAuthoritativeForEVPolicy(candidateCert.get(), policy)) {
         *trustLevel = TrustLevel::TrustAnchor;
         return SECSuccess;
       }
 #endif
     }
   }
 
   *trustLevel = TrustLevel::InheritsTrust;
--- a/security/certverifier/NSSCertDBTrustDomain.h
+++ b/security/certverifier/NSSCertDBTrustDomain.h
@@ -63,17 +63,17 @@ public:
 
   virtual SECStatus FindPotentialIssuers(
                         const SECItem* encodedIssuerName,
                         PRTime time,
                 /*out*/ mozilla::pkix::ScopedCERTCertList& results);
 
   virtual SECStatus GetCertTrust(mozilla::pkix::EndEntityOrCA endEntityOrCA,
                                  const mozilla::pkix::CertPolicyId& policy,
-                                 const CERTCertificate* candidateCert,
+                                 const SECItem& candidateCertDER,
                          /*out*/ mozilla::pkix::TrustLevel* trustLevel);
 
   virtual SECStatus VerifySignedData(const CERTSignedData* signedData,
                                      const CERTCertificate* cert);
 
   virtual SECStatus CheckRevocation(mozilla::pkix::EndEntityOrCA endEntityOrCA,
                                     const CERTCertificate* cert,
                           /*const*/ CERTCertificate* issuerCert,
--- a/security/pkix/include/pkix/pkixtypes.h
+++ b/security/pkix/include/pkix/pkixtypes.h
@@ -90,17 +90,17 @@ public:
   // *trustLevel == TrustAnchor unless the given cert is considered a trust
   // anchor *for that policy*. In particular, if the user has marked an
   // intermediate certificate as trusted, but that intermediate isn't in the
   // list of EV roots, then GetCertTrust must result in
   // *trustLevel == InheritsTrust instead of *trustLevel == TrustAnchor
   // (assuming the candidate cert is not actively distrusted).
   virtual SECStatus GetCertTrust(EndEntityOrCA endEntityOrCA,
                                  const CertPolicyId& policy,
-                                 const CERTCertificate* candidateCert,
+                                 const SECItem& candidateCertDER,
                          /*out*/ TrustLevel* trustLevel) = 0;
 
   // Find all certificates (intermediate and/or root) in the certificate
   // database that have a subject name matching |encodedIssuerName| at
   // the given time. Certificates where the given time is not within the
   // certificate's validity period may be excluded. On input, |results|
   // will be null on input. If no potential issuers are found, then this
   // function should return SECSuccess with results being either null or
--- a/security/pkix/lib/pkixcheck.cpp
+++ b/security/pkix/lib/pkixcheck.cpp
@@ -553,20 +553,18 @@ CheckIssuerIndependentProperties(TrustDo
                                  KeyPurposeId requiredEKUIfPresent,
                                  const CertPolicyId& requiredPolicy,
                                  unsigned int subCACount,
                 /*optional out*/ TrustLevel* trustLevelOut)
 {
   Result rv;
 
   TrustLevel trustLevel;
-  rv = MapSECStatus(trustDomain.GetCertTrust(endEntityOrCA,
-                                             requiredPolicy,
-                                             cert.GetNSSCert(),
-                                             &trustLevel));
+  rv = MapSECStatus(trustDomain.GetCertTrust(endEntityOrCA, requiredPolicy,
+                                             cert.GetDER(), &trustLevel));
   if (rv != Success) {
     return rv;
   }
   if (trustLevel == TrustLevel::ActivelyDistrusted) {
     return Fail(RecoverableError, SEC_ERROR_UNTRUSTED_CERT);
   }
   if (trustLevel != TrustLevel::TrustAnchor &&
       trustLevel != TrustLevel::InheritsTrust) {
--- a/security/pkix/lib/pkixutil.h
+++ b/security/pkix/lib/pkixutil.h
@@ -106,16 +106,18 @@ public:
     , nssCert(nssCert)
     , constrainedNames(nullptr)
     , includeCN(includeCN)
   {
   }
 
   Result Init();
 
+  const SECItem& GetDER() const { return nssCert->derCert; }
+
   const SECItem* encodedBasicConstraints;
   const SECItem* encodedCertificatePolicies;
   const SECItem* encodedExtendedKeyUsage;
   const SECItem* encodedKeyUsage;
   const SECItem* encodedNameConstraints;
   const SECItem* encodedInhibitAnyPolicy;
 
   BackCert* const childCert;
--- a/security/pkix/test/gtest/pkix_cert_chain_length_tests.cpp
+++ b/security/pkix/test/gtest/pkix_cert_chain_length_tests.cpp
@@ -110,20 +110,20 @@ public:
     }
 
     return true;
   }
 
 private:
   SECStatus GetCertTrust(EndEntityOrCA,
                          const CertPolicyId&,
-                         const CERTCertificate* candidateCert,
+                         const SECItem& candidateCert,
                          /*out*/ TrustLevel* trustLevel)
   {
-    if (candidateCert == certChainTail[0].get()) {
+    if (SECITEM_ItemsAreEqual(&candidateCert, &certChainTail[0]->derCert)) {
       *trustLevel = TrustLevel::TrustAnchor;
     } else {
       *trustLevel = TrustLevel::InheritsTrust;
     }
     return SECSuccess;
   }
 
   SECStatus FindPotentialIssuers(const SECItem* encodedIssuerName,