Make HSTS headers not clobber preloaded pins.

net/http/transport_security_state.cc

Index: net/http/transport_security_state.cc
diff --git a/net/http/transport_security_state.cc b/net/http/transport_security_state.cc
index 6a7a14f62ed671c84fb70a228491dd87aec80e32..59a2d3715f145923effe2df3ae48b497d827ac58 100644
--- a/net/http/transport_security_state.cc
+++ b/net/http/transport_security_state.cc
@@ -81,6 +81,16 @@ bool AddHash(const char* sha1_hash,
   return true;
 }
 
+// Updates |other| to contain the most recent observations found
+// between the states |state| and |other|.
+void MergeDomainStates(const TransportSecurityState::DomainState& state,
+                       TransportSecurityState::DomainState* other) {
+  if (other->dynamic_pkp.last_observed < state.dynamic_pkp.last_observed)
+    other->dynamic_pkp = state.dynamic_pkp;
+  if (other->dynamic_sts.last_observed < state.dynamic_sts.last_observed)
+    other->dynamic_sts = state.dynamic_sts;
+}
+
 }  // namespace
 
 TransportSecurityState::TransportSecurityState()
@@ -140,47 +150,50 @@ bool TransportSecurityState::GetDomainState(const std::string& host,
                                             DomainState* result) {
   DCHECK(CalledOnValidThread());
 
-  DomainState state;
   const std::string canonicalized_host = CanonicalizeHost(host);
   if (canonicalized_host.empty())
     return false;
 
-  bool has_preload = GetStaticDomainState(canonicalized_host, sni_enabled,
-                                          &state);
-  std::string canonicalized_preload = CanonicalizeHost(state.domain);
-  GetDynamicDomainState(host, &state);
-
+  DomainState static_state;
+  bool has_static = GetStaticDomainState(canonicalized_host, sni_enabled,
+                                         &static_state);
+  std::string canonicalized_preload = CanonicalizeHost(static_state.domain);
+  DomainState dynamic_state;
+  bool has_dynamic = GetDynamicDomainState(host, &dynamic_state);
   base::Time current_time(base::Time::Now());
 
   for (size_t i = 0; canonicalized_host[i]; i += canonicalized_host[i] + 1) {
     std::string host_sub_chunk(&canonicalized_host[i],
                                canonicalized_host.size() - i);
     // Exact match of a preload always wins.
-    if (has_preload && host_sub_chunk == canonicalized_preload) {
-      *result = state;
+    if (has_static && host_sub_chunk == canonicalized_preload) {
+      // If we have dynamic state, merge the newer parts of it into the static
+      // state. Then set the |result|.
+      if (has_dynamic)
+        MergeDomainStates(dynamic_state, &static_state);

[Ryan Sleevi, 2014/03/07 01:39:19]

BUG: static_state.domain gets all full of lies here.

Consider static state for a.b.example.com, dynamic state for b.example.com

GetDynamicDomainState will return the state for b.example.com (because it works
through the host_sub_chunks in canoncalized_host ). GetStaticDomainState will
return the state for a.b.example.com

static_state.domain will be "a.b.example.com", dynamic_state.domain will be
"b.example.com"

When you merge these, static_state.dynamic_pkp / dynamic_sts will be set to
dynamic_state.dynamic_pkp / dynamic_sts. However, .domain won't be updated.

As a result, you run the risk of now serializing back to the dynamic store
"a.b.example.com", duplicating the entry already stored under "b.example.com" -
and now no longer updated on any new observations of b.example.com.

[palmer, 2014/03/14 21:33:39]

Ah, yes. OK, I'm going to change the code so that callers always call
GetDynamicDomainState and GetStaticDomainState explicitly. That will cause a lot
of churn in this CL, so brace yourselves.

+      *result = static_state;
       return true;
     }
 
-    DomainStateMap::iterator j =
-        enabled_hosts_.find(HashHost(host_sub_chunk));
+    DomainStateMap::iterator j = enabled_hosts_.find(HashHost(host_sub_chunk));
     if (j == enabled_hosts_.end())
       continue;
 
-    if (current_time > j->second.upgrade_expiry &&
-        current_time > j->second.dynamic_spki_hashes_expiry) {
+    if (current_time > j->second.dynamic_sts.expiry &&
+        current_time > j->second.dynamic_pkp.expiry) {
       enabled_hosts_.erase(j);
       DirtyNotify();
       continue;
     }
 
-    state = j->second;
-    state.domain = DNSDomainToString(host_sub_chunk);
+    dynamic_state = j->second;
+    dynamic_state.domain = DNSDomainToString(host_sub_chunk);
 
     // Succeed if we matched the domain exactly or if subdomain matches are
     // allowed.
-    if (i == 0 || j->second.sts_include_subdomains ||
-        j->second.pkp_include_subdomains) {
-      *result = state;
+    if (i == 0 || j->second.dynamic_sts.include_subdomains ||
+        j->second.dynamic_pkp.include_subdomains) {
+      *result = dynamic_state;
       return true;
     }
 
@@ -201,18 +214,19 @@ void TransportSecurityState::DeleteAllDynamicDataSince(const base::Time& time) {
   bool dirtied = false;
   DomainStateMap::iterator i = enabled_hosts_.begin();
   while (i != enabled_hosts_.end()) {
-    if (i->second.sts_observed >= time && i->second.pkp_observed >= time) {
+    if (i->second.dynamic_sts.last_observed >= time &&
+        i->second.dynamic_pkp.last_observed >= time) {
       dirtied = true;
       enabled_hosts_.erase(i++);
       continue;
     }
 
-    if (i->second.sts_observed >= time) {
+    if (i->second.dynamic_sts.last_observed >= time) {
       dirtied = true;
-      i->second.upgrade_mode = DomainState::MODE_DEFAULT;
-    } else if (i->second.pkp_observed >= time) {
+      i->second.dynamic_sts.upgrade_mode = DomainState::MODE_DEFAULT;
+    } else if (i->second.dynamic_pkp.last_observed >= time) {
       dirtied = true;
-      i->second.dynamic_spki_hashes.clear();
+      i->second.dynamic_pkp.spki_hashes.clear();
     }
     ++i;
   }
@@ -554,22 +568,28 @@ static bool HasPreload(const struct HSTSPreload* entries, size_t num_entries,
       if (!entries[j].include_subdomains && i != 0) {
         *ret = false;
       } else {
-        out->sts_include_subdomains = entries[j].include_subdomains;
-        out->pkp_include_subdomains = entries[j].include_subdomains;
+        out->static_sts.include_subdomains = entries[j].include_subdomains;
+        out->static_sts.last_observed = base::GetBuildTime();
+        out->static_pkp.include_subdomains = entries[j].include_subdomains;
+        out->static_pkp.last_observed = base::GetBuildTime();
         *ret = true;
+        out->has_static_sts = entries[j].https_required;
         if (!entries[j].https_required)
-          out->upgrade_mode = TransportSecurityState::DomainState::MODE_DEFAULT;
+          out->static_sts.upgrade_mode =
+              TransportSecurityState::DomainState::MODE_DEFAULT;
         if (entries[j].pins.required_hashes) {
+          out->has_static_pkp = true;
           const char* const* sha1_hash = entries[j].pins.required_hashes;
           while (*sha1_hash) {
-            AddHash(*sha1_hash, &out->static_spki_hashes);
+            AddHash(*sha1_hash, &out->static_pkp.spki_hashes);
             sha1_hash++;
           }
         }
         if (entries[j].pins.excluded_hashes) {
+          out->has_static_pkp = true;
           const char* const* sha1_hash = entries[j].pins.excluded_hashes;
           while (*sha1_hash) {
-            AddHash(*sha1_hash, &out->bad_static_spki_hashes);
+            AddHash(*sha1_hash, &out->static_pkp.bad_spki_hashes);
             sha1_hash++;
           }
         }
@@ -617,14 +637,15 @@ bool TransportSecurityState::AddHSTSHeader(const std::string& host,
   base::TimeDelta max_age;
   TransportSecurityState::DomainState domain_state;
   GetDynamicDomainState(host, &domain_state);
-  if (ParseHSTSHeader(value, &max_age, &domain_state.sts_include_subdomains)) {
+  if (ParseHSTSHeader(value, &max_age,
+                      &domain_state.dynamic_sts.include_subdomains)) {
     // Handle max-age == 0
     if (max_age.InSeconds() == 0)
-      domain_state.upgrade_mode = DomainState::MODE_DEFAULT;
+      domain_state.dynamic_sts.upgrade_mode = DomainState::MODE_DEFAULT;
     else
-      domain_state.upgrade_mode = DomainState::MODE_FORCE_HTTPS;
-    domain_state.sts_observed = now;
-    domain_state.upgrade_expiry = now + max_age;
+      domain_state.dynamic_sts.upgrade_mode = DomainState::MODE_FORCE_HTTPS;
+    domain_state.dynamic_sts.last_observed = now;
+    domain_state.dynamic_sts.expiry = now + max_age;
     EnableHost(host, domain_state);
     return true;
   }
@@ -641,11 +662,11 @@ bool TransportSecurityState::AddHPKPHeader(const std::string& host,
   TransportSecurityState::DomainState domain_state;
   GetDynamicDomainState(host, &domain_state);
   if (ParseHPKPHeader(value, ssl_info.public_key_hashes,
-                      &max_age, &domain_state.pkp_include_subdomains,
-                      &domain_state.dynamic_spki_hashes)) {
+                      &max_age, &domain_state.dynamic_pkp.include_subdomains,
+                      &domain_state.dynamic_pkp.spki_hashes)) {
     // TODO(palmer): http://crbug.com/243865 handle max-age == 0.
-    domain_state.pkp_observed = now;
-    domain_state.dynamic_spki_hashes_expiry = now + max_age;
+    domain_state.dynamic_pkp.last_observed = now;
+    domain_state.dynamic_pkp.expiry = now + max_age;
     EnableHost(host, domain_state);
     return true;
   }
@@ -666,10 +687,10 @@ bool TransportSecurityState::AddHSTS(const std::string& host,
   if (i != enabled_hosts_.end())
     domain_state = i->second;
 
-  domain_state.sts_observed = base::Time::Now();
-  domain_state.sts_include_subdomains = include_subdomains;
-  domain_state.upgrade_expiry = expiry;
-  domain_state.upgrade_mode = DomainState::MODE_FORCE_HTTPS;
+  domain_state.dynamic_sts.last_observed = base::Time::Now();
+  domain_state.dynamic_sts.include_subdomains = include_subdomains;
+  domain_state.dynamic_sts.expiry = expiry;
+  domain_state.dynamic_sts.upgrade_mode = DomainState::MODE_FORCE_HTTPS;
   EnableHost(host, domain_state);
   return true;
 }
@@ -689,10 +710,10 @@ bool TransportSecurityState::AddHPKP(const std::string& host,
   if (i != enabled_hosts_.end())
     domain_state = i->second;
 
-  domain_state.pkp_observed = base::Time::Now();
-  domain_state.pkp_include_subdomains = include_subdomains;
-  domain_state.dynamic_spki_hashes_expiry = expiry;
-  domain_state.dynamic_spki_hashes = hashes;
+  domain_state.dynamic_pkp.last_observed = base::Time::Now();
+  domain_state.dynamic_pkp.include_subdomains = include_subdomains;
+  domain_state.dynamic_pkp.expiry = expiry;
+  domain_state.dynamic_pkp.spki_hashes = hashes;
   EnableHost(host, domain_state);
   return true;
 }
@@ -755,9 +776,9 @@ bool TransportSecurityState::GetStaticDomainState(
     DomainState* out) {
   DCHECK(CalledOnValidThread());
 
-  out->upgrade_mode = DomainState::MODE_FORCE_HTTPS;
-  out->sts_include_subdomains = false;
-  out->pkp_include_subdomains = false;
+  out->static_sts.upgrade_mode = DomainState::MODE_FORCE_HTTPS;
+  out->static_sts.include_subdomains = false;
+  out->static_pkp.include_subdomains = false;
 
   const bool is_build_timely = IsBuildTimely();
 
@@ -801,8 +822,8 @@ bool TransportSecurityState::GetDynamicDomainState(const std::string& host,
     if (j == enabled_hosts_.end())
       continue;
 
-    if (current_time > j->second.upgrade_expiry &&
-        current_time > j->second.dynamic_spki_hashes_expiry) {
+    if (current_time > j->second.dynamic_sts.expiry &&
+        current_time > j->second.dynamic_pkp.expiry) {
       enabled_hosts_.erase(j);
       DirtyNotify();
       continue;
@@ -813,8 +834,8 @@ bool TransportSecurityState::GetDynamicDomainState(const std::string& host,
 
     // Succeed if we matched the domain exactly or if subdomain matches are
     // allowed.
-    if (i == 0 || j->second.sts_include_subdomains ||
-        j->second.pkp_include_subdomains) {
+    if (i == 0 || j->second.dynamic_sts.include_subdomains ||
+        j->second.dynamic_pkp.include_subdomains) {
       *result = state;
       return true;
     }
@@ -825,7 +846,6 @@ bool TransportSecurityState::GetDynamicDomainState(const std::string& host,
   return false;
 }
 
-
 void TransportSecurityState::AddOrUpdateEnabledHosts(
     const std::string& hashed_host, const DomainState& state) {
   DCHECK(CalledOnValidThread());
@@ -833,12 +853,11 @@ void TransportSecurityState::AddOrUpdateEnabledHosts(
 }
 
 TransportSecurityState::DomainState::DomainState()
-    : upgrade_mode(MODE_DEFAULT),
-      sts_include_subdomains(false),
-      pkp_include_subdomains(false) {
-  base::Time now(base::Time::Now());
-  sts_observed = now;
-  pkp_observed = now;
+  : has_static_sts(false),
+    has_static_pkp(false) {
+  dynamic_sts.upgrade_mode = MODE_DEFAULT;
+  dynamic_sts.include_subdomains = false;
+  dynamic_pkp.include_subdomains = false;
 }
 
 TransportSecurityState::DomainState::~DomainState() {
@@ -855,32 +874,36 @@ bool TransportSecurityState::DomainState::CheckPublicKeyPins(
     return false;
   }
 
-  if (HashesIntersect(bad_static_spki_hashes, hashes)) {
+  if (HashesIntersect(static_pkp.bad_spki_hashes, hashes) ||
+      HashesIntersect(dynamic_pkp.bad_spki_hashes, hashes)) {
     LOG(ERROR) << "Rejecting public key chain for domain " << domain
                << ". Validated chain: " << HashesToBase64String(hashes)
                << ", matches one or more bad hashes: "
-               << HashesToBase64String(bad_static_spki_hashes);
+               << HashesToBase64String(static_pkp.bad_spki_hashes) <<
+               ", " << HashesToBase64String(dynamic_pkp.bad_spki_hashes);
     return false;
   }
 
   // If there are no pins, then any valid chain is acceptable.
-  if (dynamic_spki_hashes.empty() && static_spki_hashes.empty())
+  if (dynamic_pkp.spki_hashes.empty() && static_pkp.spki_hashes.empty()) {
     return true;
+  }
 
-  if (HashesIntersect(dynamic_spki_hashes, hashes) ||
-      HashesIntersect(static_spki_hashes, hashes)) {
+  if (HashesIntersect(dynamic_pkp.spki_hashes, hashes) ||
+      HashesIntersect(static_pkp.spki_hashes, hashes)) {
     return true;
   }
 
   LOG(ERROR) << "Rejecting public key chain for domain " << domain
              << ". Validated chain: " << HashesToBase64String(hashes)
-             << ", expected: " << HashesToBase64String(dynamic_spki_hashes)
-             << " or: " << HashesToBase64String(static_spki_hashes);
+             << ", expected: " << HashesToBase64String(dynamic_pkp.spki_hashes)
+             << " or: " << HashesToBase64String(static_pkp.spki_hashes);
   return false;
 }
 
 bool TransportSecurityState::DomainState::ShouldUpgradeToSSL() const {
-  return upgrade_mode == MODE_FORCE_HTTPS;
+  return dynamic_sts.upgrade_mode == MODE_FORCE_HTTPS ||
+         static_sts.upgrade_mode == MODE_FORCE_HTTPS;
 }
 
 bool TransportSecurityState::DomainState::ShouldSSLErrorsBeFatal() const {
@@ -888,9 +911,9 @@ bool TransportSecurityState::DomainState::ShouldSSLErrorsBeFatal() const {
 }
 
 bool TransportSecurityState::DomainState::HasPublicKeyPins() const {
-  return static_spki_hashes.size() > 0 ||
-         bad_static_spki_hashes.size() > 0 ||
-         dynamic_spki_hashes.size() > 0;
+  return static_pkp.spki_hashes.size() > 0 ||
+         static_pkp.bad_spki_hashes.size() > 0 ||
+         dynamic_pkp.spki_hashes.size() > 0;
 }
 
 }  // namespace