Skip to content

Commit

Permalink
Remove attestation_policy_observer and upload EMCert directly
Browse files Browse the repository at this point in the history
This is part of the client-side cleanup of the unused attestation
policies (see b/285556135 for more context). Since the policy
AttestationEnabledForDevice is always set to true, the machine
certificate is now uploaded directly.

Bug: b:285556135

Change-Id: Iac112f6b0b565a2d175e4df9246d050785b3cb0f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4973840
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Leon Masopust <lmasopust@google.com>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1216899}
  • Loading branch information
Leon Masopust authored and Chromium LUCI CQ committed Oct 30, 2023
1 parent 5d8e293 commit f735e83
Show file tree
Hide file tree
Showing 8 changed files with 15 additions and 172 deletions.
3 changes: 0 additions & 3 deletions chrome/browser/ash/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -656,8 +656,6 @@ source_set("ash") {
"assistant/assistant_util.h",
"attestation/attestation_ca_client.cc",
"attestation/attestation_ca_client.h",
"attestation/attestation_policy_observer.cc",
"attestation/attestation_policy_observer.h",
"attestation/certificate_util.cc",
"attestation/certificate_util.h",
"attestation/enrollment_certificate_uploader.h",
Expand Down Expand Up @@ -5227,7 +5225,6 @@ source_set("unit_tests") {
"arc/wallpaper/arc_wallpaper_service_unittest.cc",
"assistant/assistant_util_unittest.cc",
"attestation/attestation_ca_client_unittest.cc",
"attestation/attestation_policy_observer_unittest.cc",
"attestation/attestation_policy_unittest.cc",
"attestation/certificate_util_unittest.cc",
"attestation/enrollment_certificate_uploader_impl_unittest.cc",
Expand Down
51 changes: 0 additions & 51 deletions chrome/browser/ash/attestation/attestation_policy_observer.cc

This file was deleted.

51 changes: 0 additions & 51 deletions chrome/browser/ash/attestation/attestation_policy_observer.h

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <memory>
#include <string>

#include "base/check_is_test.h"
#include "base/functional/bind.h"
#include "base/functional/callback.h"
#include "base/functional/callback_helpers.h"
Expand Down Expand Up @@ -106,6 +107,14 @@ void MachineCertificateUploaderImpl::Start() {
return;
}

// We always expect a valid attestation client, except for testing scenarios.
if (!AttestationClient::Get()) {
CHECK_IS_TEST();
certificate_uploaded_ = false;
RunCallbacks(certificate_uploaded_.value());
return;
}

if (!attestation_flow_) {
std::unique_ptr<ServerProxy> attestation_ca_client(
new AttestationCAClient());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ class MachineCertificateUploaderImpl : public MachineCertificateUploader {

raw_ptr<policy::CloudPolicyClient, DanglingUntriaged | ExperimentalAsh>
policy_client_ = nullptr;
raw_ptr<AttestationFlow, ExperimentalAsh> attestation_flow_ = nullptr;
std::unique_ptr<AttestationFlow> default_attestation_flow_;
raw_ptr<AttestationFlow, ExperimentalAsh> attestation_flow_ = nullptr;
bool refresh_certificate_ = false;
std::vector<UploadCallback> callbacks_;
int num_retries_ = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include "base/path_service.h"
#include "base/task/sequenced_task_runner.h"
#include "base/time/time.h"
#include "chrome/browser/ash/attestation/attestation_policy_observer.h"
#include "chrome/browser/ash/attestation/enrollment_certificate_uploader_impl.h"
#include "chrome/browser/ash/attestation/enrollment_id_upload_manager.h"
#include "chrome/browser/ash/attestation/machine_certificate_uploader_impl.h"
Expand Down Expand Up @@ -151,6 +150,8 @@ void DeviceCloudPolicyManagerAsh::Shutdown() {
state_keys_broker_ = nullptr;
managed_session_service_.reset();
euicc_status_uploader_.reset();
core()->Disconnect();
machine_certificate_uploader_.reset();
external_data_manager_->Disconnect();
state_keys_update_subscription_ = {};
CloudPolicyManager::Shutdown();
Expand Down Expand Up @@ -237,16 +238,14 @@ void DeviceCloudPolicyManagerAsh::StartConnection(
euicc_status_uploader_ = std::make_unique<EuiccStatusUploader>(
client(), g_browser_process->local_state());

// Don't create a MachineCertificateUploader or start the
// AttestationPolicyObserver if machine cert requests are disabled.
// Don't create a MachineCertificateUploader if machine cert requests are
// disabled.
if (!(base::CommandLine::ForCurrentProcess()->HasSwitch(
ash::switches::kDisableMachineCertRequest))) {
machine_certificate_uploader_ =
std::make_unique<ash::attestation::MachineCertificateUploaderImpl>(
client());
attestation_policy_observer_ =
std::make_unique<ash::attestation::AttestationPolicyObserver>(
machine_certificate_uploader_.get());
machine_certificate_uploader_->UploadCertificateIfNeeded(base::DoNothing());
}

// Start remote commands services now that we have setup everything they need.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ class UserEventReporterHelper;

namespace ash {
namespace attestation {
class AttestationPolicyObserver;
class EnrollmentCertificateUploader;
class EnrollmentIdUploadManager;
class MachineCertificateUploader;
Expand Down Expand Up @@ -252,8 +251,6 @@ class DeviceCloudPolicyManagerAsh : public CloudPolicyManager,
enrollment_id_upload_manager_;
std::unique_ptr<ash::attestation::MachineCertificateUploader>
machine_certificate_uploader_;
std::unique_ptr<ash::attestation::AttestationPolicyObserver>
attestation_policy_observer_;
std::unique_ptr<EuiccStatusUploader> euicc_status_uploader_;

// Uploader for remote server unlock related lookup keys.
Expand Down

0 comments on commit f735e83

Please sign in to comment.