Bug 1289455 - Obviate manual CERT_DestroyCertificate() calls in PSM. r=dkeeler
authorCykesiopka <cykesiopka.bmo@gmail.com>
Fri, 05 Aug 2016 23:57:44 +0800
changeset 308920 7afd32fc3da6479f5a534ac4c19ba0dbbd2b425e
parent 308919 f30384ba6bc35ad5ef1812266f155e08dbfdd6d6
child 308921 f2ac6cfd273b6514f4e0f947e887fc3f3e984eef
push id30552
push userkwierso@gmail.com
push dateWed, 10 Aug 2016 23:15:29 +0000
treeherdermozilla-central@65520f4cf4cc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdkeeler
bugs1289455
milestone51.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 1289455 - Obviate manual CERT_DestroyCertificate() calls in PSM. r=dkeeler MozReview-Commit-ID: Aoi1VWvkNjp
security/certverifier/ExtendedValidation.cpp
security/certverifier/ExtendedValidation.h
security/certverifier/NSSCertDBTrustDomain.cpp
security/manager/ssl/nsPKCS12Blob.cpp
--- a/security/certverifier/ExtendedValidation.cpp
+++ b/security/certverifier/ExtendedValidation.cpp
@@ -25,17 +25,17 @@ struct nsMyTrustedEVInfo
 {
   const char* dotted_oid;
   const char* oid_name; // Set this to null to signal an invalid structure,
                   // (We can't have an empty list, so we'll use a dummy entry)
   SECOidTag oid_tag;
   const unsigned char ev_root_sha256_fingerprint[SHA256_LENGTH];
   const char* issuer_base64;
   const char* serial_base64;
-  CERTCertificate* cert;
+  mozilla::UniqueCERTCertificate cert;
 };
 
 // HOWTO enable additional CA root certificates for EV:
 //
 // For each combination of "root certificate" and "policy OID",
 // one entry must be added to the array named myTrustedEVInfos.
 //
 // We use the combination of "issuer name" and "serial number" to
@@ -1284,27 +1284,27 @@ isEVPolicy(SECOidTag policyOIDTag)
   }
 
   return false;
 }
 
 namespace mozilla { namespace psm {
 
 bool
-CertIsAuthoritativeForEVPolicy(const CERTCertificate* cert,
+CertIsAuthoritativeForEVPolicy(const UniqueCERTCertificate& cert,
                                const mozilla::pkix::CertPolicyId& policy)
 {
   PR_ASSERT(cert);
   if (!cert) {
     return false;
   }
 
   for (size_t iEV = 0; iEV < PR_ARRAY_SIZE(myTrustedEVInfos); ++iEV) {
     nsMyTrustedEVInfo& entry = myTrustedEVInfos[iEV];
-    if (entry.cert && CERT_CompareCerts(cert, entry.cert)) {
+    if (entry.cert && CERT_CompareCerts(cert.get(), entry.cert.get())) {
       const SECOidData* oidData = SECOID_FindOIDByTag(entry.oid_tag);
       if (oidData && oidData->oid.len == policy.numBytes &&
           !memcmp(oidData->oid.data, policy.bytes, policy.numBytes)) {
         return true;
       }
     }
   }
 
@@ -1330,17 +1330,17 @@ IdentityInfoInit()
     PR_ASSERT(rv == SECSuccess);
     if (rv != SECSuccess) {
       SECITEM_FreeItem(&ias.derIssuer, false);
       return PR_FAILURE;
     }
 
     ias.serialNumber.type = siUnsignedInteger;
 
-    entry.cert = CERT_FindCertByIssuerAndSN(nullptr, &ias);
+    entry.cert = UniqueCERTCertificate(CERT_FindCertByIssuerAndSN(nullptr, &ias));
 
     SECITEM_FreeItem(&ias.derIssuer, false);
     SECITEM_FreeItem(&ias.serialNumber, false);
 
     // If an entry is missing in the NSS root database, it may be because the
     // root database is out of sync with what we expect (e.g. a different
     // version of system NSS is installed). We assert on debug builds, but
     // silently continue on release builds. In both cases, the root cert does
@@ -1382,17 +1382,16 @@ IdentityInfoInit()
         }
       } else {
         PR_SetError(SEC_ERROR_BAD_DATA, 0);
         rv = SECFailure;
       }
     }
 
     if (rv != SECSuccess) {
-      CERT_DestroyCertificate(entry.cert);
       entry.cert = nullptr;
       entry.oid_tag = SEC_OID_UNKNOWN;
       return PR_FAILURE;
     }
   }
 
   return PR_SUCCESS;
 }
