Fix handling of device cloud signing policy key rotation

chrome/browser/chromeos/settings/session_manager_operation.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/settings/session_manager_operation.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 28 matching lines @@
     scoped_refptr<OwnerKeyUtil> owner_key_util,
     scoped_refptr<PublicKey> public_key) {
   session_manager_client_ = session_manager_client;
   owner_key_util_ = owner_key_util;
   public_key_ = public_key;
   Run();
 }
 
 void SessionManagerOperation::RestartLoad(bool key_changed) {
   if (key_changed)
-    public_key_ = NULL;
+    public_key_ = nullptr;
 
   if (!is_loading_)
     return;
 
   // Abort previous load operations.
   weak_factory_.InvalidateWeakPtrs();
   // Mark as not loading to start loading again.
   is_loading_ = false;
   StartLoading();
 }
 
 void SessionManagerOperation::StartLoading() {
   if (is_loading_)
     return;
   is_loading_ = true;
   if (cloud_validations_) {
     EnsurePublicKey(base::Bind(&SessionManagerOperation::RetrieveDeviceSettings,
                                weak_factory_.GetWeakPtr()));
   } else {
     RetrieveDeviceSettings();
   }
 }
 
 void SessionManagerOperation::ReportResult(
     DeviceSettingsService::Status status) {
   callback_.Run(this, status);
 }
 
 void SessionManagerOperation::EnsurePublicKey(const base::Closure& callback) {
-  if (force_key_load_ || !public_key_.get() || !public_key_->is_loaded()) {
+  if (force_key_load_ || !public_key_ || !public_key_->is_loaded()) {
     scoped_refptr<base::TaskRunner> task_runner =
         content::BrowserThread::GetBlockingPool()
             ->GetTaskRunnerWithShutdownBehavior(
                 base::SequencedWorkerPool::SKIP_ON_SHUTDOWN);
     base::PostTaskAndReplyWithResult(
-        task_runner.get(),
-        FROM_HERE,
-        base::Bind(&SessionManagerOperation::LoadPublicKey,
-                   owner_key_util_,
-                   public_key_),
+        task_runner.get(), FROM_HERE,
+        base::Bind(&SessionManagerOperation::LoadPublicKey, owner_key_util_,
+                   force_key_load_ ? nullptr : public_key_),
         base::Bind(&SessionManagerOperation::StorePublicKey,
-                   weak_factory_.GetWeakPtr(),
-                   callback));
+                   weak_factory_.GetWeakPtr(), callback));
   } else {
     callback.Run();
   }
 }
 
 // static
 scoped_refptr<PublicKey> SessionManagerOperation::LoadPublicKey(
     scoped_refptr<OwnerKeyUtil> util,
     scoped_refptr<PublicKey> current_key) {
   scoped_refptr<PublicKey> public_key(new PublicKey());
 
   // Keep already-existing public key.
-  if (current_key.get() && current_key->is_loaded()) {
+  if (current_key && current_key->is_loaded()) {
     public_key->data() = current_key->data();
   }
   if (!public_key->is_loaded() && util->IsPublicKeyPresent()) {
     if (!util->ImportPublicKey(&public_key->data()))
       LOG(ERROR) << "Failed to load public owner key.";
   }
 
   return public_key;
 }
 
 void SessionManagerOperation::StorePublicKey(const base::Closure& callback,
                                              scoped_refptr<PublicKey> new_key) {
   force_key_load_ = false;
   public_key_ = new_key;
 
-  if (!public_key_.get() || !public_key_->is_loaded()) {
+  if (!public_key_ || !public_key_->is_loaded()) {
     ReportResult(DeviceSettingsService::STORE_KEY_UNAVAILABLE);
     return;
   }
 
   callback.Run();
 }
 
 void SessionManagerOperation::RetrieveDeviceSettings() {
   session_manager_client()->RetrieveDevicePolicy(
       base::Bind(&SessionManagerOperation::ValidateDeviceSettings,
@@ skipping 97 matching lines @@
 
 void LoadSettingsOperation::Run() {
   StartLoading();
 }
 
 StoreSettingsOperation::StoreSettingsOperation(
     const Callback& callback,
     std::unique_ptr<em::PolicyFetchResponse> policy)
     : SessionManagerOperation(callback),
       policy_(std::move(policy)),
-      weak_factory_(this) {}
+      weak_factory_(this) {
+  if (policy_->has_new_public_key())
+    force_key_load_ = true;

[Mattias Nissler (ping if slow), 2016/12/08 12:32:21]

This change is good as it'll make sure won't race. FWIW, I'd still advise to
keep the OWNER_KEY_SET handling intact (which you do), since for some cases keys
are generated and installed outside Chrome (e.g. consumer ownership).

+}
 
 StoreSettingsOperation::~StoreSettingsOperation() {}
 
 void StoreSettingsOperation::Run() {
   session_manager_client()->StoreDevicePolicy(
       policy_->SerializeAsString(),
       base::Bind(&StoreSettingsOperation::HandleStoreResult,
                  weak_factory_.GetWeakPtr()));
 }
 
 void StoreSettingsOperation::HandleStoreResult(bool success) {
   if (!success)
     ReportResult(DeviceSettingsService::STORE_OPERATION_FAILED);
   else
     StartLoading();
 }
 
 }  // namespace chromeos