Normalize hostnames before searching for HSTS/HPKP preloads

The HSTS/HPKP preload set is pre-normalized at construction time.
Since the queries come from the GURL, not from the DNS layer,
we need to normalize the hostname before scanning for preloads.
This used to be handled by CanonicalizeHost(), which used the
same mechanism as the resolver, but the storage of the preloads
has changed to be more efficient, and thus no longer uses the
resolver-normalized form.

BUG=461481
R=davidben@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#330834}

net/http/transport_security_state.h

@@ -193,8 +193,7 @@ class NET_EXPORT TransportSecurityState
   void ClearDynamicData();
 
   // Inserts |state| into |enabled_hosts_| under the key |hashed_host|.
-  // |hashed_host| is already in the internal representation
-  // HashHost(CanonicalizeHost(host)).
+  // |hashed_host| is already in the internal representation.
   // Note: This is only used for serializing/deserializing the
   // TransportSecurityState.
   void AddOrUpdateEnabledHosts(const std::string& hashed_host,
@@ -319,11 +318,6 @@ class NET_EXPORT TransportSecurityState
   // The new state for |host| is persisted using the Delegate (if any).
   void EnableHost(const std::string& host, const DomainState& state);
 
-  // Converts |hostname| from dotted form ("www.google.com") to the form
-  // used in DNS: "\x03www\x06google\x03com", lowercases that, and returns
-  // the result.
-  static std::string CanonicalizeHost(const std::string& hostname);
-
   // The set of hosts that have enabled TransportSecurity. |sts.domain| and
   // |pkp.domain| will always be empty for a DomainState in this map; the domain
   // comes from the map key instead.

net/http/transport_security_state_unittest.cc

@@ -71,6 +71,64 @@ class TransportSecurityStateTest : public testing::Test {
   }
 };
 
+TEST_F(TransportSecurityStateTest, DomainNameOddities) {
+  TransportSecurityState state;
+  const base::Time current_time(base::Time::Now());
+  const base::Time expiry = current_time + base::TimeDelta::FromSeconds(1000);
+
+  // DNS suffix search tests. Some DNS resolvers allow a terminal "." to
+  // indicate not perform DNS suffix searching. Ensure that regardless
+  // of how this is treated at the resolver layer, or at the URL/origin
+  // layer (that is, whether they are treated as equivalent or distinct),
+  // ensure that for policy matching, something lacking a terminal "."
+  // is equivalent to something with a terminal "."
+  EXPECT_FALSE(state.ShouldUpgradeToSSL("example.com"));
+
+  state.AddHSTS("example.com", expiry, true /* include_subdomains */);
+  EXPECT_TRUE(state.ShouldUpgradeToSSL("example.com"));
+  // Trailing '.' should be equivalent; it's just a resolver hint
+  EXPECT_TRUE(state.ShouldUpgradeToSSL("example.com."));
+  // Leading '.' should be invalid
+  EXPECT_FALSE(state.ShouldUpgradeToSSL(".example.com"));
+  // Subdomains should work regardless
+  EXPECT_TRUE(state.ShouldUpgradeToSSL("sub.example.com"));
+  EXPECT_TRUE(state.ShouldUpgradeToSSL("sub.example.com."));
+  // But invalid subdomains should be rejected
+  EXPECT_FALSE(state.ShouldUpgradeToSSL("sub..example.com"));
+  EXPECT_FALSE(state.ShouldUpgradeToSSL("sub..example.com."));
+
+  // Now try the inverse form
+  TransportSecurityState state2;
+  state2.AddHSTS("example.net.", expiry, true /* include_subdomains */);
+  EXPECT_TRUE(state2.ShouldUpgradeToSSL("example.net."));
+  EXPECT_TRUE(state2.ShouldUpgradeToSSL("example.net"));
+  EXPECT_TRUE(state2.ShouldUpgradeToSSL("sub.example.net."));
+  EXPECT_TRUE(state2.ShouldUpgradeToSSL("sub.example.net"));
+
+  // Finally, test weird things
+  TransportSecurityState state3;
+  state3.AddHSTS("", expiry, true /* include_subdomains */);
+  EXPECT_FALSE(state3.ShouldUpgradeToSSL(""));
+  EXPECT_FALSE(state3.ShouldUpgradeToSSL("."));
+  EXPECT_FALSE(state3.ShouldUpgradeToSSL("..."));
+  // Make sure it didn't somehow apply HSTS to the world
+  EXPECT_FALSE(state3.ShouldUpgradeToSSL("example.org"));
+
+  TransportSecurityState state4;
+  state4.AddHSTS(".", expiry, true /* include_subdomains */);
+  EXPECT_FALSE(state4.ShouldUpgradeToSSL(""));
+  EXPECT_FALSE(state4.ShouldUpgradeToSSL("."));
+  EXPECT_FALSE(state4.ShouldUpgradeToSSL("..."));
+  EXPECT_FALSE(state4.ShouldUpgradeToSSL("example.org"));
+
+  // Now do the same for preloaded entries
+  TransportSecurityState state5;
+  EXPECT_TRUE(state5.ShouldUpgradeToSSL("accounts.google.com"));
+  EXPECT_TRUE(state5.ShouldUpgradeToSSL("accounts.google.com."));
+  EXPECT_FALSE(state5.ShouldUpgradeToSSL("accounts..google.com"));
+  EXPECT_FALSE(state5.ShouldUpgradeToSSL("accounts..google.com."));
+}
+
 TEST_F(TransportSecurityStateTest, SimpleMatches) {
   TransportSecurityState state;
   const base::Time current_time(base::Time::Now());
@@ -123,10 +181,15 @@ TEST_F(TransportSecurityStateTest, MatchesCase2) {
   const base::Time current_time(base::Time::Now());
   const base::Time expiry = current_time + base::TimeDelta::FromSeconds(1000);
 
+  // Check dynamic entries
   EXPECT_FALSE(state.ShouldUpgradeToSSL("YAhoo.coM"));
   bool include_subdomains = false;
   state.AddHSTS("yahoo.com", expiry, include_subdomains);
   EXPECT_TRUE(state.ShouldUpgradeToSSL("YAhoo.coM"));
+
+  // Check static entries
+  EXPECT_TRUE(state.ShouldUpgradeToSSL("AccounTs.GooGle.com"));
+  EXPECT_TRUE(state.ShouldUpgradeToSSL("mail.google.COM"));
 }
 
 TEST_F(TransportSecurityStateTest, SubdomainMatches) {