Add UMA for consistency between TPM and install attributes.

chrome/browser/chromeos/policy/enterprise_install_attributes.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/policy/enterprise_install_attributes.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/files/file_util.h"
 #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"
 #include "google_apis/gaia/gaia_auth_util.h"
 
 namespace policy {
 
 namespace cryptohome_util = chromeos::cryptohome_util;
 
 namespace {
 
+// Number of TPM lock state query retries during consistency check.
+int kDbusRetryCount = 12;
+
+// Interval of TPM lock state query retries during consistency check.
+int kDbusRetryIntervalInSeconds = 5;
+
 bool ReadMapKey(const std::map<std::string, std::string>& map,
                 const std::string& key,
                 std::string* value) {
   std::map<std::string, std::string>::const_iterator entry = map.find(key);
   if (entry == map.end())
     return false;
 
   *value = entry->second;
   return true;
 }
@@ skipping 14 matching lines @@
   attribute = install_attrs_proto.add_attributes();
   attribute->set_name(EnterpriseInstallAttributes::kAttrEnterpriseUser);
   attribute->set_value(user_name);
 
   return install_attrs_proto.SerializeAsString();
 }
 
 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) {
 }
 
 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(kDbusRetryCount * kDbusRetryIntervalInSeconds);
+
+  if (!base::PathExists(cache_file))
     return;
 
   device_locked_ = true;
 
   char buf[16384];
   int len = base::ReadFile(cache_file, buf, sizeof(buf));
   if (len == -1 || len >= static_cast<int>(sizeof(buf))) {
     PLOG(ERROR) << "Failed to read " << cache_file.value();
     return;
   }
@@ skipping 63 matching lines @@
   }
   callback.Run();
 }
 
 void EnterpriseInstallAttributes::LockDevice(
     const std::string& user,
     DeviceMode device_mode,
     const std::string& device_id,
     const LockResultCallback& callback) {
   DCHECK(!callback.is_null());
+  CHECK_EQ(registration_running_, false);
   CHECK_NE(device_mode, DEVICE_MODE_PENDING);
   CHECK_NE(device_mode, DEVICE_MODE_NOT_SET);
 
   // Check for existing lock first.
   if (device_locked_) {
     if (device_mode != registration_mode_) {
       callback.Run(LOCK_WRONG_MODE);
       return;
     }
 
@@ skipping 17 matching lines @@
       case DEVICE_MODE_CONSUMER_KIOSK_AUTOLAUNCH:
         // The user parameter is ignored for consumer devices.
         break;
     }
 
     // Already locked in the right mode, signal success.
     callback.Run(LOCK_SUCCESS);
     return;
   }
 
+  registration_running_ = true;

[Mattias Nissler (ping if slow), 2015/06/24 13:26:06]

nit: It's kinda unfortunate (and fragile) to have all these duplicated lines
that reset registration_running_ and then invoke the callback. One way to avoid
this might be to wrap the callback in another function like so:

callback = base::Bind(&EnterpriseInstallAttributes::ReportLockDeviceResult,
                      weak_ptr_factory_.GetWeakPtr(),
                      callback);

And then:

void EnterpriseInstallAttributes::ReportLockDeviceResult(
    const LockResultCallback& callback,
    LockResult result) {
  registration_running_ = false;
  callback.Run(result);
}

[Thiemo Nagel, 2015/06/24 15:27:40]

I guess that one could also do

void EnterpriseInstallAttributes::RunRegistrationEndCallback(
    const LockResultCallback& callback,
    LockResult result) {
  registration_running_ = false;
  callback.Run(result);
}

and then invoke it as

RunRegistrationEndCallback(callback, result);

however for both approaches, I'm not sure whether the benefit justifies the
additional complexity.

[Mattias Nissler (ping if slow), 2015/06/24 18:37:10]

Yes, that's also a valid approach.
It'd probably reduce my personal mental load when reading this when I see that
all the finish-the-operation stuff is handled by a common function. But I'm sure
people will have different opinions, which is why I left the decision to you.

[Thiemo Nagel, 2015/06/25 09:04:06]

Alight.  I'd prefer to keep it as it is, then.

