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

net/url_request/url_request_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "build/build_config.h"
 
 #if defined(OS_WIN)
 #include <windows.h>
 #include <shlobj.h>
 #endif
@@ skipping 5388 matching lines @@
       EXPECT_EQ("insert", parts[0]);
       if (i == 0) {
         session_id = parts[1];
       } else {
         EXPECT_NE(session_id, parts[1]);
       }
     }
   }
 }
 
+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)
       : ev_enabled_(ev_enabled),
         online_rev_checking_(online_rev_checking) {
   }
 
   // SSLConfigService:
   virtual void GetSSLConfig(SSLConfig* config) OVERRIDE {
     *config = SSLConfig();
@@ skipping 694 matching lines @@
 
     EXPECT_FALSE(r.is_pending());
     EXPECT_EQ(1, d->response_started_count());
     EXPECT_FALSE(d->received_data_before_response());
     EXPECT_EQ(d->bytes_received(), static_cast<int>(file_size));
   }
 }
 #endif  // !defined(DISABLE_FTP_SUPPORT)
 
 }  // namespace net