Change the bad-certificate handler for SSL (using NSS) to return an

error.

This requires a few additional changes in the rest of the code. In
particular, we now have to teach HttpNetworkTransaction about how to
restart connections with bad certificates. This was originally
intended to be done by ReconnectIgnoringLastError(), but that API
turns out be very difficult to implement in the SSLClientSocket. So,
instead, we just create a completely new SSLClientSocket.

We also have to be careful to store a copy of the certificate from
within the bad-certificate handler, as it won't be available by the
time GetSSLInfo() is called.

And we fix a bug that would cause us to erroneously talk SSL on
reconnected TCP sockets, even though we were still supposed to
negotiate a proxy tunnel first.

Review URL: http://codereview.chromium.org/43115

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

2 files changed, 79 insertions, 7 deletions

diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index fb4a650..fbfaafe 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -201,9 +201,13 @@ class HTTPSRequestTest : public testing::Test {
 #if defined(OS_MACOSX)
 // ssl_client_socket_mac.cc crashes currently in GetSSLInfo
 // when called on a connection with an unrecognized certificate
-#define MAYBE_HTTPSGetTest   DISABLED_HTTPSGetTest
+#define MAYBE_HTTPSGetTest        DISABLED_HTTPSGetTest
+#define MAYBE_HTTPSMismatchedTest DISABLED_HTTPSMismatchedTest
+#define MAYBE_HTTPSExpiredTest    DISABLED_HTTPSExpiredTest
 #else
-#define MAYBE_HTTPSGetTest   HTTPSGetTest
+#define MAYBE_HTTPSGetTest        HTTPSGetTest
+#define MAYBE_HTTPSMismatchedTest HTTPSMismatchedTest
+#define MAYBE_HTTPSExpiredTest    HTTPSExpiredTest
 #endif
 
 TEST_F(HTTPSRequestTest, MAYBE_HTTPSGetTest) {
@@ -233,7 +237,63 @@ TEST_F(HTTPSRequestTest, MAYBE_HTTPSGetTest) {
 #endif
 }
 
-// TODO(dkegel): add test for expired and mismatched certificates here
+TEST_F(HTTPSRequestTest, MAYBE_HTTPSMismatchedTest) {
+  scoped_refptr<HTTPSTestServer> server =
+      HTTPSTestServer::CreateMismatchedServer(L"net/data/ssl");
+  ASSERT_TRUE(NULL != server.get());
+
+  bool err_allowed = true;
+  for (int i = 0; i < 2 ; i++, err_allowed = !err_allowed) {
+    TestDelegate d;
+    {
+      d.set_allow_certificate_errors(err_allowed);
+      TestURLRequest r(server->TestServerPage(""), &d);
+
+      r.Start();
+      EXPECT_TRUE(r.is_pending());
+
+      MessageLoop::current()->Run();
+
+      EXPECT_EQ(1, d.response_started_count());
+      EXPECT_FALSE(d.received_data_before_response());
+      EXPECT_TRUE(d.have_certificate_errors());
+      if (err_allowed)
+        EXPECT_NE(0, d.bytes_received());
+      else
+        EXPECT_EQ(0, d.bytes_received());
+    }
+  }
+}
+
+TEST_F(HTTPSRequestTest, MAYBE_HTTPSExpiredTest) {
+  scoped_refptr<HTTPSTestServer> server =
+      HTTPSTestServer::CreateExpiredServer(L"net/data/ssl");
+  ASSERT_TRUE(NULL != server.get());
+
+  // Iterate from false to true, just so that we do the opposite of the
+  // previous test in order to increase test coverage.
+  bool err_allowed = false;
+  for (int i = 0; i < 2 ; i++, err_allowed = !err_allowed) {
+    TestDelegate d;
+    {
+      d.set_allow_certificate_errors(err_allowed);
+      TestURLRequest r(server->TestServerPage(""), &d);
+
+      r.Start();
+      EXPECT_TRUE(r.is_pending());
+
+      MessageLoop::current()->Run();
+
+      EXPECT_EQ(1, d.response_started_count());
+      EXPECT_FALSE(d.received_data_before_response());
+      EXPECT_TRUE(d.have_certificate_errors());
+      if (err_allowed)
+        EXPECT_NE(0, d.bytes_received());
+      else
+        EXPECT_EQ(0, d.bytes_received());
+    }
+  }
+}
 
 TEST_F(URLRequestTest, CancelTest) {
   TestDelegate d;
diff --git a/net/url_request/url_request_unittest.h b/net/url_request/url_request_unittest.h
index a3627ad..88b83a6 100644
--- a/net/url_request/url_request_unittest.h
+++ b/net/url_request/url_request_unittest.h
@@ -69,11 +69,13 @@ class TestDelegate : public URLRequest::Delegate {
         cancel_in_rd_(false),
         cancel_in_rd_pending_(false),
         quit_on_complete_(true),
+        allow_certificate_errors_(false),
         response_started_count_(0),
         received_bytes_count_(0),
         received_redirect_count_(0),
         received_data_before_response_(false),
         request_failed_(false),
+        have_certificate_errors_(false),
         buf_(new net::IOBuffer(kBufferSize)) {
   }
 
@@ -158,10 +160,14 @@ class TestDelegate : public URLRequest::Delegate {
   virtual void OnSSLCertificateError(URLRequest* request,
                                      int cert_error,
                                      net::X509Certificate* cert) {
-    // Ignore SSL errors, we test the server is started and shut it down by
-    // performing GETs, no security restrictions should apply as we always want
-    // these GETs to go through.
-    request->ContinueDespiteLastError();
+    // The caller can control whether it needs all SSL requests to go through,
+    // independent of any possible errors, or whether it wants SSL errors to
+    // cancel the request.
+    have_certificate_errors_ = true;
+    if (allow_certificate_errors_)
+      request->ContinueDespiteLastError();
+    else
+      request->Cancel();
   }
 
   void set_cancel_in_received_redirect(bool val) { cancel_in_rr_ = val; }
@@ -171,6 +177,9 @@ class TestDelegate : public URLRequest::Delegate {
     cancel_in_rd_pending_ = val;
   }
   void set_quit_on_complete(bool val) { quit_on_complete_ = val; }
+  void set_allow_certificate_errors(bool val) {
+    allow_certificate_errors_ = val;
+  }
   void set_username(const std::wstring& u) { username_ = u; }
   void set_password(const std::wstring& p) { password_ = p; }
 
@@ -183,6 +192,7 @@ class TestDelegate : public URLRequest::Delegate {
     return received_data_before_response_;
   }
   bool request_failed() const { return request_failed_; }
+  bool have_certificate_errors() const { return have_certificate_errors_; }
 
  private:
   static const int kBufferSize = 4096;
@@ -192,6 +202,7 @@ class TestDelegate : public URLRequest::Delegate {
   bool cancel_in_rd_;
   bool cancel_in_rd_pending_;
   bool quit_on_complete_;
+  bool allow_certificate_errors_;
 
   std::wstring username_;
   std::wstring password_;
@@ -202,6 +213,7 @@ class TestDelegate : public URLRequest::Delegate {
   int received_redirect_count_;
   bool received_data_before_response_;
   bool request_failed_;
+  bool have_certificate_errors_;
   std::string data_received_;
 
   // our read buffer