cryptohome: tpm_manager: move TPM version fingerprint UMA
The UMA of TPM version fingerprint is "Platform.TPM.VersionFingerprint".
It doesn't make sense to report it in cryptohome.
BUG=b:164262829
TEST=FEATURES=test emerge-soraka tpm_manager cryptohome
TEST=check chrome://histograms report same value on soraka.
Change-Id: Iadaac457b3bf5c68209b4435ba7d2cf4f9249db6
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2331765
Tested-by: joe Chou <yich@google.com>
Commit-Queue: joe Chou <yich@google.com>
Reviewed-by: Leo Lai <cylai@google.com>
diff --git a/cryptohome/cryptohome_metrics.cc b/cryptohome/cryptohome_metrics.cc
index 324fec6..3647fa5 100644
--- a/cryptohome/cryptohome_metrics.cc
+++ b/cryptohome/cryptohome_metrics.cc
@@ -71,7 +71,6 @@
constexpr char kCryptohomeLESyncOutcomeHistogramSuffix[] = ".SyncOutcome";
constexpr char kHomedirEncryptionTypeHistogram[] =
"Cryptohome.HomedirEncryptionType";
-constexpr char kTPMVersionFingerprint[] = "Platform.TPM.VersionFingerprint";
constexpr char kDircryptoMigrationNoSpaceFailureFreeSpaceInMbHistogram[] =
"Cryptohome.DircryptoMigrationNoSpaceFailureFreeSpaceInMb";
constexpr char kDircryptoMigrationInitialFreeSpaceInMbHistogram[] =
@@ -435,14 +434,6 @@
g_metrics->SendEnumToUMA(hist_str, result, LE_CRED_ERROR_MAX);
}
-void ReportVersionFingerprint(int fingerprint) {
- if (!g_metrics) {
- return;
- }
-
- g_metrics->SendSparseToUMA(kTPMVersionFingerprint, fingerprint);
-}
-
void ReportDircryptoMigrationFailedNoSpace(int initial_migration_free_space_mb,
int failure_free_space_mb) {
if (!g_metrics) {
diff --git a/cryptohome/cryptohome_metrics.h b/cryptohome/cryptohome_metrics.h
index 0812728..df398b2 100644
--- a/cryptohome/cryptohome_metrics.h
+++ b/cryptohome/cryptohome_metrics.h
@@ -412,10 +412,6 @@
// to the "Cryptohome.LECredential.SyncOutcome" enum histogram.
void ReportLESyncOutcome(LECredError result);
-// Reports the TPM version fingerprint to the "Platform.TPM.VersionFingerprint"
-// histogram.
-void ReportVersionFingerprint(int fingerprint);
-
// Reports the free space in MB when the migration fails and what the free space
// was initially when the migration was started.
void ReportDircryptoMigrationFailedNoSpace(int initial_migration_free_space_mb,
diff --git a/cryptohome/tpm_init.cc b/cryptohome/tpm_init.cc
index 6df3d25..b1e3e96 100644
--- a/cryptohome/tpm_init.cc
+++ b/cryptohome/tpm_init.cc
@@ -173,15 +173,6 @@
RestoreTpmStateFromStorage();
}
- // Collect version statistics.
- if (!statistics_reported_) {
- Tpm::TpmVersionInfo version_info;
- if (get_tpm()->GetVersionInfo(&version_info)) {
- ReportVersionFingerprint(version_info.GetFingerprint());
- statistics_reported_ = true;
- }
- }
-
if (load_key) {
// load cryptohome key
LoadOrCreateCryptohomeKey(&cryptohome_key_);
diff --git a/cryptohome/tpm_init.h b/cryptohome/tpm_init.h
index 5dce8a0..f81fb7e 100644
--- a/cryptohome/tpm_init.h
+++ b/cryptohome/tpm_init.h
@@ -169,7 +169,6 @@
bool take_ownership_called_ = false;
bool took_ownership_ = false;
- bool statistics_reported_ = false;
int64_t initialization_time_ = 0;
Platform* platform_ = nullptr;
TpmPersistentState tpm_persistent_state_;
diff --git a/tpm_manager/server/mock_tpm_manager_metrics.h b/tpm_manager/server/mock_tpm_manager_metrics.h
index 09139f1..26e8bc7 100644
--- a/tpm_manager/server/mock_tpm_manager_metrics.h
+++ b/tpm_manager/server/mock_tpm_manager_metrics.h
@@ -20,6 +20,7 @@
(override));
MOCK_METHOD(void, ReportDictionaryAttackCounter, (int), (override));
+ MOCK_METHOD(void, ReportVersionFingerprint, (int), (override));
};
} // namespace tpm_manager
diff --git a/tpm_manager/server/tpm_manager_metrics.cc b/tpm_manager/server/tpm_manager_metrics.cc
index 37f3e2f..cac2d7e 100644
--- a/tpm_manager/server/tpm_manager_metrics.cc
+++ b/tpm_manager/server/tpm_manager_metrics.cc
@@ -24,4 +24,8 @@
kDictionaryAttackCounterNumBuckets);
}
+void TpmManagerMetrics::ReportVersionFingerprint(int fingerprint) {
+ metrics_library_->SendSparseToUMA(kTPMVersionFingerprint, fingerprint);
+}
+
} // namespace tpm_manager
diff --git a/tpm_manager/server/tpm_manager_metrics.h b/tpm_manager/server/tpm_manager_metrics.h
index d59ab45..9379211 100644
--- a/tpm_manager/server/tpm_manager_metrics.h
+++ b/tpm_manager/server/tpm_manager_metrics.h
@@ -23,6 +23,10 @@
virtual void ReportDictionaryAttackCounter(int counter);
+ // Reports the TPM version fingerprint to the
+ // "Platform.TPM.VersionFingerprint" histogram.
+ virtual void ReportVersionFingerprint(int fingerprint);
+
void set_metrics_library_for_testing(
MetricsLibraryInterface* metrics_library) {
metrics_library_ = metrics_library;
diff --git a/tpm_manager/server/tpm_manager_metrics_names.h b/tpm_manager/server/tpm_manager_metrics_names.h
index 55c4272..3ec03d3 100644
--- a/tpm_manager/server/tpm_manager_metrics_names.h
+++ b/tpm_manager/server/tpm_manager_metrics_names.h
@@ -11,6 +11,7 @@
"Platform.TPM.DictionaryAttackResetStatus";
constexpr char kDictionaryAttackCounterHistogram[] =
"Platform.TPM.DictionaryAttackCounter";
+constexpr char kTPMVersionFingerprint[] = "Platform.TPM.VersionFingerprint";
} // namespace tpm_manager
diff --git a/tpm_manager/server/tpm_manager_service.cc b/tpm_manager/server/tpm_manager_service.cc
index 194e18d..a4d2428 100644
--- a/tpm_manager/server/tpm_manager_service.cc
+++ b/tpm_manager/server/tpm_manager_service.cc
@@ -6,13 +6,17 @@
#include <memory>
#include <string>
+#include <utility>
#include <vector>
#include <base/bind.h>
#include <base/callback.h>
#include <base/command_line.h>
#include <base/message_loop/message_pump_type.h>
+#include <base/strings/stringprintf.h>
#include <base/synchronization/lock.h>
+#include <crypto/sha2.h>
+#include <inttypes.h>
namespace {
@@ -39,6 +43,29 @@
return false;
}
+int GetFingerprint(uint32_t family,
+ uint64_t spec_level,
+ uint32_t manufacturer,
+ uint32_t tpm_model,
+ uint64_t firmware_version,
+ std::string vendor_specific) {
+ // The exact encoding doesn't matter as long as its unambiguous, stable and
+ // contains all information present in the version fields.
+ std::string encoded_parameters =
+ base::StringPrintf("%08" PRIx32 "%016" PRIx64 "%08" PRIx32 "%08" PRIx32
+ "%016" PRIx64 "%016zx",
+ family, spec_level, manufacturer, tpm_model,
+ firmware_version, vendor_specific.size());
+ encoded_parameters.append(vendor_specific);
+ std::string hash = crypto::SHA256HashString(encoded_parameters);
+
+ // Return the first 31 bits from |hash|.
+ int result =
+ static_cast<uint8_t>(hash[0]) | static_cast<uint8_t>(hash[1]) << 8 |
+ static_cast<uint8_t>(hash[2]) << 16 | static_cast<uint8_t>(hash[3]) << 24;
+ return result & 0x7fffffff;
+}
+
} // namespace
namespace tpm_manager {
@@ -85,10 +112,31 @@
base::Closure task =
base::Bind(&TpmManagerService::InitializeTask, base::Unretained(this));
worker_thread_->task_runner()->PostNonNestableTask(FROM_HERE, task);
+ ReportVersionFingerprint();
VLOG(1) << "Worker thread started.";
return true;
}
+void TpmManagerService::ReportVersionFingerprint() {
+ auto callback = base::Bind(
+ [](tpm_manager::TpmManagerMetrics* tpm_manager_metrics,
+ const tpm_manager::GetVersionInfoReply& reply) {
+ if (reply.status() == STATUS_SUCCESS) {
+ uint32_t family = reply.family();
+ uint64_t spec_level = reply.spec_level();
+ uint32_t manufacturer = reply.manufacturer();
+ uint32_t tpm_model = reply.tpm_model();
+ uint64_t firmware_version = reply.firmware_version();
+ std::string vendor_specific = reply.vendor_specific();
+ tpm_manager_metrics->ReportVersionFingerprint(
+ GetFingerprint(family, spec_level, manufacturer, tpm_model,
+ firmware_version, vendor_specific));
+ }
+ },
+ base::Unretained(tpm_manager_metrics_));
+ GetVersionInfo(tpm_manager::GetVersionInfoRequest(), callback);
+}
+
void TpmManagerService::InitializeTask() {
VLOG(1) << "Initializing service...";
diff --git a/tpm_manager/server/tpm_manager_service.h b/tpm_manager/server/tpm_manager_service.h
index ceaf38a..dd13a8f 100644
--- a/tpm_manager/server/tpm_manager_service.h
+++ b/tpm_manager/server/tpm_manager_service.h
@@ -91,6 +91,8 @@
// any other method in this class. Returns true on success.
bool Initialize();
+ void ReportVersionFingerprint();
+
// TpmOwnershipInterface methods.
void GetTpmStatus(const GetTpmStatusRequest& request,
const GetTpmStatusCallback& callback) override;
diff --git a/tpm_manager/server/tpm_manager_service_test.cc b/tpm_manager/server/tpm_manager_service_test.cc
index 462b9da..d49f604 100644
--- a/tpm_manager/server/tpm_manager_service_test.cc
+++ b/tpm_manager/server/tpm_manager_service_test.cc
@@ -23,6 +23,7 @@
using testing::_;
using testing::AtLeast;
+using testing::AtMost;
using testing::Ge;
using testing::Invoke;
using testing::Le;
@@ -55,6 +56,8 @@
public:
~TpmManagerServiceTestBase() override = default;
void SetUp() override {
+ EXPECT_CALL(mock_tpm_manager_metrics_, ReportVersionFingerprint(_))
+ .Times(AtMost(1));
service_.reset(new TpmManagerService(
wait_for_ownership, perform_preinit, &mock_local_data_store_,
&mock_tpm_status_, &mock_tpm_initializer_, &mock_tpm_nvram_,
@@ -365,7 +368,7 @@
Run();
}
-TEST_F(TpmManagerServiceTest, GetVersionInfoSuccess) {
+TEST_F(TpmManagerServiceTest_Preinit, GetVersionInfoSuccess) {
EXPECT_CALL(mock_tpm_status_, GetVersionInfo(_, _, _, _, _, _))
.WillOnce(Invoke([](uint32_t* family, uint64_t* spec_level,
uint32_t* manufacturer, uint32_t* tpm_model,
@@ -380,6 +383,8 @@
return true;
}));
+ SetupService();
+
auto callback = [](TpmManagerServiceTestBase* self, int* call_count,
base::Lock* call_count_lock,
const GetVersionInfoReply& reply) {
@@ -413,10 +418,13 @@
Run();
}
-TEST_F(TpmManagerServiceTest, GetVersionInfoError) {
+TEST_F(TpmManagerServiceTest_Preinit, GetVersionInfoError) {
EXPECT_CALL(mock_tpm_status_, GetVersionInfo(_, _, _, _, _, _))
+ .WillOnce(Return(false))
.WillOnce(Return(false));
+ SetupService();
+
auto callback = [](TpmManagerServiceTestBase* self,
const GetVersionInfoReply& reply) {
EXPECT_EQ(STATUS_DEVICE_ERROR, reply.status());
