Update FWMP in TPM

chrome/browser/chromeos/login/enrollment/auto_enrollment_controller.cc

 // Copyright 2014 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/login/enrollment/auto_enrollment_controller.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/chromeos/policy/server_backed_state_keys_broker.h"
 #include "chromeos/chromeos_switches.h"
+#include "chromeos/dbus/cryptohome/rpc.pb.h"
+#include "chromeos/dbus/cryptohome_client.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/system/statistics_provider.h"
 #include "components/policy/core/common/cloud/device_management_service.h"
 #include "net/url_request/url_request_context_getter.h"
 
 namespace chromeos {
 
 namespace {
 
 // Maximum time to wait before forcing a decision.  Note that download time for
 // state key buckets can be non-negligible, especially on 2G connections.
@@ skipping 111 matching lines @@
       safeguard_timer_(false, false),
       client_start_weak_factory_(this) {}
 
 AutoEnrollmentController::~AutoEnrollmentController() {}
 
 void AutoEnrollmentController::Start() {
   // This method is called at the point in the OOBE/login flow at which the
   // auto-enrollment check can start. This happens either after the EULA is
   // accepted, or right after a reboot if the EULA has already been accepted.
 
+  // If a client is being created or already existing, bail out.

[Daniel Erat, 2017/03/09 22:15:22]

i'm not familiar with this code, but can you update the CL description to
mention why this is being moved earlier in the method? is it to fix a bug?

[igorcov, 2017/03/10 11:05:44]

Updated the CL description. Basically this change is done so that
safeguard_timer_ is started before we possibly trigger the remove
FWMP operation.

+  if (client_start_weak_factory_.HasWeakPtrs() || client_) {
+    LOG(ERROR) << "Auto-enrollment client is already running.";
+    return;
+  }
+
+  // Arm the belts-and-suspenders timer to avoid hangs.
+  safeguard_timer_.Start(
+      FROM_HERE, base::TimeDelta::FromSeconds(kSafeguardTimeoutSeconds),
+      base::Bind(&AutoEnrollmentController::Timeout, base::Unretained(this)));
+
   // Skip if GAIA is disabled or modulus configuration is not present.
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   if (command_line->HasSwitch(chromeos::switches::kDisableGaiaServices) ||
       (!command_line->HasSwitch(
            chromeos::switches::kEnterpriseEnrollmentInitialModulus) &&
        !command_line->HasSwitch(
            chromeos::switches::kEnterpriseEnrollmentModulusLimit))) {
     VLOG(1) << "Auto-enrollment disabled: command line.";
     UpdateState(policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT);
     return;
   }
 
   // Skip if mode comes up as none.
   if (GetMode() == MODE_NONE) {
     VLOG(1) << "Auto-enrollment disabled: no mode.";
     UpdateState(policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT);
     return;
   }
 
   fre_requirement_ = GetFRERequirement();
   VLOG(1) << FRERequirementToString(fre_requirement_);
   if (fre_requirement_ == EXPLICITLY_NOT_REQUIRED ||
       fre_requirement_ == NOT_REQUIRED) {
     UpdateState(policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT);
     return;
   }
 
-  // If a client is being created or already existing, bail out.
-  if (client_start_weak_factory_.HasWeakPtrs() || client_) {
-    LOG(ERROR) << "Auto-enrollment client is already running.";
-    return;
-  }
-
-  // Arm the belts-and-suspenders timer to avoid hangs.
-  safeguard_timer_.Start(
-      FROM_HERE, base::TimeDelta::FromSeconds(kSafeguardTimeoutSeconds),
-      base::Bind(&AutoEnrollmentController::Timeout, base::Unretained(this)));
-
   // Start by checking if the device has already been owned.
   UpdateState(policy::AUTO_ENROLLMENT_STATE_PENDING);
   DeviceSettingsService::Get()->GetOwnershipStatusAsync(
       base::Bind(&AutoEnrollmentController::OnOwnershipStatusCheckDone,
                  client_start_weak_factory_.GetWeakPtr()));
 }
 
 void AutoEnrollmentController::Cancel() {
   if (client_) {
     // Cancelling the |client_| allows it to determine whether
@@ skipping 89 matching lines @@
   state_ = new_state;
 
   // Stop the safeguard timer once a result comes in.
   switch (state_) {
     case policy::AUTO_ENROLLMENT_STATE_IDLE:
     case policy::AUTO_ENROLLMENT_STATE_PENDING:
       break;
     case policy::AUTO_ENROLLMENT_STATE_CONNECTION_ERROR:
     case policy::AUTO_ENROLLMENT_STATE_SERVER_ERROR:
     case policy::AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT:
-    case policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT:
       safeguard_timer_.Stop();
       break;
+    case policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT:
+      RemoveFirmwareManagementParameters();
+      return;
   }
 
   progress_callbacks_.Notify(state_);
 }
 
+void AutoEnrollmentController::RemoveFirmwareManagementParameters() {
+  DCHECK_EQ(policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT, state_);
+
+  cryptohome::RemoveFirmwareManagementParametersRequest request;
+  chromeos::DBusThreadManager::Get()
+      ->GetCryptohomeClient()
+      ->RemoveFirmwareManagementParametersInTpm(
+          request,
+          base::Bind(
+              &AutoEnrollmentController::OnFirmwareManagementParametersRemoved,
+              client_start_weak_factory_.GetWeakPtr()));
+}
+
+void AutoEnrollmentController::OnFirmwareManagementParametersRemoved(
+    chromeos::DBusMethodCallStatus call_status,
+    bool result,
+    const cryptohome::BaseReply& reply) {
+  if (!result)
+    LOG(ERROR) << "Failed to remove firmware management parameters, error: "

[Daniel Erat, 2017/03/09 22:15:22]

the style guide requires curly brackets around multi-line if/else bodies

[igorcov, 2017/03/10 11:05:44]

Done.

+               << reply.error();
+
+  if (safeguard_timer_.IsRunning()) {
+    safeguard_timer_.Stop();
+    progress_callbacks_.Notify(state_);
+  }
+}
+
 void AutoEnrollmentController::Timeout() {
   // TODO(mnissler): Add UMA to track results of auto-enrollment checks.
   if (client_start_weak_factory_.HasWeakPtrs() &&
       fre_requirement_ != EXPLICITLY_REQUIRED) {
     // If the callbacks to check ownership status or state keys are still
     // pending, there's a bug in the code running on the device. No use in
     // retrying anything, need to fix that bug.
     LOG(ERROR) << "Failed to start auto-enrollment check, fix the code!";
-    UpdateState(policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT);
+    state_ = policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT;
   } else {
     // This can actually happen in some cases, for example when state key
     // generation is waiting for time sync or the server just doesn't reply and
     // keeps the connection open.
     LOG(ERROR) << "AutoEnrollmentClient didn't complete within time limit.";
-    UpdateState(policy::AUTO_ENROLLMENT_STATE_CONNECTION_ERROR);
+    state_ = policy::AUTO_ENROLLMENT_STATE_CONNECTION_ERROR;
   }
 
+  safeguard_timer_.Stop();
+  progress_callbacks_.Notify(state_);
+
   // Reset state.
   Cancel();
 }
 
 }  // namespace chromeos