Skip to content

Commit 7b3fb48

Browse files
Yves ArrouyeYves Arrouye
authored andcommitted
Request and directly upload an EID if no enrollment certificate
A device that is enrolled pre-M68 may not be able to obtain an enrollment certificate until it is wiped, but can request a computation of its EID immediately and upload that to the management servers. BUG=chromium:840496 TEST=unit_tests Change-Id: Ib1c4d2652110c49d1370fcc0dfbcfddb336c2de9 Reviewed-on: https://chromium-review.googlesource.com/1069599 Reviewed-by: Pavol Marko <[email protected]> Reviewed-by: Darren Krahn <[email protected]> Reviewed-by: Maksim Ivanov <[email protected]> Commit-Queue: Yves Arrouye <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#561890}(cherry picked from commit b7bfd93) Reviewed-on: https://chromium-review.googlesource.com/1080971 Reviewed-by: Yves Arrouye <[email protected]> Cr-Commit-Position: refs/branch-heads/3440@{#72} Cr-Branched-From: 010ddcf-refs/heads/master@{#561733}
1 parent a297577 commit 7b3fb48

11 files changed

+192
-19
lines changed

chrome/browser/chromeos/attestation/enrollment_policy_observer.cc

Lines changed: 67 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,26 @@ namespace {
3535
const int kRetryDelay = 5; // Seconds.
3636
const int kRetryLimit = 100;
3737

38+
// A dbus callback which handles a string result.
39+
//
40+
// Parameters
41+
// on_success - Called when result is successful and has a value.
42+
// on_failure - Called otherwise.
43+
void DBusStringCallback(
44+
base::OnceCallback<void(const std::string&)> on_success,
45+
base::OnceClosure on_failure,
46+
const base::Location& from_here,
47+
base::Optional<chromeos::CryptohomeClient::TpmAttestationDataResult>
48+
result) {
49+
if (!result.has_value() || !result->success) {
50+
LOG(ERROR) << "Cryptohome DBus method failed: " << from_here.ToString();
51+
if (!on_failure.is_null())
52+
std::move(on_failure).Run();
53+
return;
54+
}
55+
std::move(on_success).Run(result->data);
56+
}
57+
3858
void DBusPrivacyCACallback(
3959
const base::RepeatingCallback<void(const std::string&)> on_success,
4060
const base::RepeatingCallback<
@@ -153,28 +173,44 @@ void EnrollmentPolicyObserver::GetEnrollmentCertificate() {
153173
FROM_HERE));
154174
}
155175

156-
void EnrollmentPolicyObserver::UploadCertificate(
157-
const std::string& pem_certificate_chain) {
158-
policy_client_->UploadEnterpriseEnrollmentCertificate(
159-
pem_certificate_chain,
176+
void EnrollmentPolicyObserver::GetEnrollmentId() {
177+
cryptohome_client_->TpmAttestationGetEnrollmentId(
178+
true /* ignore_cache */,
179+
base::BindOnce(
180+
DBusStringCallback,
181+
base::BindOnce(&EnrollmentPolicyObserver::HandleEnrollmentId,
182+
weak_factory_.GetWeakPtr()),
183+
base::BindOnce(&EnrollmentPolicyObserver::RescheduleGetEnrollmentId,
184+
weak_factory_.GetWeakPtr()),
185+
FROM_HERE));
186+
}
187+
188+
void EnrollmentPolicyObserver::HandleEnrollmentId(
189+
const std::string& enrollment_id) {
190+
policy_client_->UploadEnterpriseEnrollmentId(
191+
enrollment_id,
160192
base::Bind(&EnrollmentPolicyObserver::OnUploadComplete,
161-
weak_factory_.GetWeakPtr()));
193+
weak_factory_.GetWeakPtr(), "Enrollment Identifier"));
162194
}
163195

