diff options
-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_; |