+
+  // 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(),
                  user,
                  device_mode,
                  device_id,
                  callback));
 }
 
 void EnterpriseInstallAttributes::LockDeviceIfAttributesIsReady(
     const std::string& user,
     DeviceMode device_mode,
     const std::string& device_id,
     const LockResultCallback& callback,
     chromeos::DBusMethodCallStatus call_status,
     bool result) {
   if (call_status != chromeos::DBUS_METHOD_CALL_SUCCESS || !result) {
+    registration_running_ = false;
     callback.Run(LOCK_NOT_READY);
     return;
   }
 
   // Clearing the TPM password seems to be always a good deal.
   if (cryptohome_util::TpmIsEnabled() &&
       !cryptohome_util::TpmIsBeingOwned() &&
       cryptohome_util::TpmIsOwned()) {
     cryptohome_client_->CallTpmClearStoredPasswordAndBlock();
   }
 
   // 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;
   }
 
   std::string mode = GetDeviceModeString(device_mode);
   std::string registration_user;
   if (!user.empty())
     registration_user = gaia::CanonicalizeEmail(user);
 
   if (device_mode == DEVICE_MODE_CONSUMER_KIOSK_AUTOLAUNCH) {
     // Set values in the InstallAttrs and lock it.
     if (!cryptohome_util::InstallAttributesSet(kAttrConsumerKioskEnabled,
                                                "true")) {
       LOG(ERROR) << "Failed writing attributes.";
+      registration_running_ = false;
       callback.Run(LOCK_SET_ERROR);
       return;
     }
   } else {
     std::string domain = gaia::ExtractDomainName(registration_user);
     // Set values in the InstallAttrs and lock it.
     if (!cryptohome_util::InstallAttributesSet(kAttrEnterpriseOwned, "true") ||
         !cryptohome_util::InstallAttributesSet(kAttrEnterpriseUser,
                                                registration_user) ||
         !cryptohome_util::InstallAttributesSet(kAttrEnterpriseDomain,
                                                domain) ||
         !cryptohome_util::InstallAttributesSet(kAttrEnterpriseMode, mode) ||
         !cryptohome_util::InstallAttributesSet(kAttrEnterpriseDeviceId,
                                                device_id)) {
       LOG(ERROR) << "Failed writing attributes.";
+      registration_running_ = false;
       callback.Run(LOCK_SET_ERROR);
       return;
     }
   }
 
   if (!cryptohome_util::InstallAttributesFinalize() ||
       cryptohome_util::InstallAttributesIsFirstInstall()) {
     LOG(ERROR) << "Failed locking.";
+    registration_running_ = false;
     callback.Run(LOCK_FINALIZE_ERROR);
     return;
   }
 
   ReadImmutableAttributes(
       base::Bind(&EnterpriseInstallAttributes::OnReadImmutableAttributes,
                  weak_ptr_factory_.GetWeakPtr(),
                  registration_user,
                  callback));
 }
 
 void EnterpriseInstallAttributes::OnReadImmutableAttributes(
     const std::string& registration_user,
     const LockResultCallback& callback) {
 
   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);
 }
 
 bool EnterpriseInstallAttributes::IsEnterpriseDevice() {
   return device_locked_ && !registration_user_.empty();
 }
 
 bool EnterpriseInstallAttributes::IsConsumerKioskDeviceWithAutoLaunch() {
   return device_locked_ &&
          registration_mode_ == DEVICE_MODE_CONSUMER_KIOSK_AUTOLAUNCH;
@@ skipping 17 matching lines @@
   if (!IsEnterpriseDevice())
     return std::string();
 
   return registration_device_id_;
 }
 
 DeviceMode EnterpriseInstallAttributes::GetMode() {
   return registration_mode_;
 }
 
+void EnterpriseInstallAttributes::TriggerConsistencyCheck(
+    int dbus_retries) {
+  consistency_check_running_ = true;
+  cryptohome_client_->TpmIsOwned(base::Bind(
+      &EnterpriseInstallAttributes::OnTpmOwnerCheckCompleted,
+      weak_ptr_factory_.GetWeakPtr(),
+      dbus_retries));
+}
+
+void EnterpriseInstallAttributes::OnTpmOwnerCheckCompleted(
+    int dbus_retries_remaining,
+    chromeos::DBusMethodCallStatus call_status,
+    bool result) {
+  if (call_status != chromeos::DBUS_METHOD_CALL_SUCCESS &&
+      dbus_retries_remaining) {
+    base::MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&EnterpriseInstallAttributes::TriggerConsistencyCheck,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   dbus_retries_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;  // This case is not a bit mask.
+  UMA_HISTOGRAM_ENUMERATION("Enterprise.AttributesTPMConsistency", state, 9);
+
+  // 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();

[Mattias Nissler (ping if slow), 2015/06/24 13:26:06]

nit: add a post_check_action_.reset() here

[Thiemo Nagel, 2015/06/24 14:05:39]

The owner check _should_ only run once, but yes, seems like good practice.

+}
+
 // 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
 // that all changes to the constants are reflected there as well.
 const char EnterpriseInstallAttributes::kConsumerDeviceMode[] = "consumer";
 const char EnterpriseInstallAttributes::kEnterpriseDeviceMode[] = "enterprise";
 const char EnterpriseInstallAttributes::kLegacyRetailDeviceMode[] = "kiosk";
 const char EnterpriseInstallAttributes::kConsumerKioskDeviceMode[] =
     "consumer_kiosk";
 const char EnterpriseInstallAttributes::kUnknownDeviceMode[] = "unknown";
@@ skipping 77 matching lines @@
                         &consumer_kiosk_enabled) &&
              consumer_kiosk_enabled == "true") {
     registration_mode_ = DEVICE_MODE_CONSUMER_KIOSK_AUTOLAUNCH;
   } else if (enterprise_user.empty() && enterprise_owned != "true") {
     // |registration_user_| is empty on consumer devices.
     registration_mode_ = DEVICE_MODE_CONSUMER;
   }
 }
 
 }  // namespace policy