Refactor TransportSecurityState.

net/base/transport_security_state.h

Index: net/base/transport_security_state.h
===================================================================
--- net/base/transport_security_state.h	(revision 124959)
+++ net/base/transport_security_state.h	(working copy)
@@ -22,105 +22,128 @@
 
 class SSLInfo;
 
-typedef std::vector<SHA1Fingerprint> FingerprintVector;
+// In the future there will be a generic Fingerprint type, with at least two
+// implementations: SHA1 and SHA256.
+//
+// TODO(palmer): Specify and implement that in net/base/x509_cert_types.h.

[Ryan Sleevi, 2012/03/07 06:02:46]

File a bug, assign to you. Easier to track TODOs when they reference discrete
bugs :)

[palmer, 2012/03/13 20:13:39]

Done.

+//
+// Until that work is done (in a separate patch) this typedef bridges the
+// gap.
+typedef SHA1Fingerprint Fingerprint;
 
-// TransportSecurityState
+typedef std::vector<Fingerprint> FingerprintVector;
+
+// Tracks which hosts have enabled strict transport security and/or public
+// key pins.
 //
-// Tracks which hosts have enabled *-Transport-Security. This object manages
-// the in-memory store. A separate object must register itself with this object
-// in order to persist the state to disk.
+// This object manages the in-memory store. Register a Delegate
+// with |SetDelegate| to persist the state to disk.
+//
+// HTTP strict transport security (HSTS) is defined in
+// http://tools.ietf.org/html/draft-ietf-websec-strict-transport-sec-04, and
+// HTTP-based dynamic public key pinning (HPKP) is defined in
+// http://tools.ietf.org/html/draft-ietf-websec-key-pinning-01.
 class NET_EXPORT TransportSecurityState
     : NON_EXPORTED_BASE(public base::NonThreadSafe) {
  public:
-  // If non-empty, |hsts_hosts| is a JSON-formatted string to treat as if it
-  // were a built-in entry (same format as persisted metadata in the
-  // TransportSecurityState file).
-  explicit TransportSecurityState(const std::string& hsts_hosts);
+  TransportSecurityState();
+
   ~TransportSecurityState();
 
-  // A DomainState is the information that we persist about a given domain.
+  // A DomainState describes the transport security state (required upgrade
+  // to HTTPS, and/or any public key pins).
   struct NET_EXPORT DomainState {

[Ryan Sleevi, 2012/03/07 06:02:46]

I think you've reached the point where this should be a class instead (
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Structs_vs._Cl...
)

[palmer, 2012/03/13 20:13:39]

Done.

-    enum Mode {
-      // Strict mode implies:
-      //   * We generate internal redirects from HTTP -> HTTPS.
-      //   * Certificate issues are fatal.
-      MODE_STRICT = 0,
-      // This used to be opportunistic HTTPS, but we removed support.
-      MODE_OPPORTUNISTIC_REMOVED = 1,
-      // SPDY_ONLY (aka X-Bodge-Transport-Security) is a hopefully temporary
-      // measure. It implies:
-      //   * We'll request HTTP URLs over HTTPS iff we have SPDY support.
-      //   * Certificate issues are fatal.
-      MODE_SPDY_ONLY = 2,
-      // Pinning means there are no HTTP -> HTTPS redirects, however certificate
-      // issues are still fatal and there may be public key pins.
-      MODE_PINNING_ONLY = 3,
+    enum UpgradeMode {
+      MODE_DEFAULT,
+      MODE_FORCE_HTTPS,
     };
 
     DomainState();
     ~DomainState();
 
+    // Parses |value| as a Public-Key-Pins header. If successful, returns true
+    // and updates the |dynamic_spki_hashes| and |dynamic_spki_hashes_expiry|
+    // fields; otherwise, returns false without updating any fields.
+    bool ParsePinsHeader(const std::string& value, const SSLInfo& ssl_info);
+
+    // Parses |value| as a Strict-Transport-Security header. If successful,
+    // returns true and updates the |upgrade_mode|, |upgrade_expiry| and
+    // |include_subdomains| fields; otherwise, returns false without updating
+    // any fields.
+    bool ParseSTSHeader(const std::string& value);
+
     // Takes a set of SubjectPublicKeyInfo |hashes| and returns true if:
-    //   1) |bad_preloaded_spki_hashes| does not intersect |hashes|; AND
-    //   2) Both |preloaded_spki_hashes| and |dynamic_spki_hashes| are empty
+    //   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,preloaded}_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
+    // |{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
     // security because you no longer trust any CAs, but it hampers disaster
     // recovery because you can't just get a new certificate signed by the
     // CA.)
     //
-    // |bad_preloaded_spki_hashes| contains public keys that we don't want to
+    // |bad_static_spki_hashes| contains public keys that we don't want to
     // trust.
-    bool IsChainOfPublicKeysPermitted(const FingerprintVector& hashes);
+    bool IsChainOfPublicKeysPermitted(const FingerprintVector& hashes) const;
 
-    // Returns true if |this| describes a more strict policy than |other|.
-    // Used to see if a dynamic DomainState should override a preloaded one.
-    bool IsMoreStrict(const DomainState& other);
+    // Returns true if any of the FingerprintVectors |static_spki_hashes|,
+    // |bad_static_spki_hashes|, or |dynamic_spki_hashes| contains any
+    // items.
+    bool HasPins() const;
 
     // ShouldRedirectHTTPToHTTPS returns true iff, given the |mode| of this
     // DomainState, HTTP requests should be internally redirected to HTTPS.
     bool ShouldRedirectHTTPToHTTPS() const;
 
-    Mode mode;
-    base::Time created;  // when this host entry was first created
-    base::Time expiry;  // the absolute time (UTC) when this record expires
-    bool include_subdomains;  // subdomains included?
+    UpgradeMode upgrade_mode;
 
-    // Optional; hashes of preloaded "pinned" SubjectPublicKeyInfos. Unless
-    // both are empty, at least one of |preloaded_spki_hashes| and
+    // The absolute time (UTC) when this DomainState was first created.
+    //
+    // Static entries do not have a created time.
+    base::Time created;
+
+    // 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?
+    //
+    // TODO(palmer): Decide if we should have separate |pin_subdomains| and
+    // |upgrade_subdomains|. Alternately, and perhaps better, is to separate
+    // DomainState into UpgradeState and PinState (requiring also changing the
+    // serialization format?).
+    bool 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 |preloaded_spki_hashes|.
-    // That is, when performing pin validation, first check dynamic and then
-    // check preloaded.
-    FingerprintVector preloaded_spki_hashes;
+    // |dynamic_spki_hashes| take precedence over |static_spki_hashes|.
+    // That is, |IsChainOfPublicKeysPermitted| first checks dynamic pins and
+    // then checks static pins.
+    FingerprintVector static_spki_hashes;
 
-    // Optional; hashes of dynamically pinned SubjectPublicKeyInfos. (They
-    // could be set e.g. by an HTTP header or by a superfluous certificate.)
+    // Optional; hashes of dynamically pinned SubjectPublicKeyInfos.
     FingerprintVector dynamic_spki_hashes;
 
     // The absolute time (UTC) when the |dynamic_spki_hashes| expire.
     base::Time dynamic_spki_hashes_expiry;
 
-    // The max-age directive of the Public-Key-Pins header as parsed. Do not
-    // persist this; it is only for testing. TODO(palmer): Therefore, get rid
-    // of it and find a better way to test.
-    int max_age;
-
-    // Optional; hashes of preloaded known-bad SubjectPublicKeyInfos which
+    // Optional; hashes of static known-bad SubjectPublicKeyInfos which
     // MUST NOT intersect with the set of SPKIs in the TLS server's
     // certificate chain.
-    FingerprintVector bad_preloaded_spki_hashes;
+    FingerprintVector bad_static_spki_hashes;
 
-    // The following members are not valid when stored in |enabled_hosts_|.
-    bool preloaded;  // is this a preloaded entry?
-    std::string domain;  // the domain which matched
+    // 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| and |GetStaticDomainState|.
+    std::string domain;
   };
 
   class Delegate {
@@ -135,147 +158,150 @@
 
   void SetDelegate(Delegate* delegate);
 
-  // Enable TransportSecurity for |host|.
+  // 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

[Ryan Sleevi, 2012/03/07 06:02:46]

nit: Line wrapping

[palmer, 2012/03/13 20:13:39]

Done.

+  // (if any).
   void EnableHost(const std::string& host, const DomainState& state);
 
-  // Delete any entry for |host|. If |host| doesn't have an exact entry then no
-  // action is taken. Returns true iff an entry was deleted.
+  // Delete any entry for |host|. If |host| doesn't have an exact entry then
+  // no action is taken. Does not delete static entries. Returns true iff an
+  // entry was deleted.
+  //
+  // The new state for |host| is persisted using the Delegate

[Ryan Sleevi, 2012/03/07 06:02:46]

nit: Line wrapping

[palmer, 2012/03/13 20:13:39]

Done.

+  // (if any).
   bool DeleteHost(const std::string& host);
 
-  // Returns true if |host| has TransportSecurity enabled. Before operating
-  // on this result, consult |result->mode|, as the expected behavior of
-  // TransportSecurity can significantly differ based on mode.
+  // Deletes all records created since a given time.
+  void DeleteSince(const base::Time& time);
+
+  // Returns true and updates |*result| iff there is a DomainState for
+  // |host|.
   //
-  // If |sni_available| is true, searches the preloads defined for SNI-using
-  // hosts as well as the usual preload list.
+  // If |sni_enabled| is true, searches the static pins defined for
+  // SNI-using hosts as well as the rest of the pins.
   //
-  // Note that |*result| is always overwritten on every call.
-  // TODO(palmer): Only update |*result| on success.
+  // If |host| matches both an exact entry and is a subdomain of another
+  // entry, the exact match determines the return value.
   bool GetDomainState(DomainState* result,

[Ryan Sleevi, 2012/03/07 06:02:46]

Per
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Function_Param...

This should be

bool GetDomainState(const std::string& host,
                    bool sni_enabled,
                    DomainState* result) const;

See also 203-205.

[palmer, 2012/03/13 20:13:39]

Done.

                       const std::string& host,
-                      bool sni_available);
+                      bool sni_enabled) const;
 
-  // Returns true if there are any certificates pinned for |host|.
-  // If so, updates the |preloaded_spki_hashes|, |dynamic_spki_hashes|, and
-  // |bad_preloaded_spki_hashes| fields of |*result| with the pins.
+  // Returns true and updates |*result| iff there is a static DomainState for
+  // |host|.
   //
-  // Note that |*result| is always overwritten on every call, regardless of
-  // whether or not pins are enabled.
+  // |GetStaticDomainState| is identical to |GetDomainState| except that it
+  // searches only the statically-defined transport security state, ignoring
+  // all dynamically-added DomainStates.
   //
-  // If |sni_available| is true, searches the preloads defined for SNI-using
-  // hosts as well as the usual preload list.
+  // If |sni_enabled| is true, searches the static pins defined for
+  // SNI-using hosts as well as the rest of the pins.
   //
-  // TODO(palmer): Only update |*result| if pins exist.
-  bool HasPinsForHost(DomainState* result,
-                      const std::string& host,
-                      bool sni_available);
+  // If |host| matches both an exact entry and is a subdomain of another
+  // entry, the exact match determines the return value.
+  bool GetStaticDomainState(DomainState* result,
+                            const std::string& host,
+                            bool sni_enabled) const;
 
-  // Returns true and updates |*result| if there is any |DomainState|
-  // metadata for |host| in the local TransportSecurityState database;
-  // returns false otherwise. TODO(palmer): Unlike the other
-  // TransportSecurityState lookup functions in this class (e.g
-  // |HasPinsForHost|, |GetDomainState|), |*result| is updated iff metadata
-  // is found. The other functions are buggy and will be fixed to behave
-  // like this one.
+  // Serializes the transport security state (the set of DomainStates
+  // for all hosts) into |*output|. Returns true if all DomainStates were
+  // serialized correctly.
   //
-  // If |sni_available| is true, searches the preloads defined for SNI-using
-  // hosts as well as the usual preload list.
-  bool HasMetadata(DomainState* result,
-                   const std::string& host,
-                   bool sni_available);
+  // The serialization format is JSON; the JSON represents a dictionary of
+  // host:DomainState pairs (host is a string). The DomainState is
+  // represented as a dictionary containing the following keys and value
+  // types (not all keys will always be present):
+  //
+  //     "include_subdomains": true|false
+  //     "created": double
+  //     "expiry": double
+  //     "dynamic_spki_hashes_expiry": double
+  //     "mode": "always"|"never"
+  //             legacy value synonyms "strict"|"pinning-only"
+  //             legacy value "spdy-only" is unused and ignored
+  //     "static_spki_hashes": list of strings
+  //         legacy key synonym "preloaded_spki_hashes"
+  //     "bad_static_spki_hashes": list of strings
+  //         legacy key synonym "bad_preloaded_spki_hashes"
+  //     "dynamic_spki_hashes": list of strings
+  //
+  // The keys are SHA256(CanonicalizeHost(domain)). The reason for hashing
+  // them is so that the stored state does not trivially reveal a user's
+  // browing history to an attacker reading the serialized state on disk.
+  bool Serialize(std::string* output) const;
 
-  // Returns true if we have a preloaded certificate pin for the |host| and if
-  // its set of required certificates is the set we expect for Google
+  // Clears any existing non-static entries, and then re-populates the
+  // transport security state from the JSON string |state|. Returns true if
+  // all entries were parsed and deserialized correctly.
+  //
+  // Sets |*dirty| to true if the new state differs from the persisted
+  // state; false otherwise.
+  bool LoadEntries(const std::string& state, bool* dirty);
+
+  // 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 |sni_available| is true, searches the preloads defined for SNI-using
-  // hosts as well as the usual preload list.
+  // If |sni_enabled| is true, searches the static pins defined for
+  // SNI-using hosts as well as the rest of the pins.
   //
-  // Note that like HasMetadata, if |host| matches both an exact entry and is a
-  // subdomain of another entry, the exact match determines the return value.
+  // 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,
-                                     bool sni_available);
+                                     bool sni_enabled);
 
