Treat HSTS and HPKP state independently.

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 32 matching lines @@
 
    protected:
     virtual ~Delegate() {}
   };
 
   TransportSecurityState();
   ~TransportSecurityState();
 
   // A DomainState describes the transport security state (required upgrade
   // to HTTPS, and/or any public key pins).
+  //
+  // TODO(davidben): STSState and PKPState are queried and processed
+  // independently (with the exception of ShouldSSLErrorsBeFatal triggering on
+  // both and on-disk storage). DomainState should be split into the two.
   class NET_EXPORT DomainState {
    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 {
+      STSState();
+      ~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
       // MODE_FORCE_HTTPS, downgrades to MODE_DEFAULT.
       base::Time expiry;
 
       UpgradeMode upgrade_mode;
 
       // Are subdomains subject to this policy state?
       bool include_subdomains;
+
+      // The following members are not valid when stored in |enabled_hosts_|:

[Ryan Sleevi, 2015/01/13 21:56:40]

This is a public structure, so what does it mean to the caller re:
|enabled_hosts_|

Put differently, describe this comment from the public API point of view, rather
than the implementation details.

[davidben, 2015/01/13 23:30:47]

(I just copied this from below. :-P) Moved the comment to |enabled_hosts_|. For
public uses of this struct, the domain field is always valid.

(This is kind of weird and possibly also worth tweaking...)

+
+      // The domain which matched during a search for this DomainState entry.
+      // Updated by |GetDynamicDomainState| and |GetStaticDomainState|.
+      std::string domain;
     };
 
     struct PKPState {
       PKPState();
       ~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;
+
+      // The following members are not valid when stored in |enabled_hosts_|:
+
+      // The domain which matched during a search for this DomainState entry.
+      // Updated by |GetDynamicDomainState| and |GetStaticDomainState|.
+      std::string domain;
     };
 
     // 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
@@ skipping 16 matching lines @@
     // ShouldUpgradeToSSL returns true iff HTTP requests should be internally
     // redirected to HTTPS (also if WS 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;
 
     STSState sts;
     PKPState 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 |GetDynamicDomainState| and |GetStaticDomainState|.
-    std::string domain;
   };
 
   class NET_EXPORT Iterator {
    public:
     explicit Iterator(const TransportSecurityState& state);
     ~Iterator();
 
     bool HasNext() const { return iterator_ != end_; }
     void Advance() { ++iterator_; }
     const std::string& hostname() const { return iterator_->first; }
@@ skipping 48 matching lines @@
   // Deletes any dynamic data stored for |host| (e.g. HSTS or HPKP data).
   // If |host| doesn't have an exact entry then no action is taken. Does
   // not delete static (i.e. preloaded) data.  Returns true iff an entry
   // was deleted.
   //
   // If an entry is deleted, the new state will be persisted through
   // the Delegate (if any).
   bool DeleteDynamicDataForHost(const std::string& host);
 
   // Returns true and updates |*result| iff there is a static (built-in)
-  // DomainState for |host|.
-  //
-  // If |host| matches both an exact entry and is a subdomain of another entry,
-  // the exact match determines the return value.
+  // DomainState for |host|. If multiple entries match |host|, the most specific
+  // match determines the return value.
+  bool GetStaticDomainState(const std::string& host, DomainState* result) const;
+
+  // Returns true and updates |*result| iff |host| has HSTS or HPKP state (or
+  // both). The two are queried independently and combined into a single
+  // DomainState. If multiple HSTS (respectively, HPKP) entries match |host|,
+  // the most specific match determines the HSTS (respectively, HPKP) portion of
+  // the return value.
   //
   // Note that this method is not const because it opportunistically removes
   // entries that have expired.
-  bool GetStaticDomainState(const std::string& host, DomainState* result) const;
-
-  // Returns true and updates |*result| iff there is a dynamic DomainState
-  // (learned from HSTS or HPKP headers, or set by the user, or other means) for
-  // |host|.
   //
-  // If |host| matches both an exact entry and is a subdomain of another entry,
-  // the exact match determines the return value.
-  //
-  // Note that this method is not const because it opportunistically removes
-  // entries that have expired.
+  // TODO(davidben): STSState and PKPState should be queried independently at
+  // the API level too.
   bool GetDynamicDomainState(const std::string& host, DomainState* result);
 
   // Processes an HSTS header value from the host, adding entries to
   // dynamic state if necessary.
   bool AddHSTSHeader(const std::string& host, const std::string& value);
 
   // Processes an HPKP header value from the host, adding entries to
   // dynamic state if necessary.  ssl_info is used to check that
   // the specified pins overlap with the certificate chain.
   bool AddHPKPHeader(const std::string& host, const std::string& value,
                      const SSLInfo& ssl_info);
 
   // Adds explicitly-specified data as if it was processed from an
   // HSTS header (used for net-internals and unit tests).
-  bool AddHSTS(const std::string& host, const base::Time& expiry,
+  void AddHSTS(const std::string& host,
+               const base::Time& expiry,
                bool include_subdomains);
 
   // Adds explicitly-specified data as if it was processed from an
   // HPKP header (used for net-internals and unit tests).
-  bool AddHPKP(const std::string& host, const base::Time& expiry,
-               bool include_subdomains, const HashValueVector& hashes);
+  void AddHPKP(const std::string& host,
+               const base::Time& expiry,
+               bool include_subdomains,
+               const HashValueVector& hashes);
 
   // Returns true iff we have any static public key pins for the |host| and
   // iff its set of required pins is the set we expect for Google
   // properties.
   //
   // If |host| matches both an exact entry and is a subdomain of another
   // entry, the exact match determines the return value.
   static bool IsGooglePinnedProperty(const std::string& host);
 
   // The maximum number of seconds for which we'll cache an HSTS request.
@@ skipping 24 matching lines @@
 
   // Helper method for actually checking pins.
   bool CheckPublicKeyPinsImpl(const std::string& host,
                               const HashValueVector& hashes,
                               std::string* failure_log);
 
   // If a Delegate is present, notify it that the internal state has
   // changed.
   void DirtyNotify();
 
+  // Adds HSTS state to |host|.
+  void AddHSTSInternal(const std::string& host,
+                       DomainState::UpgradeMode upgrade_mode,
+                       const base::Time& expiry,
+                       bool include_subdomains);
+
+  // Adds HPKP state to |host|.
+  void AddHPKPInternal(const std::string& host,
+                       const base::Time& last_observed,
+                       const base::Time& expiry,
+                       bool include_subdomains,
+                       const HashValueVector& hashes);
+
   // Enable TransportSecurity for |host|. |state| supercedes any previous
   // state for the |host|, including static entries.
   //
   // 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.
   DomainStateMap enabled_hosts_;
 
   Delegate* delegate_;
 
   // True if static pins should be used.
   bool enable_static_pins_;
 
   DISALLOW_COPY_AND_ASSIGN(TransportSecurityState);
 };
 
 }  // namespace net
 
 #endif  // NET_HTTP_TRANSPORT_SECURITY_STATE_H_