Plumb a key from MetricsServiceClient to use when signing metrics uploads.

There will need to be a follow up CL that adds an implementation for Chrome to provide an actual key.

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

components/metrics/metrics_log_manager_unittest.cc

@@ -26,7 +26,7 @@ namespace {
 
 class MetricsLogManagerTest : public testing::Test {
  public:
-  MetricsLogManagerTest() : log_store_(&pref_service_, 0) {
+  MetricsLogManagerTest() : log_store_(&pref_service_, 0, std::string()) {
     MetricsLogStore::RegisterPrefs(pref_service_.registry());
     log_store()->LoadPersistedUnsentLogs();
   }

components/metrics/metrics_log_store.cc

@@ -41,7 +41,8 @@ void MetricsLogStore::RegisterPrefs(PrefRegistrySimple* registry) {
 }
 
 MetricsLogStore::MetricsLogStore(PrefService* local_state,
-                                 size_t max_ongoing_log_size)
+                                 size_t max_ongoing_log_size,
+                                 const std::string& signing_key)
     : unsent_logs_loaded_(false),
       initial_log_queue_(std::unique_ptr<PersistedLogsMetricsImpl>(
                              new PersistedLogsMetricsImpl()),
@@ -49,14 +50,16 @@ MetricsLogStore::MetricsLogStore(PrefService* local_state,
                          prefs::kMetricsInitialLogs,
                          kInitialLogsPersistLimit,
                          kStorageByteLimitPerLogType,
-                         0),
+                         0,
+                         signing_key),
       ongoing_log_queue_(std::unique_ptr<PersistedLogsMetricsImpl>(
                              new PersistedLogsMetricsImpl()),
                          local_state,
                          prefs::kMetricsOngoingLogs,
                          kOngoingLogsPersistLimit,
                          kStorageByteLimitPerLogType,
-                         max_ongoing_log_size) {}
+                         max_ongoing_log_size,
+                         signing_key) {}
 
 MetricsLogStore::~MetricsLogStore() {}
 

components/metrics/metrics_log_store.h

@@ -25,8 +25,11 @@ class MetricsLogStore : public LogStore {
  public:
   // Constructs a MetricsLogStore that persists data into |local_state|.
   // If max_log_size is non-zero, it will not persist ongoing logs larger than
-  // |max_ongoing_log_size| bytes.
-  MetricsLogStore(PrefService* local_state, size_t max_ongoing_log_size);
+  // |max_ongoing_log_size| bytes. |signing_key| is used to generate a signature
+  // of a log, which will be uploaded to validate data integrity.
+  MetricsLogStore(PrefService* local_state,
+                  size_t max_ongoing_log_size,
+                  const std::string& signing_key);
   ~MetricsLogStore();
 
   // Registers local state prefs used by this class.

components/metrics/metrics_log_store_unittest.cc

@@ -42,7 +42,7 @@ class MetricsLogStoreTest : public testing::Test {
 }  // namespace
 
 TEST_F(MetricsLogStoreTest, StandardFlow) {
-  MetricsLogStore log_store(&pref_service_, 0);
+  MetricsLogStore log_store(&pref_service_, 0, std::string());
   log_store.LoadPersistedUnsentLogs();
 
   // Make sure a new manager has a clean slate.
@@ -66,7 +66,7 @@ TEST_F(MetricsLogStoreTest, StoreAndLoad) {
   // Set up some in-progress logging in a scoped log manager simulating the
   // leadup to quitting, then persist as would be done on quit.
   {
-    MetricsLogStore log_store(&pref_service_, 0);
+    MetricsLogStore log_store(&pref_service_, 0, std::string());
     log_store.LoadPersistedUnsentLogs();
     EXPECT_FALSE(log_store.has_unsent_logs());
     log_store.StoreLog("a", MetricsLog::ONGOING_LOG);
@@ -77,7 +77,7 @@ TEST_F(MetricsLogStoreTest, StoreAndLoad) {
 
   // Relaunch load and store more logs.
   {
-    MetricsLogStore log_store(&pref_service_, 0);
+    MetricsLogStore log_store(&pref_service_, 0, std::string());
     log_store.LoadPersistedUnsentLogs();
     EXPECT_TRUE(log_store.has_unsent_logs());
     EXPECT_EQ(0U, TypeCount(MetricsLog::INITIAL_STABILITY_LOG));
@@ -98,7 +98,7 @@ TEST_F(MetricsLogStoreTest, StoreAndLoad) {
 
   // Relaunch and verify that once logs are handled they are not re-persisted.
   {
-    MetricsLogStore log_store(&pref_service_, 0);
+    MetricsLogStore log_store(&pref_service_, 0, std::string());
     log_store.LoadPersistedUnsentLogs();
     EXPECT_TRUE(log_store.has_unsent_logs());
 
@@ -132,7 +132,7 @@ TEST_F(MetricsLogStoreTest, StoreAndLoad) {
 
 TEST_F(MetricsLogStoreTest, StoreStagedOngoingLog) {
   // Ensure that types are preserved when storing staged logs.
-  MetricsLogStore log_store(&pref_service_, 0);
+  MetricsLogStore log_store(&pref_service_, 0, std::string());
   log_store.LoadPersistedUnsentLogs();
   log_store.StoreLog("a", MetricsLog::ONGOING_LOG);
   log_store.StageNextLog();
@@ -144,7 +144,7 @@ TEST_F(MetricsLogStoreTest, StoreStagedOngoingLog) {
 
 TEST_F(MetricsLogStoreTest, StoreStagedInitialLog) {
   // Ensure that types are preserved when storing staged logs.
-  MetricsLogStore log_store(&pref_service_, 0);
+  MetricsLogStore log_store(&pref_service_, 0, std::string());
   log_store.LoadPersistedUnsentLogs();
   log_store.StoreLog("b", MetricsLog::INITIAL_STABILITY_LOG);
   log_store.StageNextLog();
@@ -156,7 +156,7 @@ TEST_F(MetricsLogStoreTest, StoreStagedInitialLog) {
 
 TEST_F(MetricsLogStoreTest, LargeLogDiscarding) {
   // Set the size threshold very low, to verify that it's honored.
-  MetricsLogStore log_store(&pref_service_, 1);
+  MetricsLogStore log_store(&pref_service_, 1, std::string());
   log_store.LoadPersistedUnsentLogs();
 
   log_store.StoreLog("persisted", MetricsLog::INITIAL_STABILITY_LOG);
@@ -171,7 +171,7 @@ TEST_F(MetricsLogStoreTest, LargeLogDiscarding) {
 TEST_F(MetricsLogStoreTest, DiscardOrder) {
   // Ensure that the correct log is discarded if new logs are pushed while
   // a log is staged.
-  MetricsLogStore log_store(&pref_service_, 0);
+  MetricsLogStore log_store(&pref_service_, 0, std::string());
   log_store.LoadPersistedUnsentLogs();
 
   log_store.StoreLog("a", MetricsLog::ONGOING_LOG);

components/metrics/metrics_reporting_service.cc

@@ -36,7 +36,9 @@ void MetricsReportingService::RegisterPrefs(PrefRegistrySimple* registry) {
 MetricsReportingService::MetricsReportingService(MetricsServiceClient* client,
                                                  PrefService* local_state)
     : ReportingService(client, local_state, kUploadLogAvoidRetransmitSize),
-      metrics_log_store_(local_state, kUploadLogAvoidRetransmitSize) {}
+      metrics_log_store_(local_state,
+                         kUploadLogAvoidRetransmitSize,
+                         client->GetUploadSigningKey()) {}
 
 MetricsReportingService::~MetricsReportingService() {}
 

components/metrics/metrics_service_client.cc

@@ -4,6 +4,7 @@
 
 #include "components/metrics/metrics_service_client.h"
 
+#include "base/strings/string_util.h"
 #include "components/metrics/url_constants.h"
 
 namespace metrics {
@@ -62,4 +63,8 @@ std::string MetricsServiceClient::GetAppPackageName() {
   return std::string();
 }
 
+std::string MetricsServiceClient::GetUploadSigningKey() {
+  return std::string();
+}
+
 }  // namespace metrics

components/metrics/metrics_service_client.h

@@ -134,6 +134,10 @@ class MetricsServiceClient {
   // name in Android WebView, or an empty string on other platforms.
   virtual std::string GetAppPackageName();
 
+  // Gets the key used to sign metrics uploads. This will be used to compute an
+  // HMAC-SHA256 signature of an uploaded log.
+  virtual std::string GetUploadSigningKey();
+
   // Sets the callback to run MetricsServiceManager::UpdateRunningServices.
   void SetUpdateRunningServicesCallback(const base::Closure& callback);
 

components/metrics/persisted_logs.cc

@@ -52,7 +52,8 @@ PersistedLogs::LogInfo::~LogInfo() {}
 
 void PersistedLogs::LogInfo::Init(PersistedLogsMetrics* metrics,
                                   const std::string& log_data,
-                                  const std::string& log_timestamp) {
+                                  const std::string& log_timestamp,
+                                  const std::string& signing_key) {
   DCHECK(!log_data.empty());
 
   if (!compression::GzipCompress(log_data, &compressed_log_data)) {
@@ -69,7 +70,7 @@ void PersistedLogs::LogInfo::Init(PersistedLogsMetrics* metrics,
   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()) ||
+  if (!hmac.Init(signing_key) ||
       !hmac.Sign(log_data, hmac_data, digest_length)) {
     NOTREACHED() << "HMAC signing failed";
   }
@@ -82,13 +83,15 @@ PersistedLogs::PersistedLogs(std::unique_ptr<PersistedLogsMetrics> metrics,
                              const char* pref_name,
                              size_t min_log_count,
                              size_t min_log_bytes,
-                             size_t max_log_size)
+                             size_t max_log_size,
+                             const std::string& signing_key)
     : metrics_(std::move(metrics)),
       local_state_(local_state),
       pref_name_(pref_name),
       min_log_count_(min_log_count),
       min_log_bytes_(min_log_bytes),
       max_log_size_(max_log_size != 0 ? max_log_size : static_cast<size_t>(-1)),
+      signing_key_(signing_key),
       staged_log_index_(-1) {
   DCHECK(local_state_);
   // One of the limit arguments must be non-zero.
@@ -160,7 +163,8 @@ void PersistedLogs::LoadPersistedUnsentLogs() {
 void PersistedLogs::StoreLog(const std::string& log_data) {
   list_.push_back(LogInfo());
   list_.back().Init(metrics_.get(), log_data,
-                    base::Int64ToString(base::Time::Now().ToTimeT()));
+                    base::Int64ToString(base::Time::Now().ToTimeT()),
+                    signing_key_);
 }
 
 void PersistedLogs::Purge() {

components/metrics/persisted_logs.h

@@ -35,12 +35,17 @@ class PersistedLogs : public LogStore {
   //
   // If the optional |max_log_size| parameter is non-zero, all logs larger than
   // that limit will be skipped when writing to disk.
+  //
+  // |signing_key| is used to produce an HMAC-SHA256 signature of the logged
+  // data, which will be uploaded with the log and used to validate data
+  // integrity.
   PersistedLogs(std::unique_ptr<PersistedLogsMetrics> metrics,
                 PrefService* local_state,
                 const char* pref_name,
                 size_t min_log_count,
                 size_t min_log_bytes,
-                size_t max_log_size);
+                size_t max_log_size,
+                const std::string& signing_key);
   ~PersistedLogs();
 
   // LogStore:
@@ -93,17 +98,26 @@ class PersistedLogs : public LogStore {
   // Logs greater than this size will not be written to disk.
   const size_t max_log_size_;
 
+  // Used to create a signature of log data, in order to verify reported data is
+  // authentic.
+  const std::string signing_key_;
+
   struct LogInfo {
     LogInfo();
     LogInfo(const LogInfo& other);
     ~LogInfo();
 
-    // Initializes the members based on uncompressed |log_data| and
-    // |log_timestamp|.
+    // Initializes the members based on uncompressed |log_data|,
+    // |log_timestamp|, and |signing_key|. |log_data| is the uncompressed
+    // serialized log protobuf. A hash and a signature are computed from
+    // |log_data|. The signature is produced using |signing_key|. |log_data|
+    // will be compressed and storred in |compressed_log_data|. |log_timestamp|
+    // is stored as is.
     // |metrics| is the parent's metrics_ object, and should not be held.
     void Init(PersistedLogsMetrics* metrics,
               const std::string& log_data,
-              const std::string& log_timestamp);
+              const std::string& log_timestamp,
+              const std::string& signing_key);
 
     // Compressed log data - a serialized protobuf that's been gzipped.
     std::string compressed_log_data;

components/metrics/persisted_logs_unittest.cc

@@ -65,12 +65,24 @@ class TestPersistedLogs : public PersistedLogs {
  public:
   TestPersistedLogs(PrefService* service, size_t min_log_bytes)
       : PersistedLogs(std::unique_ptr<PersistedLogsMetricsImpl>(
-                             new PersistedLogsMetricsImpl()),
+                          new PersistedLogsMetricsImpl()),
                       service,
                       kTestPrefName,
                       kLogCountLimit,
                       min_log_bytes,
-                      0) {}
+                      0,
+                      std::string()) {}
+  TestPersistedLogs(PrefService* service,
+                    size_t min_log_bytes,
+                    const std::string& signing_key)
+      : PersistedLogs(std::unique_ptr<PersistedLogsMetricsImpl>(
+                          new PersistedLogsMetricsImpl()),
+                      service,
+                      kTestPrefName,
+                      kLogCountLimit,
+                      min_log_bytes,
+                      0,
+                      signing_key) {}
 
   // Stages and removes the next log, while testing it's value.
   void ExpectNextLog(const std::string& expected_log) {
@@ -298,13 +310,39 @@ TEST_F(PersistedLogsTest, Signatures) {
 
   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);
+  // The expected signature as a base 64 encoded string. The value was obtained
+  // by running the test with an empty expected_signature_base64 and taking the
+  // actual value from the test failure message. Can be verifying by the
+  // following python code:
+  // import hmac, hashlib, base64
+  // key = ''
+  // print(base64.b64encode(
+  //   hmac.new(key, msg='foo', digestmod=hashlib.sha256).digest()).decode())
+  std::string expected_signature_base64 =
+      "DA2Y9+PZ1F5y6Id7wbEEMn77nAexjy/+ztdtgTB/H/8=";
+
+  std::string actual_signature_base64;
+  base::Base64Encode(persisted_logs.staged_log_signature(),
+                     &actual_signature_base64);
+  EXPECT_EQ(expected_signature_base64, actual_signature_base64);
+
+  // Test a different key results in a different signature.
+  std::string key = "secret key, don't tell anyone";
+  TestPersistedLogs persisted_logs_different_key(&prefs_, kLogByteLimit, key);
+
+  persisted_logs_different_key.StoreLog(kFooText);
+  persisted_logs_different_key.StageNextLog();
+
+  EXPECT_EQ(Compress(kFooText), persisted_logs_different_key.staged_log());
+
+  // Base 64 encoded signature obtained in similar fashion to previous
+  // signature. To use previous python code change:
+  // key = "secret key, don't tell anyone"
+  expected_signature_base64 = "DV7z8wdDrjLkQrCzrXR3UjWsR3/YVM97tIhMnhUvfXM=";
+  base::Base64Encode(persisted_logs_different_key.staged_log_signature(),
+                     &actual_signature_base64);
+
+  EXPECT_EQ(expected_signature_base64, actual_signature_base64);
 }
 
 }  // namespace metrics

components/ukm/ukm_reporting_service.cc

@@ -11,6 +11,7 @@
 #include "base/metrics/field_trial_params.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/metrics/histogram_macros.h"
+#include "components/metrics/metrics_service_client.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/ukm/persisted_logs_metrics_impl.h"
 #include "components/ukm/ukm_pref_names.h"
@@ -65,7 +66,8 @@ UkmReportingService::UkmReportingService(metrics::MetricsServiceClient* client,
                       prefs::kUkmPersistedLogs,
                       kMinPersistedLogs,
                       kMinPersistedBytes,
-                      kMaxLogRetransmitSize) {}
+                      kMaxLogRetransmitSize,
+                      client->GetUploadSigningKey()) {}
 
 UkmReportingService::~UkmReportingService() {}
 

components/ukm/ukm_service_unittest.cc

@@ -151,7 +151,7 @@ class UkmServiceTest : public testing::Test {
         prefs::kUkmPersistedLogs,
         3,     // log count limit
         1000,  // byte limit
-        0);
+        0, std::string());
 
     result_persisted_logs.LoadPersistedUnsentLogs();
     result_persisted_logs.StageNextLog();