Revert 289433 "Refactor pooling logic into a helper method"

(Test-only?) leaks (see bug). The lsan suppressions file tells me to
revert, not suppress.

BUG=404833

> Refactor pooling logic into a helper method
> Disable pooling when there are cert errors.
> Disable pooling when pinning does not match for the new host.
> 
> BUG=398925
> 
> Review URL: https://codereview.chromium.org/425803014

TBR=rch@chromium.org

Review URL: https://codereview.chromium.org/483043002

Cr-Commit-Position: refs/heads/master@{#290384}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@290384 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 15 insertions, 219 deletions

diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc
index 51c6ee7..64e2c04 100644
--- a/net/spdy/spdy_session.cc
+++ b/net/spdy/spdy_session.cc
@@ -29,12 +29,10 @@
 #include "net/base/net_log.h"
 #include "net/base/net_util.h"
 #include "net/cert/asn1_util.h"
-#include "net/cert/cert_verify_result.h"
 #include "net/http/http_log_util.h"
 #include "net/http/http_network_session.h"
 #include "net/http/http_server_properties.h"
 #include "net/http/http_util.h"
-#include "net/http/transport_security_state.h"
 #include "net/spdy/spdy_buffer_producer.h"
 #include "net/spdy/spdy_frame_builder.h"
 #include "net/spdy/spdy_http_utils.h"
@@ -531,47 +529,9 @@ SpdySession::PushedStreamInfo::PushedStreamInfo(
 
 SpdySession::PushedStreamInfo::~PushedStreamInfo() {}
 
-// static
-bool SpdySession::CanPool(TransportSecurityState* transport_security_state,
-                          const SSLInfo& ssl_info,
-                          const std::string& old_hostname,
-                          const std::string& new_hostname) {
-  // Pooling is prohibited if the server cert is not valid for the new domain,
-  // and for connections on which client certs were sent. It is also prohibited
-  // when channel ID was sent if the hosts are from different eTLDs+1.
-  if (IsCertStatusError(ssl_info.cert_status))
-    return false;
-
-  if (ssl_info.client_cert_sent)
-    return false;
-
-  if (ssl_info.channel_id_sent &&
-      ChannelIDService::GetDomainForHost(new_hostname) !=
-      ChannelIDService::GetDomainForHost(old_hostname)) {
-    return false;
-  }
-
-  bool unused = false;
-  if (!ssl_info.cert->VerifyNameMatch(new_hostname, &unused))
-    return false;
-
-  std::string pinning_failure_log;
-  if (!transport_security_state->CheckPublicKeyPins(
-          new_hostname,
-          true, /* sni_available */
-          ssl_info.is_issued_by_known_root,
-          ssl_info.public_key_hashes,
-          &pinning_failure_log)) {
-    return false;
-  }
-
-  return true;
-}
-
 SpdySession::SpdySession(
     const SpdySessionKey& spdy_session_key,
     const base::WeakPtr<HttpServerProperties>& http_server_properties,
-    TransportSecurityState* transport_security_state,
     bool verify_domain_authentication,
     bool enable_sending_initial_data,
     bool enable_compression,
@@ -587,7 +547,6 @@ SpdySession::SpdySession(
       spdy_session_key_(spdy_session_key),
       pool_(NULL),
       http_server_properties_(http_server_properties),
-      transport_security_state_(transport_security_state),
       read_buffer_(new IOBuffer(kReadBufferSize)),
       stream_hi_water_mark_(kFirstStreamId),
       num_pushed_streams_(0u),
@@ -755,8 +714,18 @@ bool SpdySession::VerifyDomainAuthentication(const std::string& domain) {
   if (!GetSSLInfo(&ssl_info, &was_npn_negotiated, &protocol_negotiated))
     return true;   // This is not a secure session, so all domains are okay.
 
-  return CanPool(transport_security_state_, ssl_info,
-                 host_port_pair().host(), domain);
+  // Disable pooling for secure sessions.
+  // TODO(rch): re-enable this.
+  return false;
+#if 0
+  bool unused = false;
+  return
+      !ssl_info.client_cert_sent &&
+      (!ssl_info.channel_id_sent ||
+       (ChannelIDService::GetDomainForHost(domain) ==
+        ChannelIDService::GetDomainForHost(host_port_pair().host()))) &&
+      ssl_info.cert->VerifyNameMatch(domain, &unused);
+#endif
 }
 
 int SpdySession::GetPushStream(
diff --git a/net/spdy/spdy_session.h b/net/spdy/spdy_session.h
index d2da863..4037e03 100644
--- a/net/spdy/spdy_session.h
+++ b/net/spdy/spdy_session.h
@@ -70,7 +70,6 @@ class BoundNetLog;
 struct LoadTimingInfo;
 class SpdyStream;
 class SSLInfo;
-class TransportSecurityState;
 
 // NOTE: There's an enum of the same name (also with numeric suffixes)
 // in histograms.xml. Be sure to add new values there also.
@@ -223,13 +222,6 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
     FLOW_CONTROL_STREAM_AND_SESSION
   };
 
-  // Returns true if |hostname| can be pooled into an existing connection
-  // associated with |ssl_info|.
-  static bool CanPool(TransportSecurityState* transport_security_state,
-                      const SSLInfo& ssl_info,
-                      const std::string& old_hostname,
-                      const std::string& new_hostname);
-
   // Create a new SpdySession.
   // |spdy_session_key| is the host/port that this session connects to, privacy
   // and proxy configuration settings that it's using.
@@ -237,7 +229,6 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
   // network events to.
   SpdySession(const SpdySessionKey& spdy_session_key,
               const base::WeakPtr<HttpServerProperties>& http_server_properties,
-              TransportSecurityState* transport_security_state,
               bool verify_domain_authentication,
               bool enable_sending_initial_data,
               bool enable_compression,
@@ -972,8 +963,6 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
   SpdySessionPool* pool_;
   const base::WeakPtr<HttpServerProperties> http_server_properties_;
 
-  TransportSecurityState* transport_security_state_;
-
   // The socket handle for this session.
   scoped_ptr<ClientSocketHandle> connection_;
 
diff --git a/net/spdy/spdy_session_pool.cc b/net/spdy/spdy_session_pool.cc
index d839785..189c945 100644
--- a/net/spdy/spdy_session_pool.cc
+++ b/net/spdy/spdy_session_pool.cc
@@ -31,7 +31,6 @@ SpdySessionPool::SpdySessionPool(
     HostResolver* resolver,
     SSLConfigService* ssl_config_service,
     const base::WeakPtr<HttpServerProperties>& http_server_properties,
-    TransportSecurityState* transport_security_state,
     bool force_single_domain,
     bool enable_compression,
     bool enable_ping_based_connection_checking,
@@ -42,7 +41,6 @@ SpdySessionPool::SpdySessionPool(
     SpdySessionPool::TimeFunc time_func,
     const std::string& trusted_spdy_proxy)
     : http_server_properties_(http_server_properties),
-      transport_security_state_(transport_security_state),
       ssl_config_service_(ssl_config_service),
       resolver_(resolver),
       verify_domain_authentication_(true),
@@ -100,7 +98,6 @@ base::WeakPtr<SpdySession> SpdySessionPool::CreateAvailableSessionFromSocket(
   scoped_ptr<SpdySession> new_session(
       new SpdySession(key,
                       http_server_properties_,
-                      transport_security_state_,
                       verify_domain_authentication_,
                       enable_sending_initial_data_,
                       enable_compression_,
diff --git a/net/spdy/spdy_session_pool.h b/net/spdy/spdy_session_pool.h
index 2fbb030..0fad5d2 100644
--- a/net/spdy/spdy_session_pool.h
+++ b/net/spdy/spdy_session_pool.h
@@ -34,7 +34,6 @@ class ClientSocketHandle;
 class HostResolver;
 class HttpServerProperties;
 class SpdySession;
-class TransportSecurityState;
 
 // This is a very simple pool for open SpdySessions.
 class NET_EXPORT SpdySessionPool
@@ -51,7 +50,6 @@ class NET_EXPORT SpdySessionPool
       HostResolver* host_resolver,
       SSLConfigService* ssl_config_service,
       const base::WeakPtr<HttpServerProperties>& http_server_properties,
-      TransportSecurityState* transport_security_state,
       bool force_single_domain,
       bool enable_compression,
       bool enable_ping_based_connection_checking,
@@ -193,8 +191,6 @@ class NET_EXPORT SpdySessionPool
 
   const base::WeakPtr<HttpServerProperties> http_server_properties_;
 
-  TransportSecurityState* transport_security_state_;
-
   // The set of all sessions. This is a superset of the sessions in
   // |available_sessions_|.
   //
diff --git a/net/spdy/spdy_session_unittest.cc b/net/spdy/spdy_session_unittest.cc
index 8194fe7..1fa5f2e 100644
--- a/net/spdy/spdy_session_unittest.cc
+++ b/net/spdy/spdy_session_unittest.cc
@@ -4,7 +4,6 @@
 
 #include "net/spdy/spdy_session.h"
 
-#include "base/base64.h"
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
@@ -2376,7 +2375,7 @@ TEST_P(SpdySessionTest, CloseActivatedStreamThatClosesSession) {
   EXPECT_TRUE(session == NULL);
 }
 
-TEST_P(SpdySessionTest, VerifyDomainAuthentication) {
+TEST_P(SpdySessionTest, DISABLED_VerifyDomainAuthentication) {
   session_deps_.host_resolver->set_synchronous_mode(true);
 
   MockConnect connect_data(SYNCHRONOUS, OK);
@@ -2418,7 +2417,8 @@ TEST_P(SpdySessionTest, VerifyDomainAuthentication) {
   EXPECT_FALSE(session->VerifyDomainAuthentication("mail.google.com"));
 }
 
-TEST_P(SpdySessionTest, ConnectionPooledWithTlsChannelId) {
+// TODO(rch): re-enable this.
+TEST_P(SpdySessionTest, DISABLED_ConnectionPooledWithTlsChannelId) {
   session_deps_.host_resolver->set_synchronous_mode(true);
 
   MockConnect connect_data(SYNCHRONOUS, OK);
@@ -5001,108 +5001,4 @@ TEST(MapNetErrorToGoAwayStatus, MapsValue) {
   CHECK_EQ(GOAWAY_PROTOCOL_ERROR, MapNetErrorToGoAwayStatus(ERR_UNEXPECTED));
 }
 
-TEST(CanPoolTest, CanPool) {
-  // Load a cert that is valid for:
-  //   www.example.org
-  //   mail.example.org
-  //   www.example.com
-
-  TransportSecurityState tss;
-  SSLInfo ssl_info;
-  ssl_info.cert = ImportCertFromFile(GetTestCertsDirectory(),
-                                     "spdy_pooling.pem");
-
-  EXPECT_TRUE(SpdySession::CanPool(
-      &tss, ssl_info, "www.example.org", "www.example.org"));
-  EXPECT_TRUE(SpdySession::CanPool(
-      &tss, ssl_info, "www.example.org", "mail.example.org"));
-  EXPECT_TRUE(SpdySession::CanPool(
-      &tss, ssl_info, "www.example.org", "mail.example.com"));
-  EXPECT_FALSE(SpdySession::CanPool(
-      &tss, ssl_info, "www.example.org", "mail.google.com"));
-}
-
-TEST(CanPoolTest, CanNotPoolWithCertErrors) {
-  // Load a cert that is valid for:
-  //   www.example.org
-  //   mail.example.org
-  //   www.example.com
-
-  TransportSecurityState tss;
-  SSLInfo ssl_info;
-  ssl_info.cert = ImportCertFromFile(GetTestCertsDirectory(),
-                                     "spdy_pooling.pem");
-  ssl_info.cert_status = CERT_STATUS_REVOKED;
-
-  EXPECT_FALSE(SpdySession::CanPool(
-      &tss, ssl_info, "www.example.org", "mail.example.org"));
-}
-
-TEST(CanPoolTest, CanNotPoolWithClientCerts) {
-  // Load a cert that is valid for:
-  //   www.example.org
-  //   mail.example.org
-  //   www.example.com
-
-  TransportSecurityState tss;
-  SSLInfo ssl_info;
-  ssl_info.cert = ImportCertFromFile(GetTestCertsDirectory(),
-                                     "spdy_pooling.pem");
-  ssl_info.client_cert_sent = true;
-
-  EXPECT_FALSE(SpdySession::CanPool(
-      &tss, ssl_info, "www.example.org", "mail.example.org"));
-}
-
-TEST(CanPoolTest, CanNotPoolAcrossETLDsWithChannelID) {
-  // Load a cert that is valid for:
-  //   www.example.org
-  //   mail.example.org
-  //   www.example.com
-
-  TransportSecurityState tss;
-  SSLInfo ssl_info;
-  ssl_info.cert = ImportCertFromFile(GetTestCertsDirectory(),
-                                     "spdy_pooling.pem");
-  ssl_info.channel_id_sent = true;
-
-  EXPECT_TRUE(SpdySession::CanPool(
-      &tss, ssl_info, "www.example.org", "mail.example.org"));
-  EXPECT_FALSE(SpdySession::CanPool(
-      &tss, ssl_info, "www.example.org", "www.example.com"));
-}
-
-TEST(CanPoolTest, CanNotPoolWithBadPins) {
-  uint8 primary_pin = 1;
-  uint8 backup_pin = 2;
-  uint8 bad_pin = 3;
-  TransportSecurityState tss;
-  test::AddPin(&tss, "mail.example.org", primary_pin, backup_pin);
-
-  SSLInfo ssl_info;
-  ssl_info.cert = ImportCertFromFile(GetTestCertsDirectory(),
-                                     "spdy_pooling.pem");
-  ssl_info.is_issued_by_known_root = true;
-  ssl_info.public_key_hashes.push_back(test::GetTestHashValue(bad_pin));
-
-  EXPECT_FALSE(SpdySession::CanPool(
-      &tss, ssl_info, "www.example.org", "mail.example.org"));
-}
-
-TEST(CanPoolTest, CanPoolWithAcceptablePins) {
-  uint8 primary_pin = 1;
-  uint8 backup_pin = 2;
-  TransportSecurityState tss;
-  test::AddPin(&tss, "mail.example.org", primary_pin, backup_pin);
-
-  SSLInfo ssl_info;
-  ssl_info.cert = ImportCertFromFile(GetTestCertsDirectory(),
-                                     "spdy_pooling.pem");
-  ssl_info.is_issued_by_known_root = true;
-  ssl_info.public_key_hashes.push_back(test::GetTestHashValue(primary_pin));
-
-  EXPECT_TRUE(SpdySession::CanPool(
-      &tss, ssl_info, "www.example.org", "mail.example.org"));
-}
-
 }  // namespace net
diff --git a/net/spdy/spdy_test_utils.cc b/net/spdy/spdy_test_utils.cc
index 7627abe..e33a08b 100644
--- a/net/spdy/spdy_test_utils.cc
+++ b/net/spdy/spdy_test_utils.cc
@@ -7,13 +7,10 @@
 #include <cstring>
 #include <vector>
 
-#include "base/base64.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/sys_byteorder.h"
-#include "net/http/transport_security_state.h"
-#include "net/ssl/ssl_info.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace net {
@@ -142,36 +139,6 @@ std::string a2b_hex(const char* hex_data) {
   return result;
 }
 
-HashValue GetTestHashValue(uint8_t label) {
-  HashValue hash_value(HASH_VALUE_SHA256);
-  memset(hash_value.data(), label, hash_value.size());
-  return hash_value;
-}
-
-std::string GetTestPin(uint8_t label) {
-  HashValue hash_value = GetTestHashValue(label);
-  std::string base64;
-  base::Base64Encode(base::StringPiece(
-      reinterpret_cast<char*>(hash_value.data()), hash_value.size()), &base64);
-
-  return std::string("pin-sha256=\"") + base64 + "\"";
-}
-
-void AddPin(TransportSecurityState* state,
-            const std::string& host,
-            uint8_t primary_label,
-            uint8_t backup_label) {
-  std::string primary_pin = GetTestPin(primary_label);
-  std::string backup_pin = GetTestPin(backup_label);
-  std::string header = "max-age = 10000; " + primary_pin + "; " + backup_pin;
-
-  // Construct a fake SSLInfo that will pass AddHPKPHeader's checks.
-  SSLInfo ssl_info;
-  ssl_info.is_issued_by_known_root = true;
-  ssl_info.public_key_hashes.push_back(GetTestHashValue(primary_label));
-  EXPECT_TRUE(state->AddHPKPHeader(host, header, ssl_info));
-}
-
 }  // namespace test
 
 }  // namespace net
diff --git a/net/spdy/spdy_test_utils.h b/net/spdy/spdy_test_utils.h
index 14bc8ef..439311e 100644
--- a/net/spdy/spdy_test_utils.h
+++ b/net/spdy/spdy_test_utils.h
@@ -5,17 +5,12 @@
 #ifndef NET_SPDY_TEST_UTILS_H_
 #define NET_SPDY_TEST_UTILS_H_
 
-#include <stdint.h>
-
 #include <string>
 
 #include "net/spdy/spdy_protocol.h"
 
 namespace net {
 
-class HashValue;
-class TransportSecurityState;
-
 namespace test {
 
 std::string HexDumpWithMarks(const unsigned char* data, int length,
@@ -38,19 +33,6 @@ void SetFrameLength(SpdyFrame* frame,
 
 std::string a2b_hex(const char* hex_data);
 
-// Returns a SHA1 HashValue in which each byte has the value |label|.
-HashValue GetTestHashValue(uint8_t label);
-
-// Returns SHA1 pinning header for the of the base64 encoding of
-// GetTestHashValue(|label|).
-std::string GetTestPin(uint8_t label);
-
-// Adds a pin for |host| to |state|.
-void AddPin(TransportSecurityState* state,
-            const std::string& host,
-            uint8_t primary_label,
-            uint8_t backup_label);
-
 }  // namespace test
 
 }  // namespace net