-  // Reports UMA statistics upon public key pin failure. Reports only down to
-  // the second-level domain of |host| (e.g. google.com if |host| is
-  // mail.google.com), and only if |host| is a preloaded STS host.
-  static void ReportUMAOnPinFailure(const std::string& host);
+  // Decodes a pin string |value| (e.g. "sha1/hvfkN/qlp/zhXR3cuerq6jd2Z7g=").
+  // If parsing succeeded, updates |*out| and returns true; otherwise returns
+  // false without updating |*out|.
+  static bool ParsePin(const std::string& value, Fingerprint* out);
 
-  // Parses |cert|'s Subject Public Key Info structure, hashes it, and writes
-  // the hash into |spki_hash|. Returns true on parse success, false on
-  // failure.
-  static bool GetPublicKeyHash(const X509Certificate& cert,
-                               SHA1Fingerprint* spki_hash);
+  // The maximum number of seconds for which we'll cache an HSTS request.
+  static const long int kMaxHSTSAgeSecs;
 
-  // Decodes a pin string |value| (e.g. "sha1/hvfkN/qlp/zhXR3cuerq6jd2Z7g=")
-  // and populates |out|.
-  static bool ParsePin(const std::string& value, SHA1Fingerprint* out);
-
-  // Deletes all records created since a given time.
-  void DeleteSince(const base::Time& time);
-
-  // Parses |value| as a Public-Key-Pins header. If successful, returns |true|
-  // and updates the |dynamic_spki_hashes| and |dynamic_spki_hashes_expiry|
-  // fields of |*state|; otherwise, returns |false| without updating |*state|.
-  static bool ParsePinsHeader(const std::string& value,
-                              const SSLInfo& ssl_info,
-                              DomainState* state);
-
-  // Returns |true| if |value| parses as a valid *-Transport-Security
-  // header value.  The values of max-age and and includeSubDomains are
-  // returned in |max_age| and |include_subdomains|, respectively.  The out
-  // parameters are not modified if the function returns |false|.
-  static bool ParseHeader(const std::string& value,
-                          int* max_age,
-                          bool* include_subdomains);
-
-  // ParseSidePin attempts to parse a side pin from |side_info| which signs the
-  // SubjectPublicKeyInfo in |leaf_spki|. A side pin is a way for a site to
-  // sign their public key with a key that is offline but still controlled by
-  // them. If successful, the hash of the public key used to sign |leaf_spki|
-  // is put into |out_pub_key_hash|.
+  // Parses |side_info| as a side pin. If
+  // successful, returns true and appends the hash of the public key that signed

