Implement SSL certificate error handling on the Mac. If the user gives

us bad certs to allow, we tell SecureTransport to not verify the server
cert, and only allow the cert to be one of the bad certs the user allows.

In the future we should figure out how to verify the server cert ourselves.

R=avi,eroman
BUG=http://crbug.com/11983
TEST=Visit https://www.ssl247.com/ and https://alioth.debian.org/.  Clicking
the "Proceed anyway" button should bring you to the site with a red
"https" in the location bar. 
Review URL: http://codereview.chromium.org/165191

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

6 files changed, 102 insertions, 52 deletions

diff --git a/net/base/ssl_config_service.h b/net/base/ssl_config_service.h
index 388b255..5354b3e 100644
--- a/net/base/ssl_config_service.h
+++ b/net/base/ssl_config_service.h
@@ -2,10 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef NET_BASE_SSL_CONFIG_SERVICE_H__
-#define NET_BASE_SSL_CONFIG_SERVICE_H__
+#ifndef NET_BASE_SSL_CONFIG_SERVICE_H_
+#define NET_BASE_SSL_CONFIG_SERVICE_H_
 
-#include <set>
+#include <vector>
 
 #include "base/time.h"
 #include "net/base/x509_certificate.h"
@@ -30,11 +30,27 @@ struct SSLConfig {
   // TODO(wtc): move the following members to a new SSLParams structure.  They
   // are not SSL configuration settings.
 
-  // Add any known-bad SSL certificates to allowed_bad_certs_ that should not
-  // trigger an ERR_CERT_*_INVALID error when calling SSLClientSocket::Connect.
-  // This would normally be done in response to the user explicitly accepting
-  // the bad certificate.
-  std::set<scoped_refptr<X509Certificate> > allowed_bad_certs_;
+  struct CertAndStatus {
+    scoped_refptr<X509Certificate> cert;
+    int cert_status;
+  };
+
+  // Returns true if |cert| is one of the certs in |allowed_bad_certs|.
+  // TODO(wtc): Move this to a .cc file.  ssl_config_service.cc is Windows
+  // only right now, so I can't move it there.
+  bool IsAllowedBadCert(X509Certificate* cert) const {
+    for (size_t i = 0; i < allowed_bad_certs.size(); ++i) {
+      if (cert == allowed_bad_certs[i].cert)
+        return true;
+    }
+    return false;
+  }
+
+  // Add any known-bad SSL certificate (with its cert status) to
+  // |allowed_bad_certs| that should not trigger an ERR_CERT_* error when
+  // calling SSLClientSocket::Connect.  This would normally be done in
+  // response to the user explicitly accepting the bad certificate.
+  std::vector<CertAndStatus> allowed_bad_certs;
 
   // True if we should send client_cert to the server.
   bool send_client_cert;
@@ -87,4 +103,4 @@ class SSLConfigService {
 
 }  // namespace net
 
-#endif  // NET_BASE_SSL_CONFIG_SERVICE_H__
+#endif  // NET_BASE_SSL_CONFIG_SERVICE_H_
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 1a1b2f0..a3f5513 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -1378,7 +1378,10 @@ int HttpNetworkTransaction::HandleCertificateError(int error) {
     // SSL info object. This data structure will be consulted after calling
     // RestartIgnoringLastError(). And the user will be asked interactively
     // before RestartIgnoringLastError() is ever called.
-    ssl_config_.allowed_bad_certs_.insert(response_.ssl_info.cert);
+    SSLConfig::CertAndStatus bad_cert;
+    bad_cert.cert = response_.ssl_info.cert;
+    bad_cert.cert_status = response_.ssl_info.cert_status;
+    ssl_config_.allowed_bad_certs.push_back(bad_cert);
   }
   return error;
 }
diff --git a/net/socket/ssl_client_socket_mac.cc b/net/socket/ssl_client_socket_mac.cc
index dd56e35..2fe9743 100644
--- a/net/socket/ssl_client_socket_mac.cc
+++ b/net/socket/ssl_client_socket_mac.cc
@@ -241,6 +241,24 @@ int KeySizeOfCipherSuite(SSLCipherSuite suite) {
   }
 }
 
