summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--chrome/browser/ui/cocoa/certificate_viewer.mm25
-rw-r--r--chrome/browser/ui/views/certificate_viewer_win.cc15
-rw-r--r--net/base/scoped_cert_chain_context.h30
-rw-r--r--net/base/x509_certificate.h68
-rw-r--r--net/base/x509_certificate_mac.cc23
-rw-r--r--net/base/x509_certificate_win.cc80
-rw-r--r--net/net.gyp1
7 files changed, 163 insertions, 79 deletions
diff --git a/chrome/browser/ui/cocoa/certificate_viewer.mm b/chrome/browser/ui/cocoa/certificate_viewer.mm
index 3b6a656..8fbe2af 100644
--- a/chrome/browser/ui/cocoa/certificate_viewer.mm
+++ b/chrome/browser/ui/cocoa/certificate_viewer.mm
@@ -9,30 +9,15 @@
#include <vector>
-#include "base/logging.h"
+#include "base/mac/foundation_util.h"
#include "base/mac/scoped_cftyperef.h"
#include "net/base/x509_certificate.h"
void ShowCertificateViewer(gfx::NativeWindow parent,
net::X509Certificate* cert) {
- SecCertificateRef cert_mac = cert->os_cert_handle();
- if (!cert_mac)
- return;
-
- base::mac::ScopedCFTypeRef<CFMutableArrayRef> certificates(
- CFArrayCreateMutable(kCFAllocatorDefault, 0, &kCFTypeArrayCallBacks));
- if (!certificates.get()) {
- NOTREACHED();
- return;
- }
- CFArrayAppendValue(certificates, cert_mac);
-
- // Server certificate must be first in the array; subsequent certificates
- // in the chain can be in any order.
- const std::vector<SecCertificateRef>& ca_certs =
- cert->GetIntermediateCertificates();
- for (size_t i = 0; i < ca_certs.size(); ++i)
- CFArrayAppendValue(certificates, ca_certs[i]);
+ base::mac::ScopedCFTypeRef<CFArrayRef> cert_chain(
+ cert->CreateOSCertChainForCert());
+ NSArray* certificates = base::mac::CFToNSCast(cert_chain.get());
// Explicitly disable revocation checking, regardless of user preferences
// or system settings. The behaviour of SFCertificatePanel is to call
@@ -77,7 +62,7 @@ void ShowCertificateViewer(gfx::NativeWindow parent,
modalDelegate:nil
didEndSelector:NULL
contextInfo:NULL
- certificates:reinterpret_cast<NSArray*>(certificates.get())
+ certificates:certificates
showGroup:YES];
// The SFCertificatePanel releases itself when the sheet is dismissed.
}
diff --git a/chrome/browser/ui/views/certificate_viewer_win.cc b/chrome/browser/ui/views/certificate_viewer_win.cc
index ba54267..9e5007c 100644
--- a/chrome/browser/ui/views/certificate_viewer_win.cc
+++ b/chrome/browser/ui/views/certificate_viewer_win.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// 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.
@@ -8,10 +8,16 @@
#include <cryptuiapi.h>
#pragma comment(lib, "cryptui.lib")
+#include "base/logging.h"
#include "net/base/x509_certificate.h"
void ShowCertificateViewer(gfx::NativeWindow parent,
net::X509Certificate* cert) {
+ // Create a new cert context and store containing just the certificate
+ // and its intermediate certificates.
+ PCCERT_CONTEXT cert_list = cert->CreateOSCertChainForCert();
+ CHECK(cert_list);
+
CRYPTUI_VIEWCERTIFICATE_STRUCT view_info = { 0 };
view_info.dwSize = sizeof(view_info);
// We set our parent to the tab window. This makes the cert dialog created
@@ -19,9 +25,8 @@ void ShowCertificateViewer(gfx::NativeWindow parent,
view_info.hwndParent = parent;
view_info.dwFlags = CRYPTUI_DISABLE_EDITPROPERTIES |
CRYPTUI_DISABLE_ADDTOSTORE;
- view_info.pCertContext = cert->os_cert_handle();
- // Search the cert store that 'cert' is in when building the cert chain.
- HCERTSTORE cert_store = view_info.pCertContext->hCertStore;
+ view_info.pCertContext = cert_list;
+ HCERTSTORE cert_store = cert_list->hCertStore;
view_info.cStores = 1;
view_info.rghStores = &cert_store;
BOOL properties_changed;
@@ -29,4 +34,6 @@ void ShowCertificateViewer(gfx::NativeWindow parent,
// This next call blocks but keeps processing windows messages, making it
// modal to the browser window.
BOOL rv = ::CryptUIDlgViewCertificate(&view_info, &properties_changed);
+
+ CertFreeCertificateContext(cert_list);
}
diff --git a/net/base/scoped_cert_chain_context.h b/net/base/scoped_cert_chain_context.h
deleted file mode 100644
index 668e46e..0000000
--- a/net/base/scoped_cert_chain_context.h
+++ /dev/null
@@ -1,30 +0,0 @@
-// 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.
-
-#ifndef NET_BASE_SCOPED_CERT_CHAIN_CONTEXT_H_
-#define NET_BASE_SCOPED_CERT_CHAIN_CONTEXT_H_
-#pragma once
-
-#include <windows.h>
-#include <wincrypt.h>
-
-#include "base/memory/scoped_ptr.h"
-
-namespace net {
-
-// This class wraps the CertFreeCertificateChain function in a class that can
-// be passed as a template argument to scoped_ptr_malloc.
-class ScopedPtrMallocFreeCertChain {
- public:
- void operator()(const CERT_CHAIN_CONTEXT* x) const {
- CertFreeCertificateChain(x);
- }
-};
-
-typedef scoped_ptr_malloc<const CERT_CHAIN_CONTEXT,
- ScopedPtrMallocFreeCertChain> ScopedCertChainContext;
-
-} // namespace net
-
-#endif // NET_BASE_SCOPED_CERT_CHAIN_CONTEXT_H_
diff --git a/net/base/x509_certificate.h b/net/base/x509_certificate.h
index 8a16088..d862110 100644
--- a/net/base/x509_certificate.h
+++ b/net/base/x509_certificate.h
@@ -28,7 +28,7 @@
#include "base/synchronization/lock.h"
#elif defined(USE_OPENSSL)
// Forward declaration; real one in <x509.h>
-struct x509_st;
+typedef struct x509_st X509;
typedef struct x509_store_st X509_STORE;
#elif defined(USE_NSS)
// Forward declaration; real one in <cert.h>
@@ -56,15 +56,15 @@ 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 a 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.
#if defined(OS_WIN)
typedef PCCERT_CONTEXT OSCertHandle;
#elif defined(OS_MACOSX)
typedef SecCertificateRef OSCertHandle;
#elif defined(USE_OPENSSL)
- typedef struct x509_st* OSCertHandle;
+ typedef X509* OSCertHandle;
#elif defined(USE_NSS)
typedef struct CERTCertificateStr* OSCertHandle;
#else
@@ -288,6 +288,13 @@ class NET_EXPORT X509Certificate
// Creates the chain of certs to use for this client identity cert.
CFArrayRef CreateClientCertificateChain() const;
+
+ // Returns a new CFArrayRef containing this certificate and its intermediate
+ // certificates in the form expected by Security.framework and Keychain
+ // Services, or NULL on failure.
+ // The first item in the array will be this certificate, followed by its
+ // intermediates, if any.
+ CFArrayRef CreateOSCertChainForCert() const;
#endif
#if defined(OS_WIN)
@@ -299,6 +306,52 @@ class NET_EXPORT X509Certificate
// this store so that we can close the system store when we finish
// searching for client certificates.
static HCERTSTORE cert_store();
+
+ // Returns a new PCCERT_CONTEXT containing this certificate and its
+ // intermediate certificates, or NULL on failure. The returned
+ // PCCERT_CONTEXT *MUST NOT* be stored in an X509Certificate, as then
+ // os_cert_handle() will not return the correct result. This function is
+ // only necessary if the CERT_CONTEXT.hCertStore member will be accessed or
+ // enumerated, which is generally true for any CryptoAPI functions involving
+ // certificate chains, including validation or certificate display.
+ //
+ // Remarks:
+ // Depending on the CryptoAPI function, Windows may need to access the
+ // HCERTSTORE that the passed-in PCCERT_CONTEXT belongs to, such as to
+ // locate additional intermediates. However, in the current X509Certificate
+ // implementation on Windows, all X509Certificate::OSCertHandles belong to
+ // the same HCERTSTORE - X509Certificate::cert_store(). Since certificates
+ // may be created and accessed on any number of threads, if CryptoAPI is
+ // trying to read this global store while additional certificates are being
+ // added, it may return inconsistent results while enumerating the store.
+ // While the memory accesses themselves are thread-safe, the resultant view
+ // of what is in the store may be altered.
+ //
+ // If OSCertHandles were instead added to a NULL HCERTSTORE, which is valid
+ // in CryptoAPI, then Windows would be unable to locate any of the
+ // intermediates supplied in |intermediate_ca_certs_|, because the
+ // hCertStore will refer to a magic value that indicates "only this
+ // certificate."
+ //
+ // To avoid these problems, a new in-memory HCERTSTORE is created containing
+ // just this certificate and its intermediates. The handle to the version of
+ // this certificate in the new HCERTSTORE is then returned, with the
+ // HCERTSTORE set to be automatically freed when the returned certificate
+ // is freed.
+ //
+ // This function is only needed when the HCERTSTORE of the os_cert_handle()
+ // will be accessed, which is generally only during certificate validation
+ // or display. While the returned PCCERT_CONTEXT and its HCERTSTORE can
+ // safely be used on multiple threads if no further modifications happen, it
+ // is generally preferable for each thread that needs such a context to
+ // obtain its own, rather than risk thread-safety issues by sharing.
+ //
+ // Because of how X509Certificate caching is implemented, attempting to
+ // create an X509Certificate from the returned PCCERT_CONTEXT may result in
+ // the original handle (and thus the originall HCERTSTORE) being returned by
+ // os_cert_handle(). For this reason, the returned PCCERT_CONTEXT *MUST NOT*
+ // be stored in an X509Certificate.
+ PCCERT_CONTEXT CreateOSCertChainForCert() const;
#endif
#if defined(OS_ANDROID)
@@ -345,6 +398,11 @@ class NET_EXPORT X509Certificate
// The content of the DER encoded certificate is written to |encoded|.
bool GetDEREncoded(std::string* encoded);
+ // Returns the OSCertHandle of this object. Because of caching, this may
+ // differ from the OSCertHandle originally supplied during initialization.
+ // Note: On Windows, CryptoAPI may return unexpected results if this handle
+ // is used across multiple threads. For more details, see
+ // CreateOSCertChainForCert().
OSCertHandle os_cert_handle() const { return cert_handle_; }
// Returns true if two OSCertHandles refer to identical certificates.
diff --git a/net/base/x509_certificate_mac.cc b/net/base/x509_certificate_mac.cc
index 3adaed4..61dbbe2 100644
--- a/net/base/x509_certificate_mac.cc
+++ b/net/base/x509_certificate_mac.cc
@@ -747,14 +747,7 @@ int X509Certificate::VerifyInternal(const std::string& hostname,
// array of certificates, the first of which is the certificate we're
// verifying, and the subsequent (optional) certificates are used for
// chain building.
- CFMutableArrayRef cert_array = CFArrayCreateMutable(kCFAllocatorDefault, 0,
- &kCFTypeArrayCallBacks);
- if (!cert_array)
- return ERR_OUT_OF_MEMORY;
- ScopedCFTypeRef<CFArrayRef> scoped_cert_array(cert_array);
- CFArrayAppendValue(cert_array, cert_handle_);
- for (size_t i = 0; i < intermediate_ca_certs_.size(); ++i)
- CFArrayAppendValue(cert_array, intermediate_ca_certs_[i]);
+ ScopedCFTypeRef<CFArrayRef> cert_array(CreateOSCertChainForCert());
// From here on, only one thread can be active at a time. We have had a number
// of sporadic crashes in the SecTrustEvaluate call below, way down inside
@@ -1343,6 +1336,20 @@ CFArrayRef X509Certificate::CreateClientCertificateChain() const {
return chain.release();
}
+CFArrayRef X509Certificate::CreateOSCertChainForCert() const {
+ CFMutableArrayRef cert_list =
+ CFArrayCreateMutable(kCFAllocatorDefault, 0,
+ &kCFTypeArrayCallBacks);
+ if (!cert_list)
+ return NULL;
+
+ CFArrayAppendValue(cert_list, os_cert_handle());
+ for (size_t i = 0; i < intermediate_ca_certs_.size(); ++i)
+ CFArrayAppendValue(cert_list, intermediate_ca_certs_[i]);
+
+ return cert_list;
+}
+
// static
X509Certificate::OSCertHandle
X509Certificate::ReadOSCertHandleFromPickle(const Pickle& pickle,
diff --git a/net/base/x509_certificate_win.cc b/net/base/x509_certificate_win.cc
index 0c355b6..efa5e5c 100644
--- a/net/base/x509_certificate_win.cc
+++ b/net/base/x509_certificate_win.cc
@@ -9,6 +9,7 @@
#include "base/lazy_instance.h"
#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
#include "base/pickle.h"
#include "base/sha1.h"
#include "base/string_tokenizer.h"
@@ -21,7 +22,6 @@
#include "net/base/cert_verify_result.h"
#include "net/base/ev_root_ca_metadata.h"
#include "net/base/net_errors.h"
-#include "net/base/scoped_cert_chain_context.h"
#include "net/base/test_root_certs.h"
#include "net/base/x509_certificate_known_roots_win.h"
@@ -33,11 +33,6 @@ namespace net {
namespace {
-typedef crypto::ScopedCAPIHandle<
- HCERTSTORE,
- crypto::CAPIDestroyerWithFlags<HCERTSTORE,
- CertCloseStore, 0> > ScopedHCERTSTORE;
-
struct FreeChainEngineFunctor {
void operator()(HCERTCHAINENGINE engine) const {
if (engine)
@@ -45,9 +40,35 @@ struct FreeChainEngineFunctor {
}
};
+struct FreeCertContextFunctor {
+ void operator()(PCCERT_CONTEXT context) const {
+ if (context)
+ CertFreeCertificateContext(context);
+ }
+};
+
+struct FreeCertChainContextFunctor {
+ void operator()(PCCERT_CHAIN_CONTEXT chain_context) const {
+ if (chain_context)
+ CertFreeCertificateChain(chain_context);
+ }
+};
+
typedef crypto::ScopedCAPIHandle<HCERTCHAINENGINE, FreeChainEngineFunctor>
ScopedHCERTCHAINENGINE;
+typedef crypto::ScopedCAPIHandle<
+ HCERTSTORE,
+ crypto::CAPIDestroyerWithFlags<HCERTSTORE,
+ CertCloseStore, 0> > ScopedHCERTSTORE;
+
+typedef scoped_ptr_malloc<const CERT_CONTEXT,
+ FreeCertContextFunctor> ScopedPCCERT_CONTEXT;
+
+typedef scoped_ptr_malloc<const CERT_CHAIN_CONTEXT,
+ FreeCertChainContextFunctor>
+ ScopedPCCERT_CHAIN_CONTEXT;
+
//-----------------------------------------------------------------------------
// TODO(wtc): This is a copy of the MapSecurityError function in
@@ -695,6 +716,40 @@ HCERTSTORE X509Certificate::cert_store() {
return g_cert_store.Get().cert_store();
}
+PCCERT_CONTEXT X509Certificate::CreateOSCertChainForCert() const {
+ // Create an in-memory certificate store to hold this certificate and
+ // any intermediate certificates in |intermediate_ca_certs_|. The store
+ // will be referenced in the returned PCCERT_CONTEXT, and will not be freed
+ // until the PCCERT_CONTEXT is freed.
+ ScopedHCERTSTORE store(CertOpenStore(
+ CERT_STORE_PROV_MEMORY, 0, NULL,
+ CERT_STORE_DEFER_CLOSE_UNTIL_LAST_FREE_FLAG, NULL));
+ if (!store.get())
+ return NULL;
+
+ // NOTE: This preserves all of the properties of |os_cert_handle()| except
+ // for CERT_KEY_PROV_HANDLE_PROP_ID and CERT_KEY_CONTEXT_PROP_ID - the two
+ // properties that hold access to already-opened private keys. If a handle
+ // has already been unlocked (eg: PIN prompt), then the first time that the
+ // identity is used for client auth, it may prompt the user again.
+ PCCERT_CONTEXT primary_cert;
+ BOOL ok = CertAddCertificateContextToStore(store.get(), os_cert_handle(),
+ CERT_STORE_ADD_ALWAYS,
+ &primary_cert);
+ if (!ok || !primary_cert)
+ return NULL;
+
+ for (size_t i = 0; i < intermediate_ca_certs_.size(); ++i) {
+ CertAddCertificateContextToStore(store.get(), intermediate_ca_certs_[i],
+ CERT_STORE_ADD_ALWAYS, NULL);
+ }
+
+ // Note: |store| is explicitly not released, as the call to CertCloseStore()
+ // when |store| goes out of scope will not actually free the store. Instead,
+ // the store will be freed when |primary_cert| is freed.
+ return primary_cert;
+}
+
int X509Certificate::VerifyInternal(const std::string& hostname,
int flags,
CRLSet* crl_set,
@@ -762,21 +817,23 @@ int X509Certificate::VerifyInternal(const std::string& hostname,
if (TestRootCerts::HasInstance())
chain_engine.reset(TestRootCerts::GetInstance()->GetChainEngine());
+ ScopedPCCERT_CONTEXT cert_list(CreateOSCertChainForCert());
PCCERT_CHAIN_CONTEXT chain_context;
// IE passes a non-NULL pTime argument that specifies the current system
// time. IE passes CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT as the
// chain_flags argument.
if (!CertGetCertificateChain(
chain_engine,
- cert_handle_,
+ cert_list.get(),
NULL, // current system time
- cert_handle_->hCertStore,
+ cert_list->hCertStore,
&chain_para,
chain_flags,
NULL, // reserved
&chain_context)) {
return MapSecurityError(GetLastError());
}
+
if (chain_context->TrustStatus.dwErrorStatus &
CERT_TRUST_IS_NOT_VALID_FOR_USAGE) {
ev_policy_oid = NULL;
@@ -785,9 +842,9 @@ int X509Certificate::VerifyInternal(const std::string& hostname,
CertFreeCertificateChain(chain_context);
if (!CertGetCertificateChain(
chain_engine,
- cert_handle_,
+ cert_list.get(),
NULL, // current system time
- cert_handle_->hCertStore,
+ cert_list->hCertStore,
&chain_para,
chain_flags,
NULL, // reserved
@@ -795,7 +852,8 @@ int X509Certificate::VerifyInternal(const std::string& hostname,
return MapSecurityError(GetLastError());
}
}
- ScopedCertChainContext scoped_chain_context(chain_context);
+
+ ScopedPCCERT_CHAIN_CONTEXT scoped_chain_context(chain_context);
GetCertChainInfo(chain_context, verify_result);
verify_result->cert_status |= MapCertChainErrorStatusToCertStatus(
diff --git a/net/net.gyp b/net/net.gyp
index a6fd6ed..98446ce 100644
--- a/net/net.gyp
+++ b/net/net.gyp
@@ -195,7 +195,6 @@
'base/rand_callback.h',
'base/registry_controlled_domain.cc',
'base/registry_controlled_domain.h',
- 'base/scoped_cert_chain_context.h',
'base/sdch_filter.cc',
'base/sdch_filter.h',
'base/sdch_manager.cc',
generated by cgit v1.1 at 2025-02-14 23:21:05 +0000