Bug 1289455 - Obviate manual CERT_DestroyCertificate() calls in PSM. draft
authorCykesiopka <cykesiopka.bmo@gmail.com>
Fri, 05 Aug 2016 23:57:44 +0800
changeset 397352 428f39e7e8847dd17b5d7565ed9ae8de055be244
parent 397351 32b74662a9e38a53e7f054992e9739c08881833f
child 527426 64ecf42710e3fed7fd4287c78394f4f868be944b
push id25265
push usercykesiopka.bmo@gmail.com
push dateFri, 05 Aug 2016 17:30:07 +0000
bugs1289455
milestone51.0a1
Bug 1289455 - Obviate manual CERT_DestroyCertificate() calls in PSM. 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());