Add UMA for consistency between TPM and install attributes.

chrome/browser/chromeos/policy/enterprise_install_attributes.cc

Index: chrome/browser/chromeos/policy/enterprise_install_attributes.cc
diff --git a/chrome/browser/chromeos/policy/enterprise_install_attributes.cc b/chrome/browser/chromeos/policy/enterprise_install_attributes.cc
index 3fbbc8d00d2e38def39a61379e44f1b2625da2e1..ac3d372fab67a0967a25e814ef1b5693dd24e39e 100644
--- a/chrome/browser/chromeos/policy/enterprise_install_attributes.cc
+++ b/chrome/browser/chromeos/policy/enterprise_install_attributes.cc
@@ -11,6 +11,9 @@
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
+#include "base/metrics/histogram_base.h"
+#include "base/metrics/histogram_macros.h"
+#include "base/time/time.h"
 #include "chrome/browser/chromeos/policy/proto/install_attributes.pb.h"
 #include "chromeos/cryptohome/cryptohome_util.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
@@ -22,6 +25,12 @@ namespace cryptohome_util = chromeos::cryptohome_util;
 
 namespace {
 
+// Retry interval for consistency check against TPM lock state.
+int kDbusRetryIntervalInSeconds = 5;
+
+// Total time during which of TPM lock state queries are retried.
+int kDbusRetryDurationInSeconds = 60;
+
 bool ReadMapKey(const std::map<std::string, std::string>& map,
                 const std::string& key,
                 std::string* value) {
@@ -56,6 +65,8 @@ EnterpriseInstallAttributes::GetEnterpriseOwnedInstallAttributesBlobForTesting(
 EnterpriseInstallAttributes::EnterpriseInstallAttributes(
     chromeos::CryptohomeClient* cryptohome_client)
     : device_locked_(false),
+      consistency_check_running_(false),
+      registration_running_(false),
       registration_mode_(DEVICE_MODE_PENDING),
       cryptohome_client_(cryptohome_client),
       weak_ptr_factory_(this) {
@@ -63,9 +74,15 @@ EnterpriseInstallAttributes::EnterpriseInstallAttributes(
 
 EnterpriseInstallAttributes::~EnterpriseInstallAttributes() {}
 
-void EnterpriseInstallAttributes::ReadCacheFile(
-    const base::FilePath& cache_file) {
-  if (device_locked_ || !base::PathExists(cache_file))
+void EnterpriseInstallAttributes::Init(const base::FilePath& cache_file) {
+  DCHECK_EQ(false, device_locked_);
+
+  // The actual check happens asynchronously, thus it is ok to trigger it before
+  // Init() has completed.
+  TriggerConsistencyCheck(
+      kDbusRetryDurationInSeconds / kDbusRetryIntervalInSeconds);
+
+  if (!base::PathExists(cache_file))
     return;
 
   device_locked_ = true;
@@ -149,6 +166,7 @@ void EnterpriseInstallAttributes::LockDevice(
     const std::string& device_id,
     const LockResultCallback& callback) {
   DCHECK(!callback.is_null());
+  DCHECK_EQ(registration_running_, false);

[Mattias Nissler (ping if slow), 2015/06/24 11:55:03]

Let's make this a CHECK_EQ so we fail fast if this gets triggered by races in
callers and whatnot that may slip through QA.

[Thiemo Nagel, 2015/06/24 12:49:52]

Done.

   CHECK_NE(device_mode, DEVICE_MODE_PENDING);
   CHECK_NE(device_mode, DEVICE_MODE_NOT_SET);
 
@@ -186,6 +204,21 @@ void EnterpriseInstallAttributes::LockDevice(
     return;
   }
 
+  registration_running_ = true;
+
+  // In case the consistency check is still running, postpone the device locking
+  // until it has finished.  This should not introduce additional delay since
+  // device locking must wait for TPM initialization anyways.
+  if (consistency_check_running_) {
+    post_check_action_ = base::Bind(&EnterpriseInstallAttributes::LockDevice,
+                                    weak_ptr_factory_.GetWeakPtr(),
+                                    user,
+                                    device_mode,
+                                    device_id,
+                                    callback);
+    return;
+  }
+
   cryptohome_client_->InstallAttributesIsReady(
       base::Bind(&EnterpriseInstallAttributes::LockDeviceIfAttributesIsReady,
                  weak_ptr_factory_.GetWeakPtr(),
@@ -203,6 +236,7 @@ void EnterpriseInstallAttributes::LockDeviceIfAttributesIsReady(
     chromeos::DBusMethodCallStatus call_status,
     bool result) {
   if (call_status != chromeos::DBUS_METHOD_CALL_SUCCESS || !result) {
+    registration_running_ = false;
     callback.Run(LOCK_NOT_READY);
     return;
   }
@@ -217,12 +251,14 @@ void EnterpriseInstallAttributes::LockDeviceIfAttributesIsReady(
   // Make sure we really have a working InstallAttrs.
   if (cryptohome_util::InstallAttributesIsInvalid()) {
     LOG(ERROR) << "Install attributes invalid.";
+    registration_running_ = false;
     callback.Run(LOCK_BACKEND_INVALID);
     return;
   }
 
   if (!cryptohome_util::InstallAttributesIsFirstInstall()) {
     LOG(ERROR) << "Install attributes already installed.";
+    registration_running_ = false;
     callback.Run(LOCK_ALREADY_LOCKED);
     return;
   }
@@ -237,6 +273,7 @@ void EnterpriseInstallAttributes::LockDeviceIfAttributesIsReady(
     if (!cryptohome_util::InstallAttributesSet(kAttrConsumerKioskEnabled,
                                                "true")) {
       LOG(ERROR) << "Failed writing attributes.";
+      registration_running_ = false;
       callback.Run(LOCK_SET_ERROR);
       return;
     }
@@ -252,6 +289,7 @@ void EnterpriseInstallAttributes::LockDeviceIfAttributesIsReady(
         !cryptohome_util::InstallAttributesSet(kAttrEnterpriseDeviceId,
                                                device_id)) {
       LOG(ERROR) << "Failed writing attributes.";
+      registration_running_ = false;
       callback.Run(LOCK_SET_ERROR);
       return;
     }
@@ -260,6 +298,7 @@ void EnterpriseInstallAttributes::LockDeviceIfAttributesIsReady(
   if (!cryptohome_util::InstallAttributesFinalize() ||
       cryptohome_util::InstallAttributesIsFirstInstall()) {
     LOG(ERROR) << "Failed locking.";
+    registration_running_ = false;
     callback.Run(LOCK_FINALIZE_ERROR);
     return;
   }
@@ -277,10 +316,12 @@ void EnterpriseInstallAttributes::OnReadImmutableAttributes(
 
   if (GetRegistrationUser() != registration_user) {
     LOG(ERROR) << "Locked data doesn't match.";
+    registration_running_ = false;
     callback.Run(LOCK_READBACK_ERROR);
     return;
   }
 
+  registration_running_ = false;
   callback.Run(LOCK_SUCCESS);
 }
 
@@ -318,6 +359,45 @@ DeviceMode EnterpriseInstallAttributes::GetMode() {
   return registration_mode_;
 }
 
+void EnterpriseInstallAttributes::TriggerConsistencyCheck(
+    int dbus_tries_remaining) {
+  consistency_check_running_ = true;
+  cryptohome_client_->TpmIsOwned(base::Bind(
+      &EnterpriseInstallAttributes::OnTpmOwnerCheckCompleted,
+      weak_ptr_factory_.GetWeakPtr(),
+      dbus_tries_remaining));
+}
+
+void EnterpriseInstallAttributes::OnTpmOwnerCheckCompleted(
+    int dbus_tries_remaining,
+    chromeos::DBusMethodCallStatus call_status,
+    bool result) {
+  if (call_status != chromeos::DBUS_METHOD_CALL_SUCCESS &&
+      dbus_tries_remaining) {
+    base::MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&EnterpriseInstallAttributes::TriggerConsistencyCheck,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   dbus_tries_remaining - 1),
+        base::TimeDelta::FromSeconds(kDbusRetryIntervalInSeconds));
+    return;
+  }
+
+  base::HistogramBase::Sample state = device_locked_;
+  state |= 0x2 * (registration_mode_ == DEVICE_MODE_ENTERPRISE);
+  if (call_status == chromeos::DBUS_METHOD_CALL_SUCCESS)
+    state |= 0x4 * result;
+  else
+    state |= 0x8;
+  UMA_HISTOGRAM_ENUMERATION("Enterprise.AttributesTPMConsistency", state, 12);
+
+  // Run any action (LockDevice call) that might have queued behind the
+  // consistency check.
+  consistency_check_running_ = false;
+  if (!post_check_action_.is_null())
+    post_check_action_.Run();
+}
+
 // Warning: The values for these keys (but not the keys themselves) are stored
 // in the protobuf with a trailing zero.  Also note that some of these constants
 // have been copied to login_manager/device_policy_service.cc.  Please make sure