Make HSTS headers not clobber preloaded pins.

net/http/http_security_headers_unittest.cc

Index: net/http/http_security_headers_unittest.cc
diff --git a/net/http/http_security_headers_unittest.cc b/net/http/http_security_headers_unittest.cc
index 42a5ee9896062504e21575f23519983d5caa9652..2d778e781370f8e8348c45cd083fee0071240377 100644
--- a/net/http/http_security_headers_unittest.cc
+++ b/net/http/http_security_headers_unittest.cc
@@ -450,13 +450,15 @@ TEST_F(HttpSecurityHeadersTest, ValidPKPHeadersSHA256) {
 
 TEST_F(HttpSecurityHeadersTest, UpdateDynamicPKPOnly) {
   TransportSecurityState state;
-  TransportSecurityState::DomainState domain_state;
+  TransportSecurityState::DomainState static_domain_state;
 
   // docs.google.com has preloaded pins.
+  const bool sni_enabled = true;
   std::string domain = "docs.google.com";
-  EXPECT_TRUE(state.GetDomainState(domain, true, &domain_state));
-  EXPECT_GT(domain_state.static_spki_hashes.size(), 1UL);
-  HashValueVector saved_hashes = domain_state.static_spki_hashes;
+  EXPECT_TRUE(
+      state.GetStaticDomainState(domain, sni_enabled, &static_domain_state));
+  EXPECT_GT(static_domain_state.pkp.spki_hashes.size(), 1UL);
+  HashValueVector saved_hashes = static_domain_state.pkp.spki_hashes;
 
   // Add a header, which should only update the dynamic state.
   HashValue good_hash = GetTestHashValue(1, HASH_VALUE_SHA1);
@@ -471,49 +473,99 @@ TEST_F(HttpSecurityHeadersTest, UpdateDynamicPKPOnly) {
   ssl_info.public_key_hashes.push_back(saved_hashes[0]);
   EXPECT_TRUE(state.AddHPKPHeader(domain, header, ssl_info));
 
-  // Expect the preloaded state to remain unchanged.
-  std::string canonicalized_host = TransportSecurityState::CanonicalizeHost(
-      domain);
-  TransportSecurityState::DomainState static_domain_state;
-  EXPECT_TRUE(state.GetStaticDomainState(canonicalized_host,
-                                         true,
-                                         &static_domain_state));
+  // Expect the static state to remain unchanged.
+  TransportSecurityState::DomainState new_static_domain_state;
+  EXPECT_TRUE(state.GetStaticDomainState(
+      domain, sni_enabled, &new_static_domain_state));
   for (size_t i = 0; i < saved_hashes.size(); ++i) {
-    EXPECT_TRUE(HashValuesEqual(
-        saved_hashes[i])(static_domain_state.static_spki_hashes[i]));
+    EXPECT_TRUE(HashValuesEqual(saved_hashes[i])(
+        new_static_domain_state.pkp.spki_hashes[i]));
   }
 
   // Expect the dynamic state to reflect the header.
   TransportSecurityState::DomainState dynamic_domain_state;
   EXPECT_TRUE(state.GetDynamicDomainState(domain, &dynamic_domain_state));
-  EXPECT_EQ(2UL, dynamic_domain_state.dynamic_spki_hashes.size());
+  EXPECT_EQ(2UL, dynamic_domain_state.pkp.spki_hashes.size());
 
-  HashValueVector::const_iterator hash = std::find_if(
-      dynamic_domain_state.dynamic_spki_hashes.begin(),
-      dynamic_domain_state.dynamic_spki_hashes.end(),
-      HashValuesEqual(good_hash));
-  EXPECT_NE(dynamic_domain_state.dynamic_spki_hashes.end(), hash);
+  HashValueVector::const_iterator hash =
+      std::find_if(dynamic_domain_state.pkp.spki_hashes.begin(),
+                   dynamic_domain_state.pkp.spki_hashes.end(),
+                   HashValuesEqual(good_hash));
+  EXPECT_NE(dynamic_domain_state.pkp.spki_hashes.end(), hash);
 
-  hash = std::find_if(
-      dynamic_domain_state.dynamic_spki_hashes.begin(),
-      dynamic_domain_state.dynamic_spki_hashes.end(),
-      HashValuesEqual(backup_hash));
-  EXPECT_NE(dynamic_domain_state.dynamic_spki_hashes.end(), hash);
+  hash = std::find_if(dynamic_domain_state.pkp.spki_hashes.begin(),
+                      dynamic_domain_state.pkp.spki_hashes.end(),
+                      HashValuesEqual(backup_hash));
+  EXPECT_NE(dynamic_domain_state.pkp.spki_hashes.end(), hash);
 
   // Expect the overall state to reflect the header, too.
-  EXPECT_TRUE(state.GetDomainState(domain, true, &domain_state));
-  EXPECT_EQ(2UL, domain_state.dynamic_spki_hashes.size());
+  EXPECT_TRUE(state.HasPublicKeyPins(domain, sni_enabled));
+  HashValueVector hashes;
+  hashes.push_back(good_hash);
+  std::string failure_log;
+  EXPECT_TRUE(
+      state.CheckPublicKeyPins(domain, sni_enabled, hashes, &failure_log));
+
+  TransportSecurityState::DomainState new_dynamic_domain_state;
+  EXPECT_TRUE(state.GetDynamicDomainState(domain, &new_dynamic_domain_state));
+  EXPECT_EQ(2UL, new_dynamic_domain_state.pkp.spki_hashes.size());
 
-  hash = std::find_if(domain_state.dynamic_spki_hashes.begin(),
-                      domain_state.dynamic_spki_hashes.end(),
+  hash = std::find_if(new_dynamic_domain_state.pkp.spki_hashes.begin(),
+                      new_dynamic_domain_state.pkp.spki_hashes.end(),
                       HashValuesEqual(good_hash));
-  EXPECT_NE(domain_state.dynamic_spki_hashes.end(), hash);
+  EXPECT_NE(new_dynamic_domain_state.pkp.spki_hashes.end(), hash);
+
+  hash = std::find_if(new_dynamic_domain_state.pkp.spki_hashes.begin(),
+                      new_dynamic_domain_state.pkp.spki_hashes.end(),
+                      HashValuesEqual(backup_hash));
+  EXPECT_NE(new_dynamic_domain_state.pkp.spki_hashes.end(), hash);
+}
+
+TEST_F(HttpSecurityHeadersTest, NoClobberPins) {

[Ryan Sleevi, 2014/04/24 21:40:21]

Document test

// Tests that when a static HSTS and a static HPKP entry are present,
// adding a dynamic HSTS header does not clobber the static HPKP entry.
// Further, adding a dynamic HPKP entry could not affect the HSTS entry
// for the site.

[palmer, 2014/04/25 00:59:59]

Done.

+  TransportSecurityState state;
+  TransportSecurityState::DomainState domain_state;
+
+  // accounts.google.com has preloaded pins.
+  std::string domain = "accounts.google.com";
+
+  // Retrieve the DomainState as it is by default, including its known good
+  // pins.
+  const bool sni_enabled = true;
+  EXPECT_TRUE(state.GetStaticDomainState(domain, sni_enabled, &domain_state));
+  HashValueVector saved_hashes = domain_state.pkp.spki_hashes;
+  EXPECT_TRUE(domain_state.ShouldUpgradeToSSL());
+  EXPECT_TRUE(domain_state.HasPublicKeyPins());
+  EXPECT_TRUE(state.ShouldUpgradeToSSL(domain, sni_enabled));
+  EXPECT_TRUE(state.HasPublicKeyPins(domain, sni_enabled));
+
+  // Add a dynamic HSTS header. CheckPublicKeyPins should still pass when given
+  // the original |saved_hashes|, indicating that the static PKP data is still
+  // configured for the domain.
+  EXPECT_TRUE(state.AddHSTSHeader(domain, "includesubdomains; max-age=10000"));
+  EXPECT_TRUE(state.ShouldUpgradeToSSL(domain, sni_enabled));
+  std::string failure_log;
+  EXPECT_TRUE(state.CheckPublicKeyPins(
+      domain, sni_enabled, saved_hashes, &failure_log));
+
+  // Add an HPKP header, which should only update the dynamic state.
+  HashValue good_hash = GetTestHashValue(1, HASH_VALUE_SHA1);
+  std::string good_pin = GetTestPin(1, HASH_VALUE_SHA1);
+  std::string backup_pin = GetTestPin(2, HASH_VALUE_SHA1);
+  std::string header = "max-age = 10000; " + good_pin + "; " + backup_pin;
 
-  hash = std::find_if(
-      domain_state.dynamic_spki_hashes.begin(),
-      domain_state.dynamic_spki_hashes.end(),
-      HashValuesEqual(backup_hash));
-  EXPECT_NE(domain_state.dynamic_spki_hashes.end(), hash);
+  // Construct a fake SSLInfo that will pass AddHPKPHeader's checks.
+  SSLInfo ssl_info;
+  ssl_info.public_key_hashes.push_back(good_hash);
+  ssl_info.public_key_hashes.push_back(saved_hashes[0]);
+  EXPECT_TRUE(state.AddHPKPHeader(domain, header, ssl_info));
+
+  EXPECT_TRUE(state.AddHPKPHeader(domain, header, ssl_info));
+  // HSTS should still be configured for this domain.
+  EXPECT_TRUE(domain_state.ShouldUpgradeToSSL());
+  EXPECT_TRUE(state.ShouldUpgradeToSSL(domain, sni_enabled));
+  // Check that a good pin is still valid.
+  EXPECT_TRUE(state.CheckPublicKeyPins(
+      domain, sni_enabled, saved_hashes, &failure_log));
 }
 
 };    // namespace net