Serialize and deserialize dynamic Expect-CT state

net/http/transport_security_persister.cc

Index: net/http/transport_security_persister.cc
diff --git a/net/http/transport_security_persister.cc b/net/http/transport_security_persister.cc
index a37d2379db2993f847c9461dc42e8ef8d49fd30f..ce8dd62055ef20062d9e5b5a170f7b3d928d57e5 100644
--- a/net/http/transport_security_persister.cc
+++ b/net/http/transport_security_persister.cc
@@ -9,6 +9,7 @@
 
 #include "base/base64.h"
 #include "base/bind.h"
+#include "base/feature_list.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/json/json_reader.h"
@@ -81,6 +82,10 @@ const char kCreated[] = "created";
 const char kStsObserved[] = "sts_observed";
 const char kPkpObserved[] = "pkp_observed";
 const char kReportUri[] = "report-uri";
+const char kExpectCTExpiry[] = "expect_ct_expiry";
+const char kExpectCTObserved[] = "expect_ct_observed";
+const char kExpectCTEnforce[] = "expect_ct_enforce";
+const char kExpectCTReportUri[] = "expect_ct_report_uri";
 
 std::string LoadState(const base::FilePath& path) {
   std::string result;
@@ -90,56 +95,44 @@ std::string LoadState(const base::FilePath& path) {
   return result;
 }
 
-}  // namespace
-
-TransportSecurityPersister::TransportSecurityPersister(
-    TransportSecurityState* state,
-    const base::FilePath& profile_path,
-    const scoped_refptr<base::SequencedTaskRunner>& background_runner,
-    bool readonly)
-    : transport_security_state_(state),
-      writer_(profile_path.AppendASCII("TransportSecurity"), background_runner),
-      foreground_runner_(base::ThreadTaskRunnerHandle::Get()),
-      background_runner_(background_runner),
-      readonly_(readonly),
-      weak_ptr_factory_(this) {
-  transport_security_state_->SetDelegate(this);
-
-  base::PostTaskAndReplyWithResult(
-      background_runner_.get(), FROM_HERE,
-      base::Bind(&LoadState, writer_.path()),
-      base::Bind(&TransportSecurityPersister::CompleteLoad,
-                 weak_ptr_factory_.GetWeakPtr()));
+bool IsDynamicExpectCTEnabled() {
+  return base::FeatureList::IsEnabled(
+      TransportSecurityState::kDynamicExpectCTFeature);
 }
 
-TransportSecurityPersister::~TransportSecurityPersister() {
-  DCHECK(foreground_runner_->RunsTasksOnCurrentThread());
-
-  if (writer_.HasPendingWrite())
-    writer_.DoScheduledWrite();
+// Populates |host| with default values for the STS and PKP states.
+// These default values represent "null" states and are only useful to keep
+// the entries in the resulting JSON consistent. The deserializer will ignore
+// "null" states.
+// TODO(davidben): This can be removed when the STS and PKP states are stored

[mattm, 2017/04/15 04:45:01]

I don't know the history here, but is there anything that can be done here
instead of just continuing the current course?

Is the end goal to have them in different files or just a nicer organization
within a single file? If it's the first, then we could just put expect-CT data
in a new file right from the start. If it's the latter then it may be a bit more
complex.. maybe store all the expect-ct keys in a sub-dictionary under each
hostname instead?

+// independently on disk. https://crbug.com/470295
+void PopulateEntryWithDefaults(base::DictionaryValue* host) {
+  host->Clear();
 
-  transport_security_state_->SetDelegate(NULL);
-}
+  // STS default values.
+  host->SetBoolean(kStsIncludeSubdomains, false);
+  host->SetDouble(kStsObserved, 0.0);
+  host->SetDouble(kExpiry, 0.0);
+  host->SetString(kMode, kDefault);
 
-void TransportSecurityPersister::StateIsDirty(
-    TransportSecurityState* state) {
-  DCHECK(foreground_runner_->RunsTasksOnCurrentThread());
-  DCHECK_EQ(transport_security_state_, state);
+  // PKP default values.
+  host->SetBoolean(kPkpIncludeSubdomains, false);
+  host->SetDouble(kPkpObserved, 0.0);
+  host->SetDouble(kDynamicSPKIHashesExpiry, 0.0);
 
-  if (!readonly_)
-    writer_.ScheduleWrite(this);
+  // Expect-CT default values.

[mattm, 2017/04/15 04:45:01]

Should kExpectCTReportUri be in here?

[estark, 2017/04/15 20:18:03]

Removed this section per your comment below.

+  if (IsDynamicExpectCTEnabled()) {
+    host->SetBoolean(kExpectCTObserved, 0.0);
+    host->SetBoolean(kExpectCTExpiry, 0.0);
+    host->SetBoolean(kExpectCTEnforce, false);
+  }
 }
 
-bool TransportSecurityPersister::SerializeData(std::string* output) {
-  DCHECK(foreground_runner_->RunsTasksOnCurrentThread());
-
-  base::DictionaryValue toplevel;
-  base::Time now = base::Time::Now();
-
-  // TODO(davidben): Fix the serialization format by splitting the on-disk
-  // representation of the STS and PKP states. https://crbug.com/470295.
-  TransportSecurityState::STSStateIterator sts_iterator(
-      *transport_security_state_);
+// Serializes STS data from |state| into |toplevel|. Any existing state in
+// |toplevel| for each item is overwritten.
+void SerializeSTSData(TransportSecurityState* state,
+                      base::DictionaryValue* toplevel) {
+  TransportSecurityState::STSStateIterator sts_iterator(*state);
   for (; sts_iterator.HasNext(); sts_iterator.Advance()) {
     const std::string& hostname = sts_iterator.hostname();
     const TransportSecurityState::STSState& sts_state =
@@ -166,11 +159,17 @@ bool TransportSecurityPersister::SerializeData(std::string* output) {
         continue;
     }
 
-    toplevel.Set(key, std::move(serialized));
+    toplevel->Set(key, std::move(serialized));
   }
+}
 
-  TransportSecurityState::PKPStateIterator pkp_iterator(
-      *transport_security_state_);
+// Serializes PKP data from |state| into |toplevel|. For each PKP item in
+// |state|, if |toplevel| already contains an item for that hostname, the item
+// is updated with the PKP data.
+void SerializePKPData(TransportSecurityState* state,
+                      base::DictionaryValue* toplevel) {
+  base::Time now = base::Time::Now();
+  TransportSecurityState::PKPStateIterator pkp_iterator(*state);
   for (; pkp_iterator.HasNext(); pkp_iterator.Advance()) {
     const std::string& hostname = pkp_iterator.hostname();
     const TransportSecurityState::PKPState& pkp_state =
@@ -180,12 +179,12 @@ bool TransportSecurityPersister::SerializeData(std::string* output) {
     // that entry.
     const std::string key = HashedDomainToExternalString(hostname);
     base::DictionaryValue* serialized = nullptr;
-    if (!toplevel.GetDictionary(key, &serialized)) {
+    if (!toplevel->GetDictionary(key, &serialized)) {
       std::unique_ptr<base::DictionaryValue> serialized_scoped(
           new base::DictionaryValue);
       serialized = serialized_scoped.get();
       PopulateEntryWithDefaults(serialized);
-      toplevel.Set(key, std::move(serialized_scoped));
+      toplevel->Set(key, std::move(serialized_scoped));
     }
 
     serialized->SetBoolean(kPkpIncludeSubdomains, pkp_state.include_subdomains);
@@ -205,6 +204,122 @@ bool TransportSecurityPersister::SerializeData(std::string* output) {
 
     serialized->SetString(kReportUri, pkp_state.report_uri.spec());
   }
+}
+
+// Serializes Expect-CT data from |state| into |toplevel|. For each Expect-CT
+// item in |state|, if |toplevel| already contains an item for that hostname,
+// the item is updated with the Expect-CT data.
+void SerializeExpectCTData(TransportSecurityState* state,
+                           base::DictionaryValue* toplevel) {
+  if (!IsDynamicExpectCTEnabled())
+    return;
+  TransportSecurityState::ExpectCTStateIterator expect_ct_iterator(*state);
+  for (; expect_ct_iterator.HasNext(); expect_ct_iterator.Advance()) {
+    const std::string& hostname = expect_ct_iterator.hostname();
+    const TransportSecurityState::ExpectCTState& expect_ct_state =
+        expect_ct_iterator.domain_state();
+
+    // See if the current |hostname| already has STS/PKP state and, if so,
+    // update that entry.
+    const std::string key = HashedDomainToExternalString(hostname);
+    base::DictionaryValue* serialized = nullptr;
+    if (!toplevel->GetDictionary(key, &serialized)) {
+      std::unique_ptr<base::DictionaryValue> serialized_scoped(
+          new base::DictionaryValue);
+      serialized = serialized_scoped.get();
+      PopulateEntryWithDefaults(serialized);
+      toplevel->Set(key, std::move(serialized_scoped));
+    }
+
+    serialized->SetDouble(kExpectCTObserved,
+                          expect_ct_state.last_observed.ToDoubleT());
+    serialized->SetDouble(kExpectCTExpiry, expect_ct_state.expiry.ToDoubleT());
+    serialized->SetBoolean(kExpectCTEnforce, expect_ct_state.enforce);
+    serialized->SetString(kExpectCTReportUri,
+                          expect_ct_state.report_uri.spec());
+  }
+}
+
+bool DeserializeExpectCTState(const base::DictionaryValue* parsed,
+                              TransportSecurityState::ExpectCTState* state) {
+  double observed;
+  bool has_observed = parsed->GetDouble(kExpectCTObserved, &observed);
+  double expiry;
+  bool has_expiry = parsed->GetDouble(kExpectCTExpiry, &expiry);
+  bool enforce;
+  bool has_enforce = parsed->GetBoolean(kExpectCTEnforce, &enforce);
+  std::string report_uri_str;
+  bool has_report_uri = parsed->GetString(kExpectCTReportUri, &report_uri_str);
+
+  // Entries are not required to have Expect-CT data.
+  if (!has_observed && !has_expiry && !has_enforce)

[mattm, 2017/04/15 04:45:01]

Is the PopulateEntryWithDefaults change necessary if you already check whether
the entries are present?

[estark, 2017/04/15 20:18:03]

Hmm no it's not, removed.

+    return true;
+
+  // If there is Expect-CT data, the default keys (kExpectCTObserved,
+  // kExpectCTExpiry, kExpectCTEnforce) are expected to be present.
+  if (!has_observed || !has_expiry || !has_enforce)
+    return false;
+
+  state->last_observed = base::Time::FromDoubleT(observed);
+  state->expiry = base::Time::FromDoubleT(expiry);
+  state->enforce = enforce;
+  if (has_report_uri) {
+    GURL report_uri(report_uri_str);
+    if (report_uri.is_valid())
+      state->report_uri = report_uri;
+  }
+  return true;
+}
+
+}  // namespace
+
+TransportSecurityPersister::TransportSecurityPersister(
+    TransportSecurityState* state,
+    const base::FilePath& profile_path,
+    const scoped_refptr<base::SequencedTaskRunner>& background_runner,
+    bool readonly)
+    : transport_security_state_(state),
+      writer_(profile_path.AppendASCII("TransportSecurity"), background_runner),
+      foreground_runner_(base::ThreadTaskRunnerHandle::Get()),
+      background_runner_(background_runner),
+      readonly_(readonly),
+      weak_ptr_factory_(this) {
+  transport_security_state_->SetDelegate(this);
+
+  base::PostTaskAndReplyWithResult(
+      background_runner_.get(), FROM_HERE,
+      base::Bind(&LoadState, writer_.path()),
+      base::Bind(&TransportSecurityPersister::CompleteLoad,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+TransportSecurityPersister::~TransportSecurityPersister() {
+  DCHECK(foreground_runner_->RunsTasksOnCurrentThread());
+
+  if (writer_.HasPendingWrite())
+    writer_.DoScheduledWrite();
+
+  transport_security_state_->SetDelegate(NULL);
+}
+
+void TransportSecurityPersister::StateIsDirty(TransportSecurityState* state) {
+  DCHECK(foreground_runner_->RunsTasksOnCurrentThread());
+  DCHECK_EQ(transport_security_state_, state);
+
+  if (!readonly_)
+    writer_.ScheduleWrite(this);
+}
+
+bool TransportSecurityPersister::SerializeData(std::string* output) {
+  DCHECK(foreground_runner_->RunsTasksOnCurrentThread());
+
+  base::DictionaryValue toplevel;
+
+  // TODO(davidben): Fix the serialization format by splitting the on-disk
+  // representation of the STS and PKP states. https://crbug.com/470295.
+  SerializeSTSData(transport_security_state_, &toplevel);
+  SerializePKPData(transport_security_state_, &toplevel);
+  SerializeExpectCTData(transport_security_state_, &toplevel);
 
   base::JSONWriter::WriteWithOptions(
       toplevel, base::JSONWriter::OPTIONS_PRETTY_PRINT, output);
@@ -241,6 +356,7 @@ bool TransportSecurityPersister::Deserialize(const std::string& serialized,
 
     TransportSecurityState::STSState sts_state;
     TransportSecurityState::PKPState pkp_state;
+    TransportSecurityState::ExpectCTState expect_ct_state;
 
     // kIncludeSubdomains is a legacy synonym for kStsIncludeSubdomains and
     // kPkpIncludeSubdomains. Parse at least one of these properties,
@@ -323,13 +439,21 @@ bool TransportSecurityPersister::Deserialize(const std::string& serialized,
       pkp_state.last_observed = base::Time::Now();
     }
 
+    if (!DeserializeExpectCTState(parsed, &expect_ct_state)) {
+      continue;
+    }
+
     bool has_sts =
         sts_state.expiry > current_time && sts_state.ShouldUpgradeToSSL();
     bool has_pkp =
         pkp_state.expiry > current_time && pkp_state.HasPublicKeyPins();
-    if (!has_sts && !has_pkp) {
+    bool has_expect_ct =
+        expect_ct_state.expiry > current_time &&
+        (expect_ct_state.enforce || !expect_ct_state.report_uri.is_empty());
+    if (!has_sts && !has_pkp && !has_expect_ct) {
       // Make sure we dirty the state if we drop an entry. The entries can only
-      // be dropped when both the STS and PKP states are expired or invalid.
+      // be dropped when all the STS, PKP, and Expect-CT states are expired or
+      // invalid.
       dirtied = true;
       continue;
     }
@@ -346,28 +470,14 @@ bool TransportSecurityPersister::Deserialize(const std::string& serialized,
       state->AddOrUpdateEnabledSTSHosts(hashed, sts_state);
     if (has_pkp)
       state->AddOrUpdateEnabledPKPHosts(hashed, pkp_state);
+    if (has_expect_ct)
+      state->AddOrUpdateEnabledExpectCTHosts(hashed, expect_ct_state);
   }
 
   *dirty = dirtied;
   return true;
 }
 
-void TransportSecurityPersister::PopulateEntryWithDefaults(
-    base::DictionaryValue* host) {
-  host->Clear();
-
-  // STS default values.
-  host->SetBoolean(kStsIncludeSubdomains, false);
-  host->SetDouble(kStsObserved, 0.0);
-  host->SetDouble(kExpiry, 0.0);
-  host->SetString(kMode, kDefault);
-
-  // PKP default values.
-  host->SetBoolean(kPkpIncludeSubdomains, false);
-  host->SetDouble(kPkpObserved, 0.0);
-  host->SetDouble(kDynamicSPKIHashesExpiry, 0.0);
-}
-
 void TransportSecurityPersister::CompleteLoad(const std::string& state) {
   DCHECK(foreground_runner_->RunsTasksOnCurrentThread());