+// Returns the server's certificate.  The caller must release a reference
+// to the return value when done.  Returns NULL on failure.
+X509Certificate* GetServerCert(SSLContextRef ssl_context) {
+  CFArrayRef certs;
+  OSStatus status = SSLCopyPeerCertificates(ssl_context, &certs);
+  if (status != noErr)
+    return NULL;
+
+  DCHECK_GT(CFArrayGetCount(certs), 0);
+
+  SecCertificateRef server_cert = static_cast<SecCertificateRef>(
+      const_cast<void*>(CFArrayGetValueAtIndex(certs, 0)));
+  CFRetain(server_cert);
+  CFRelease(certs);
+  return X509Certificate::CreateFromHandle(
+      server_cert, X509Certificate::SOURCE_FROM_NETWORK);
+}
+
 }  // namespace
 
 //-----------------------------------------------------------------------------
@@ -305,10 +323,23 @@ int SSLClientSocketMac::Connect(CompletionCallback* callback) {
   if (status)
     return NetErrorFromOSStatus(status);
 
-  status = SSLSetPeerDomainName(ssl_context_, hostname_.c_str(),
-                                hostname_.length());
-  if (status)
-    return NetErrorFromOSStatus(status);
+  if (ssl_config_.allowed_bad_certs.empty()) {
+    // We're going to use the default certificate verification that the system
+    // does, and accept its answer for the cert status.
+    status = SSLSetPeerDomainName(ssl_context_, hostname_.data(),
+                                  hostname_.length());
+    if (status)
+      return NetErrorFromOSStatus(status);
+
+    // TODO(wtc): for now, always check revocation.
+    server_cert_status_ = CERT_STATUS_REV_CHECKING_ENABLED;
+  } else {
+    // Disable certificate chain validation.  We will only allow the certs in
+    // ssl_config_.allowed_bad_certs.
+    status = SSLSetEnableCertVerify(ssl_context_, false);
+    if (status)
+      return NetErrorFromOSStatus(status);
+  }
 
   next_state_ = STATE_HANDSHAKE;
   int rv = DoLoop(OK);
@@ -394,26 +425,14 @@ void SSLClientSocketMac::GetSSLInfo(SSLInfo* ssl_info) {
   ssl_info->Reset();
 
   // set cert
-  CFArrayRef certs;
-  OSStatus status = SSLCopyPeerCertificates(ssl_context_, &certs);
-  if (!status) {
-    DCHECK(CFArrayGetCount(certs) > 0);
-
-    SecCertificateRef client_cert =
-        static_cast<SecCertificateRef>(
-          const_cast<void*>(CFArrayGetValueAtIndex(certs, 0)));
-    CFRetain(client_cert);
-    ssl_info->cert = X509Certificate::CreateFromHandle(
-        client_cert, X509Certificate::SOURCE_FROM_NETWORK);
-    CFRelease(certs);
-  }
+  ssl_info->cert = server_cert_;
 
   // update status
   ssl_info->cert_status = server_cert_status_;
 
   // security info
   SSLCipherSuite suite;
-  status = SSLGetNegotiatedCipher(ssl_context_, &suite);
+  OSStatus status = SSLGetNegotiatedCipher(ssl_context_, &suite);
   if (!status)
     ssl_info->security_bits = KeySizeOfCipherSuite(suite);
 }
@@ -485,26 +504,36 @@ int SSLClientSocketMac::DoLoop(int last_io_result) {
 
 int SSLClientSocketMac::DoHandshake() {
   OSStatus status = SSLHandshake(ssl_context_);
-
-  if (status == errSSLWouldBlock)
-    next_state_ = STATE_HANDSHAKE;
-
-  if (status == noErr)
-    completed_handshake_ = true;
-
   int net_error = NetErrorFromOSStatus(status);
 
-  // At this point we have a connection. For now, we're going to use the default
-  // certificate verification that the system does, and accept its answer for
-  // the cert status. In the future, we'll need to call SSLSetEnableCertVerify
-  // to disable cert verification and do the verification ourselves. This allows
-  // very fine-grained control over what we'll accept for certification.
-  // TODO(avi): ditto
-
-  // TODO(wtc): for now, always check revocation.
-  server_cert_status_ = CERT_STATUS_REV_CHECKING_ENABLED;
-  if (net_error)
+  if (status == errSSLWouldBlock) {
+    next_state_ = STATE_HANDSHAKE;
+  } else if (status == noErr) {
+    completed_handshake_ = true;  // We have a connection.
+
+    server_cert_ = GetServerCert(ssl_context_);
+    DCHECK(server_cert_);
+    if (!ssl_config_.allowed_bad_certs.empty()) {
+      // Check server_cert_ because SecureTransport didn't verify it.
+      // TODO(wtc): If server_cert_ is not one of the allowed bad certificates,
+      // we should verify server_cert_ ourselves.  Since we don't know how to
+      // do that yet, treat it as an invalid certificate.
+      net_error = ERR_CERT_INVALID;
+      server_cert_status_ |= CERT_STATUS_INVALID;
+
+      for (size_t i = 0; i < ssl_config_.allowed_bad_certs.size(); ++i) {
+        if (server_cert_ == ssl_config_.allowed_bad_certs[i].cert) {
+          net_error = OK;
+          server_cert_status_ = ssl_config_.allowed_bad_certs[i].cert_status;
+          break;
+        }
+      }
+    }
+  } else if (IsCertStatusError(net_error)) {
+    server_cert_ = GetServerCert(ssl_context_);
+    DCHECK(server_cert_);
     server_cert_status_ |= MapNetErrorToCertStatus(net_error);
+  }
 
   return net_error;
 }
diff --git a/net/socket/ssl_client_socket_mac.h b/net/socket/ssl_client_socket_mac.h
index f9a08af..906bde5 100644
--- a/net/socket/ssl_client_socket_mac.h
+++ b/net/socket/ssl_client_socket_mac.h
@@ -84,6 +84,8 @@ class SSLClientSocketMac : public SSLClientSocket {
   State next_state_;
   State next_io_state_;
 
+  // Set when handshake finishes.
+  scoped_refptr<X509Certificate> server_cert_;
   int server_cert_status_;
 
   bool completed_handshake_;
diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index 96a8c6d..e6b3e96 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -332,7 +332,7 @@ int SSLClientSocketNSS::Connect(CompletionCallback* callback) {
 
 void SSLClientSocketNSS::InvalidateSessionIfBadCertificate() {
   if (UpdateServerCert() != NULL &&
-      ssl_config_.allowed_bad_certs_.count(server_cert_)) {
+      ssl_config_.IsAllowedBadCert(server_cert_)) {
     SSL_InvalidateSession(nss_fd_);
   }
 }
@@ -782,10 +782,10 @@ int SSLClientSocketNSS::DoVerifyCertComplete(int result) {
   // result of verifier_.Verify.
   // Eventually, we should cache the cert verification results so that we don't
   // need to call verifier_.Verify repeatedly.  But for now we need to do this.
-  // Alternatively, we might be able to store the cert's status along with
-  // the cert in the allowed_bad_certs_ set.
+  // Alternatively, we could use the cert's status that we stored along with
+  // the cert in the allowed_bad_certs vector.
   if (IsCertificateError(result) &&
-      ssl_config_.allowed_bad_certs_.count(server_cert_)) {
+      ssl_config_.IsAllowedBadCert(server_cert_)) {
     LOG(INFO) << "accepting bad SSL certificate, as user told us to";
     result = OK;
   }
diff --git a/net/socket/ssl_client_socket_win.cc b/net/socket/ssl_client_socket_win.cc
index fba04ea..474f630 100644
--- a/net/socket/ssl_client_socket_win.cc
+++ b/net/socket/ssl_client_socket_win.cc
@@ -873,10 +873,10 @@ int SSLClientSocketWin::DoVerifyCertComplete(int result) {
   // result of verifier_.Verify.
   // Eventually, we should cache the cert verification results so that we don't
   // need to call verifier_.Verify repeatedly. But for now we need to do this.
-  // Alternatively, we might be able to store the cert's status along with
-  // the cert in the allowed_bad_certs_ set.
+  // Alternatively, we could use the cert's status that we stored along with
+  // the cert in the allowed_bad_certs vector.
   if (IsCertificateError(result) &&
-      ssl_config_.allowed_bad_certs_.count(server_cert_))
+      ssl_config_.IsAllowedBadCert(server_cert_))
     result = OK;
 
   LogConnectionTypeMetrics();