Bug 1041343: Use references instead of pointers for TrustLevel output parameters, r=cviecco
authorBrian Smith <brian@briansmith.org>
Sun, 20 Jul 2014 11:06:26 -0700
changeset 197204 a6147f19dc56aecfcce19a019d8f966db8a32492
parent 197203 68fe3e5c7181331a5bf13e865cb3c559bfe81400
child 197205 8da59dd9fc7fe1422d1bf26fa169f8089034a57e
push id27236
push useremorley@mozilla.com
push dateFri, 01 Aug 2014 15:52:48 +0000
treeherdermozilla-central@44e5072476b7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscviecco
bugs1041343
milestone34.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 1041343: Use references instead of pointers for TrustLevel output parameters, r=cviecco
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/pkixbuild.cpp
security/pkix/lib/pkixcheck.cpp
security/pkix/lib/pkixcheck.h
security/pkix/lib/pkixocsp.cpp
security/pkix/test/gtest/pkixbuild_tests.cpp
security/pkix/test/gtest/pkixcert_extension_tests.cpp
security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp
security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp
--- a/security/apps/AppTrustDomain.cpp
+++ b/security/apps/AppTrustDomain.cpp
@@ -132,22 +132,21 @@ AppTrustDomain::FindIssuer(const SECItem
 
   return Success;
 }
 
 Result
 AppTrustDomain::GetCertTrust(EndEntityOrCA endEntityOrCA,
                              const CertPolicyId& policy,
                              const SECItem& candidateCertDER,
-                     /*out*/ TrustLevel* trustLevel)
+                             /*out*/ TrustLevel& trustLevel)
 {
   MOZ_ASSERT(policy.IsAnyPolicy());
-  MOZ_ASSERT(trustLevel);
   MOZ_ASSERT(mTrustedRoot);
-  if (!trustLevel || !policy.IsAnyPolicy()) {
+  if (!policy.IsAnyPolicy()) {
     return Result::FATAL_ERROR_INVALID_ARGS;
   }
   if (!mTrustedRoot) {
     return Result::FATAL_ERROR_INVALID_STATE;
   }
 
   // Handle active distrust of the certificate.
 
@@ -171,28 +170,28 @@ AppTrustDomain::GetCertTrust(EndEntityOr
     // 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
                               ? CERTDB_TRUSTED_CA
                               : CERTDB_TRUSTED;
     if (((flags & (relevantTrustBit | CERTDB_TERMINAL_RECORD)))
             == CERTDB_TERMINAL_RECORD) {
-      *trustLevel = TrustLevel::ActivelyDistrusted;
+      trustLevel = TrustLevel::ActivelyDistrusted;
       return Success;
     }
   }
 
   // mTrustedRoot is the only trust anchor for this validation.
   if (CERT_CompareCerts(mTrustedRoot.get(), candidateCert.get())) {
-    *trustLevel = TrustLevel::TrustAnchor;
+    trustLevel = TrustLevel::TrustAnchor;
     return Success;
   }
 
-  *trustLevel = TrustLevel::InheritsTrust;
+  trustLevel = TrustLevel::InheritsTrust;
   return Success;
 }
 
 Result
 AppTrustDomain::VerifySignedData(const SignedDataWithSignature& signedData,
                                  const SECItem& subjectPublicKeyInfo)
 {
   return ::mozilla::pkix::VerifySignedData(signedData, subjectPublicKeyInfo,
--- a/security/apps/AppTrustDomain.h
+++ b/security/apps/AppTrustDomain.h
@@ -21,17 +21,17 @@ public:
 
   AppTrustDomain(ScopedCERTCertList&, void* pinArg);
 
   SECStatus SetTrustedRoot(AppTrustedRoot trustedRoot);
 
   virtual Result GetCertTrust(mozilla::pkix::EndEntityOrCA endEntityOrCA,
                               const mozilla::pkix::CertPolicyId& policy,
                               const SECItem& candidateCertDER,
-                              /*out*/ mozilla::pkix::TrustLevel* trustLevel)
+                              /*out*/ mozilla::pkix::TrustLevel& trustLevel)
                               MOZ_OVERRIDE;
   virtual Result FindIssuer(const SECItem& encodedIssuerName,
                             IssuerChecker& checker,
                             PRTime time) MOZ_OVERRIDE;
   virtual Result CheckRevocation(mozilla::pkix::EndEntityOrCA endEntityOrCA,
                                  const mozilla::pkix::CertID& certID, PRTime time,
                                  /*optional*/ const SECItem* stapledOCSPresponse,
                                  /*optional*/ const SECItem* aiaExtension);
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -138,23 +138,18 @@ NSSCertDBTrustDomain::FindIssuer(const S
 
   return Success;
 }
 
 Result
 NSSCertDBTrustDomain::GetCertTrust(EndEntityOrCA endEntityOrCA,
                                    const CertPolicyId& policy,
                                    const SECItem& candidateCertDER,
-                                   /*out*/ TrustLevel* trustLevel)
+                                   /*out*/ TrustLevel& trustLevel)
 {
-  PR_ASSERT(trustLevel);
-  if (!trustLevel) {
-    return Result::FATAL_ERROR_INVALID_ARGS;
-  }
-
 #ifdef MOZ_NO_EV_CERTS
   if (!policy.IsAnyPolicy()) {
     return Result::ERROR_POLICY_VALIDATION_FAILED;
   }
 #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
@@ -184,38 +179,38 @@ NSSCertDBTrustDomain::GetCertTrust(EndEn
     // 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 ? CERTDB_TRUSTED_CA
                                                : CERTDB_TRUSTED;
     if (((flags & (relevantTrustBit|CERTDB_TERMINAL_RECORD)))
             == CERTDB_TERMINAL_RECORD) {
-      *trustLevel = TrustLevel::ActivelyDistrusted;
+      trustLevel = TrustLevel::ActivelyDistrusted;
       return Success;
     }
 
     // For TRUST, we only use the CERTDB_TRUSTED_CA bit, because Gecko hasn't
     // 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;
+        trustLevel = TrustLevel::TrustAnchor;
         return Success;
       }
 #ifndef MOZ_NO_EV_CERTS
       if (CertIsAuthoritativeForEVPolicy(candidateCert.get(), policy)) {
-        *trustLevel = TrustLevel::TrustAnchor;
+        trustLevel = TrustLevel::TrustAnchor;
         return Success;
       }
 #endif
     }
   }
 
