metrics: base/hash/sha1 -> crypto/obsolete/sha1
LogInfo uses SHA-1 for hashing log data for unsent logs. This is used to
store a hash of the log SHA-1 to catch errors from memory corruption
within `unsent_log_store`. This gets written and restored from disk.
SHA-1 is also being used for setting the kStabilitySavedSystemProfile
hash within `environment_recorder.cc`.
Because these could affect retrieving prior user preference data and
metrics, this CL migrates to using the crypto/obsolete SHA-1 library.
Mechanical change, no expected behavior change.
Bug: 430344933
Change-Id: Icc224caf4af32162a416c0fb0f62e524b507dbf1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7159550
Reviewed-by: Giovanni Ortuno Urquidi <ortuno@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Antonio Alphonse <alphonsea@google.com>
Reviewed-by: David Benjamin <davidben@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1554241}
NOKEYCHECK=True
GitOrigin-RevId: fe056840db0f047b59790e1db2b6813fd5e036f0
diff --git a/environment_recorder.cc b/environment_recorder.cc
index 49e4fe7..c8f94ae 100644
--- a/environment_recorder.cc
+++ b/environment_recorder.cc
@@ -5,24 +5,24 @@
#include "components/metrics/environment_recorder.h"
#include "base/base64.h"
-#include "base/hash/sha1.h"
#include "base/strings/string_number_conversions.h"
#include "components/metrics/metrics_pref_names.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"
+#include "crypto/obsolete/sha1.h"
#include "third_party/metrics_proto/system_profile.pb.h"
namespace metrics {
-namespace {
-
-// Computes a SHA-1 hash of |data| and returns it as a hex string.
-std::string ComputeSHA1(const std::string& data) {
- return base::HexEncode(base::SHA1Hash(base::as_byte_span(data)));
+// Computes a SHA-1 hash of |data| and returns it as a hex string. This
+// function is intentionally declared in a separate header file
+// "crypto/obsolete/sha1.h", so as to easily monitor current usage of SHA-1 in
+// Chrome, since SHA-1 is now discouraged for new code.
+std::string Sha1AsHexForSystemProfile(std::string_view data) {
+ return base::HexEncode(
+ crypto::obsolete::Sha1::Hash(base::as_byte_span(data)));
}
-} // namespace
-
EnvironmentRecorder::EnvironmentRecorder(PrefService* local_state)
: local_state_(local_state) {}
@@ -38,8 +38,9 @@
base::Base64Encode(serialized_system_profile);
local_state_->SetString(prefs::kStabilitySavedSystemProfile,
base64_system_profile);
- local_state_->SetString(prefs::kStabilitySavedSystemProfileHash,
- ComputeSHA1(serialized_system_profile));
+ local_state_->SetString(
+ prefs::kStabilitySavedSystemProfileHash,
+ Sha1AsHexForSystemProfile(serialized_system_profile));
}
return serialized_system_profile;
@@ -61,7 +62,8 @@
if (!base::Base64Decode(base64_system_profile, &serialized_system_profile)) {
return false;
}
- if (ComputeSHA1(serialized_system_profile) != system_profile_hash) {
+ if (Sha1AsHexForSystemProfile(serialized_system_profile) !=
+ system_profile_hash) {
return false;
}
if (!system_profile->ParseFromString(serialized_system_profile)) {
diff --git a/reporting_service_unittest.cc b/reporting_service_unittest.cc
index de076ef..e4b976f 100644
--- a/reporting_service_unittest.cc
+++ b/reporting_service_unittest.cc
@@ -12,8 +12,8 @@
#include <string_view>
#include "base/functional/bind.h"
-#include "base/hash/sha1.h"
#include "base/strings/string_util.h"
+#include "base/strings/string_view_util.h"
#include "base/test/task_environment.h"
#include "components/metrics/log_store.h"
#include "components/metrics/metrics_log.h"
@@ -21,6 +21,7 @@
#include "components/metrics/metrics_upload_scheduler.h"
#include "components/metrics/test/test_metrics_service_client.h"
#include "components/prefs/testing_pref_service.h"
+#include "crypto/obsolete/sha1.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/zlib/google/compression_utils.h"
@@ -71,7 +72,7 @@
}
void StageNextLog() override {
if (has_unsent_logs()) {
- staged_log_hash_ = base::SHA1HashString(logs_.front().log);
+ staged_log_hash_ = metrics::Sha1ForUnsentLogStore(logs_.front().log);
}
}
void DiscardStagedLog(std::string_view reason) override {
diff --git a/unsent_log_store.cc b/unsent_log_store.cc
index 4d12dd3..09748d1 100644
--- a/unsent_log_store.cc
+++ b/unsent_log_store.cc
@@ -11,7 +11,6 @@
#include <utility>
#include "base/base64.h"
-#include "base/hash/sha1.h"
#include "base/metrics/histogram_macros.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h"
@@ -22,6 +21,7 @@
#include "components/prefs/pref_service.h"
#include "components/prefs/scoped_user_pref_update.h"
#include "crypto/hmac.h"
+#include "crypto/obsolete/sha1.h"
#include "third_party/zlib/google/compression_utils.h"
namespace metrics {
@@ -154,7 +154,7 @@
return;
}
- hash = base::SHA1HashString(log_data);
+ hash = Sha1ForUnsentLogStore(log_data);
signature = ComputeHMACForLog(log_data, signing_key);
timestamp = log_timestamp;
@@ -582,4 +582,14 @@
}
}
+// Computes a SHA-1 hash of |data| and returns it as a string. This is
+// required for backward compatibility with existing on-disk data. This function
+// is intentionally declared in a separate header file "crypto/obsolete/sha1.h",
+// so as to easily monitor current usage of SHA-1 in Chrome, since SHA-1 is now
+// discouraged for new code.
+std::string Sha1ForUnsentLogStore(std::string_view data) {
+ return std::string(base::as_string_view(
+ crypto::obsolete::Sha1::Hash(base::as_byte_span(data))));
+}
+
} // namespace metrics
diff --git a/unsent_log_store_unittest.cc b/unsent_log_store_unittest.cc
index 1d0e349..0cc7b89 100644
--- a/unsent_log_store_unittest.cc
+++ b/unsent_log_store_unittest.cc
@@ -5,16 +5,18 @@
#include "components/metrics/unsent_log_store.h"
#include <stddef.h>
+
#include <limits>
#include "base/base64.h"
-#include "base/hash/sha1.h"
#include "base/rand_util.h"
+#include "base/strings/string_view_util.h"
#include "base/values.h"
#include "components/metrics/unsent_log_store_metrics_impl.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/scoped_user_pref_update.h"
#include "components/prefs/testing_pref_service.h"
+#include "crypto/obsolete/sha1.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/zlib/google/compression_utils.h"
@@ -41,8 +43,9 @@
// Since the size check is done against a compressed log, generate enough
// data that compresses to larger than |log_size|.
std::string rand_bytes = base::RandBytesAsString(min_compressed_size);
- while (Compress(rand_bytes).size() < min_compressed_size)
+ while (Compress(rand_bytes).size() < min_compressed_size) {
rand_bytes.append(base::RandBytesAsString(min_compressed_size));
+ }
SCOPED_TRACE(testing::Message()
<< "Using random data " << base::Base64Encode(rand_bytes));
return rand_bytes;
@@ -183,9 +186,10 @@
LogMetadata log_metadata;
size_t log_count = kLogCountLimit * 5;
- for (size_t i = 0; i < log_count; ++i)
+ for (size_t i = 0; i < log_count; ++i) {
unsent_log_store.StoreLog("x", log_metadata,
MetricsLogsEventManager::CreateReason::kUnknown);
+ }
EXPECT_EQ(log_count, unsent_log_store.size());
unsent_log_store.TrimAndPersistUnsentLogs(/*overwrite_in_memory_store=*/true);
@@ -491,8 +495,9 @@
}
TEST_F(UnsentLogStoreTest, Hashes) {
- const char kFooText[] = "foo";
- const std::string foo_hash = base::SHA1HashString(kFooText);
+ const std::string kFooText = "foo";
+ const std::string foo_hash = Sha1ForUnsentLogStore(kFooText);
+
LogMetadata log_metadata;
TestUnsentLogStore unsent_log_store(&prefs_, kLogByteLimit);
