Ensure X509Certificate::OSCertHandles are safe to be used on both UI and IO threads on Win

net/base/x509_certificate_nss.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/base/x509_certificate.h"
 
 #include <cert.h>
 #include <cryptohi.h>
 #include <keyhi.h>
 #include <nss.h>
@@ skipping 746 matching lines @@
 void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
   dns_names->clear();
 
   // Compare with CERT_VerifyCertName().
   GetCertSubjectAltNamesOfType(cert_handle_, certDNSName, dns_names);
 
   if (dns_names->empty())
     dns_names->push_back(subject_.common_name);
 }
 
+X509Certificate::OSCertListHandle
+X509Certificate::CreateOSCertListHandle() const {
+  return CERT_DupCertificate(cert_handle_);
+}

[wtc, 2011/10/04 00:26:34]

Change VerifyInternal to use CreateOSCertListHandle().

[Ryan Sleevi, 2011/10/04 03:38:07]

I don't think this is desirable for NSS, since then it would have to be scoped
to un-dup.

[wtc, 2011/10/04 18:00:51]

The reason I suggested having VerifyInternal use
CreateOSCertListHandle on all platforms is that the
consistency makes it easier to explain when one should
use os_cert_handle() and when one should use CreateOSCertListHandle().

With your proposal, we will need to explain why
CreateOSCertListHandle() is necessary only for Mac (which
needs an array of SecCertificateRef) and Windows (for
thread safety of certificate store enumeration).  It still
works, but it makes it harder for a future maintainer to
understand this code.

+
 int X509Certificate::VerifyInternal(const std::string& hostname,
                                     int flags,
                                     CertVerifyResult* verify_result) const {
   // Make sure that the hostname matches with the common name of the cert.
   SECStatus status = CERT_VerifyCertName(cert_handle_, hostname.c_str());
   if (status != SECSuccess)
     verify_result->cert_status |= CERT_STATUS_COMMON_NAME_INVALID;
 
   // Make sure that the cert is valid now.
   SECCertTimeValidity validity = CERT_CheckCertValidTimes(
@@ skipping 189 matching lines @@
     OSCertHandle cert_handle) {
   return CERT_DupCertificate(cert_handle);
 }
 
 // static
 void X509Certificate::FreeOSCertHandle(OSCertHandle cert_handle) {
   CERT_DestroyCertificate(cert_handle);
 }
 
 // static
+void X509Certificate::FreeOSCertListHandle(OSCertListHandle cert_list) {

[wtc, 2011/10/04 00:26:34]

cert_list => cert_list_handle

+  CERT_DestroyCertificate(cert_list);
+}
+
+// static
 SHA1Fingerprint X509Certificate::CalculateFingerprint(
     OSCertHandle cert) {
   SHA1Fingerprint sha1;
   memset(sha1.data, 0, sizeof(sha1.data));
 
   DCHECK(NULL != cert->derCert.data);
   DCHECK_NE(0U, cert->derCert.len);
 
   SECStatus rv = HASH_HashBuf(HASH_AlgSHA1, sha1.data,
                               cert->derCert.data, cert->derCert.len);
@@ skipping 16 matching lines @@
 
 // static
 bool X509Certificate::WriteOSCertHandleToPickle(OSCertHandle cert_handle,
                                                 Pickle* pickle) {
   return pickle->WriteData(
       reinterpret_cast<const char*>(cert_handle->derCert.data),
       cert_handle->derCert.len);
 }
 
 }  // namespace net