Revert "Added incident report for variations seed signature mismatch."

> Added incident report for variations seed signature mismatch.
>
> - Keep record of a bad signature in VariationsSeedStore::LoadSeed.
> - Register VerifyVariationsSeedSignature in SafeBrowsingService::RegisterAllDelayedAnalysis to send an incident report, if a bad signature is encountered.
>
> BUG=423467
>
> Review URL: https://codereview.chromium.org/646733002
>
> Cr-Commit-Position: refs/heads/master@{#299964}

This reverts commit 37907039 because it
accesses g_browser_process->variations_service() off of the main thread,
leading to a DCHECK.

BUG=423467
TBR=grt@chromium.org

Review URL: https://codereview.chromium.org/664333002

Cr-Commit-Position: refs/heads/master@{#300479}

chrome/browser/metrics/variations/variations_seed_store.cc

@@ -120,7 +120,6 @@ VariationsSeedStore::~VariationsSeedStore() {
 }
 
 bool VariationsSeedStore::LoadSeed(variations::VariationsSeed* seed) {
-  invalid_base64_signature_.clear();
   const std::string base64_seed_data =
       local_state_->GetString(prefs::kVariationsSeed);
   if (base64_seed_data.empty()) {
@@ -151,8 +150,6 @@ bool VariationsSeedStore::LoadSeed(variations::VariationsSeed* seed) {
               << "with result: " << result << ". Clearing the pref.";
       ClearPrefs();
       RecordVariationSeedEmptyHistogram(VARIATIONS_SEED_INVALID_SIGNATURE);
-      // Record the invalid signature.
-      invalid_base64_signature_ = base64_seed_signature;
       return false;
     }
   }
@@ -275,8 +272,4 @@ VariationsSeedStore::VerifySeedSignature(
   return VARIATIONS_SEED_SIGNATURE_INVALID_SEED;
 }
 
-std::string VariationsSeedStore::GetInvalidSignature() const {
-  return invalid_base64_signature_;
-}
-
 }  // namespace chrome_variations

chrome/browser/metrics/variations/variations_seed_store.h

@@ -55,10 +55,6 @@ class VariationsSeedStore {
   // Registers Local State prefs used by this class.
   static void RegisterPrefs(PrefRegistrySimple* registry);
 
-  // Returns the invalid signature in base64 format, or an empty string if the
-  // signature was valid, missing, or if signature verification is disabled.
-  std::string GetInvalidSignature() const;
-
  protected:
   // Note: UMA histogram enum - don't re-order or remove entries.
   enum VerifySignatureResult {
@@ -92,9 +88,6 @@ class VariationsSeedStore {
   // Cached serial number from the most recently fetched variations seed.
   std::string variations_serial_number_;
 
-  // Keeps track of an invalid signature.
-  std::string invalid_base64_signature_;
-
   DISALLOW_COPY_AND_ASSIGN(VariationsSeedStore);
 };
 

chrome/browser/metrics/variations/variations_seed_store_unittest.cc

@@ -126,60 +126,6 @@ TEST(VariationsSeedStoreTest, LoadSeed) {
   EXPECT_FALSE(seed_store.LoadSeed(&loaded_seed));
 }
 
-TEST(VariationsSeedStoreTest, GetInvalidSignature) {
-  const variations::VariationsSeed seed = CreateTestSeed();
-  std::string seed_hash;
-  const std::string base64_seed = SerializeSeedBase64(seed, &seed_hash);
-
-  TestingPrefServiceSimple prefs;
-  VariationsSeedStore::RegisterPrefs(prefs.registry());
-  prefs.SetString(prefs::kVariationsSeed, base64_seed);
-
-  // The below seed and signature pair were generated using the server's
-  // private key.
-  const std::string base64_seed_data =
-      "CigxZDI5NDY0ZmIzZDc4ZmYxNTU2ZTViNTUxYzY0NDdjYmM3NGU1ZmQwEr0BCh9VTUEtVW5p"
-      "Zm9ybWl0eS1UcmlhbC0xMC1QZXJjZW50GICckqUFOAFCB2RlZmF1bHRKCwoHZGVmYXVsdBAB"
-      "SgwKCGdyb3VwXzAxEAFKDAoIZ3JvdXBfMDIQAUoMCghncm91cF8wMxABSgwKCGdyb3VwXzA0"
-      "EAFKDAoIZ3JvdXBfMDUQAUoMCghncm91cF8wNhABSgwKCGdyb3VwXzA3EAFKDAoIZ3JvdXBf"
-      "MDgQAUoMCghncm91cF8wORAB";
-  const std::string base64_seed_signature =
-      "MEQCIDD1IVxjzWYncun+9IGzqYjZvqxxujQEayJULTlbTGA/AiAr0oVmEgVUQZBYq5VLOSvy"
-      "96JkMYgzTkHPwbv7K/CmgA==";
-  const std::string base64_seed_signature_invalid =
-      "AEQCIDD1IVxjzWYncun+9IGzqYjZvqxxujQEayJULTlbTGA/AiAr0oVmEgVUQZBYq5VLOSvy"
-      "96JkMYgzTkHPwbv7K/CmgA==";
-
-  // Set seed and valid signature in prefs.
-  prefs.SetString(prefs::kVariationsSeed, base64_seed_data);
-  prefs.SetString(prefs::kVariationsSeedSignature, base64_seed_signature);
-
-  VariationsSeedStore seed_store(&prefs);
-  variations::VariationsSeed loaded_seed;
-  seed_store.LoadSeed(&loaded_seed);
-  std::string invalid_signature = seed_store.GetInvalidSignature();
-  // Valid signature so we get an empty string.
-  EXPECT_EQ(std::string(), invalid_signature);
-
-  prefs.SetString(prefs::kVariationsSeedSignature,
-                  base64_seed_signature_invalid);
-  seed_store.LoadSeed(&loaded_seed);
-  // Invalid signature, so we should get the signature itself, except on mobile
-  // where we should get an empty string because verification is not enabled.
-  invalid_signature = seed_store.GetInvalidSignature();
-#if defined(OS_IOS) || defined(OS_ANDROID)
-  EXPECT_EQ(std::string(), invalid_signature);
-#else
-  EXPECT_EQ(base64_seed_signature_invalid, invalid_signature);
-#endif
-
-  prefs.SetString(prefs::kVariationsSeedSignature, std::string());
-  seed_store.LoadSeed(&loaded_seed);
-  invalid_signature = seed_store.GetInvalidSignature();
-  // Empty signature, not considered invalid.
-  EXPECT_EQ(std::string(), invalid_signature);
-}
-
 TEST(VariationsSeedStoreTest, StoreSeedData) {
   const variations::VariationsSeed seed = CreateTestSeed();
   const std::string serialized_seed = SerializeSeed(seed);

chrome/browser/metrics/variations/variations_service.cc

@@ -694,8 +694,4 @@ void VariationsService::RecordLastFetchTime() {
   }
 }
 
-std::string VariationsService::GetInvalidVariationsSeedSignature() const {
-  return seed_store_.GetInvalidSignature();
-}
-
 }  // namespace chrome_variations

chrome/browser/metrics/variations/variations_service.h

@@ -133,11 +133,6 @@ class VariationsService
     policy_pref_service_ = service;
   }
 
-  // Returns the invalid variations seed signature in base64 format, or an empty
-  // string if the signature was valid, missing, or if signature verification is
-  // disabled.
-  std::string GetInvalidVariationsSeedSignature() const;
-
  protected:
   // Starts the fetching process once, where |OnURLFetchComplete| is called with
   // the response.

chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc

@@ -30,7 +30,6 @@
 #include "chrome/browser/safe_browsing/incident_reporting/incident_report_uploader_impl.h"
 #include "chrome/browser/safe_browsing/incident_reporting/preference_validation_delegate.h"
 #include "chrome/browser/safe_browsing/incident_reporting/tracked_preference_incident_handlers.h"
-#include "chrome/browser/safe_browsing/incident_reporting/variations_seed_signature_incident_handlers.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/safe_browsing/csd.pb.h"
@@ -50,9 +49,8 @@ enum IncidentType {
   TRACKED_PREFERENCE = 1,
   BINARY_INTEGRITY = 2,
   BLACKLIST_LOAD = 3,
-  VARIATIONS_SEED_SIGNATURE = 4,
   // Values for new incident types go here.
-  NUM_INCIDENT_TYPES = 5
+  NUM_INCIDENT_TYPES = 4
 };
 
 // The action taken for an incident; used for user metrics (see
@@ -91,8 +89,6 @@ size_t CountIncidents(const ClientIncidentReport_IncidentData& incident) {
     ++result;
   if (incident.has_blacklist_load())
     ++result;
-  if (incident.has_variations_seed_signature())
-    ++result;
   // Add detection for new incident types here.
   return result;
 }
@@ -106,11 +102,9 @@ IncidentType GetIncidentType(
     return BINARY_INTEGRITY;
   if (incident_data.has_blacklist_load())
     return BLACKLIST_LOAD;
-  if (incident_data.has_variations_seed_signature())
-    return VARIATIONS_SEED_SIGNATURE;
 
   // Add detection for new incident types here.
-  COMPILE_ASSERT(VARIATIONS_SEED_SIGNATURE + 1 == NUM_INCIDENT_TYPES,
+  COMPILE_ASSERT(BLACKLIST_LOAD + 1 == NUM_INCIDENT_TYPES,
                  add_support_for_new_types);
   NOTREACHED();
   return NUM_INCIDENT_TYPES;
@@ -147,13 +141,9 @@ PersistentIncidentState ComputeIncidentState(
       state.key = GetBlacklistLoadIncidentKey(incident);
       state.digest = GetBlacklistLoadIncidentDigest(incident);
       break;
-    case VARIATIONS_SEED_SIGNATURE:
-      state.key = GetVariationsSeedSignatureIncidentKey(incident);
-      state.digest = GetVariationsSeedSignatureIncidentDigest(incident);
-      break;
     // Add handling for new incident types here.
     default:
-      COMPILE_ASSERT(VARIATIONS_SEED_SIGNATURE + 1 == NUM_INCIDENT_TYPES,
+      COMPILE_ASSERT(BLACKLIST_LOAD + 1 == NUM_INCIDENT_TYPES,
                      add_support_for_new_types);
       NOTREACHED();
       break;

chrome/browser/safe_browsing/incident_reporting/variations_seed_signature_analyzer.cc

-// Copyright 2014 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.
-
-#include "chrome/browser/safe_browsing/incident_reporting/variations_seed_signature_analyzer.h"
-
-#include <string>
-
-#include "chrome/browser/browser_process.h"
-#include "chrome/browser/metrics/variations/variations_service.h"
-#include "chrome/browser/safe_browsing/safe_browsing_service.h"
-#include "chrome/common/safe_browsing/csd.pb.h"
-
-namespace safe_browsing {
-
-void RegisterVariationsSeedSignatureAnalysis() {
-  scoped_refptr<SafeBrowsingService> safe_browsing_service(
-      g_browser_process->safe_browsing_service());
-
-  safe_browsing_service->RegisterDelayedAnalysisCallback(
-      base::Bind(&VerifyVariationsSeedSignature));
-}
-
-void VerifyVariationsSeedSignature(const AddIncidentCallback& callback) {
-  if (g_browser_process->variations_service() == nullptr)
-    return;
-  std::string invalid_signature = g_browser_process->variations_service()
-                                      ->GetInvalidVariationsSeedSignature();
-  if (!invalid_signature.empty()) {
-    scoped_ptr<ClientIncidentReport_IncidentData> incident_data(
-        new ClientIncidentReport_IncidentData());
-    ClientIncidentReport_IncidentData_VariationsSeedSignatureIncident*
-        variations_seed_signature =
-            incident_data->mutable_variations_seed_signature();
-    variations_seed_signature->set_variations_seed_signature(invalid_signature);
-    callback.Run(incident_data.Pass());
-  }
-}
-
-}  // namespace safe_browsing

chrome/browser/safe_browsing/incident_reporting/variations_seed_signature_analyzer.h

-// Copyright 2014 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 CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_VARIATIONS_SEED_SIGNATURE_ANALYZER_H_
-#define CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_VARIATIONS_SEED_SIGNATURE_ANALYZER_H_
-
-#include "base/strings/string16.h"
-#include "chrome/browser/safe_browsing/incident_reporting/add_incident_callback.h"
-
-namespace safe_browsing {
-
-// Registers a process-wide analysis with the incident reporting service that
-// will verify the variations seed signature.
-void RegisterVariationsSeedSignatureAnalysis();
-
-// Callback to pass to the incident reporting service. The incident reporting
-// service will verify if the variations seed signature is invalid.
-void VerifyVariationsSeedSignature(const AddIncidentCallback& callback);
-
-}  // namespace safe_browsing
-
-#endif  // CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_VARIATIONS_SEED_SIGNATURE_ANALYZER_H_

chrome/browser/safe_browsing/incident_reporting/variations_seed_signature_incident_handlers.cc

-// Copyright 2014 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.
-
-#include "chrome/browser/safe_browsing/incident_reporting/variations_seed_signature_incident_handlers.h"
-
-#include "base/logging.h"
-#include "chrome/browser/safe_browsing/incident_reporting/incident_handler_util.h"
-#include "chrome/common/safe_browsing/csd.pb.h"
-
-namespace safe_browsing {
-
-std::string GetVariationsSeedSignatureIncidentKey(
-    const ClientIncidentReport_IncidentData& incident_data) {
-  DCHECK(incident_data.has_variations_seed_signature());
-  return "variations_seed_signature";
-}
-
-uint32_t GetVariationsSeedSignatureIncidentDigest(
-    const ClientIncidentReport_IncidentData& incident_data) {
-  DCHECK(incident_data.has_variations_seed_signature());
-  return HashMessage(incident_data.variations_seed_signature());
-}
-
-}  // namespace safe_browsing

chrome/browser/safe_browsing/incident_reporting/variations_seed_signature_incident_handlers.h

-// Copyright 2014 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 CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_VARIATIONS_SEED_SIGNATURE_INCIDENT_HANDLERS_H_
-#define CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_VARIATIONS_SEED_SIGNATURE_INCIDENT_HANDLERS_H_
-
-#include <stdint.h>
-
-#include <string>
-
-namespace safe_browsing {
-
-class ClientIncidentReport_IncidentData;
-
-// All variations seed signature incidents return the same constant key so only
-// one incident report is sent.
-std::string GetVariationsSeedSignatureIncidentKey(
-    const ClientIncidentReport_IncidentData& incident_data);
-
-// Returns a digest of the invalid seed signature value.
-uint32_t GetVariationsSeedSignatureIncidentDigest(
-    const ClientIncidentReport_IncidentData& incident_data);
-
-}  // namespace safe_browsing
-
-#endif  // CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_VARIATIONS_SEED_SIGNATURE_INCIDENT_HANDLERS_H_

chrome/browser/safe_browsing/incident_reporting/variations_seed_signature_incident_handlers_unittest.cc

-// Copyright 2014 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.
-
-#include "chrome/browser/safe_browsing/incident_reporting/variations_seed_signature_incident_handlers.h"
-
-#include "base/memory/scoped_ptr.h"
-#include "chrome/common/safe_browsing/csd.pb.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace safe_browsing {
-
-namespace {
-
-scoped_ptr<ClientIncidentReport_IncidentData> MakeIncident() {
-  scoped_ptr<ClientIncidentReport_IncidentData> incident(
-      new ClientIncidentReport_IncidentData);
-  incident->mutable_variations_seed_signature()->set_variations_seed_signature(
-      "MEUCID+QmAfajh/kk4zZyv0IUisZ84sIddnjiW9yAXjFJIMFAiEAtVUHhFA/4M6Bff2GazL"
-      "7tXVLhURxUQcpiMg9eMLWO0U=");
-  return incident.Pass();
-}
-
-}  // namespace
-
-// Tests that GetKey returns a constant.
-TEST(GetVariationsSeedSignatureIncidentKey, KeyIsConstant) {
-  ClientIncidentReport_IncidentData incident;
-
-  incident.mutable_variations_seed_signature()->set_variations_seed_signature(
-      "MEUCID+QmAfajh/kk4zZyv0IUisZ84sIddnjiW9yAXjFJIMFAiEAtVUHhFA/4M6Bff2GazL"
-      "7tXVLhURxUQcpiMg9eMLWO0U=");
-  EXPECT_EQ(std::string("variations_seed_signature"),
-            GetVariationsSeedSignatureIncidentKey(incident));
-  incident.mutable_variations_seed_signature()->set_variations_seed_signature(
-      "AEUCID+QmAfajh/kk4zZyv0IUisZ84sIddnjiW9yAXjFJIMFAiEAtVUHhFA/4M6Bff2GazL"
-      "7tXVLhURxUQcpiMg9eMLWO0U=");
-  EXPECT_EQ("variations_seed_signature",
-            GetVariationsSeedSignatureIncidentKey(incident));
-}
-
-// Tests that GetDigest returns the same value for the same incident.
-TEST(GetVariationsSeedSignatureIncidentDigest, SameIncidentSameDigest) {
-  scoped_ptr<ClientIncidentReport_IncidentData> incident(MakeIncident());
-
-  uint32_t digest = GetVariationsSeedSignatureIncidentDigest(*incident);
-  EXPECT_EQ(digest, GetVariationsSeedSignatureIncidentDigest(*MakeIncident()));
-}
-
-// Tests that GetDigest returns different values for different incidents.
-TEST(GetVariationsSeedSignatureIncidentDigest,
-     DifferentIncidentDifferentDigest) {
-  scoped_ptr<ClientIncidentReport_IncidentData> incident(MakeIncident());
-  incident->mutable_variations_seed_signature()->set_variations_seed_signature(
-      "MEUCID+QmAfajh/kk4zZyv0IUisZ84sIddnjiW9yAXjFJIMFAiEAtVUHhFA/4M6Bff2GazL"
-      "7tXVLhURxUQcpiMg9eMLWO0U=");
-
-  scoped_ptr<ClientIncidentReport_IncidentData> incident2(MakeIncident());
-  incident2->mutable_variations_seed_signature()->set_variations_seed_signature(
-      "AEUCID+QmAfajh/kk4zZyv0IUisZ84sIddnjiW9yAXjFJIMFAiEAtVUHhFA/4M6Bff2GazL"
-      "7tXVLhURxUQcpiMg9eMLWO0U=");
-
-  EXPECT_NE(GetVariationsSeedSignatureIncidentDigest(*incident),
-            GetVariationsSeedSignatureIncidentDigest(*incident2));
-}
-
-}  // namespace safe_browsing

chrome/browser/safe_browsing/safe_browsing_service.cc

@@ -30,7 +30,6 @@
 #include "chrome/browser/safe_browsing/incident_reporting/binary_integrity_analyzer.h"
 #include "chrome/browser/safe_browsing/incident_reporting/blacklist_load_analyzer.h"
 #include "chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.h"
-#include "chrome/browser/safe_browsing/incident_reporting/variations_seed_signature_analyzer.h"
 #include "chrome/browser/safe_browsing/malware_details.h"
 #include "chrome/browser/safe_browsing/ping_manager.h"
 #include "chrome/browser/safe_browsing/protocol_manager.h"
@@ -363,7 +362,6 @@ SafeBrowsingDatabaseManager* SafeBrowsingService::CreateDatabaseManager() {
 void SafeBrowsingService::RegisterAllDelayedAnalysis() {
   safe_browsing::RegisterBinaryIntegrityAnalysis();
   safe_browsing::RegisterBlacklistLoadAnalysis();
-  safe_browsing::RegisterVariationsSeedSignatureAnalysis();
 }
 
 void SafeBrowsingService::InitURLRequestContextOnIOThread(

chrome/chrome_browser.gypi

@@ -2617,10 +2617,6 @@
       'browser/safe_browsing/incident_reporting/preference_validation_delegate.h',
       'browser/safe_browsing/incident_reporting/tracked_preference_incident_handlers.cc',
       'browser/safe_browsing/incident_reporting/tracked_preference_incident_handlers.h',
-      'browser/safe_browsing/incident_reporting/variations_seed_signature_analyzer.cc',
-      'browser/safe_browsing/incident_reporting/variations_seed_signature_analyzer.h',
-      'browser/safe_browsing/incident_reporting/variations_seed_signature_incident_handlers.cc',
-      'browser/safe_browsing/incident_reporting/variations_seed_signature_incident_handlers.h',
       'browser/safe_browsing/path_sanitizer.cc',
       'browser/safe_browsing/path_sanitizer.h',
       'browser/safe_browsing/pe_image_reader_win.cc',

chrome/chrome_tests_unit.gypi

@@ -681,7 +681,6 @@
       'browser/safe_browsing/incident_reporting/module_integrity_verifier_win_unittest.cc',
       'browser/safe_browsing/incident_reporting/preference_validation_delegate_unittest.cc',
       'browser/safe_browsing/incident_reporting/tracked_preference_incident_handlers_unittest.cc',
-      'browser/safe_browsing/incident_reporting/variations_seed_signature_incident_handlers_unittest.cc',
       'browser/safe_browsing/local_two_phase_testserver.cc',
       'browser/safe_browsing/malware_details_unittest.cc',
       'browser/safe_browsing/path_sanitizer_unittest.cc',

chrome/common/safe_browsing/csd.proto

@@ -362,14 +362,10 @@ message ClientIncidentReport {
       optional string version = 3;
       optional bool blacklist_initialized = 4;
     }
-    message VariationsSeedSignatureIncident {
-      optional string variations_seed_signature = 1;
-    }
     optional int64 incident_time_msec = 1;
     optional TrackedPreferenceIncident tracked_preference = 2;
     optional BinaryIntegrityIncident binary_integrity = 3;
     optional BlacklistLoadIncident blacklist_load = 4;
-    optional VariationsSeedSignatureIncident variations_seed_signature = 5;
   }
 
   repeated IncidentData incident = 1;

tools/metrics/histograms/histograms.xml

@@ -45102,7 +45102,6 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   <int value="1" label="TrackedPreference"/>
   <int value="2" label="BinaryIntegrity"/>
   <int value="3" label="BlacklistLoad"/>
-  <int value="4" label="VariationsSeedSignature"/>
 </enum>
 
 <enum name="Inconsistencies" type="int">