Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(686)

Unified Diff: components/variations/variations_seed_store.cc

Issue 2935623004: [Cleanup] Clean up the VariationsSeedStore's histograms. (Closed)
Patch Set: Rebase Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/variations/variations_seed_store.cc
diff --git a/components/variations/variations_seed_store.cc b/components/variations/variations_seed_store.cc
index 27382acefeb97ffc39036bfe427c50e17514eb52..2bf003aefa304319853f797f4ef827ef4a7db6e0 100644
--- a/components/variations/variations_seed_store.cc
+++ b/components/variations/variations_seed_store.cc
@@ -29,16 +29,6 @@ namespace variations {
namespace {
-// Signature verification is disabled on mobile platforms for now, since it
-// adds about ~15ms to the startup time on mobile (vs. a couple ms on desktop).
-bool SignatureVerificationEnabled() {
-#if defined(OS_IOS) || defined(OS_ANDROID)
- return false;
-#else
- return true;
-#endif
-}
-
// The ECDSA public key of the variations server for verifying variations seed
// signatures.
const uint8_t kPublicKey[] = {
@@ -51,55 +41,36 @@ const uint8_t kPublicKey[] = {
0x2b, 0xff, 0x82, 0x4d, 0xd3, 0xfe, 0xc5, 0xef, 0x20, 0xc6, 0xa3, 0x10, 0xbf,
};
-// Note: UMA histogram enum - don't re-order or remove entries.
-enum VariationSeedEmptyState {
- VARIATIONS_SEED_NOT_EMPTY,
- VARIATIONS_SEED_EMPTY,
- VARIATIONS_SEED_CORRUPT,
- VARIATIONS_SEED_INVALID_SIGNATURE,
- VARIATIONS_SEED_CORRUPT_BASE64,
- VARIATIONS_SEED_CORRUPT_PROTOBUF,
- VARIATIONS_SEED_CORRUPT_GZIP,
- VARIATIONS_SEED_EMPTY_ENUM_SIZE,
-};
+// Verifies a variations seed (the serialized proto bytes) with the specified
+// base-64 encoded signature that was received from the server and returns the
+// result. The signature is assumed to be an "ECDSA with SHA-256" signature
+// (see kECDSAWithSHA256AlgorithmID in the .cc file). Returns the result of
+// signature verification.
+VerifySignatureResult VerifySeedSignature(
+ const std::string& seed_bytes,
+ const std::string& base64_seed_signature) {
+ if (base64_seed_signature.empty())
+ return VerifySignatureResult::MISSING_SIGNATURE;
-void RecordVariationSeedEmptyHistogram(VariationSeedEmptyState state) {
- UMA_HISTOGRAM_ENUMERATION("Variations.SeedEmpty", state,
- VARIATIONS_SEED_EMPTY_ENUM_SIZE);
-}
+ std::string signature;
+ if (!base::Base64Decode(base64_seed_signature, &signature))
+ return VerifySignatureResult::DECODE_FAILED;
-enum VariationsSeedStoreResult {
- VARIATIONS_SEED_STORE_SUCCESS,
- VARIATIONS_SEED_STORE_FAILED_EMPTY,
- VARIATIONS_SEED_STORE_FAILED_PARSE,
- VARIATIONS_SEED_STORE_FAILED_SIGNATURE,
- VARIATIONS_SEED_STORE_FAILED_GZIP,
- // DELTA_COUNT is not so much a result of the seed store, but rather counting
- // the number of delta-compressed seeds the SeedStore() function saw. Kept in
- // the same histogram for convenience of comparing against the other values.
- VARIATIONS_SEED_STORE_DELTA_COUNT,
- VARIATIONS_SEED_STORE_FAILED_DELTA_READ_SEED,
- VARIATIONS_SEED_STORE_FAILED_DELTA_APPLY,
- VARIATIONS_SEED_STORE_FAILED_DELTA_STORE,
- VARIATIONS_SEED_STORE_FAILED_UNGZIP,
- VARIATIONS_SEED_STORE_FAILED_EMPTY_GZIP_CONTENTS,
- VARIATIONS_SEED_STORE_FAILED_UNSUPPORTED_SEED_FORMAT,
- VARIATIONS_SEED_STORE_RESULT_ENUM_SIZE,
-};
+ crypto::SignatureVerifier verifier;
+ if (!verifier.VerifyInit(crypto::SignatureVerifier::ECDSA_SHA256,
+ reinterpret_cast<const uint8_t*>(signature.data()),
+ signature.size(), kPublicKey,
+ arraysize(kPublicKey))) {
+ return VerifySignatureResult::INVALID_SIGNATURE;
+ }
-void RecordSeedStoreHistogram(VariationsSeedStoreResult result) {
- UMA_HISTOGRAM_ENUMERATION("Variations.SeedStoreResult", result,
- VARIATIONS_SEED_STORE_RESULT_ENUM_SIZE);
-}
+ verifier.VerifyUpdate(reinterpret_cast<const uint8_t*>(seed_bytes.data()),
+ seed_bytes.size());
+ if (!verifier.VerifyFinal())
+ return VerifySignatureResult::INVALID_SEED;
-// Note: UMA histogram enum - don't re-order or remove entries.
-enum VariationsSeedDateChangeState {
- SEED_DATE_NO_OLD_DATE,
- SEED_DATE_NEW_DATE_OLDER,
- SEED_DATE_SAME_DAY,
- SEED_DATE_NEW_DAY,
- SEED_DATE_ENUM_SIZE,
-};
+ return VerifySignatureResult::VALID_SIGNATURE;
+}
// Truncates a time to the start of the day in UTC. If given a time representing
// 2014-03-11 10:18:03.1 UTC, it will return a time representing
@@ -118,36 +89,20 @@ base::Time TruncateToUTCDay(const base::Time& time) {
return out_time;
}
-VariationsSeedDateChangeState GetSeedDateChangeState(
+UpdateSeedDateResult GetSeedDateChangeState(
const base::Time& server_seed_date,
const base::Time& stored_seed_date) {
if (server_seed_date < stored_seed_date)
- return SEED_DATE_NEW_DATE_OLDER;
+ return UpdateSeedDateResult::NEW_DATE_IS_OLDER;
if (TruncateToUTCDay(server_seed_date) !=
TruncateToUTCDay(stored_seed_date)) {
- // The server date is earlier than the stored date, and they are from
+ // The server date is later than the stored date, and they are from
// different UTC days, so |server_seed_date| is a valid new day.
- return SEED_DATE_NEW_DAY;
+ return UpdateSeedDateResult::NEW_DAY;
}
- return SEED_DATE_SAME_DAY;
-}
-
-#if defined(OS_ANDROID)
-enum FirstRunResult {
- FIRST_RUN_SEED_IMPORT_SUCCESS,
- FIRST_RUN_SEED_IMPORT_FAIL_NO_CALLBACK,
- FIRST_RUN_SEED_IMPORT_FAIL_NO_FIRST_RUN_SEED,
- FIRST_RUN_SEED_IMPORT_FAIL_STORE_FAILED,
- FIRST_RUN_SEED_IMPORT_FAIL_INVALID_RESPONSE_DATE,
- FIRST_RUN_RESULT_ENUM_SIZE,
-};
-
-void RecordFirstRunResult(FirstRunResult result) {
- UMA_HISTOGRAM_ENUMERATION("Variations.FirstRunResult", result,
- FIRST_RUN_RESULT_ENUM_SIZE);
+ return UpdateSeedDateResult::SAME_DAY;
}
-#endif // OS_ANDROID
} // namespace
@@ -166,33 +121,37 @@ bool VariationsSeedStore::LoadSeed(VariationsSeed* seed) {
#endif // OS_ANDROID
std::string seed_data;
- if (!ReadSeedData(&seed_data))
+ LoadSeedResult read_result = ReadSeedData(&seed_data);
+ if (read_result != LoadSeedResult::SUCCESS) {
+ RecordLoadSeedResult(read_result);
return false;
+ }
+
+ if (SignatureVerificationEnabled()) {
+ const std::string base64_signature =
+ local_state_->GetString(prefs::kVariationsSeedSignature);
- const std::string base64_seed_signature =
- local_state_->GetString(prefs::kVariationsSeedSignature);
- const VerifySignatureResult result =
- VerifySeedSignature(seed_data, base64_seed_signature);
- if (result != VARIATIONS_SEED_SIGNATURE_ENUM_SIZE) {
+ const VerifySignatureResult result =
+ VerifySeedSignature(seed_data, base64_signature);
UMA_HISTOGRAM_ENUMERATION("Variations.LoadSeedSignature", result,
- VARIATIONS_SEED_SIGNATURE_ENUM_SIZE);
- if (result != VARIATIONS_SEED_SIGNATURE_VALID) {
- ClearPrefs();
- RecordVariationSeedEmptyHistogram(VARIATIONS_SEED_INVALID_SIGNATURE);
+ VerifySignatureResult::ENUM_SIZE);
+ if (result != VerifySignatureResult::VALID_SIGNATURE) {
// Record the invalid signature.
- invalid_base64_signature_ = base64_seed_signature;
+ invalid_base64_signature_ = base64_signature;
+ ClearPrefs();
+ RecordLoadSeedResult(LoadSeedResult::INVALID_SIGNATURE);
return false;
}
}
if (!seed->ParseFromString(seed_data)) {
ClearPrefs();
- RecordVariationSeedEmptyHistogram(VARIATIONS_SEED_CORRUPT_PROTOBUF);
+ RecordLoadSeedResult(LoadSeedResult::CORRUPT_PROTOBUF);
return false;
}
variations_serial_number_ = seed->serial_number();
- RecordVariationSeedEmptyHistogram(VARIATIONS_SEED_NOT_EMPTY);
+ RecordLoadSeedResult(LoadSeedResult::SUCCESS);
return true;
}
@@ -209,8 +168,7 @@ bool VariationsSeedStore::StoreSeedData(
if (is_gzip_compressed) {
if (compression::GzipUncompress(data, &ungzipped_data)) {
if (ungzipped_data.empty()) {
- RecordSeedStoreHistogram(
- VARIATIONS_SEED_STORE_FAILED_EMPTY_GZIP_CONTENTS);
+ RecordStoreSeedResult(StoreSeedResult::FAILED_EMPTY_GZIP_CONTENTS);
return false;
}
@@ -220,7 +178,7 @@ bool VariationsSeedStore::StoreSeedData(
UMA_HISTOGRAM_COUNTS_1000("Variations.StoreSeed.GzipSize",
data.length() / 1024);
} else {
- RecordSeedStoreHistogram(VARIATIONS_SEED_STORE_FAILED_UNGZIP);
+ RecordStoreSeedResult(StoreSeedResult::FAILED_UNGZIP);
return false;
}
} else {
@@ -240,17 +198,18 @@ bool VariationsSeedStore::StoreSeedData(
// If the data is delta compressed, first decode it.
DCHECK(invalid_base64_signature_.empty());
- RecordSeedStoreHistogram(VARIATIONS_SEED_STORE_DELTA_COUNT);
+ RecordStoreSeedResult(StoreSeedResult::DELTA_COUNT);
std::string existing_seed_data;
std::string updated_seed_data;
- if (!ReadSeedData(&existing_seed_data)) {
- RecordSeedStoreHistogram(VARIATIONS_SEED_STORE_FAILED_DELTA_READ_SEED);
+ LoadSeedResult read_result = ReadSeedData(&existing_seed_data);
+ if (read_result != LoadSeedResult::SUCCESS) {
+ RecordStoreSeedResult(StoreSeedResult::FAILED_DELTA_READ_SEED);
return false;
}
if (!ApplyDeltaPatch(existing_seed_data, ungzipped_data,
&updated_seed_data)) {
- RecordSeedStoreHistogram(VARIATIONS_SEED_STORE_FAILED_DELTA_APPLY);
+ RecordStoreSeedResult(StoreSeedResult::FAILED_DELTA_APPLY);
return false;
}
@@ -266,14 +225,14 @@ bool VariationsSeedStore::StoreSeedData(
UMA_HISTOGRAM_COUNTS_1000("Variations.StoreSeed.DeltaSize",
ungzipped_data.length() / 1024);
} else {
- RecordSeedStoreHistogram(VARIATIONS_SEED_STORE_FAILED_DELTA_STORE);
+ RecordStoreSeedResult(StoreSeedResult::FAILED_DELTA_STORE);
}
return result;
}
void VariationsSeedStore::UpdateSeedDateAndLogDayChange(
const base::Time& server_date_fetched) {
- VariationsSeedDateChangeState date_change = SEED_DATE_NO_OLD_DATE;
+ UpdateSeedDateResult result = UpdateSeedDateResult::NO_OLD_DATE;
if (local_state_->HasPrefPath(prefs::kVariationsSeedDate)) {
const int64_t stored_date_value =
@@ -281,11 +240,11 @@ void VariationsSeedStore::UpdateSeedDateAndLogDayChange(
const base::Time stored_date =
base::Time::FromInternalValue(stored_date_value);
- date_change = GetSeedDateChangeState(server_date_fetched, stored_date);
+ result = GetSeedDateChangeState(server_date_fetched, stored_date);
}
- UMA_HISTOGRAM_ENUMERATION("Variations.SeedDateChange", date_change,
- SEED_DATE_ENUM_SIZE);
+ UMA_HISTOGRAM_ENUMERATION("Variations.SeedDateChange", result,
+ UpdateSeedDateResult::ENUM_SIZE);
local_state_->SetInt64(prefs::kVariationsSeedDate,
server_date_fetched.ToInternalValue());
@@ -304,33 +263,15 @@ void VariationsSeedStore::RegisterPrefs(PrefRegistrySimple* registry) {
registry->RegisterStringPref(prefs::kVariationsSeedSignature, std::string());
}
-VariationsSeedStore::VerifySignatureResult
-VariationsSeedStore::VerifySeedSignature(
- const std::string& seed_bytes,
- const std::string& base64_seed_signature) {
- if (!SignatureVerificationEnabled())
- return VARIATIONS_SEED_SIGNATURE_ENUM_SIZE;
-
- if (base64_seed_signature.empty())
- return VARIATIONS_SEED_SIGNATURE_MISSING;
-
- std::string signature;
- if (!base::Base64Decode(base64_seed_signature, &signature))
- return VARIATIONS_SEED_SIGNATURE_DECODE_FAILED;
-
- crypto::SignatureVerifier verifier;
- if (!verifier.VerifyInit(crypto::SignatureVerifier::ECDSA_SHA256,
- reinterpret_cast<const uint8_t*>(signature.data()),
- signature.size(), kPublicKey,
- arraysize(kPublicKey))) {
- return VARIATIONS_SEED_SIGNATURE_INVALID_SIGNATURE;
- }
-
- verifier.VerifyUpdate(reinterpret_cast<const uint8_t*>(seed_bytes.data()),
- seed_bytes.size());
- if (verifier.VerifyFinal())
- return VARIATIONS_SEED_SIGNATURE_VALID;
- return VARIATIONS_SEED_SIGNATURE_INVALID_SEED;
+bool VariationsSeedStore::SignatureVerificationEnabled() {
+#if defined(OS_IOS) || defined(OS_ANDROID)
+ // Signature verification is disabled on mobile platforms for now, since it
+ // adds about ~15ms to the startup time on mobile (vs. a couple ms on
+ // desktop).
+ return false;
+#else
+ return true;
+#endif
}
void VariationsSeedStore::ClearPrefs() {
@@ -352,50 +293,48 @@ void VariationsSeedStore::ImportFirstRunJavaSeed() {
android::GetVariationsFirstRunSeed(&seed_data, &seed_signature, &seed_country,
&response_date, &is_gzip_compressed);
if (seed_data.empty()) {
- RecordFirstRunResult(FIRST_RUN_SEED_IMPORT_FAIL_NO_FIRST_RUN_SEED);
+ RecordFirstRunSeedImportResult(
+ FirstRunSeedImportResult::FAIL_NO_FIRST_RUN_SEED);
return;
}
base::Time current_date;
if (!base::Time::FromUTCString(response_date.c_str(), &current_date)) {
- RecordFirstRunResult(FIRST_RUN_SEED_IMPORT_FAIL_INVALID_RESPONSE_DATE);
+ RecordFirstRunSeedImportResult(
+ FirstRunSeedImportResult::FAIL_INVALID_RESPONSE_DATE);
LOG(WARNING) << "Invalid response date: " << response_date;
return;
}
if (!StoreSeedData(seed_data, seed_signature, seed_country, current_date,
false, is_gzip_compressed, nullptr)) {
- RecordFirstRunResult(FIRST_RUN_SEED_IMPORT_FAIL_STORE_FAILED);
+ RecordFirstRunSeedImportResult(FirstRunSeedImportResult::FAIL_STORE_FAILED);
LOG(WARNING) << "First run variations seed is invalid.";
return;
}
- RecordFirstRunResult(FIRST_RUN_SEED_IMPORT_SUCCESS);
+ RecordFirstRunSeedImportResult(FirstRunSeedImportResult::SUCCESS);
}
#endif // OS_ANDROID
-bool VariationsSeedStore::ReadSeedData(std::string* seed_data) {
+LoadSeedResult VariationsSeedStore::ReadSeedData(std::string* seed_data) {
std::string base64_seed_data =
local_state_->GetString(prefs::kVariationsCompressedSeed);
- if (base64_seed_data.empty()) {
- RecordVariationSeedEmptyHistogram(VARIATIONS_SEED_EMPTY);
- return false;
- }
+ if (base64_seed_data.empty())
+ return LoadSeedResult::EMPTY;
// If the decode process fails, assume the pref value is corrupt and clear it.
std::string decoded_data;
if (!base::Base64Decode(base64_seed_data, &decoded_data)) {
ClearPrefs();
- RecordVariationSeedEmptyHistogram(VARIATIONS_SEED_CORRUPT_BASE64);
- return false;
+ return LoadSeedResult::CORRUPT_BASE64;
}
if (!compression::GzipUncompress(decoded_data, seed_data)) {
ClearPrefs();
- RecordVariationSeedEmptyHistogram(VARIATIONS_SEED_CORRUPT_GZIP);
- return false;
+ return LoadSeedResult::CORRUPT_GZIP;
}
- return true;
+ return LoadSeedResult::SUCCESS;
}
bool VariationsSeedStore::StoreSeedDataNoDelta(
@@ -405,24 +344,24 @@ bool VariationsSeedStore::StoreSeedDataNoDelta(
const base::Time& date_fetched,
VariationsSeed* parsed_seed) {
if (seed_data.empty()) {
- RecordSeedStoreHistogram(VARIATIONS_SEED_STORE_FAILED_EMPTY_GZIP_CONTENTS);
+ RecordStoreSeedResult(StoreSeedResult::FAILED_EMPTY_GZIP_CONTENTS);
return false;
}
// Only store the seed data if it parses correctly.
VariationsSeed seed;
if (!seed.ParseFromString(seed_data)) {
- RecordSeedStoreHistogram(VARIATIONS_SEED_STORE_FAILED_PARSE);
+ RecordStoreSeedResult(StoreSeedResult::FAILED_PARSE);
return false;
}
- const VerifySignatureResult result =
- VerifySeedSignature(seed_data, base64_seed_signature);
- if (result != VARIATIONS_SEED_SIGNATURE_ENUM_SIZE) {
+ if (SignatureVerificationEnabled()) {
+ const VerifySignatureResult result =
+ VerifySeedSignature(seed_data, base64_seed_signature);
UMA_HISTOGRAM_ENUMERATION("Variations.StoreSeedSignature", result,
- VARIATIONS_SEED_SIGNATURE_ENUM_SIZE);
- if (result != VARIATIONS_SEED_SIGNATURE_VALID) {
- RecordSeedStoreHistogram(VARIATIONS_SEED_STORE_FAILED_SIGNATURE);
+ VerifySignatureResult::ENUM_SIZE);
+ if (result != VerifySignatureResult::VALID_SIGNATURE) {
+ RecordStoreSeedResult(StoreSeedResult::FAILED_SIGNATURE);
return false;
}
}
@@ -430,7 +369,7 @@ bool VariationsSeedStore::StoreSeedDataNoDelta(
// Compress the seed before base64-encoding and storing.
std::string compressed_seed_data;
if (!compression::GzipCompress(seed_data, &compressed_seed_data)) {
- RecordSeedStoreHistogram(VARIATIONS_SEED_STORE_FAILED_GZIP);
+ RecordStoreSeedResult(StoreSeedResult::FAILED_GZIP);
return false;
}
@@ -459,7 +398,7 @@ bool VariationsSeedStore::StoreSeedDataNoDelta(
if (parsed_seed)
seed.Swap(parsed_seed);
- RecordSeedStoreHistogram(VARIATIONS_SEED_STORE_SUCCESS);
+ RecordStoreSeedResult(StoreSeedResult::SUCCESS);
return true;
}
@@ -512,8 +451,7 @@ bool VariationsSeedStore::ApplyDeltaPatch(const std::string& existing_data,
}
void VariationsSeedStore::ReportUnsupportedSeedFormatError() {
- RecordSeedStoreHistogram(
- VARIATIONS_SEED_STORE_FAILED_UNSUPPORTED_SEED_FORMAT);
+ RecordStoreSeedResult(StoreSeedResult::FAILED_UNSUPPORTED_SEED_FORMAT);
}
} // namespace variations
« no previous file with comments | « components/variations/variations_seed_store.h ('k') | components/variations/variations_seed_store_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698