[Ryan Sleevi, 2012/03/07 06:02:46]

nit: Line wrapping.

[palmer, 2012/03/13 20:13:39]

Done.

+  // |leaf_spki| to |*out_pub_key_hash|.
+  //
+  // A side pin is a way for a site to sign their public key with a key that is
+  // offline but still controlled by them.
+  //
+  // TODO(agl): Document the format of a side pin.
   static bool ParseSidePin(const base::StringPiece& leaf_spki,
                            const base::StringPiece& side_info,
                            FingerprintVector* out_pub_key_hash);
 
-  bool Serialise(std::string* output);
-  // Existing non-preloaded entries are cleared and repopulated from the
-  // passed JSON string.
-  bool LoadEntries(const std::string& state, bool* dirty);
-
-  // The maximum number of seconds for which we'll cache an HSTS request.
-  static const long int kMaxHSTSAgeSecs;
-
  private:
-  FRIEND_TEST_ALL_PREFIXES(TransportSecurityStateTest, IsPreloaded);
+  // 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);
 
-  // If we have a callback configured, call it to let our serialiser know that
-  // our state is dirty.
+  // If we have a Delegate, call its |StateIsDirty| function.
   void DirtyNotify();
-  bool IsPreloadedSTS(const std::string& canonicalized_host,
-                      bool sni_available,
-                      DomainState* out);
 
