Make HSTS headers not clobber preloaded pins.

net/http/transport_security_state.h

 // 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.
 
 #ifndef NET_HTTP_TRANSPORT_SECURITY_STATE_H_
 #define NET_HTTP_TRANSPORT_SECURITY_STATE_H_
 
 #include <map>
 #include <string>
 #include <utility>
@@ skipping 43 matching lines @@
    public:
     enum UpgradeMode {
       // These numbers must match those in hsts_view.js, function modeToString.
       MODE_FORCE_HTTPS = 0,
       MODE_DEFAULT = 1,
     };
 
     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
     //      or at least one of them intersects |hashes|.
     //
     // |{dynamic,static}_spki_hashes| contain trustworthy public key hashes,
     // any one of which is sufficient to validate the certificate chain in
     // question. The public keys could be of a root CA, intermediate CA, or
     // leaf certificate, depending on the security vs. disaster recovery
     // tradeoff selected. (Pinning only to leaf certifiates increases
@@ skipping 12 matching lines @@
 
     // ShouldUpgradeToSSL returns true iff, given the |mode| of this
     // DomainState, HTTP requests should be internally redirected to HTTPS
     // (also if the "ws" WebSocket request should be upgraded to "wss")
     bool ShouldUpgradeToSSL() const;
 
     // ShouldSSLErrorsBeFatal returns true iff HTTPS errors should cause
     // 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.
+    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.
     //
-    // 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
-    // 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_|:
 
     // The domain which matched during a search for this DomainState entry.
     // Updated by |GetDomainState|, |GetDynamicDomainState|, and
     // |GetStaticDomainState|.
     std::string domain;
   };
 
   class NET_EXPORT Iterator {
@@ skipping 174 matching lines @@
   DomainStateMap enabled_hosts_;
 
   Delegate* delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(TransportSecurityState);
 };
 
 }  // namespace net
 
 #endif  // NET_HTTP_TRANSPORT_SECURITY_STATE_H_