net: add a test to ensure that our TLS handshake doesn't get too large.

net/url_request/url_request_unittest.cc

Index: net/url_request/url_request_unittest.cc
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index e3a3be87753fd552f242db38558a6d87c7b6e522..51e2eae0eec15794c19424edac50d5ba28993649 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -5406,6 +5406,44 @@ TEST_F(HTTPSRequestTest, SSLSessionCacheShardTest) {
   }
 }
 
+TEST_F(HTTPSRequestTest, ClientHelloTest) {
+  // Check that our ClientHello doesn't get too large because that's known to
+  // cause issues.
+
+  SpawnedTestServer::SSLOptions ssl_options;
+  SpawnedTestServer test_server(
+      SpawnedTestServer::TYPE_HTTPS,
+      ssl_options,
+      base::FilePath(FILE_PATH_LITERAL("net/data/ssl")));
+  ASSERT_TRUE(test_server.Start());
+
+  HttpStreamFactory::EnableNpnSpdy4a2();

[Ryan Sleevi, 2013/07/23 20:50:02]

Need a comment explaining why this is being called - and why the test is leaving
this state enabled if any ASSERTs fail / when the test ends.

[agl, 2013/07/24 18:09:18]

Done. This will be reset by NetTestSuite; have noted that in the comment.

+
+  TestDelegate d;
+  URLRequest r(
+      test_server.GetURL("client-hello-length"), &d, &default_context_);
+
+  r.Start();
+  EXPECT_TRUE(r.is_pending());
+
+  base::MessageLoop::current()->Run();
+
+  // The response will be a single, base 10 number which is the number of bytes
+  // in the ClientHello, including the handshake message type byte and 3 byte
+  // length at the beginning.
+
+  EXPECT_EQ(1, d.response_started_count());
+  unsigned client_hello_length;
+  ASSERT_TRUE(base::StringToUint(d.data_received(), &client_hello_length));
+  const unsigned sni_overhead = 2 /* extension type */ +
+                                2 /* extension length */ +
+                                4 /* lengths in the server_name extension */ +
+                                strlen("www.wellsfargo.com");

[Ryan Sleevi, 2013/07/23 20:50:02]

I don't quite get why you chose this, or incorporated into the length check.

It seems we'd run just as much risk with "www2.wellsfargo.com" hitting the 256
byte limit.

Would we kill a ciphersuite for that server? If not, I'm not sure why the SNI
overhead check.

[agl, 2013/07/24 18:09:18]

www.wellsfargo.com is probably the biggest site with this issue, so we would
want to know if we're going to break it.

I agree, it would be nice to assert a stronger bound so that www2.wellsfargo.com
doesn't have a problem, but we need to do some work to shrink the ClientHello
first.

+  static const unsigned session_id_overhead = 32;
+
+  EXPECT_LT(client_hello_length + sni_overhead + session_id_overhead, 256u);
+}
+
 class TestSSLConfigService : public SSLConfigService {
  public:
   TestSSLConfigService(bool ev_enabled, bool online_rev_checking)