-  *trustLevel = TrustLevel::InheritsTrust;
+  trustLevel = TrustLevel::InheritsTrust;
   return Success;
 }
 
 Result
 NSSCertDBTrustDomain::VerifySignedData(const SignedDataWithSignature& signedData,
                                        const SECItem& subjectPublicKeyInfo)
 {
   return ::mozilla::pkix::VerifySignedData(signedData, subjectPublicKeyInfo,
--- a/security/certverifier/NSSCertDBTrustDomain.h
+++ b/security/certverifier/NSSCertDBTrustDomain.h
@@ -58,17 +58,17 @@ public:
 
   virtual Result FindIssuer(const SECItem& encodedIssuerName,
                             IssuerChecker& checker,
                             PRTime time) MOZ_OVERRIDE;
 
   virtual Result GetCertTrust(mozilla::pkix::EndEntityOrCA endEntityOrCA,
                               const mozilla::pkix::CertPolicyId& policy,
                               const SECItem& candidateCertDER,
-                              /*out*/ mozilla::pkix::TrustLevel* trustLevel)
+                              /*out*/ mozilla::pkix::TrustLevel& trustLevel)
                               MOZ_OVERRIDE;
 
   virtual Result CheckPublicKey(const SECItem& subjectPublicKeyInfo)
                                 MOZ_OVERRIDE;
 
   virtual Result VerifySignedData(
                    const mozilla::pkix::SignedDataWithSignature& signedData,
                    const SECItem& subjectPublicKeyInfo) MOZ_OVERRIDE;
--- a/security/pkix/include/pkix/pkixtypes.h
+++ b/security/pkix/include/pkix/pkixtypes.h
@@ -187,26 +187,26 @@ public:
   virtual ~TrustDomain() { }
 
   // Determine the level of trust in the given certificate for the given role.
   // This will be called for every certificate encountered during path
   // building.
   //
   // When policy.IsAnyPolicy(), then no policy-related checking should be done.
   // When !policy.IsAnyPolicy(), then GetCertTrust MUST NOT return with
-  // *trustLevel == TrustAnchor unless the given cert is considered a trust
+  // 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
+  // trustLevel == InheritsTrust instead of trustLevel == TrustAnchor
   // (assuming the candidate cert is not actively distrusted).
   virtual Result GetCertTrust(EndEntityOrCA endEntityOrCA,
                               const CertPolicyId& policy,
                               const SECItem& candidateCertDER,
-                              /*out*/ TrustLevel* trustLevel) = 0;
+                              /*out*/ TrustLevel& trustLevel) = 0;
 
   class IssuerChecker
   {
   public:
     // potentialIssuerDER is the complete DER encoding of the certificate to
     // be checked as a potential issuer.
     //
     // If additionalNameConstraints is not nullptr then it must point to an
--- a/security/pkix/lib/pkixbuild.cpp
+++ b/security/pkix/lib/pkixbuild.cpp
@@ -217,17 +217,17 @@ BuildForward(TrustDomain& trustDomain,
 
   TrustLevel trustLevel;
   // If this is an end-entity and not a trust anchor, we defer reporting
   // any error found here until after attempting to find a valid chain.
   // See the explanation of error prioritization in pkix.h.
   rv = CheckIssuerIndependentProperties(trustDomain, subject, time,
                                         requiredKeyUsageIfPresent,
                                         requiredEKUIfPresent, requiredPolicy,
-                                        subCACount, &trustLevel);
+                                        subCACount, trustLevel);
   Result deferredEndEntityError = Success;
   if (rv != Success) {
     if (subject.endEntityOrCA == EndEntityOrCA::MustBeEndEntity &&
         trustLevel != TrustLevel::TrustAnchor) {
       deferredEndEntityError = rv;
     } else {
       return rv;
     }
--- a/security/pkix/lib/pkixcheck.cpp
+++ b/security/pkix/lib/pkixcheck.cpp
@@ -621,39 +621,35 @@ CheckExtendedKeyUsage(EndEntityOrCA endE
 Result
 CheckIssuerIndependentProperties(TrustDomain& trustDomain,
                                  const BackCert& cert,
                                  PRTime time,
                                  KeyUsage requiredKeyUsageIfPresent,
                                  KeyPurposeId requiredEKUIfPresent,
                                  const CertPolicyId& requiredPolicy,
                                  unsigned int subCACount,
-                /*optional out*/ TrustLevel* trustLevelOut)
+                                 /*out*/ TrustLevel& trustLevel)
 {
   Result rv;
 
   const EndEntityOrCA endEntityOrCA = cert.endEntityOrCA;
 
-  TrustLevel trustLevel;
   rv = trustDomain.GetCertTrust(endEntityOrCA, requiredPolicy, cert.GetDER(),
-                                &trustLevel);
+                                trustLevel);
   if (rv != Success) {
     return rv;
   }
   if (trustLevel == TrustLevel::ActivelyDistrusted) {
     return Result::ERROR_UNTRUSTED_CERT;
   }
   if (trustLevel != TrustLevel::TrustAnchor &&
       trustLevel != TrustLevel::InheritsTrust) {
     // The TrustDomain returned a trust level that we weren't expecting.
     return Result::FATAL_ERROR_INVALID_STATE;
   }
-  if (trustLevelOut) {
-    *trustLevelOut = trustLevel;
-  }
 
   // 4.2.1.1. Authority Key Identifier is ignored (see bug 965136).
 
   // 4.2.1.2. Subject Key Identifier is ignored (see bug 965136).
 
   // 4.2.1.3. Key Usage
   rv = CheckKeyUsage(endEntityOrCA, cert.GetKeyUsage(),
                      requiredKeyUsageIfPresent);
--- a/security/pkix/lib/pkixcheck.h
+++ b/security/pkix/lib/pkixcheck.h
@@ -34,17 +34,17 @@ namespace mozilla { namespace pkix {
 Result CheckIssuerIndependentProperties(
           TrustDomain& trustDomain,
           const BackCert& cert,
           PRTime time,
           KeyUsage requiredKeyUsageIfPresent,
           KeyPurposeId requiredEKUIfPresent,
           const CertPolicyId& requiredPolicy,
           unsigned int subCACount,
-          /*optional out*/ TrustLevel* trustLevel = nullptr);
+          /*out*/ TrustLevel& trustLevel);
 
 Result CheckNameConstraints(const SECItem& encodedNameConstraints,
                             const BackCert& firstChild,
                             KeyPurposeId requiredEKUIfPresent);
 
 } } // namespace mozilla::pkix
 
 #endif // mozilla_pkix__pkixcheck_h
--- a/security/pkix/lib/pkixocsp.cpp
+++ b/security/pkix/lib/pkixocsp.cpp
@@ -108,20 +108,22 @@ CheckOCSPResponseSignerCert(TrustDomain&
   //
   // Note that CheckIssuerIndependentProperties processes
   // SEC_OID_OCSP_RESPONDER in the way that the OCSP specification requires us
   // to--in particular, it doesn't allow SEC_OID_OCSP_RESPONDER to be implied
   // by a missing EKU extension, unlike other EKUs.
   //
   // TODO(bug 926261): If we're validating for a policy then the policy OID we
   // are validating for should be passed to CheckIssuerIndependentProperties.
+  TrustLevel unusedTrustLevel;
   rv = CheckIssuerIndependentProperties(trustDomain, potentialSigner, time,
                                         KeyUsage::noParticularKeyUsageRequired,
                                         KeyPurposeId::id_kp_OCSPSigning,
-                                        CertPolicyId::anyPolicy, 0);
+                                        CertPolicyId::anyPolicy, 0,
+                                        unusedTrustLevel);
   if (rv != Success) {
     return rv;
   }
 
   // It is possible that there exists a certificate with the same key as the
   // issuer but with a different name, so we need to compare names
   // XXX(bug 926270) XXX(bug 1008133) XXX(bug 980163): Improve name
   // comparison.
--- a/security/pkix/test/gtest/pkixbuild_tests.cpp
+++ b/security/pkix/test/gtest/pkixbuild_tests.cpp
@@ -112,22 +112,22 @@ public:
     }
 
     return true;
   }
 
 private:
   virtual Result GetCertTrust(EndEntityOrCA, const CertPolicyId&,
                               const SECItem& candidateCert,
-                              /*out*/ TrustLevel* trustLevel)
+                              /*out*/ TrustLevel& trustLevel)
   {
     if (SECITEM_ItemsAreEqual(&candidateCert, &certChainTail[0]->derCert)) {
-      *trustLevel = TrustLevel::TrustAnchor;
+      trustLevel = TrustLevel::TrustAnchor;
     } else {
-      *trustLevel = TrustLevel::InheritsTrust;
+      trustLevel = TrustLevel::InheritsTrust;
     }
     return Success;
   }
 
   virtual Result FindIssuer(const SECItem& encodedIssuerName,
                             IssuerChecker& checker, PRTime time)
   {
     ScopedCERTCertList
--- a/security/pkix/test/gtest/pkixcert_extension_tests.cpp
+++ b/security/pkix/test/gtest/pkixcert_extension_tests.cpp
@@ -72,19 +72,19 @@ CreateCert(PLArenaPool* arena, const cha
   return CreateCert(arena, subjectStr, extensions, subjectKey);
 }
 
 class TrustEverythingTrustDomain : public TrustDomain
 {
 private:
   virtual Result GetCertTrust(EndEntityOrCA, const CertPolicyId&,
                               const SECItem& candidateCert,
-                              /*out*/ TrustLevel* trustLevel)
+                              /*out*/ TrustLevel& trustLevel)
   {
-    *trustLevel = TrustLevel::TrustAnchor;
+    trustLevel = TrustLevel::TrustAnchor;
     return Success;
   }
 
   virtual Result FindIssuer(const SECItem& /*encodedIssuerName*/,
                             IssuerChecker& /*checker*/, PRTime /*time*/)
   {
     ADD_FAILURE();
     return Result::FATAL_ERROR_LIBRARY_FAILURE;
--- a/security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp
+++ b/security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp
@@ -31,17 +31,17 @@
 
 using namespace mozilla::pkix;
 using namespace mozilla::pkix::test;
 
 class CreateEncodedOCSPRequestTrustDomain : public TrustDomain
 {
 private:
   virtual Result GetCertTrust(EndEntityOrCA, const CertPolicyId&,
-                              const SECItem&, /*out*/ TrustLevel*)
+                              const SECItem&, /*out*/ TrustLevel&)
   {
     ADD_FAILURE();
     return Result::FATAL_ERROR_LIBRARY_FAILURE;
   }
 
   virtual Result FindIssuer(const SECItem&, IssuerChecker&, PRTime)
   {
     ADD_FAILURE();
--- a/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp
+++ b/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp
@@ -40,21 +40,20 @@ class OCSPTestTrustDomain : public Trust
 {
 public:
   OCSPTestTrustDomain()
   {
   }
 
   virtual Result GetCertTrust(EndEntityOrCA endEntityOrCA, const CertPolicyId&,
                               const SECItem& candidateCert,
-                              /*out*/ TrustLevel* trustLevel)
+                              /*out*/ TrustLevel& trustLevel)
   {
     EXPECT_EQ(endEntityOrCA, EndEntityOrCA::MustBeEndEntity);
-    EXPECT_TRUE(trustLevel);
-    *trustLevel = TrustLevel::InheritsTrust;
+    trustLevel = TrustLevel::InheritsTrust;
     return Success;
   }
 
   virtual Result FindIssuer(const SECItem&, IssuerChecker&, PRTime)
   {
     ADD_FAILURE();
     return Result::FATAL_ERROR_LIBRARY_FAILURE;
   }
@@ -833,23 +832,22 @@ public:
       this->certDER = certDER;
       this->certTrustLevel = certTrustLevel;
       return true;
     }
   private:
     virtual Result GetCertTrust(EndEntityOrCA endEntityOrCA,
                                 const CertPolicyId&,
                                 const SECItem& candidateCert,
-                                /*out*/ TrustLevel* trustLevel)
+                                /*out*/ TrustLevel& trustLevel)
     {
       EXPECT_EQ(endEntityOrCA, EndEntityOrCA::MustBeEndEntity);
-      EXPECT_TRUE(trustLevel);
       EXPECT_TRUE(certDER);
       EXPECT_TRUE(SECITEM_ItemsAreEqual(certDER, &candidateCert));
-      *trustLevel = certTrustLevel;
+      trustLevel = certTrustLevel;
       return Success;
     }
 
     const SECItem* certDER; // weak pointer
     TrustLevel certTrustLevel;
   };
 
   TrustDomain trustDomain;