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 211530 a6147f19dc56aecfcce19a019d8f966db8a32492
parent 211529 68fe3e5c7181331a5bf13e865cb3c559bfe81400
child 211531 8da59dd9fc7fe1422d1bf26fa169f8089034a57e
push idunknown
push userunknown
push dateunknown
reviewerscviecco
bugs1041343
milestone34.0a1
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;