Make HSTS headers not clobber preloaded pins.

net/http/transport_security_state.h

Index: net/http/transport_security_state.h
diff --git a/net/http/transport_security_state.h b/net/http/transport_security_state.h
index 3511b6916f582db5d126c0968fe7e2980e61d813..03520a92d602e983ce6bb6f642d33ad4809f63a0 100644
--- a/net/http/transport_security_state.h
+++ b/net/http/transport_security_state.h
@@ -61,6 +61,41 @@ class NET_EXPORT TransportSecurityState
     DomainState();
     ~DomainState();
 
+    struct STSState {
+      // The absolute time (UTC) when the |upgrade_mode| (and other state) was
+      // observed.
+      base::Time last_observed;
+
+      // The absolute time (UTC) when the |upgrade_mode|, if set to
+      // UPGRADE_ALWAYS, downgrades to UPGRADE_NEVER.
+      base::Time expiry;
+
+      UpgradeMode upgrade_mode;
+
+      // Are subdomains subject to this policy state?
+      bool include_subdomains;
+    };
+
+    struct PKPState {
+      // The absolute time (UTC) when the |spki_hashes| (and other state) were
+      // observed.
+      base::Time last_observed;
+
+      // The absolute time (UTC) when the |spki_hashes| expire.
+      base::Time expiry;
+
+      // Optional; hashes of pinned SubjectPublicKeyInfos.
+      HashValueVector spki_hashes;
+
+      // Optional; hashes of static known-bad SubjectPublicKeyInfos which MUST
+      // NOT intersect with the set of SPKIs in the TLS server's certificate
+      // chain.
+      HashValueVector bad_spki_hashes;
+
+      // Are subdomains subject to this policy state?
+      bool include_subdomains;
+    };
+
     // Takes a set of SubjectPublicKeyInfo |hashes| and returns true if:
     //   1) |bad_static_spki_hashes| does not intersect |hashes|; AND
     //   2) Both |static_spki_hashes| and |dynamic_spki_hashes| are empty
@@ -93,51 +128,20 @@ class NET_EXPORT TransportSecurityState
     // hard-fail behavior (e.g. if HSTS is set for the domain)
     bool ShouldSSLErrorsBeFatal() const;
 
-    UpgradeMode upgrade_mode;
+    bool has_static_sts;
+    STSState static_sts;
+    STSState dynamic_sts;
 
-    // The absolute time (UTC) when the |upgrade_mode| was observed.
-    //
-    // TODO(palmer): Perhaps static entries should have an "observed" time.
-    base::Time sts_observed;
-
-    // The absolute time (UTC) when the |dynamic_spki_hashes| (and other
-    // |dynamic_*| state) were observed.
-    //
-    // TODO(palmer): Perhaps static entries should have an "observed" time.
-    base::Time pkp_observed;
-
-    // The absolute time (UTC) when the |upgrade_mode|, if set to
-    // UPGRADE_ALWAYS, downgrades to UPGRADE_NEVER.
-    base::Time upgrade_expiry;
-
-    // Are subdomains subject to this DomainState, for the purposes of
-    // upgrading to HTTPS?
-    bool sts_include_subdomains;
-
-    // Are subdomains subject to this DomainState, for the purposes of
-    // Pin Validation?
-    bool pkp_include_subdomains;
-
-    // Optional; hashes of static pinned SubjectPublicKeyInfos. Unless both
-    // are empty, at least one of |static_spki_hashes| and
-    // |dynamic_spki_hashes| MUST intersect with the set of SPKIs in the TLS
-    // server's certificate chain.
-    //
-    // |dynamic_spki_hashes| take precedence over |static_spki_hashes|.
-    // That is, |IsChainOfPublicKeysPermitted| first checks dynamic pins and
-    // then checks static pins.
-    HashValueVector static_spki_hashes;
-
-    // Optional; hashes of dynamically pinned SubjectPublicKeyInfos.
-    HashValueVector dynamic_spki_hashes;
-
-    // The absolute time (UTC) when the |dynamic_spki_hashes| expire.
-    base::Time dynamic_spki_hashes_expiry;
-
-    // Optional; hashes of static known-bad SubjectPublicKeyInfos which
-    // MUST NOT intersect with the set of SPKIs in the TLS server's
+    bool has_static_pkp;
+    // Unless both |{dynamic,static}_hpkp_state.spki_hashes| are empty, at least
+    // one of them MUST intersect with the set of SPKIs in the TLS server's
     // certificate chain.
-    HashValueVector bad_static_spki_hashes;
+    //
+    // |dynamic_hpkp_state| takes precedence over |static_hpkp_state|. That is,
+    // |IsChainOfPublicKeysPermitted| first checks dynamic state and then checks
+    // static state.

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

Let's expand/reword this comment.

// When checking whether or not a pin is valid, both dynamic and
// static HPKP state are checked. While dynamic state is
// checked first, static state is always consulted. Unless BOTH
// static and dynamic states are empty, the set of SPKIs from the
// TLS peer MUST be present in one of the states.


That is, your comment about "dynamic state takes precedence" makes me think we
ignore static state when we have dynamic state - but that's not true, is it?

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

Well, wouldn't it be a good idea to allow site operators to turn off pinning by
dynamic means? The I-D says:

"""UAs that support additional sources of pinning information MUST use the most
recently observed pinning information when performing Pin Validation for a host.
 The most recently observed pinning information is determined based upon the
most recent Effective Pin Date, as described in Section 2.3.2."""

Suggesting that it should, indeed, be possible for Chrome to ignore static state
if there is newer dynamic state that calls for no pinning.

[Ryan Sleevi, 2014/03/14 21:55:29]

Yes, but your comment is unclear on that, which is why I raised it.

"First checks dynamic state, and then checks static state" - except IIRC, you
were/are always checking static state (eg: not allowing dynamic state to
override)

Some degree of rewording the comment is good, perhaps as simple as "first checks
dynamic state, if present, and if not, continues to check static state"

Indicating that static state is *never* checked if dynamic state is present.

Or, you know, indicates that whichever is 'newer' is checked :)

+    PKPState static_pkp;
+    PKPState dynamic_pkp;
 
     // The following members are not valid when stored in |enabled_hosts_|: