Do not assume OSCertHandle is OpenSSL's X509 structure.

net/socket/ssl_client_socket_openssl.cc

Index: net/socket/ssl_client_socket_openssl.cc
diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc
index d04670fc14fcab9b1e7465a3525886b6b798b03a..4ae855d60dbad617c448c02ef1be0a219a43e18a 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -332,16 +332,17 @@ class SSLClientSocketOpenSSL::SSLContext {
 // and the other elements are in the order given by the server.
 class SSLClientSocketOpenSSL::PeerCertificateChain {
  public:
-  explicit PeerCertificateChain(SSL* ssl) { Reset(ssl); }
+  explicit PeerCertificateChain(STACK_OF(X509) * chain) { Reset(chain); }

[wtc, 2014/03/19 18:49:35]

Nit: remove the space before '*'.

[haavardm, 2014/03/20 13:56:29]

Done.

   PeerCertificateChain(const PeerCertificateChain& other) { *this = other; }
   ~PeerCertificateChain() {}
   PeerCertificateChain& operator=(const PeerCertificateChain& other);
 
-  // Resets the PeerCertificateChain to the set of certificates supplied by the
-  // peer of |ssl|, which may be NULL, indicating to empty the store
+  // Resets the PeerCertificateChain to the set of certificates supplied by
+  // |chain|, which may be NULL, indicating to empty the store

[wtc, 2014/03/19 18:49:35]

Nit: change "supplied by |chain|" to "in |chain|".

[haavardm, 2014/03/20 13:56:29]

Done.

   // certificates. Note: If an error occurs, such as being unable to parse the
   // certificates,  this will behave as if Reset(NULL) was called.
-  void Reset(SSL* ssl);
+  void Reset(STACK_OF(X509)* chain);
+
   // Note that when USE_OPENSSL is defined, OSCertHandle is X509*
   const scoped_refptr<X509Certificate>& AsOSChain() const { return os_chain_; }
 
@@ -356,6 +357,8 @@ class SSLClientSocketOpenSSL::PeerCertificateChain {
     return sk_X509_value(openssl_chain_.get(), index);
   }
 
+  bool IsValid() { return os_chain_.get() && openssl_chain_.get(); }
+
  private:
   static void FreeX509Stack(STACK_OF(X509)* cert_chain) {
     sk_X509_pop_free(cert_chain, X509_free);
@@ -389,14 +392,11 @@ SSLClientSocketOpenSSL::PeerCertificateChain::operator=(
 #if defined(USE_OPENSSL)
 // When OSCertHandle is typedef'ed to X509, this implementation does a short cut
 // to avoid converting back and forth between der and X509 struct.
-void SSLClientSocketOpenSSL::PeerCertificateChain::Reset(SSL* ssl) {
+void SSLClientSocketOpenSSL::PeerCertificateChain::Reset(
+    STACK_OF(X509)* chain) {
   openssl_chain_.reset(NULL);
   os_chain_ = NULL;
 
-  if (ssl == NULL)
-    return;
-
-  STACK_OF(X509)* chain = SSL_get_peer_cert_chain(ssl);
   if (!chain)
     return;
 
@@ -418,14 +418,11 @@ void SSLClientSocketOpenSSL::PeerCertificateChain::Reset(SSL* ssl) {
   }
 }
 #else  // !defined(USE_OPENSSL)
-void SSLClientSocketOpenSSL::PeerCertificateChain::Reset(SSL* ssl) {
+void SSLClientSocketOpenSSL::PeerCertificateChain::Reset(
+    STACK_OF(X509)* chain) {
   openssl_chain_.reset(NULL);
   os_chain_ = NULL;
 
-  if (ssl == NULL)
-    return;
-
-  STACK_OF(X509)* chain = SSL_get_peer_cert_chain(ssl);
   if (!chain)
     return;
 
@@ -1058,8 +1055,12 @@ void SSLClientSocketOpenSSL::DoConnectCallback(int rv) {
 }
 
 X509Certificate* SSLClientSocketOpenSSL::UpdateServerCert() {
-  server_cert_chain_->Reset(ssl_);
+  server_cert_chain_->Reset(SSL_get_peer_cert_chain(ssl_));
   server_cert_ = server_cert_chain_->AsOSChain();
+
+  if (!server_cert_chain_->IsValid())
+    DVLOG(1) << "UpdateServerCert received invalid certificate from peer";

[wtc, 2014/03/19 18:49:35]

Nit: add "chain" to the error message?

[haavardm, 2014/03/20 13:56:29]

Done.

+
   return server_cert_.get();
 }
 
@@ -1502,10 +1503,11 @@ int SSLClientSocketOpenSSL::CertVerifyCallback(X509_STORE_CTX* store_ctx) {
     return 1;
   }
 
-  if (X509Certificate::IsSameOSCert(server_cert_->os_cert_handle(),
-                                    sk_X509_value(store_ctx->untrusted, 0))) {
+  CHECK(server_cert_.get());
+
+  PeerCertificateChain chain(store_ctx->chain);
+  if (chain.IsValid() && server_cert_->Equals(chain.AsOSChain()))
     return 1;
-  }
 
   LOG(ERROR) << "Server certificate changed between handshakes";

[wtc, 2014/03/19 18:49:35]

Nit: consider emitting a different error message if chain.IsValid() is false.

[haavardm, 2014/03/20 13:56:29]

Done.

   return 0;