diff options
author | wtc@chromium.org <wtc@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-12-09 19:35:54 +0000 |
---|---|---|
committer | wtc@chromium.org <wtc@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-12-09 19:35:54 +0000 |
commit | eef15aa68c1ead785af67f8e4aaf8fcdb7ed6c04 (patch) | |
tree | 63fbe28673d68bdf64a66c881ddad6c827dc8b8f | |
parent | 32a4eeb62d7eb1d4e8178eabbc5ffa3bd651d7cb (diff) | |
download | chromium_src-eef15aa68c1ead785af67f8e4aaf8fcdb7ed6c04.zip chromium_src-eef15aa68c1ead785af67f8e4aaf8fcdb7ed6c04.tar.gz chromium_src-eef15aa68c1ead785af67f8e4aaf8fcdb7ed6c04.tar.bz2 |
Define X509Certificate::intermediate_ca_certs_ as a std::vector of
OSCertHandle so that we can also use it on Windows.
Remove the unused SSLClientSocketMac::intermediate_certs_ member.
R=hawk
BUG=28744
TEST=Can visit good HTTPS sites with no certificate errors. Clicking
the "Certificate information" button in the page security information
window should show a complete certificate chain (as opposed to just
the server certificate).
Review URL: http://codereview.chromium.org/452042
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@34175 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/cocoa/page_info_window_mac.mm | 13 | ||||
-rw-r--r-- | net/base/x509_certificate.cc | 12 | ||||
-rw-r--r-- | net/base/x509_certificate.h | 30 | ||||
-rw-r--r-- | net/base/x509_certificate_mac.cc | 22 | ||||
-rw-r--r-- | net/socket/ssl_client_socket_mac.cc | 6 | ||||
-rw-r--r-- | net/socket/ssl_client_socket_mac.h | 1 |
6 files changed, 33 insertions, 51 deletions
diff --git a/chrome/browser/cocoa/page_info_window_mac.mm b/chrome/browser/cocoa/page_info_window_mac.mm index 910ce3f..73074ea 100644 --- a/chrome/browser/cocoa/page_info_window_mac.mm +++ b/chrome/browser/cocoa/page_info_window_mac.mm @@ -76,13 +76,12 @@ void PageInfoWindowMac::ShowCertDialog(int) { } CFArrayAppendValue(certificates, cert_mac); - CFArrayRef ca_certs = cert->GetIntermediateCertificates(); - if (ca_certs) { - // Server certificate must be first in the array; subsequent certificates - // in the chain can be in any order. - CFArrayAppendArray(certificates, ca_certs, - CFRangeMake(0, CFArrayGetCount(ca_certs))); - } + // 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]); [[SFCertificatePanel sharedCertificatePanel] beginSheetForWindow:[controller_ window] diff --git a/net/base/x509_certificate.cc b/net/base/x509_certificate.cc index e8db7c7..24388d8 100644 --- a/net/base/x509_certificate.cc +++ b/net/base/x509_certificate.cc @@ -170,9 +170,6 @@ X509Certificate* X509Certificate::CreateFromBytes(const char* data, X509Certificate::X509Certificate(OSCertHandle cert_handle, Source source) : cert_handle_(cert_handle), -#if defined(OS_MACOSX) - intermediate_ca_certs_(NULL), -#endif source_(source) { Initialize(); } @@ -186,9 +183,6 @@ X509Certificate::X509Certificate(const std::string& subject, valid_start_(start_date), valid_expiry_(expiration_date), cert_handle_(NULL), -#if defined(OS_MACOSX) - intermediate_ca_certs_(NULL), -#endif source_(SOURCE_UNUSED) { memset(fingerprint_.data, 0, sizeof(fingerprint_.data)); } @@ -198,9 +192,9 @@ X509Certificate::~X509Certificate() { X509Certificate::Cache::GetInstance()->Remove(this); if (cert_handle_) FreeOSCertHandle(cert_handle_); -#if defined(OS_MACOSX) - if (intermediate_ca_certs_) - CFRelease(intermediate_ca_certs_); +#if defined(OS_MACOSX) || defined(OS_WIN) + for (size_t i = 0; i < intermediate_ca_certs_.size(); ++i) + FreeOSCertHandle(intermediate_ca_certs_[i]); #endif } diff --git a/net/base/x509_certificate.h b/net/base/x509_certificate.h index 2628dbb..b83bd8a 100644 --- a/net/base/x509_certificate.h +++ b/net/base/x509_certificate.h @@ -209,15 +209,19 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> { // now. bool HasExpired() const; -#if defined(OS_MACOSX) - // Adds an untrusted intermediate certificate that may be needed for - // chain building. - void AddIntermediateCertificate(SecCertificateRef cert); - - // Returns intermediate certificates added via AddIntermediateCertificate(). - // Ownership follows the "get" rule: it is the caller's responsibility to - // retain the result. - CFArrayRef GetIntermediateCertificates() { return intermediate_ca_certs_; } +#if defined(OS_MACOSX) || defined(OS_WIN) + // Adds an untrusted intermediate certificate that may be needed for + // chain building. + void AddIntermediateCertificate(OSCertHandle cert) { + intermediate_ca_certs_.push_back(cert); + } + + // Returns intermediate certificates added via AddIntermediateCertificate(). + // Ownership follows the "get" rule: it is the caller's responsibility to + // retain the elements of the result. + const std::vector<OSCertHandle>& GetIntermediateCertificates() const { + return intermediate_ca_certs_; + } #endif // Verifies the certificate against the given hostname. Returns OK if @@ -310,10 +314,10 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> { // A handle to the certificate object in the underlying crypto library. OSCertHandle cert_handle_; -#if defined(OS_MACOSX) - // Untrusted intermediate certificates associated with this certificate - // that may be needed for chain building. - CFMutableArrayRef intermediate_ca_certs_; +#if defined(OS_MACOSX) || defined(OS_WIN) + // Untrusted intermediate certificates associated with this certificate + // that may be needed for chain building. + std::vector<OSCertHandle> intermediate_ca_certs_; #endif // Where the certificate comes from. diff --git a/net/base/x509_certificate_mac.cc b/net/base/x509_certificate_mac.cc index d5b597f..2b96be4 100644 --- a/net/base/x509_certificate_mac.cc +++ b/net/base/x509_certificate_mac.cc @@ -454,14 +454,8 @@ int X509Certificate::Verify(const std::string& hostname, int flags, return ERR_OUT_OF_MEMORY; scoped_cftyperef<CFArrayRef> scoped_cert_array(cert_array); CFArrayAppendValue(cert_array, cert_handle_); - if (intermediate_ca_certs_) { - CFIndex intermediate_count = CFArrayGetCount(intermediate_ca_certs_); - for (CFIndex i = 0; i < intermediate_count; ++i) { - SecCertificateRef intermediate_cert = static_cast<SecCertificateRef>( - const_cast<void*>(CFArrayGetValueAtIndex(intermediate_ca_certs_, i))); - CFArrayAppendValue(cert_array, intermediate_cert); - } - } + for (size_t i = 0; i < intermediate_ca_certs_.size(); ++i) + CFArrayAppendValue(cert_array, intermediate_ca_certs_[i]); SecTrustRef trust_ref = NULL; status = SecTrustCreateWithCertificates(cert_array, ssl_policy, &trust_ref); @@ -655,18 +649,6 @@ bool X509Certificate::VerifyEV() const { return false; } -void X509Certificate::AddIntermediateCertificate(SecCertificateRef cert) { - if (cert) { - if (!intermediate_ca_certs_) { - intermediate_ca_certs_ = CFArrayCreateMutable(kCFAllocatorDefault, 0, - &kCFTypeArrayCallBacks); - } - if (intermediate_ca_certs_) { - CFArrayAppendValue(intermediate_ca_certs_, cert); - } - } -} - // static X509Certificate::OSCertHandle X509Certificate::CreateOSCertHandleFromBytes( const char* data, int length) { diff --git a/net/socket/ssl_client_socket_mac.cc b/net/socket/ssl_client_socket_mac.cc index 5e085eb..ec67afa 100644 --- a/net/socket/ssl_client_socket_mac.cc +++ b/net/socket/ssl_client_socket_mac.cc @@ -286,10 +286,14 @@ X509Certificate* GetServerCert(SSLContextRef ssl_context) { // Add each of the intermediate certificates in the server's chain to the // server's X509Certificate object. This makes them available to // X509Certificate::Verify() for chain building. + // TODO(wtc): Since X509Certificate::CreateFromHandle may return a cached + // X509Certificate object, we may be adding intermediate CA certificates to + // it repeatedly! CFIndex certs_length = CFArrayGetCount(certs); for (CFIndex i = 1; i < certs_length; ++i) { SecCertificateRef cert_ref = reinterpret_cast<SecCertificateRef>( const_cast<void*>(CFArrayGetValueAtIndex(certs, i))); + CFRetain(cert_ref); x509_cert->AddIntermediateCertificate(cert_ref); } @@ -848,7 +852,7 @@ OSStatus SSLClientSocketMac::SSLReadCallback(SSLConnectionRef connection, if (rv < 0) return OSStatusFromNetError(rv); - else if (rv == 0) // stream closed + else if (rv == 0) // stream closed return errSSLClosedGraceful; else return noErr; diff --git a/net/socket/ssl_client_socket_mac.h b/net/socket/ssl_client_socket_mac.h index a520abc..704090c 100644 --- a/net/socket/ssl_client_socket_mac.h +++ b/net/socket/ssl_client_socket_mac.h @@ -106,7 +106,6 @@ class SSLClientSocketMac : public SSLClientSocket { State next_handshake_state_; scoped_refptr<X509Certificate> server_cert_; - std::vector<scoped_refptr<X509Certificate> > intermediate_certs_; scoped_ptr<CertVerifier> verifier_; CertVerifyResult server_cert_verify_result_; |