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

net/base/x509_certificate.h

Index: net/base/x509_certificate.h
diff --git a/net/base/x509_certificate.h b/net/base/x509_certificate.h
index 1a8b23324e947fff9805b5626ea5ee9735120866..5cfa582398c60d764093b5e3b3e0e0cdd8c10e1d 100644
--- a/net/base/x509_certificate.h
+++ b/net/base/x509_certificate.h
@@ -27,8 +27,10 @@
 
 #include "base/synchronization/lock.h"
 #elif defined(USE_OPENSSL)
+#include <openssl/safestack.h>
 // Forward declaration; real one in <x509.h>
-struct x509_st;
+typedef struct x509_st X509;
+PREDECLARE_STACK_OF(X509);
 typedef struct x509_store_st X509_STORE;
 #elif defined(USE_NSS)
 // Forward declaration; real one in <cert.h>
@@ -55,20 +57,49 @@ typedef std::vector<scoped_refptr<X509Certificate> > CertificateList;
 class NET_EXPORT X509Certificate
     : public base::RefCountedThreadSafe<X509Certificate> {
  public:
-  // A handle to the certificate object in the underlying crypto library.
-  // We assume that OSCertHandle is a pointer type on all platforms and
-  // NULL is an invalid OSCertHandle.
+  // An OSCertHandle is a handle to the certificate object in the underlying
+  // crypto library. We assume that OSCertHandle is a pointer type on all
+  // platforms and that NULL represents an invalid OSCertHandle.
+  //
+  // An OSCertListHandle is a handle to the object in the underlying crypto
+  // library that represents a collection of certificates, with one of the
+  // certificates marked as an identity certificate and the remaining
+  // certificates marked as supplementary certificates for path building. Like
+  // OSCertHandle, it is assumed to be a pointer type on all platforms and
+  // that NULL represents an invalid OSCertListHandle.
+  //
+  // Depending on the underlying cryptographic library, an OSCertHandle or
+  // or OSCertListHandle may not be thread-safe. To avoid threading issues,
+  // each thread that is sharing an X509Certificate and needs access to an
+  // OSCertListHandle should use CreateOSCertListHandle() or only allow it to

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

In "only allow it to be used on a single thread", it's not
clear what "it" refers to.

The goal of this paragraph is to explain to people when they
can use os_cert_handle(), and when they must use CreateOSCertListHandle().
Please make sure the distinction between the two types is
addressed.  (I'll let you judge it.)

+  // be used on a single thread.
 #if defined(OS_WIN)
   typedef PCCERT_CONTEXT OSCertHandle;
+  // Though the same type as an OSCertHandle, a different PCCERT_CONTEXT is
+  // returned, beloning to a unique, temporary HCERTSTORE containing just the
+  // intermediate certificates.
+  typedef PCCERT_CONTEXT OSCertListHandle;
 #elif defined(OS_MACOSX)
   typedef SecCertificateRef OSCertHandle;
+  // Apple's certificate chain and identity functions use a CFArrayRef, with
+  // the first item in the array being the certificate that is to be
+  // verified/viewed/modified, and the remaining items containing optional
+  // additional certificates to use in path building or verification.
+  typedef CFArrayRef OSCertListHandle;
 #elif defined(USE_OPENSSL)
-  typedef struct x509_st* OSCertHandle;
+  typedef X509* OSCertHandle;
+  typedef STACK_OF(X509)* OSCertListHandle;
 #elif defined(USE_NSS)
   typedef struct CERTCertificateStr* OSCertHandle;
+  // Currently, because of how chain building/verification is used with NSS,
+  // it is not necessary to provide a separate type for the NSS native
+  // certificate chains (CERT_CertificateList or CERTCertList, depending on
+  // API).
+  typedef OSCertHandle OSCertListHandle;
 #else
   // TODO(ericroman): not implemented
   typedef void* OSCertHandle;
+  typedef OSCertHandle OSCertListHandle;
 #endif
 
   typedef std::vector<OSCertHandle> OSCertHandles;
@@ -254,6 +285,12 @@ class NET_EXPORT X509Certificate
   // Returns true if I already contain all the given intermediate certs.
   bool HasIntermediateCertificates(const OSCertHandles& certs);
 
+  // Returns a new OSCertListHandle representing the certificate and any
+  // associated intermediates certificates, or NULL on failure. Ownership is
+  // transferred to the caller and may be released by calling
+  // FreeOSCertListHandle() with the returned value.
+  OSCertListHandle CreateOSCertListHandle() const;
+
 #if defined(OS_MACOSX)
   // Does this certificate's usage allow SSL client authentication?
   bool SupportsSSLClientAuth() const;
@@ -368,6 +405,9 @@ class NET_EXPORT X509Certificate
   // Frees (or releases a reference to) an OS certificate handle.
   static void FreeOSCertHandle(OSCertHandle cert_handle);
 
+  // Frees (or releases a reference to) an OS certificate list handle.
+  static void FreeOSCertListHandle(OSCertListHandle cert_list_handle);
+
   // Calculates the SHA-1 fingerprint of the certificate.  Returns an empty
   // (all zero) fingerprint on failure.
   static SHA1Fingerprint CalculateFingerprint(OSCertHandle cert_handle);