164-
void EnrollmentPolicyObserver::OnUploadComplete(bool status) {
165-
if (!status) {
166-
VLOG(1) << "Failed to upload Enterprise Enrollment Certificate"
167-
<< " to DMServer.";
168-
return;
196+
void EnrollmentPolicyObserver::RescheduleGetEnrollmentId() {
197+
if (++num_retries_ < retry_limit_) {
198+
content::BrowserThread::PostDelayedTask(
199+
content::BrowserThread::UI, FROM_HERE,
200+
base::BindOnce(&EnrollmentPolicyObserver::GetEnrollmentId,
201+
weak_factory_.GetWeakPtr()),
202+
base::TimeDelta::FromSeconds(retry_delay_));
203+
} else {
204+
LOG(WARNING) << "EnrollmentPolicyObserver: Retry limit exceeded.";
169205
}
170-
VLOG(1) << "Enterprise Enrollment Certificate uploaded to DMServer.";
171206
}
172207

173208
void EnrollmentPolicyObserver::HandleGetCertificateFailure(
174209
AttestationStatus status) {
175210
if (status == ATTESTATION_SERVER_BAD_REQUEST_FAILURE) {
176-
// We cannot get an enrollment cert (no EID) so upload an empty one.
177-
UploadCertificate("");
211+
// We cannot get an enrollment cert (no EID). However we can compute the
212+
// EID we will have after a device wipe, and should upload that.
213+
GetEnrollmentId();
178214
} else if (++num_retries_ < retry_limit_) {
179215
content::BrowserThread::PostDelayedTask(
180216
content::BrowserThread::UI, FROM_HERE,
@@ -186,5 +222,23 @@ void EnrollmentPolicyObserver::HandleGetCertificateFailure(
186222
}
187223
}
188224

225+
void EnrollmentPolicyObserver::UploadCertificate(
226+
const std::string& pem_certificate_chain) {
227+
policy_client_->UploadEnterpriseEnrollmentCertificate(
228+
pem_certificate_chain,
229+
base::BindRepeating(&EnrollmentPolicyObserver::OnUploadComplete,
230+
weak_factory_.GetWeakPtr(),
231+
"Enterprise Enrollment Certificate"));
232+
}
233+
234+
void EnrollmentPolicyObserver::OnUploadComplete(const std::string& what,
235+
bool status) {
236+
if (!status) {
237+
LOG(ERROR) << "Failed to upload " << what << " to DMServer.";
238+
return;
239+
}
240+
VLOG(1) << what << " uploaded to DMServer.";
241+
}
242+
189243
} // namespace attestation
190244
} // namespace chromeos

chrome/browser/chromeos/attestation/enrollment_policy_observer.h

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,16 +59,26 @@ class EnrollmentPolicyObserver : public DeviceSettingsService::Observer {
5959
// Gets an enrollment certificate.
6060
void GetEnrollmentCertificate();
6161

62-
// Uploads an enrollment certificate to the policy server.
63-
void UploadCertificate(const std::string& pem_certificate_chain);
62+
// Gets an enrollment identifier directly.
63+
void GetEnrollmentId();
6464

65-
// Called when a certificate upload operation completes. On success, |status|
66-
// will be true.
67-
void OnUploadComplete(bool status);
65+
// Handles an enrollment identifer obtained directly.
66+
void HandleEnrollmentId(const std::string& enrollment_id);
67+
68+
// Reschedule an attempt to get an enrollment identifier directly.
69+
void RescheduleGetEnrollmentId();
6870

6971
// Handles a failure to get a certificate.
7072
void HandleGetCertificateFailure(AttestationStatus status);
7173

74+
// Uploads an enrollment certificate to the policy server.
75+
void UploadCertificate(const std::string& pem_certificate_chain);
76+
77+
// Called when a certificate or enrollment identifier upload operation
78+
// completes. On success, |status| will be true. The string |what| is
79+
// used in logging.
80+
void OnUploadComplete(const std::string& what, bool status);
81+
7282
DeviceSettingsService* device_settings_service_;
7383
policy::CloudPolicyClient* policy_client_;
7484
CryptohomeClient* cryptohome_client_;

chrome/browser/chromeos/attestation/enrollment_policy_observer_unittest.cc

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,15 @@ class EnrollmentPolicyObserverTest : public DeviceSettingsTestBase {
6464
}
6565

6666
protected:
67+
static constexpr char kEnrollmentId[] =
68+
"6fcc0ebddec3db95cdcf82476d594f4d60db934c5b47fa6085c707b2a93e205b";
69+
70+
void SetUp() override {
71+
DeviceSettingsTestBase::SetUp();
72+
cryptohome_client_.set_tpm_attestation_enrollment_id(
73+
true /* ignore_cache */, kEnrollmentId);
74+
}
75+
6776
void SetUpEnrollmentIdNeeded(bool enrollment_id_needed) {
6877
if (enrollment_id_needed) {
6978
EXPECT_CALL(attestation_flow_, GetCertificate(_, _, _, _, _))
@@ -96,6 +105,8 @@ class EnrollmentPolicyObserverTest : public DeviceSettingsTestBase {
96105
StrictMock<policy::MockCloudPolicyClient> policy_client_;
97106
};
98107

108+
constexpr char EnrollmentPolicyObserverTest::kEnrollmentId[];
109+
99110
TEST_F(EnrollmentPolicyObserverTest, UploadEnterpriseEnrollmentCertificate) {
100111
SetUpEnrollmentIdNeeded(true);
101112
Run();
@@ -121,7 +132,7 @@ TEST_F(EnrollmentPolicyObserverTest, GetCertificateUnspecifiedFailure) {
121132
TEST_F(EnrollmentPolicyObserverTest, GetCertificateBadRequestFailure) {
122133
EXPECT_CALL(attestation_flow_, GetCertificate(_, _, _, _, _))
123134
.WillOnce(WithArgs<4>(Invoke(CertCallbackBadRequestFailure)));
124-
EXPECT_CALL(policy_client_, UploadEnterpriseEnrollmentCertificate("", _))
135+
EXPECT_CALL(policy_client_, UploadEnterpriseEnrollmentId(kEnrollmentId, _))
125136
.WillOnce(WithArgs<1>(Invoke(StatusCallbackSuccess)));
126137
SetUpDevicePolicy(true);
127138
Run();

chromeos/dbus/cryptohome_client.cc

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,21 @@ class CryptohomeClientImpl : public CryptohomeClient {
417417
return CallBoolMethod(&method_call, std::move(callback));
418418
}
419419

420+
// CryptohomeClient override.
421+
void TpmAttestationGetEnrollmentId(
422+
bool ignore_cache,
423+
DBusMethodCallback<TpmAttestationDataResult> callback) override {
424+
dbus::MethodCall method_call(
425+
cryptohome::kCryptohomeInterface,
426+
cryptohome::kCryptohomeTpmAttestationGetEnrollmentId);
427+
dbus::MessageWriter writer(&method_call);
428+
writer.AppendBool(ignore_cache);
429+
proxy_->CallMethod(
430+
&method_call, kTpmDBusTimeoutMs,
431+
base::BindOnce(&CryptohomeClientImpl::OnTpmAttestationDataMethod,
432+
weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
433+
}
434+
420435
// CryptohomeClient override.
421436
void TpmAttestationIsEnrolled(DBusMethodCallback<bool> callback) override {
422437
dbus::MethodCall method_call(

chromeos/dbus/cryptohome_client.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,13 @@ class CHROMEOS_EXPORT CryptohomeClient : public DBusClient {
287287
// when the operation completes.
288288
virtual void TpmAttestationIsPrepared(DBusMethodCallback<bool> callback) = 0;
289289

290+
// Requests the device's enrollment identifier (EID). The |callback| will be
291+
// called with the EID. If |ignore_cache| is true, the EID is calculated
292+
// even if the attestation database already contains a cached version.
293+
virtual void TpmAttestationGetEnrollmentId(
294+
bool ignore_cache,
295+
DBusMethodCallback<TpmAttestationDataResult> callback) = 0;
296+
290297
// Calls the TpmAttestationIsEnrolled dbus method. The callback is called
291298
// when the operation completes.
292299
virtual void TpmAttestationIsEnrolled(DBusMethodCallback<bool> callback) = 0;

chromeos/dbus/fake_cryptohome_client.cc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,18 @@ void FakeCryptohomeClient::TpmAttestationIsPrepared(
313313
FROM_HERE, base::BindOnce(std::move(callback), result));
314314
}
315315

316+
void FakeCryptohomeClient::TpmAttestationGetEnrollmentId(
317+
bool ignore_cache,
318+
DBusMethodCallback<TpmAttestationDataResult> callback) {
319+
base::ThreadTaskRunnerHandle::Get()->PostTask(
320+
FROM_HERE,
321+
base::BindOnce(
322+
std::move(callback),
323+
TpmAttestationDataResult{
324+
true, ignore_cache ? tpm_attestation_enrollment_id_ignore_cache_
325+
: tpm_attestation_enrollment_id_}));
326+
}
327+
316328
void FakeCryptohomeClient::TpmAttestationIsEnrolled(
317329
DBusMethodCallback<bool> callback) {
318330
auto result = service_is_available_

chromeos/dbus/fake_cryptohome_client.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,9 @@ class CHROMEOS_EXPORT FakeCryptohomeClient : public CryptohomeClient {
8585
bool InstallAttributesIsInvalid(bool* is_invalid) override;
8686
bool InstallAttributesIsFirstInstall(bool* is_first_install) override;
8787
void TpmAttestationIsPrepared(DBusMethodCallback<bool> callback) override;
88+
void TpmAttestationGetEnrollmentId(
89+
bool ignore_cache,
90+
DBusMethodCallback<TpmAttestationDataResult> callback) override;
8891
void TpmAttestationIsEnrolled(DBusMethodCallback<bool> callback) override;
8992
void AsyncTpmAttestationCreateEnrollRequest(
9093
chromeos::attestation::PrivacyCAType pca_type,
@@ -236,6 +239,15 @@ class CHROMEOS_EXPORT FakeCryptohomeClient : public CryptohomeClient {
236239
cryptohome_error_ = error;
237240
}
238241

242+
void set_tpm_attestation_enrollment_id(bool ignore_cache,
243+
const std::string& eid) {
244+
if (ignore_cache) {
245+
tpm_attestation_enrollment_id_ignore_cache_ = eid;
246+
} else {
247+
tpm_attestation_enrollment_id_ = eid;
248+
}
249+
}
250+
239251
void set_tpm_attestation_is_enrolled(bool enrolled) {
240252
tpm_attestation_is_enrolled_ = enrolled;
241253
}
@@ -369,6 +381,10 @@ class CHROMEOS_EXPORT FakeCryptohomeClient : public CryptohomeClient {
369381
uint64_t dircrypto_migration_progress_;
370382

371383
bool needs_dircrypto_migration_ = false;
384+
std::string tpm_attestation_enrollment_id_ignore_cache_ =
385+
"6fcc0ebddec3db95cdcf82476d594f4d60db934c5b47fa6085c707b2a93e205b";
386+
std::string tpm_attestation_enrollment_id_ =
387+
"6fcc0ebddec3db95cdcf82476d594f4d60db934c5b47fa6085c707b2a93e205b";
372388
bool tpm_attestation_is_enrolled_ = true;
373389
bool tpm_attestation_is_prepared_ = true;
374390
bool tpm_attestation_does_key_exist_should_succeed_ = true;

components/policy/core/common/cloud/cloud_policy_client.cc

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,29 @@ void CloudPolicyClient::UploadEnterpriseEnrollmentCertificate(
413413
em::DeviceCertUploadRequest::ENTERPRISE_ENROLLMENT_CERTIFICATE, callback);
414414
}
415415

416+
void CloudPolicyClient::UploadEnterpriseEnrollmentId(
417+
const std::string& enrollment_id,
418+
const CloudPolicyClient::StatusCallback& callback) {
419+
CHECK(is_registered());
420+
std::unique_ptr<DeviceManagementRequestJob> request_job(
421+
service_->CreateJob(DeviceManagementRequestJob::TYPE_UPLOAD_CERTIFICATE,
422+
GetRequestContext()));
423+
request_job->SetDMToken(dm_token_);
424+
request_job->SetClientID(client_id_);
425+
426+
em::DeviceManagementRequest* request = request_job->GetRequest();
427+
em::DeviceCertUploadRequest* upload_request =
428+
request->mutable_cert_upload_request();
429+
upload_request->set_enrollment_id(enrollment_id);
430+
431+
const DeviceManagementRequestJob::Callback job_callback = base::BindRepeating(
432+
&CloudPolicyClient::OnCertificateUploadCompleted,
433+
weak_ptr_factory_.GetWeakPtr(), request_job.get(), callback);
434+
435+
request_jobs_.push_back(std::move(request_job));
436+
request_jobs_.back()->Start(job_callback);
437+
}
438+
416439
void CloudPolicyClient::UploadDeviceStatus(
417440
const em::DeviceStatusReportRequest* device_status,
418441
const em::SessionStatusReportRequest* session_status,

components/policy/core/common/cloud/cloud_policy_client.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,13 @@ class POLICY_EXPORT CloudPolicyClient {
189189
const std::string& certificate_data,
190190
const StatusCallback& callback);
191191

192+
// Upload an enrollment identifier to the server. Like FetchPolicy, this
193+
// method requires that the client is in a registered state.
194+
// |enrollment_id| must hold an enrollment identifier. The |callback| will be
195+
// called when the operation completes.
196+
virtual void UploadEnterpriseEnrollmentId(const std::string& enrollment_id,
197+
const StatusCallback& callback);
198+
192199
// Uploads device/session status to the server. As above, the client must be
193200
// in a registered state. If non-null, |device_status| and |session_status|
194201
// will be included in the upload status request. The |callback| will be

components/policy/core/common/cloud/cloud_policy_client_unittest.cc

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ const char kDMToken[] = "fake-dm-token";
5050
const char kDeviceDMToken[] = "fake-device-dm-token";
5151
const char kMachineCertificate[] = "fake-machine-certificate";
5252
const char kEnrollmentCertificate[] = "fake-enrollment-certificate";
53+
const char kEnrollmentId[] = "fake-enrollment-id";
5354
const char kRequisition[] = "fake-requisition";
5455
const char kStateKey[] = "fake-state-key";
5556
const char kPayload[] = "input_payload";
@@ -180,6 +181,8 @@ class CloudPolicyClientTest : public testing::Test {
180181
upload_enrollment_certificate_request_.mutable_cert_upload_request()
181182
->set_certificate_type(
182183
em::DeviceCertUploadRequest::ENTERPRISE_ENROLLMENT_CERTIFICATE);
184+
upload_enrollment_id_request_.mutable_cert_upload_request()
185+
->set_enrollment_id(kEnrollmentId);
183186
upload_certificate_response_.mutable_cert_upload_response();
184187

185188
upload_status_request_.mutable_device_status_report_request();
@@ -468,6 +471,7 @@ class CloudPolicyClientTest : public testing::Test {
468471
em::DeviceManagementRequest unregistration_request_;
469472
em::DeviceManagementRequest upload_machine_certificate_request_;
470473
em::DeviceManagementRequest upload_enrollment_certificate_request_;
474+
em::DeviceManagementRequest upload_enrollment_id_request_;
471475
em::DeviceManagementRequest upload_status_request_;
472476
em::DeviceManagementRequest chrome_desktop_report_request_;
473477
em::DeviceManagementRequest remote_command_request_;
@@ -977,6 +981,18 @@ TEST_F(CloudPolicyClientTest, UploadCertificateFailure) {
977981
EXPECT_EQ(DM_STATUS_REQUEST_FAILED, client_->status());
978982
}
979983

984+
TEST_F(CloudPolicyClientTest, UploadEnterpriseEnrollmentId) {
985+
Register();
986+
987+
ExpectUploadCertificate(upload_enrollment_id_request_);
988+
EXPECT_CALL(callback_observer_, OnCallbackComplete(true)).Times(1);
989+
CloudPolicyClient::StatusCallback callback =
990+
base::BindRepeating(&MockStatusCallbackObserver::OnCallbackComplete,
991+
base::Unretained(&callback_observer_));
992+
client_->UploadEnterpriseEnrollmentId(kEnrollmentId, callback);
993+
EXPECT_EQ(DM_STATUS_SUCCESS, client_->status());
994+
}
995+
980996
TEST_F(CloudPolicyClientTest, UploadStatus) {
981997
Register();
982998

0 commit comments

Comments
 (0)