Do not cache Strict-Transport-Security and Public-Key-Pins headers.

This stops them from being honored (since they are absent) when loading
pages from cache. These headers should only take effect on live, error-free
HTTPS connections.

BUG=110817
TEST=net_unittests, HTTPSRequestTest.HTTPSErrorsNoClobberTSSTest

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

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

5 files changed, 114 insertions, 4 deletions

diff --git a/net/http/http_response_headers.cc b/net/http/http_response_headers.cc
index 61fed43..4916498 100644
--- a/net/http/http_response_headers.cc
+++ b/net/http/http_response_headers.cc
@@ -56,6 +56,15 @@ const char* const kCookieResponseHeaders[] = {
   "set-cookie2"
 };
 
+// By default, do not cache Strict-Transport-Security or Public-Key-Pins.
+// This avoids erroneously re-processing them on page loads from cache ---
+// they are defined to be valid only on live and error-free HTTPS
+// connections.
+const char* const kSecurityStateHeaders[] = {
+  "strict-transport-security",
+  "public-key-pins"
+};
+
 // These response headers are not copied from a 304/206 response to the cached
 // response headers.  This list is based on Mozilla's nsHttpResponseHead.cpp.
 const char* const kNonUpdatedHeaders[] = {
@@ -191,6 +200,9 @@ void HttpResponseHeaders::Persist(Pickle* pickle, PersistOptions options) {
   if ((options & PERSIST_SANS_RANGES) == PERSIST_SANS_RANGES)
     AddHopContentRangeHeaders(&filter_headers);
 
+  if ((options & PERSIST_SANS_SECURITY_STATE) == PERSIST_SANS_SECURITY_STATE)
+    AddSecurityStateHeaders(&filter_headers);
+
   std::string blob;
   blob.reserve(raw_headers_.size());
 
@@ -832,6 +844,11 @@ void HttpResponseHeaders::AddHopContentRangeHeaders(HeaderSet* result) {
   result->insert("content-range");
 }
 
+void HttpResponseHeaders::AddSecurityStateHeaders(HeaderSet* result) {
+  for (size_t i = 0; i < arraysize(kSecurityStateHeaders); ++i)
+    result->insert(std::string(kSecurityStateHeaders[i]));
+}
+
 void HttpResponseHeaders::GetMimeTypeAndCharset(std::string* mime_type,
                                                 std::string* charset) const {
   mime_type->clear();
diff --git a/net/http/http_response_headers.h b/net/http/http_response_headers.h
index c4636a6..0ab162e 100644
--- a/net/http/http_response_headers.h
+++ b/net/http/http_response_headers.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -37,6 +37,7 @@ class NET_EXPORT HttpResponseHeaders
   static const PersistOptions PERSIST_SANS_HOP_BY_HOP = 1 << 2;
   static const PersistOptions PERSIST_SANS_NON_CACHEABLE = 1 << 3;
   static const PersistOptions PERSIST_SANS_RANGES = 1 << 4;
+  static const PersistOptions PERSIST_SANS_SECURITY_STATE = 1 << 5;
 
   // Parses the given raw_headers.  raw_headers should be formatted thus:
   // includes the http status response line, each line is \0-terminated, and
@@ -335,6 +336,9 @@ class NET_EXPORT HttpResponseHeaders
   // Adds the set of content range response headers.
   static void AddHopContentRangeHeaders(HeaderSet* header_names);
 
+  // Adds the set of transport security state headers.
+  static void AddSecurityStateHeaders(HeaderSet* header_names);
+
   // We keep a list of ParsedHeader objects.  These tell us where to locate the
   // header-value pairs within raw_headers_.
   HeaderList parsed_;
diff --git a/net/http/http_response_headers_unittest.cc b/net/http/http_response_headers_unittest.cc
index eec259a..775946c 100644
--- a/net/http/http_response_headers_unittest.cc
+++ b/net/http/http_response_headers_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -452,6 +452,18 @@ TEST(HttpResponseHeadersTest, Persist) {
       "Content-Length: 450\n"
       "Content-Encoding: gzip\n"
     },
+    // Test filtering of transport security state headers.
+    { net::HttpResponseHeaders::PERSIST_SANS_SECURITY_STATE,
+      "HTTP/1.1 200 OK\n"
+      "Strict-Transport-Security: max-age=1576800\n"
+      "Bar: 1\n"
+      "Public-Key-Pins: max-age=100000; "
+          "pin-sha1=\"ObT42aoSpAqWdY9WfRfL7i0HsVk=\";"
+          "pin-sha1=\"7kW49EVwZG0hSNx41ZO/fUPN0ek=\"",
+
+      "HTTP/1.1 200 OK\n"
+      "Bar: 1\n"
+    },
   };
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
diff --git a/net/http/http_response_info.cc b/net/http/http_response_info.cc
index 50cd388..951e2b6 100644
--- a/net/http/http_response_info.cc
+++ b/net/http/http_response_info.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -250,7 +250,8 @@ void HttpResponseInfo::Persist(Pickle* pickle,
         net::HttpResponseHeaders::PERSIST_SANS_CHALLENGES |
         net::HttpResponseHeaders::PERSIST_SANS_HOP_BY_HOP |
         net::HttpResponseHeaders::PERSIST_SANS_NON_CACHEABLE |
-        net::HttpResponseHeaders::PERSIST_SANS_RANGES;
+        net::HttpResponseHeaders::PERSIST_SANS_RANGES |
+        net::HttpResponseHeaders::PERSIST_SANS_SECURITY_STATE;
   }
 
   headers->Persist(pickle, persist_options);
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index 287e21b..e6c4c31 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -123,6 +123,20 @@ void CheckSSLInfo(const SSLInfo& ssl_info) {
   EXPECT_NE(0, cipher_suite);
 }
 
+bool FingerprintsEqual(const FingerprintVector& a, const FingerprintVector& b) {
+  size_t size = a.size();
+
+  if (size != b.size())
+    return false;
+
+  for (size_t i = 0; i < size; ++i) {
+    if (!a[i].Equals(b[i]))
+      return false;
+  }
+
+  return true;
+}
+
 }  // namespace
 
 // A network delegate that blocks requests, optionally cancelling or redirecting
@@ -1379,6 +1393,68 @@ TEST_F(HTTPSRequestTest, HTTPSPreloadedHSTSTest) {
   EXPECT_TRUE(d.certificate_errors_are_fatal());
 }
 
+// This tests that cached HTTPS page loads do not cause any updates to the
+// TransportSecurityState.
+TEST_F(HTTPSRequestTest, HTTPSErrorsNoClobberTSSTest) {
+  // The actual problem -- CERT_MISMATCHED_NAME in this case -- doesn't
+  // matter. It just has to be any error.
+  TestServer::HTTPSOptions https_options(
+      TestServer::HTTPSOptions::CERT_MISMATCHED_NAME);
+  TestServer test_server(https_options,
+                         FilePath(FILE_PATH_LITERAL("net/data/ssl")));
+  ASSERT_TRUE(test_server.Start());
+
+  // We require that the URL be www.google.com in order to pick up the
+  // preloaded and dynamic HSTS and public key pin entries in the
+  // TransportSecurityState. This means that we have to use a
+  // MockHostResolver in order to direct www.google.com to the testserver.
+
+  MockHostResolver host_resolver;
+  host_resolver.rules()->AddRule("www.google.com", "127.0.0.1");
+  TestNetworkDelegate network_delegate;  // must outlive URLRequest
+  scoped_refptr<TestURLRequestContext> context(new TestURLRequestContext(true));
+  context->set_network_delegate(&network_delegate);
+  context->set_host_resolver(&host_resolver);
+  TransportSecurityState transport_security_state("");
+  TransportSecurityState::DomainState domain_state;
+  EXPECT_TRUE(transport_security_state.HasMetadata(&domain_state,
+                                                   "www.google.com", true));
+  context->set_transport_security_state(&transport_security_state);
+  context->Init();
+
+  TestDelegate d;
+  TestURLRequest r(GURL(StringPrintf("https://www.google.com:%d",
+                                     test_server.host_port_pair().port())),
+                   &d);
+  r.set_context(context);
+
+  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());
+  EXPECT_TRUE(d.certificate_errors_are_fatal());
+
+  // Get a fresh copy of the state, and check that it hasn't been updated.
+  TransportSecurityState::DomainState new_domain_state;
+  EXPECT_TRUE(transport_security_state.HasMetadata(&new_domain_state,
+                                                   "www.google.com", true));
+  EXPECT_EQ(new_domain_state.mode, domain_state.mode);
+  EXPECT_EQ(new_domain_state.created.ToTimeT(), domain_state.created.ToTimeT());
+  EXPECT_EQ(new_domain_state.expiry.ToTimeT(), domain_state.expiry.ToTimeT());
+  EXPECT_EQ(new_domain_state.include_subdomains,
+            domain_state.include_subdomains);
+  EXPECT_TRUE(FingerprintsEqual(new_domain_state.preloaded_spki_hashes,
+                                domain_state.preloaded_spki_hashes));
+  EXPECT_TRUE(FingerprintsEqual(new_domain_state.dynamic_spki_hashes,
+                                domain_state.dynamic_spki_hashes));
+  EXPECT_TRUE(FingerprintsEqual(new_domain_state.bad_preloaded_spki_hashes,
+                                domain_state.bad_preloaded_spki_hashes));
+}
+
 namespace {
 
 class SSLClientAuthTestDelegate : public TestDelegate {