-  static std::string CanonicalizeHost(const std::string& host);
-  static bool Deserialise(const std::string& state,
+  static bool Deserialize(const std::string& state,
                           bool* dirty,
                           std::map<std::string, DomainState>* out);
 
-  // The set of hosts that have enabled TransportSecurity. The keys here
-  // are SHA256(DNSForm(domain)) where DNSForm converts from dotted form
-  // ('www.google.com') to the form used in DNS: "\x03www\x06google\x03com"
+  // The set of hosts that have enabled TransportSecurity.
   std::map<std::string, DomainState> enabled_hosts_;
 
-  // These hosts are extra rules to treat as built-in, passed in the
-  // constructor (typically originating from the command line).
+  // Extra entries, provided by the user at run-time, to treat as if they
+  // were static.
   std::map<std::string, DomainState> forced_hosts_;
 
-  // Our delegate who gets notified when we are dirtied, or NULL.
   Delegate* delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(TransportSecurityState);
 };
 
+// Parse |json| (in the format documented in
+// |TransportSecurityState::Serialize|) and update |state| with the state
+// entries. Returns true if all records in |json| were correctly parsed.
+bool ParseJSONSecurityState(const std::string& json,
+                            TransportSecurityState* state);
+
 }  // namespace net
 
 #endif  // NET_BASE_TRANSPORT_SECURITY_STATE_H_