@@ -1405,20 +1404,17 @@ EnsureIdentityInfoLoaded()
   (void) PR_CallOnce(&sIdentityInfoCallOnce, IdentityInfoInit);
 }
 
 void
 CleanupIdentityInfo()
 {
   for (size_t iEV = 0; iEV < PR_ARRAY_SIZE(myTrustedEVInfos); ++iEV) {
     nsMyTrustedEVInfo &entry = myTrustedEVInfos[iEV];
-    if (entry.cert) {
-      CERT_DestroyCertificate(entry.cert);
-      entry.cert = nullptr;
-    }
+    entry.cert = nullptr;
   }
 
   memset(&sIdentityInfoCallOnce, 0, sizeof(PRCallOnceType));
 }
 
 // Find the first policy OID that is known to be an EV policy OID.
 SECStatus
 GetFirstEVPolicy(CERTCertificate* cert,
--- a/security/certverifier/ExtendedValidation.h
+++ b/security/certverifier/ExtendedValidation.h
@@ -1,31 +1,32 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#ifndef mozilla_psm_ExtendedValidation_h
-#define mozilla_psm_ExtendedValidation_h
+#ifndef ExtendedValidation_h
+#define ExtendedValidation_h
 
+#include "ScopedNSSTypes.h"
 #include "certt.h"
 #include "prtypes.h"
 
 namespace mozilla { namespace pkix { struct CertPolicyId; } }
 
 namespace mozilla { namespace psm {
 
 #ifndef MOZ_NO_EV_CERTS
 void EnsureIdentityInfoLoaded();
 void CleanupIdentityInfo();
 SECStatus GetFirstEVPolicy(CERTCertificate* cert,
                            /*out*/ mozilla::pkix::CertPolicyId& policy,
                            /*out*/ SECOidTag& policyOidTag);
 
 // CertIsAuthoritativeForEVPolicy does NOT evaluate whether the cert is trusted
 // or distrusted.
-bool CertIsAuthoritativeForEVPolicy(const CERTCertificate* cert,
+bool CertIsAuthoritativeForEVPolicy(const UniqueCERTCertificate& cert,
                                     const mozilla::pkix::CertPolicyId& policy);
 #endif
 
 } } // namespace mozilla::psm
 
-#endif // mozilla_psm_ExtendedValidation_h
+#endif // ExtendedValidation_h
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -241,17 +241,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 Success;
       }
 #ifndef MOZ_NO_EV_CERTS
-      if (CertIsAuthoritativeForEVPolicy(candidateCert.get(), policy)) {
+      if (CertIsAuthoritativeForEVPolicy(candidateCert, policy)) {
         trustLevel = TrustLevel::TrustAnchor;
         return Success;
       }
 #endif
     }
   }
 
   trustLevel = TrustLevel::InheritsTrust;
--- a/security/manager/ssl/nsPKCS12Blob.cpp
+++ b/security/manager/ssl/nsPKCS12Blob.cpp
@@ -645,22 +645,21 @@ nsPKCS12Blob::nickname_collision(SECItem
     // that nickname.  Keep updating the count until we find a nickname
     // without a corresponding cert.
     //  XXX If a user imports *many* certs without the 'friendly name'
     //      attribute, then this may take a long time.  :(
     nickname = nickFromPropC;
     if (count > 1) {
       nickname.AppendPrintf(" #%d", count);
     }
-    CERTCertificate *cert = CERT_FindCertByNickname(CERT_GetDefaultCertDB(),
-                                           const_cast<char*>(nickname.get()));
+    UniqueCERTCertificate cert(CERT_FindCertByNickname(CERT_GetDefaultCertDB(),
+                                                       nickname.get()));
     if (!cert) {
       break;
     }
-    CERT_DestroyCertificate(cert);
     count++;
   }
   SECItem *newNick = new SECItem;
   if (!newNick)
     return nullptr;
 
   newNick->type = siAsciiString;
   newNick->data = (unsigned char*) strdup(nickname.get());