Enable SSLClientSocketTest unit tests on Mac OS X by implementing our own certificate validation code. This gives us proper hostname matching, multiple error codes (e.g., before a certificate could be marked as expired or untrusted, but not both), revocation checking, and EV certificate checking.

BUG=19286,10910,14733
TEST=https://www.paypal.com should work without warning. https://paypal.com should get a warning about a hostname mismatch. https://test-ssev.verisign.com:1443/test-SSEV-expired-verisign.html should give a warning about an expired certificate.
Review URL: http://codereview.chromium.org/174102

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

4 files changed, 130 insertions, 81 deletions

diff --git a/net/socket/ssl_client_socket_mac.cc b/net/socket/ssl_client_socket_mac.cc
index 6684398..56b19d1 100644
--- a/net/socket/ssl_client_socket_mac.cc
+++ b/net/socket/ssl_client_socket_mac.cc
@@ -4,8 +4,10 @@
 
 #include "net/socket/ssl_client_socket_mac.h"
 
+#include "base/scoped_cftyperef.h"
 #include "base/singleton.h"
 #include "base/string_util.h"
+#include "net/base/cert_verifier.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/base/ssl_info.h"
@@ -276,7 +278,6 @@ SSLClientSocketMac::SSLClientSocketMac(ClientSocket* transport_socket,
       user_callback_(NULL),
       next_state_(STATE_NONE),
       next_io_state_(STATE_NONE),
-      server_cert_status_(0),
       completed_handshake_(false),
       ssl_context_(NULL),
       pending_send_error_(OK),
@@ -325,23 +326,11 @@ int SSLClientSocketMac::Connect(CompletionCallback* callback) {
   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);
-  }
+  // Disable certificate verification within Secure Transport; we'll
+  // be handling that ourselves.
+  status = SSLSetEnableCertVerify(ssl_context_, false);
+  if (status)
+    return NetErrorFromOSStatus(status);
 
   next_state_ = STATE_HANDSHAKE;
   int rv = DoLoop(OK);
