We don't handle certificate errors during SSL renegotiation.

In the common case, the server sends the same certificate during
renegotiation.  Since the certificate has been verified, we can
assume the certificate is good or has been accepted by the user.
If the server sends a different certificate that has an error,
we need to return an error code that won't trigger our
certificate error handling code, which doesn't handle this case
correctly.  Add the ERR_CERT_ERROR_IN_SSL_RENEGOTIATION error
for this purpose.

R=rvargas
BUG=http://crbug.com/13226
TEST=See http://crbug.com/13226 comment 9
Review URL: http://codereview.chromium.org/118410

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@17919 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 36 insertions, 8 deletions

diff --git a/net/base/net_error_list.h b/net/base/net_error_list.h
index ae7d272..e22e11f 100644
--- a/net/base/net_error_list.h
+++ b/net/base/net_error_list.h
@@ -99,6 +99,13 @@ NET_ERROR(SSL_RENEGOTIATION_REQUESTED, -114)
 // The proxy requested authentication (for tunnel establishment).
 NET_ERROR(PROXY_AUTH_REQUESTED, -115)
 
+// During SSL renegotiation (rehandshake), the server sent a certificate with
+// an error.
+//
+// Note: this error is not in the -2xx range so that it won't be handled as a
+// certificate error.
+NET_ERROR(CERT_ERROR_IN_SSL_RENEGOTIATION, -116)
+
 // Certificate error codes
 //
 // The values of certificate error codes must be consecutive.
diff --git a/net/base/ssl_client_socket_win.cc b/net/base/ssl_client_socket_win.cc
index 7c76b99..a839bb3 100644
--- a/net/base/ssl_client_socket_win.cc
+++ b/net/base/ssl_client_socket_win.cc
@@ -60,6 +60,12 @@ static int MapSecurityError(SECURITY_STATUS err) {
   }
 }
 
+// Returns true if the two CERT_CONTEXTs contain the same certificate.
+bool SameCert(PCCERT_CONTEXT a, PCCERT_CONTEXT b) {
+  return a->cbCertEncoded == b->cbCertEncoded &&
+         memcmp(a->pbCertEncoded, b->pbCertEncoded, b->cbCertEncoded) == 0;
+}
+
 //-----------------------------------------------------------------------------
 
 // A bitmask consisting of these bit flags encodes which versions of the SSL
@@ -727,11 +733,7 @@ int SSLClientSocketWin::DoVerifyCertComplete(int result) {
 
   LogConnectionTypeMetrics();
   if (renegotiating_) {
-    // A rehandshake, started in the middle of a Read, has completed.
-    renegotiating_ = false;
-    // Pick up where we left off.  Go back to reading data.
-    if (result == OK)
-      SetNextStateForRead();
+    DidCompleteRenegotiation(result);
   } else {
     // The initial handshake, kicked off by a Connect, has completed.
     completed_handshake_ = true;
@@ -998,13 +1000,31 @@ int SSLClientSocketWin::DidCompleteHandshake() {
     DLOG(ERROR) << "QueryContextAttributes failed: " << status;
     return MapSecurityError(status);
   }
-  server_cert_ = X509Certificate::CreateFromHandle(
-      server_cert_handle, X509Certificate::SOURCE_FROM_NETWORK);
+  if (renegotiating_ &&
+      SameCert(server_cert_->os_cert_handle(), server_cert_handle)) {
+    // We already verified the server certificate.  Either it is good or the
+    // user has accepted the certificate error.
+    CertFreeCertificateContext(server_cert_handle);
+    DidCompleteRenegotiation(OK);
+  } else {
+    server_cert_ = X509Certificate::CreateFromHandle(
+        server_cert_handle, X509Certificate::SOURCE_FROM_NETWORK);
 
-  next_state_ = STATE_VERIFY_CERT;
+    next_state_ = STATE_VERIFY_CERT;
+  }
   return OK;
 }
 
+// Called when a renegotiation is completed.  |result| is the verification
+// result of the server certificate received during renegotiation.
+void SSLClientSocketWin::DidCompleteRenegotiation(int result) {
+  // A rehandshake, started in the middle of a Read, has completed.
+  renegotiating_ = false;
+  // Pick up where we left off.  Go back to reading data.
+  if (result == OK)
+    SetNextStateForRead();
+}
+
 void SSLClientSocketWin::LogConnectionTypeMetrics() const {
   UpdateConnectionTypeHistograms(CONNECTION_SSL);
   if (server_cert_verify_result_.has_md5)
diff --git a/net/base/ssl_client_socket_win.h b/net/base/ssl_client_socket_win.h
index 9bdb5a1..35f1e9e 100644
--- a/net/base/ssl_client_socket_win.h
+++ b/net/base/ssl_client_socket_win.h
@@ -66,6 +66,7 @@ class SSLClientSocketWin : public SSLClientSocket {
 
   int DidCallInitializeSecurityContext();
   int DidCompleteHandshake();
+  void DidCompleteRenegotiation(int result);
   void LogConnectionTypeMetrics() const;
   void SetNextStateForRead();
   void FreeSendBuffer();