Adding a log signature to staged logs in log stores.

Bug: 906202
Change-Id: I8607465f4657eb5c7c96e1ad85a51f4e8206cbfc
Reviewed-on: https://chromium-review.googlesource.com/c/1340532Reviewed-by: Adam Langley <agl@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Commit-Queue: Jesse Doherty <jwd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611362}

components/metrics/BUILD.gn

@@ -115,6 +115,7 @@ jumbo_static_library("metrics") {
     "//components/prefs",
     "//components/variations",
     "//components/version_info:version_info",
+    "//crypto",
     "//extensions/buildflags",
     "//third_party/zlib/google:compression_utils",
   ]

components/metrics/DEPS

@@ -10,6 +10,7 @@ include_rules = [
   "+components/variations",
   "+components/version_info",
   "+content/public/test",
+  "+crypto",
   "+extensions/buildflags",
   "+mojo/public/cpp",
   "+services/service_manager/public/cpp",

components/metrics/log_store.h

@@ -24,10 +24,15 @@ class LogStore {
   // Will trigger a DCHECK if there is no staged log.
   virtual const std::string& staged_log() const = 0;
 
-  // The SHA1 hash of the staged log.
+  // The SHA1 hash of the staged log. This is used to detect log corruption.
   // Will trigger a DCHECK if there is no staged log.
   virtual const std::string& staged_log_hash() const = 0;
 
+  // The HMAC-SHA256 signature of the staged log. This is used to validate that
+  // a log originated from Chrome, and to detect corruption.
+  // Will trigger a DCHECK if there is no staged log.
+  virtual const std::string& staged_log_signature() const = 0;
+
   // Populates staged_log() with the next stored log to send.
   // The order in which logs are staged is up to the implementor.
   // The staged_log must remain the same even if additional logs are added.

components/metrics/metrics_log_store.cc

@@ -100,6 +100,12 @@ const std::string& MetricsLogStore::staged_log_hash() const {
              : ongoing_log_queue_.staged_log_hash();
 }
 
+const std::string& MetricsLogStore::staged_log_signature() const {
+  return initial_log_queue_.has_staged_log()
+             ? initial_log_queue_.staged_log_signature()
+             : ongoing_log_queue_.staged_log_signature();
+}
+
 void MetricsLogStore::StageNextLog() {
   DCHECK(!has_staged_log());
   if (initial_log_queue_.has_unsent_logs())

components/metrics/metrics_log_store.h

@@ -40,6 +40,7 @@ class MetricsLogStore : public LogStore {
   bool has_staged_log() const override;
   const std::string& staged_log() const override;
   const std::string& staged_log_hash() const override;
+  const std::string& staged_log_signature() const override;
   void StageNextLog() override;
   void DiscardStagedLog() override;
   void PersistUnsentLogs() const override;

components/metrics/persisted_logs.cc

@@ -13,10 +13,12 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/sha1.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/string_util.h"
 #include "base/timer/elapsed_timer.h"
 #include "components/metrics/persisted_logs_metrics.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
+#include "crypto/hmac.h"
 #include "third_party/zlib/google/compression_utils.h"
 
 namespace metrics {
@@ -24,6 +26,7 @@ namespace metrics {
 namespace {
 
 const char kLogHashKey[] = "hash";
+const char kLogSignatureKey[] = "signature";
 const char kLogTimestampKey[] = "timestamp";
 const char kLogDataKey[] = "data";
 
@@ -43,6 +46,10 @@ std::string DecodeFromBase64(const std::string& to_convert) {
 
 }  // namespace
 
+PersistedLogs::LogInfo::LogInfo() {}
+PersistedLogs::LogInfo::LogInfo(const PersistedLogs::LogInfo& other) = default;
+PersistedLogs::LogInfo::~LogInfo() {}
+
 void PersistedLogs::LogInfo::Init(PersistedLogsMetrics* metrics,
                                   const std::string& log_data,
                                   const std::string& log_timestamp) {
@@ -56,6 +63,17 @@ void PersistedLogs::LogInfo::Init(PersistedLogsMetrics* metrics,
   metrics->RecordCompressionRatio(compressed_log_data.size(), log_data.size());
 
   hash = base::SHA1HashString(log_data);
+
+  // TODO(crbug.com/906202): Add an actual key for signing.
+  crypto::HMAC hmac(crypto::HMAC::SHA256);
+  const size_t digest_length = hmac.DigestLength();
+  unsigned char* hmac_data = reinterpret_cast<unsigned char*>(
+      base::WriteInto(&signature, digest_length + 1));
+  if (!hmac.Init(std::string()) ||
+      !hmac.Sign(log_data, hmac_data, digest_length)) {
+    NOTREACHED() << "HMAC signing failed";
+  }
+
   timestamp = log_timestamp;
 }
 
@@ -88,18 +106,24 @@ bool PersistedLogs::has_staged_log() const {
   return staged_log_index_ != -1;
 }
 
-// Returns the element in the front of the list.
+// Returns the compressed data of the element in the front of the list.
 const std::string& PersistedLogs::staged_log() const {
   DCHECK(has_staged_log());
   return list_[staged_log_index_].compressed_log_data;
 }
 
-// Returns the element in the front of the list.
+// Returns the hash of element in the front of the list.
 const std::string& PersistedLogs::staged_log_hash() const {
   DCHECK(has_staged_log());
   return list_[staged_log_index_].hash;
 }
 
+// Returns the signature of element in the front of the list.
+const std::string& PersistedLogs::staged_log_signature() const {
+  DCHECK(has_staged_log());
+  return list_[staged_log_index_].signature;
+}
+
 // Returns the timestamp of the element in the front of the list.
 const std::string& PersistedLogs::staged_log_timestamp() const {
   DCHECK(has_staged_log());
@@ -162,7 +186,8 @@ void PersistedLogs::ReadLogsFromPrefList(const base::ListValue& list_value) {
     const base::DictionaryValue* dict;
     if (!list_value.GetDictionary(i, &dict) ||
         !dict->GetString(kLogDataKey, &list_[i].compressed_log_data) ||
-        !dict->GetString(kLogHashKey, &list_[i].hash)) {
+        !dict->GetString(kLogHashKey, &list_[i].hash) ||
+        !dict->GetString(kLogSignatureKey, &list_[i].signature)) {
       list_.clear();
       metrics_->RecordLogReadStatus(
           PersistedLogsMetrics::LOG_STRING_CORRUPTION);
@@ -172,6 +197,8 @@ void PersistedLogs::ReadLogsFromPrefList(const base::ListValue& list_value) {
     list_[i].compressed_log_data =
         DecodeFromBase64(list_[i].compressed_log_data);
     list_[i].hash = DecodeFromBase64(list_[i].hash);
+    list_[i].signature = DecodeFromBase64(list_[i].signature);
+
     // Ignoring the success of this step as timestamp might not be there for
     // older logs.
     // NOTE: Should be added to the check with other fields once migration is
@@ -215,6 +242,7 @@ void PersistedLogs::WriteLogsToPrefList(base::ListValue* list_value) const {
     std::unique_ptr<base::DictionaryValue> dict_value(
         new base::DictionaryValue);
     dict_value->SetString(kLogHashKey, EncodeToBase64(list_[i].hash));
+    dict_value->SetString(kLogSignatureKey, EncodeToBase64(list_[i].signature));
     dict_value->SetString(kLogDataKey,
                           EncodeToBase64(list_[i].compressed_log_data));
     dict_value->SetString(kLogTimestampKey, list_[i].timestamp);

components/metrics/persisted_logs.h

@@ -48,6 +48,7 @@ class PersistedLogs : public LogStore {
   bool has_staged_log() const override;
   const std::string& staged_log() const override;
   const std::string& staged_log_hash() const override;
+  const std::string& staged_log_signature() const override;
   void StageNextLog() override;
   void DiscardStagedLog() override;
   void PersistUnsentLogs() const override;
@@ -93,6 +94,10 @@ class PersistedLogs : public LogStore {
   const size_t max_log_size_;
 
   struct LogInfo {
+    LogInfo();
+    LogInfo(const LogInfo& other);
+    ~LogInfo();
+
     // Initializes the members based on uncompressed |log_data| and
     // |log_timestamp|.
     // |metrics| is the parent's metrics_ object, and should not be held.
@@ -103,9 +108,15 @@ class PersistedLogs : public LogStore {
     // Compressed log data - a serialized protobuf that's been gzipped.
     std::string compressed_log_data;
 
-    // The SHA1 hash of log, stored to catch errors from memory corruption.
+    // The SHA1 hash of the log. Computed in Init and stored to catch errors
+    // from memory corruption.
     std::string hash;
 
+    // The HMAC-SHA256 signature of the log, used to validate the log came from
+    // Chrome. It's computed in Init and stored, instead of computed on demand,
+    // to catch errors from memory corruption.
+    std::string signature;
+
     // The timestamp of when the log was created as a time_t value.
     std::string timestamp;
   };

components/metrics/persisted_logs_unittest.cc

@@ -115,6 +115,8 @@ TEST_F(PersistedLogsTest, SingleElementLogList) {
   EXPECT_EQ(persisted_logs.staged_log(), result_persisted_logs.staged_log());
   EXPECT_EQ(persisted_logs.staged_log_hash(),
             result_persisted_logs.staged_log_hash());
+  EXPECT_EQ(persisted_logs.staged_log_signature(),
+            result_persisted_logs.staged_log_signature());
   EXPECT_EQ(persisted_logs.staged_log_timestamp(),
             result_persisted_logs.staged_log_timestamp());
 }
@@ -287,4 +289,22 @@ TEST_F(PersistedLogsTest, Hashes) {
   EXPECT_EQ(foo_hash, persisted_logs.staged_log_hash());
 }
 
+TEST_F(PersistedLogsTest, Signatures) {
+  const char kFooText[] = "foo";
+
+  TestPersistedLogs persisted_logs(&prefs_, kLogByteLimit);
+  persisted_logs.StoreLog(kFooText);
+  persisted_logs.StageNextLog();
+
+  EXPECT_EQ(Compress(kFooText), persisted_logs.staged_log());
+
+  // Decode the expected signature from a base 64 encoded string.
+  std::string expected_signature;
+  base::Base64Decode("DA2Y9+PZ1F5y6Id7wbEEMn77nAexjy/+ztdtgTB/H/8=",
+                     &expected_signature);
+
+  std::string actual_signature = persisted_logs.staged_log_signature();
+  EXPECT_EQ(expected_signature, actual_signature);
+}
+
 }  // namespace metrics

components/metrics/reporting_service_unittest.cc

@@ -13,6 +13,7 @@
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/sha1.h"
+#include "base/strings/string_util.h"
 #include "base/test/test_simple_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/metrics/log_store.h"
@@ -42,6 +43,9 @@ class TestLogStore : public LogStore {
   const std::string& staged_log_hash() const override {
     return staged_log_hash_;
   }
+  const std::string& staged_log_signature() const override {
+    return base::EmptyString();
+  }
   void StageNextLog() override {
     if (has_unsent_logs())
       staged_log_hash_ = base::SHA1HashString(logs_.front());