@@ -430,7 +419,7 @@ void SSLClientSocketMac::GetSSLInfo(SSLInfo* ssl_info) {
   ssl_info->cert = server_cert_;
 
   // update status
-  ssl_info->cert_status = server_cert_status_;
+  ssl_info->cert_status = server_cert_verify_result_.cert_status;
 
   // security info
   SSLCipherSuite suite;
@@ -483,6 +472,14 @@ int SSLClientSocketMac::DoLoop(int last_io_result) {
         // Do the SSL/TLS handshake.
         rv = DoHandshake();
         break;
+      case STATE_VERIFY_CERT:
+        // Kick off server certificate validation.
+        rv = DoVerifyCert();
+        break;
+      case STATE_VERIFY_CERT_COMPLETE:
+        // Check the results of the  server certificate validation.
+        rv = DoVerifyCertComplete(rv);
+        break;
       case STATE_READ_COMPLETE:
         // A read off the network is complete; do the paperwork.
         rv = DoReadComplete(rv);
@@ -506,38 +503,60 @@ int SSLClientSocketMac::DoLoop(int last_io_result) {
 
 int SSLClientSocketMac::DoHandshake() {
   OSStatus status = SSLHandshake(ssl_context_);
-  int net_error = NetErrorFromOSStatus(status);
 
-  if (status == errSSLWouldBlock) {
+  if (status == errSSLWouldBlock)
     next_state_ = STATE_HANDSHAKE;
-  } else if (status == noErr) {
-    completed_handshake_ = true;  // We have a connection.
 
+  if (status == noErr) {
     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 (IsCertificateError(net_error)) {
-    server_cert_ = GetServerCert(ssl_context_);
-    DCHECK(server_cert_);
-    server_cert_status_ |= MapNetErrorToCertStatus(net_error);
+    if (!server_cert_)
+      return ERR_UNEXPECTED;
+    next_state_ = STATE_VERIFY_CERT;
+  }
+
+  return NetErrorFromOSStatus(status);
+}
+
+int SSLClientSocketMac::DoVerifyCert() {
+  next_state_ = STATE_VERIFY_CERT_COMPLETE;
+
+  if (!server_cert_)
+    return ERR_UNEXPECTED;
+
+  // 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.
+  CFArrayRef certs;
+  OSStatus status = SSLCopyPeerCertificates(ssl_context_, &certs);
+  if (status != noErr || !certs)
+    return ERR_UNEXPECTED;
+  scoped_cftyperef<CFArrayRef> scoped_certs(certs);
+  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)));
+    server_cert_->AddIntermediateCertificate(cert_ref);
   }
 
-  return net_error;
+  // TODO(hawk): set flags based on the SSLConfig, once SSLConfig is
+  // fully fleshed out on Mac OS X.
+  int flags = 0;
+  verifier_.reset(new CertVerifier);
+  return verifier_->Verify(server_cert_, hostname_, flags,
+                           &server_cert_verify_result_, &io_callback_);
+}
+
+int SSLClientSocketMac::DoVerifyCertComplete(int result) {
+  DCHECK(verifier_.get());
+  verifier_.reset();
+
+  if (IsCertificateError(result) && ssl_config_.IsAllowedBadCert(server_cert_))
+    result = OK;
+
+  completed_handshake_ = true;
+  DCHECK(next_state_ == STATE_NONE);
+
+  return result;
 }
 
 int SSLClientSocketMac::DoReadComplete(int result) {
diff --git a/net/socket/ssl_client_socket_mac.h b/net/socket/ssl_client_socket_mac.h
index 906bde5..a447055 100644
--- a/net/socket/ssl_client_socket_mac.h
+++ b/net/socket/ssl_client_socket_mac.h
@@ -11,12 +11,15 @@
 #include <vector>
 
 #include "base/scoped_ptr.h"
+#include "net/base/cert_verify_result.h"
 #include "net/base/completion_callback.h"
 #include "net/base/ssl_config_service.h"
 #include "net/socket/ssl_client_socket.h"
 
 namespace net {
 
+class CertVerifier;
+
 // An SSL client socket implemented with Secure Transport.
 class SSLClientSocketMac : public SSLClientSocket {
  public:
@@ -51,6 +54,8 @@ class SSLClientSocketMac : public SSLClientSocket {
   int DoPayloadRead();
   int DoPayloadWrite();
   int DoHandshake();
+  int DoVerifyCert();
+  int DoVerifyCertComplete(int result);
   int DoReadComplete(int result);
   void OnWriteComplete(int result);
 
@@ -79,14 +84,17 @@ class SSLClientSocketMac : public SSLClientSocket {
     STATE_PAYLOAD_READ,
     STATE_PAYLOAD_WRITE,
     STATE_HANDSHAKE,
+    STATE_VERIFY_CERT,
+    STATE_VERIFY_CERT_COMPLETE,
     STATE_READ_COMPLETE,
   };
   State next_state_;
   State next_io_state_;
 
-  // Set when handshake finishes.
   scoped_refptr<X509Certificate> server_cert_;
-  int server_cert_status_;
+  std::vector<scoped_refptr<X509Certificate> > intermediate_certs_;
+  scoped_ptr<CertVerifier> verifier_;
+  CertVerifyResult server_cert_verify_result_;
 
   bool completed_handshake_;
   SSLContextRef ssl_context_;
diff --git a/net/socket/ssl_client_socket_unittest.cc b/net/socket/ssl_client_socket_unittest.cc
index d565ab6..aa94ff8 100644
--- a/net/socket/ssl_client_socket_unittest.cc
+++ b/net/socket/ssl_client_socket_unittest.cc
@@ -56,35 +56,7 @@ class SSLClientSocketTest : public PlatformTest {
 
 //-----------------------------------------------------------------------------
 
-#if defined(OS_MACOSX)
-// Status 6/19/09:
-//
-// If these tests are enabled on OSX, we choke at the point
-// SSLHandshake() (Security framework call) is called from
-// SSLClientSocketMac::DoHandshake().  Return value is -9812 (cert
-// valid but root not trusted), but if you don't have the cert in your
-// keychain as documented on
-// http://dev.chromium.org/developers/testing, the -9812 becomes a
-// -9813 (no root cert).
-//
-// See related handshake failures exhibited by disabled tests in
-// net/url_request/url_request_unittest.cc.
-#define MAYBE_Connect DISABLED_Connect
-#define MAYBE_ConnectExpired DISABLED_ConnectExpired
-#define MAYBE_ConnectMismatched DISABLED_ConnectMismatched
-#define MAYBE_Read DISABLED_Read
-#define MAYBE_Read_SmallChunks DISABLED_Read_SmallChunks
-#define MAYBE_Read_Interrupted DISABLED_Read_Interrupted
-#else
-#define MAYBE_Connect Connect
-#define MAYBE_ConnectExpired ConnectExpired
-#define MAYBE_ConnectMismatched ConnectMismatched
-#define MAYBE_Read Read
-#define MAYBE_Read_SmallChunks Read_SmallChunks
-#define MAYBE_Read_Interrupted Read_Interrupted
-#endif
-
-TEST_F(SSLClientSocketTest, MAYBE_Connect) {
+TEST_F(SSLClientSocketTest, Connect) {
   StartOKServer();
 
   net::AddressList addr;
@@ -121,7 +93,7 @@ TEST_F(SSLClientSocketTest, MAYBE_Connect) {
   EXPECT_FALSE(sock->IsConnected());
 }
 
-TEST_F(SSLClientSocketTest, MAYBE_ConnectExpired) {
+TEST_F(SSLClientSocketTest, ConnectExpired) {
   StartExpiredServer();
 
   net::AddressList addr;
@@ -157,7 +129,7 @@ TEST_F(SSLClientSocketTest, MAYBE_ConnectExpired) {
   // leave it connected.
 }
 
-TEST_F(SSLClientSocketTest, MAYBE_ConnectMismatched) {
+TEST_F(SSLClientSocketTest, ConnectMismatched) {
   StartMismatchedServer();
 
   net::AddressList addr;
@@ -199,7 +171,7 @@ TEST_F(SSLClientSocketTest, MAYBE_ConnectMismatched) {
 //   - Server closes the underlying TCP connection directly.
 //   - Server sends data unexpectedly.
 
-TEST_F(SSLClientSocketTest, MAYBE_Read) {
+TEST_F(SSLClientSocketTest, Read) {
   StartOKServer();
 
   net::AddressList addr;
@@ -259,7 +231,7 @@ TEST_F(SSLClientSocketTest, MAYBE_Read) {
   }
 }
 
-TEST_F(SSLClientSocketTest, MAYBE_Read_SmallChunks) {
+TEST_F(SSLClientSocketTest, Read_SmallChunks) {
   StartOKServer();
 
   net::AddressList addr;
@@ -314,7 +286,7 @@ TEST_F(SSLClientSocketTest, MAYBE_Read_SmallChunks) {
   }
 }
 
-TEST_F(SSLClientSocketTest, MAYBE_Read_Interrupted) {
+TEST_F(SSLClientSocketTest, Read_Interrupted) {
   StartOKServer();
 
   net::AddressList addr;
diff --git a/net/socket/ssl_test_util.cc b/net/socket/ssl_test_util.cc
index b0fa20e..7e73f1e 100644
--- a/net/socket/ssl_test_util.cc
+++ b/net/socket/ssl_test_util.cc
@@ -24,6 +24,10 @@
 #include <pk11pub.h>
 #undef Lock
 #include "base/nss_init.h"
+#elif defined(OS_MACOSX)
+#include <Security/Security.h>
+#include "base/scoped_cftyperef.h"
+#include "net/base/x509_certificate.h"
 #endif
 
 #include "base/file_util.h"
@@ -81,10 +85,48 @@ static CERTCertificate* LoadTemporaryCert(const FilePath& filename) {
 }
 #endif
 
+#if defined(OS_MACOSX)
+static net::X509Certificate* LoadTemporaryCert(const FilePath& filename) {
+  std::string rawcert;
+  if (!file_util::ReadFileToString(filename.ToWStringHack(), &rawcert)) {
+    LOG(ERROR) << "Can't load certificate " << filename.ToWStringHack();
+    return NULL;
+  }
+
+  CFDataRef pem = CFDataCreate(kCFAllocatorDefault,
+                               reinterpret_cast<const UInt8*>(rawcert.data()),
+                               static_cast<CFIndex>(rawcert.size()));
+  if (!pem)
+    return NULL;
+  scoped_cftyperef<CFDataRef> scoped_pem(pem);
+
+  SecExternalFormat input_format = kSecFormatUnknown;
+  SecExternalItemType item_type = kSecItemTypeUnknown;
+  CFArrayRef cert_array = NULL;
+  if (SecKeychainItemImport(pem, NULL, &input_format, &item_type, 0, NULL, NULL,
+                            &cert_array))
+    return NULL;
+  scoped_cftyperef<CFArrayRef> scoped_cert_array(cert_array);
+
+  if (!CFArrayGetCount(cert_array))
+    return NULL;
+
+  SecCertificateRef cert_ref = static_cast<SecCertificateRef>(
+      const_cast<void*>(CFArrayGetValueAtIndex(cert_array, 0)));
+  CFRetain(cert_ref);
+  return net::X509Certificate::CreateFromHandle(cert_ref,
+      net::X509Certificate::SOURCE_FROM_NETWORK);
+}
+#endif
+
 }  // namespace
 
 namespace net {
 
+#if defined(OS_MACOSX)
+void SetMacTestCertificate(X509Certificate* cert);
+#endif
+
 // static
 const char TestServerLauncher::kHostName[] = "127.0.0.1";
 const char TestServerLauncher::kMismatchedHostName[] = "localhost";
@@ -317,6 +359,8 @@ TestServerLauncher::~TestServerLauncher() {
 #if defined(OS_LINUX)
   if (cert_)
     CERT_DestroyCertificate(reinterpret_cast<CERTCertificate*>(cert_));
+#elif defined(OS_MACOSX)
+  SetMacTestCertificate(NULL);
 #endif
   Stop();
 }
@@ -353,6 +397,12 @@ bool TestServerLauncher::LoadTestRootCert() {
           LoadTemporaryCert(GetRootCertPath()));
   DCHECK(cert_);
   return (cert_ != NULL);
+#elif defined(OS_MACOSX)
+  X509Certificate* cert = LoadTemporaryCert(GetRootCertPath());
+  if (!cert)
+    return false;
+  SetMacTestCertificate(cert);
+  return true;
 #else
   return true;
 #endif