Fix the hash generation algorithm to be consistent with prior implementation.

BUG=321680
R=bauerb@chromium.org,gab@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@244512 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/prefs/pref_hash_calculator.cc

@@ -16,6 +16,32 @@
 
 namespace {
 
+// Calculates an HMAC of |message| using |key|, encoded as a hexadecimal string.
+std::string GetDigestString(const std::string& key,
+                            const std::string& message) {
+  crypto::HMAC hmac(crypto::HMAC::SHA256);
+  std::vector<uint8> digest(hmac.DigestLength());
+  if (!hmac.Init(key) || !hmac.Sign(message, &digest[0], digest.size())) {
+    NOTREACHED();
+    return std::string();
+  }
+  return base::HexEncode(&digest[0], digest.size());
+}
+
+// Verifies that |digest_string| is a valid HMAC of |message| using |key|.
+// |digest_string| must be encoded as a hexadecimal string.
+bool VerifyDigestString(const std::string& key,
+                        const std::string& message,
+                        const std::string& digest_string) {
+  crypto::HMAC hmac(crypto::HMAC::SHA256);
+  std::vector<uint8> digest;
+  return base::HexStringToBytes(digest_string, &digest) &&
+      hmac.Init(key) &&
+      hmac.Verify(message,
+                  base::StringPiece(reinterpret_cast<char*>(&digest[0]),
+                                    digest.size()));
+}
+
 // Renders |value| as a string. |value| may be NULL, in which case the result
 // is an empty string.
 std::string ValueAsString(const base::Value* value) {
@@ -38,53 +64,63 @@ std::string ValueAsString(const base::Value* value) {
 }
 
 // Common helper for all hash algorithms.
-std::string CalculateFromValueAndComponents(
-    const std::string& seed,
+std::string GetMessageFromValueAndComponents(
     const base::Value* value,
     const std::vector<std::string>& extra_components) {
-  static const size_t kSHA256DigestSize = 32;
+  return JoinString(extra_components, "") + ValueAsString(value);
+}
 
-  std::string message = JoinString(extra_components, "") + ValueAsString(value);
 
-  crypto::HMAC hmac(crypto::HMAC::SHA256);
-  unsigned char digest[kSHA256DigestSize];
-  if (!hmac.Init(seed) || !hmac.Sign(message, digest, arraysize(digest))) {
-    NOTREACHED();
+// Generates a device ID based on the input device ID. The derived device ID has
+// no useful properties beyond those of the input device ID except that it is
+// consistent with previous implementations.
+std::string GenerateDeviceIdLikePrefMetricsServiceDid(
+    const std::string& original_device_id) {
+  if (original_device_id.empty())
     return std::string();
-  }
+  return StringToLowerASCII(
+      GetDigestString(original_device_id, "PrefMetricsService"));
+}
 
-  return base::HexEncode(digest, arraysize(digest));
+// Verifies a hash using a deprecated hash algorithm. For validating old
+// hashes during migration.
+bool VerifyLegacyHash(const std::string& seed,
+                      const base::Value* value,
+                      const std::string& digest_string) {
+  return VerifyDigestString(
+      seed,
+      GetMessageFromValueAndComponents(value, std::vector<std::string>()),
+      digest_string);
 }
 
 }  // namespace
 
 PrefHashCalculator::PrefHashCalculator(const std::string& seed,
                                        const std::string& device_id)
-    : seed_(seed), device_id_(device_id) {}
+    : seed_(seed),
+      device_id_(GenerateDeviceIdLikePrefMetricsServiceDid(device_id)) {}
 
 std::string PrefHashCalculator::Calculate(const std::string& path,
                                           const base::Value* value) const {
-  std::vector<std::string> components;
-  if (!device_id_.empty())
-    components.push_back(device_id_);
-  components.push_back(path);
-  return CalculateFromValueAndComponents(seed_, value, components);
+  return GetDigestString(seed_, GetMessage(path, value));
 }
 
 PrefHashCalculator::ValidationResult PrefHashCalculator::Validate(
     const std::string& path,
     const base::Value* value,
-    const std::string& hash) const {
-  if (hash == Calculate(path, value))
+    const std::string& digest_string) const {
+  if (VerifyDigestString(seed_, GetMessage(path, value), digest_string))
     return VALID;
-  if (hash == CalculateLegacyHash(path, value))
+  if (VerifyLegacyHash(seed_, value, digest_string))
     return VALID_LEGACY;
   return INVALID;
 }
 
-std::string PrefHashCalculator::CalculateLegacyHash(
-    const std::string& path, const base::Value* value) const {
-  return CalculateFromValueAndComponents(seed_,
-                                         value,
-                                         std::vector<std::string>());
+std::string PrefHashCalculator::GetMessage(const std::string& path,
+                                           const base::Value* value) const {
+  std::vector<std::string> components;
+  if (!device_id_.empty())
+    components.push_back(device_id_);
+  components.push_back(path);
+  return GetMessageFromValueAndComponents(value, components);
 }

chrome/browser/prefs/pref_hash_calculator.h

@@ -39,10 +39,9 @@ class PrefHashCalculator {
                             const std::string& hash) const;
 
  private:
-  // Calculate a hash using a deprecated hash algorithm. For validating old
-  // hashes during migration.
-  std::string CalculateLegacyHash(const std::string& path,
-                                  const base::Value* value) const;
+  // Concatenates |device_id_|, |path|, and |value| to give the hash input.
+  std::string GetMessage(const std::string& path,
+                         const base::Value* value) const;
 
   std::string seed_;
   std::string device_id_;

chrome/browser/prefs/pref_hash_calculator_unittest.cc

@@ -71,25 +71,24 @@ TEST(PrefHashCalculatorTest, TestCurrentAlgorithm) {
 
 // Tests the output against a known value to catch unexpected algorithm changes.
 TEST(PrefHashCalculatorTest, CatchHashChanges) {
-  const char* kDeviceId = "test_device_id1";
+  static const char kSeed[] = "0123456789ABCDEF0123456789ABCDEF";
+  static const char kDeviceId[] = "test_device_id1";
   {
     static const char kExpectedValue[] =
-        "5CE37D7EBCBC9BE510F0F5E7C326CA92C1673713C3717839610AEA1A217D8BB8";
+        "D8137B8E767D3D910DCD3821CAC61D26ABB042E6EC406AEB0E347ED73A3A4EC1";
 
     base::ListValue list;
     list.Set(0, new base::FundamentalValue(true));
     list.Set(1, new base::FundamentalValue(100));
     list.Set(2, new base::FundamentalValue(1.0));
 
-    // 32 NULL bytes is the seed that was used to generate the hash in old
-    // tests. Use it again to ensure that we haven't altered the algorithm.
     EXPECT_EQ(PrefHashCalculator::VALID,
-              PrefHashCalculator(std::string(32u, 0), kDeviceId).Validate(
+              PrefHashCalculator(kSeed, kDeviceId).Validate(
                   "pref.path2", &list, kExpectedValue));
   }
   {
     static const char kExpectedValue[] =
-        "A50FE7EB31BFBC32B8A27E71730AF15421178A9B5815644ACE174B18966735B9";
+        "3F947A044DE9E421A735525385B4C789693682E6F6E3E4CB4741E58724B28F96";
 
     base::DictionaryValue dict;
     dict.Set("a", new base::StringValue("foo"));
@@ -97,18 +96,40 @@ TEST(PrefHashCalculatorTest, CatchHashChanges) {
     dict.Set("b", new base::StringValue("bar"));
     dict.Set("c", new base::StringValue("baz"));
 
-    // 32 NULL bytes is the seed that was used to generate the hash in old
-    // tests. Use it again to ensure that we haven't altered the algorithm.
     EXPECT_EQ(PrefHashCalculator::VALID,
-              PrefHashCalculator(std::string(32u, 0), kDeviceId).Validate(
+              PrefHashCalculator(kSeed, kDeviceId).Validate(
                   "pref.path1", &dict, kExpectedValue));
   }
 }
 
+TEST(PrefHashCalculatorTest, TestCompatibilityWithPrefMetricsService) {
+  static const char kSeed[] = {
+    0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+    0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F,
+    0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+    0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F,
+    0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+    0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F,
+    0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+    0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F
+  };
+  static const char kDeviceId[] =
+      "D730D9CBD98C734A4FB097A1922275FE9F7E026A4EA1BE0E84";
+  static const char kExpectedValue[] =
+      "845EF34663FF8D32BE6707F40258FBA531C2BFC532E3B014AFB3476115C2A9DE";
+
+  base::ListValue startup_urls;
+  startup_urls.Set(0, new base::StringValue("http://www.chromium.org/"));
+
+  EXPECT_EQ(PrefHashCalculator::VALID,
+            PrefHashCalculator(std::string(kSeed, arraysize(kSeed)), kDeviceId).
+            Validate("session.startup_urls", &startup_urls, kExpectedValue));
+}
+
 TEST(PrefHashCalculatorTest, TestLegacyAlgorithm) {
-  const char* kExpectedValue =
+  static const char kExpectedValue[] =
       "C503FB7C65EEFD5C07185F616A0AA67923C069909933F362022B1F187E73E9A2";
-  const char* kDeviceId = "deviceid";
+  static const char kDeviceId[] = "not_used";
 
   base::DictionaryValue dict;
   dict.Set("a", new base::StringValue("foo"));
@@ -120,5 +141,4 @@ TEST(PrefHashCalculatorTest, TestLegacyAlgorithm) {
   EXPECT_EQ(PrefHashCalculator::VALID_LEGACY,
             PrefHashCalculator(std::string(32u, 0), kDeviceId).Validate(
                 "pref.path1", &dict, kExpectedValue));
-
 }