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}
nwjs · May 31, 2018 · 7b3fb48 · 7b3fb48
1 parent a297577
commit 7b3fb48
Show file tree

Hide file tree

Showing 11 changed files with 192 additions and 19 deletions.
diff --git a/chrome/browser/chromeos/attestation/enrollment_policy_observer.cc b/chrome/browser/chromeos/attestation/enrollment_policy_observer.cc
@@ -35,6 +35,26 @@ namespace {
 const int kRetryDelay = 5;  // Seconds.
 const int kRetryLimit = 100;
 
+// A dbus callback which handles a string result.
+//
+// Parameters
+//   on_success - Called when result is successful and has a value.
+//   on_failure - Called otherwise.
+void DBusStringCallback(
+    base::OnceCallback<void(const std::string&)> on_success,
+    base::OnceClosure on_failure,
+    const base::Location& from_here,
+    base::Optional<chromeos::CryptohomeClient::TpmAttestationDataResult>
+        result) {
+  if (!result.has_value() || !result->success) {
+    LOG(ERROR) << "Cryptohome DBus method failed: " << from_here.ToString();
+    if (!on_failure.is_null())
+      std::move(on_failure).Run();
+    return;
+  }
+  std::move(on_success).Run(result->data);
+}
+
 void DBusPrivacyCACallback(
     const base::RepeatingCallback<void(const std::string&)> on_success,
     const base::RepeatingCallback<
@@ -153,28 +173,44 @@ void EnrollmentPolicyObserver::GetEnrollmentCertificate() {
           FROM_HERE));
 }
 
-void EnrollmentPolicyObserver::UploadCertificate(
-    const std::string& pem_certificate_chain) {
-  policy_client_->UploadEnterpriseEnrollmentCertificate(
-      pem_certificate_chain,
+void EnrollmentPolicyObserver::GetEnrollmentId() {
+  cryptohome_client_->TpmAttestationGetEnrollmentId(
+      true /* ignore_cache */,
+      base::BindOnce(
+          DBusStringCallback,
+          base::BindOnce(&EnrollmentPolicyObserver::HandleEnrollmentId,
+                         weak_factory_.GetWeakPtr()),
+          base::BindOnce(&EnrollmentPolicyObserver::RescheduleGetEnrollmentId,
+                         weak_factory_.GetWeakPtr()),
+          FROM_HERE));
+}
+
+void EnrollmentPolicyObserver::HandleEnrollmentId(
+    const std::string& enrollment_id) {
+  policy_client_->UploadEnterpriseEnrollmentId(
+      enrollment_id,
       base::Bind(&EnrollmentPolicyObserver::OnUploadComplete,
-                 weak_factory_.GetWeakPtr()));
+                 weak_factory_.GetWeakPtr(), "Enrollment Identifier"));
 }
 
-void EnrollmentPolicyObserver::OnUploadComplete(bool status) {
-  if (!status) {
-    VLOG(1) << "Failed to upload Enterprise Enrollment Certificate"
-            << " to DMServer.";
-    return;
+void EnrollmentPolicyObserver::RescheduleGetEnrollmentId() {
+  if (++num_retries_ < retry_limit_) {
+    content::BrowserThread::PostDelayedTask(
+        content::BrowserThread::UI, FROM_HERE,
+        base::BindOnce(&EnrollmentPolicyObserver::GetEnrollmentId,
+                       weak_factory_.GetWeakPtr()),
+        base::TimeDelta::FromSeconds(retry_delay_));
+  } else {
+    LOG(WARNING) << "EnrollmentPolicyObserver: Retry limit exceeded.";
   }
-  VLOG(1) << "Enterprise Enrollment Certificate uploaded to DMServer.";
 }
 
 void EnrollmentPolicyObserver::HandleGetCertificateFailure(
     AttestationStatus status) {
   if (status == ATTESTATION_SERVER_BAD_REQUEST_FAILURE) {
-    // We cannot get an enrollment cert (no EID) so upload an empty one.
-    UploadCertificate("");
+    // We cannot get an enrollment cert (no EID). However we can compute the
+    // EID we will have after a device wipe, and should upload that.
+    GetEnrollmentId();
   } else if (++num_retries_ < retry_limit_) {
     content::BrowserThread::PostDelayedTask(
         content::BrowserThread::UI, FROM_HERE,
@@ -186,5 +222,23 @@ void EnrollmentPolicyObserver::HandleGetCertificateFailure(
   }
 }
 
+void EnrollmentPolicyObserver::UploadCertificate(
+    const std::string& pem_certificate_chain) {
+  policy_client_->UploadEnterpriseEnrollmentCertificate(
+      pem_certificate_chain,
+      base::BindRepeating(&EnrollmentPolicyObserver::OnUploadComplete,
+                          weak_factory_.GetWeakPtr(),
+                          "Enterprise Enrollment Certificate"));
+}
+
+void EnrollmentPolicyObserver::OnUploadComplete(const std::string& what,
+                                                bool status) {
+  if (!status) {
+    LOG(ERROR) << "Failed to upload " << what << " to DMServer.";
+    return;
+  }
+  VLOG(1) << what << " uploaded to DMServer.";
+}
+
 }  // namespace attestation
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/attestation/enrollment_policy_observer.h b/chrome/browser/chromeos/attestation/enrollment_policy_observer.h
@@ -59,16 +59,26 @@ class EnrollmentPolicyObserver : public DeviceSettingsService::Observer {
   // Gets an enrollment certificate.
   void GetEnrollmentCertificate();
 
-  // Uploads an enrollment certificate to the policy server.
-  void UploadCertificate(const std::string& pem_certificate_chain);
+  // Gets an enrollment identifier directly.
+  void GetEnrollmentId();
 
-  // Called when a certificate upload operation completes.  On success, |status|
-  // will be true.
-  void OnUploadComplete(bool status);
+  // Handles an enrollment identifer obtained directly.
+  void HandleEnrollmentId(const std::string& enrollment_id);
+
+  // Reschedule an attempt to get an enrollment identifier directly.
+  void RescheduleGetEnrollmentId();
 
   // Handles a failure to get a certificate.
   void HandleGetCertificateFailure(AttestationStatus status);
 
+  // Uploads an enrollment certificate to the policy server.
+  void UploadCertificate(const std::string& pem_certificate_chain);
+
+  // Called when a certificate or enrollment identifier upload operation
+  // completes. On success, |status| will be true. The string |what| is
+  // used in logging.
+  void OnUploadComplete(const std::string& what, bool status);
+
   DeviceSettingsService* device_settings_service_;
   policy::CloudPolicyClient* policy_client_;
   CryptohomeClient* cryptohome_client_;

diff --git a/chrome/browser/chromeos/attestation/enrollment_policy_observer_unittest.cc b/chrome/browser/chromeos/attestation/enrollment_policy_observer_unittest.cc
@@ -64,6 +64,15 @@ class EnrollmentPolicyObserverTest : public DeviceSettingsTestBase {
   }
 
  protected:
+  static constexpr char kEnrollmentId[] =
+      "6fcc0ebddec3db95cdcf82476d594f4d60db934c5b47fa6085c707b2a93e205b";
+
+  void SetUp() override {
+    DeviceSettingsTestBase::SetUp();
+    cryptohome_client_.set_tpm_attestation_enrollment_id(
+        true /* ignore_cache */, kEnrollmentId);
+  }
+
   void SetUpEnrollmentIdNeeded(bool enrollment_id_needed) {
     if (enrollment_id_needed) {
       EXPECT_CALL(attestation_flow_, GetCertificate(_, _, _, _, _))
@@ -96,6 +105,8 @@ class EnrollmentPolicyObserverTest : public DeviceSettingsTestBase {
   StrictMock<policy::MockCloudPolicyClient> policy_client_;
 };
 
+constexpr char EnrollmentPolicyObserverTest::kEnrollmentId[];
+
 TEST_F(EnrollmentPolicyObserverTest, UploadEnterpriseEnrollmentCertificate) {
   SetUpEnrollmentIdNeeded(true);
   Run();
@@ -121,7 +132,7 @@ TEST_F(EnrollmentPolicyObserverTest, GetCertificateUnspecifiedFailure) {
 TEST_F(EnrollmentPolicyObserverTest, GetCertificateBadRequestFailure) {
   EXPECT_CALL(attestation_flow_, GetCertificate(_, _, _, _, _))
       .WillOnce(WithArgs<4>(Invoke(CertCallbackBadRequestFailure)));
-  EXPECT_CALL(policy_client_, UploadEnterpriseEnrollmentCertificate("", _))
+  EXPECT_CALL(policy_client_, UploadEnterpriseEnrollmentId(kEnrollmentId, _))
       .WillOnce(WithArgs<1>(Invoke(StatusCallbackSuccess)));
   SetUpDevicePolicy(true);
   Run();

diff --git a/chromeos/dbus/cryptohome_client.cc b/chromeos/dbus/cryptohome_client.cc
@@ -417,6 +417,21 @@ class CryptohomeClientImpl : public CryptohomeClient {
     return CallBoolMethod(&method_call, std::move(callback));
   }
 
+  // CryptohomeClient override.
+  void TpmAttestationGetEnrollmentId(
+      bool ignore_cache,
+      DBusMethodCallback<TpmAttestationDataResult> callback) override {
+    dbus::MethodCall method_call(
+        cryptohome::kCryptohomeInterface,
+        cryptohome::kCryptohomeTpmAttestationGetEnrollmentId);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendBool(ignore_cache);
+    proxy_->CallMethod(
+        &method_call, kTpmDBusTimeoutMs,
+        base::BindOnce(&CryptohomeClientImpl::OnTpmAttestationDataMethod,
+                       weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+  }
+
   // CryptohomeClient override.
   void TpmAttestationIsEnrolled(DBusMethodCallback<bool> callback) override {
     dbus::MethodCall method_call(

diff --git a/chromeos/dbus/cryptohome_client.h b/chromeos/dbus/cryptohome_client.h
@@ -287,6 +287,13 @@ class CHROMEOS_EXPORT CryptohomeClient : public DBusClient {
   // when the operation completes.
   virtual void TpmAttestationIsPrepared(DBusMethodCallback<bool> callback) = 0;
 
+  // Requests the device's enrollment identifier (EID). The |callback| will be
+  // called with the EID. If |ignore_cache| is true, the EID is calculated
+  // even if the attestation database already contains a cached version.
+  virtual void TpmAttestationGetEnrollmentId(
+      bool ignore_cache,
+      DBusMethodCallback<TpmAttestationDataResult> callback) = 0;
+
   // Calls the TpmAttestationIsEnrolled dbus method.  The callback is called
   // when the operation completes.
   virtual void TpmAttestationIsEnrolled(DBusMethodCallback<bool> callback) = 0;

diff --git a/chromeos/dbus/fake_cryptohome_client.cc b/chromeos/dbus/fake_cryptohome_client.cc
@@ -313,6 +313,18 @@ void FakeCryptohomeClient::TpmAttestationIsPrepared(
       FROM_HERE, base::BindOnce(std::move(callback), result));
 }
 
+void FakeCryptohomeClient::TpmAttestationGetEnrollmentId(
+    bool ignore_cache,
+    DBusMethodCallback<TpmAttestationDataResult> callback) {
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          std::move(callback),
+          TpmAttestationDataResult{
+              true, ignore_cache ? tpm_attestation_enrollment_id_ignore_cache_
+                                 : tpm_attestation_enrollment_id_}));
+}
+
 void FakeCryptohomeClient::TpmAttestationIsEnrolled(
     DBusMethodCallback<bool> callback) {
   auto result = service_is_available_

diff --git a/chromeos/dbus/fake_cryptohome_client.h b/chromeos/dbus/fake_cryptohome_client.h
@@ -85,6 +85,9 @@ class CHROMEOS_EXPORT FakeCryptohomeClient : public CryptohomeClient {
   bool InstallAttributesIsInvalid(bool* is_invalid) override;
   bool InstallAttributesIsFirstInstall(bool* is_first_install) override;
   void TpmAttestationIsPrepared(DBusMethodCallback<bool> callback) override;
+  void TpmAttestationGetEnrollmentId(
+      bool ignore_cache,
+      DBusMethodCallback<TpmAttestationDataResult> callback) override;
   void TpmAttestationIsEnrolled(DBusMethodCallback<bool> callback) override;
   void AsyncTpmAttestationCreateEnrollRequest(
       chromeos::attestation::PrivacyCAType pca_type,
@@ -236,6 +239,15 @@ class CHROMEOS_EXPORT FakeCryptohomeClient : public CryptohomeClient {
     cryptohome_error_ = error;
   }
 
+  void set_tpm_attestation_enrollment_id(bool ignore_cache,
+                                         const std::string& eid) {
+    if (ignore_cache) {
+      tpm_attestation_enrollment_id_ignore_cache_ = eid;
+    } else {
+      tpm_attestation_enrollment_id_ = eid;
+    }
+  }
+
   void set_tpm_attestation_is_enrolled(bool enrolled) {
     tpm_attestation_is_enrolled_ = enrolled;
   }
@@ -369,6 +381,10 @@ class CHROMEOS_EXPORT FakeCryptohomeClient : public CryptohomeClient {
   uint64_t dircrypto_migration_progress_;
 
   bool needs_dircrypto_migration_ = false;
+  std::string tpm_attestation_enrollment_id_ignore_cache_ =
+      "6fcc0ebddec3db95cdcf82476d594f4d60db934c5b47fa6085c707b2a93e205b";
+  std::string tpm_attestation_enrollment_id_ =
+      "6fcc0ebddec3db95cdcf82476d594f4d60db934c5b47fa6085c707b2a93e205b";
   bool tpm_attestation_is_enrolled_ = true;
   bool tpm_attestation_is_prepared_ = true;
   bool tpm_attestation_does_key_exist_should_succeed_ = true;

diff --git a/components/policy/core/common/cloud/cloud_policy_client.cc b/components/policy/core/common/cloud/cloud_policy_client.cc
@@ -413,6 +413,29 @@ void CloudPolicyClient::UploadEnterpriseEnrollmentCertificate(
       em::DeviceCertUploadRequest::ENTERPRISE_ENROLLMENT_CERTIFICATE, callback);
 }
 
+void CloudPolicyClient::UploadEnterpriseEnrollmentId(
+    const std::string& enrollment_id,
+    const CloudPolicyClient::StatusCallback& callback) {
+  CHECK(is_registered());
+  std::unique_ptr<DeviceManagementRequestJob> request_job(
+      service_->CreateJob(DeviceManagementRequestJob::TYPE_UPLOAD_CERTIFICATE,
+                          GetRequestContext()));
+  request_job->SetDMToken(dm_token_);
+  request_job->SetClientID(client_id_);
+
+  em::DeviceManagementRequest* request = request_job->GetRequest();
+  em::DeviceCertUploadRequest* upload_request =
+      request->mutable_cert_upload_request();
+  upload_request->set_enrollment_id(enrollment_id);
+
+  const DeviceManagementRequestJob::Callback job_callback = base::BindRepeating(
+      &CloudPolicyClient::OnCertificateUploadCompleted,
+      weak_ptr_factory_.GetWeakPtr(), request_job.get(), callback);
+
+  request_jobs_.push_back(std::move(request_job));
+  request_jobs_.back()->Start(job_callback);
+}
+
 void CloudPolicyClient::UploadDeviceStatus(
     const em::DeviceStatusReportRequest* device_status,
     const em::SessionStatusReportRequest* session_status,

diff --git a/components/policy/core/common/cloud/cloud_policy_client.h b/components/policy/core/common/cloud/cloud_policy_client.h
@@ -189,6 +189,13 @@ class POLICY_EXPORT CloudPolicyClient {
       const std::string& certificate_data,
       const StatusCallback& callback);
 
+  // Upload an enrollment identifier to the server. Like FetchPolicy, this
+  // method requires that the client is in a registered state.
+  // |enrollment_id| must hold an enrollment identifier. The |callback| will be
+  // called when the operation completes.
+  virtual void UploadEnterpriseEnrollmentId(const std::string& enrollment_id,
+                                            const StatusCallback& callback);
+
   // Uploads device/session status to the server. As above, the client must be
   // in a registered state. If non-null, |device_status| and |session_status|
   // will be included in the upload status request. The |callback| will be

diff --git a/components/policy/core/common/cloud/cloud_policy_client_unittest.cc b/components/policy/core/common/cloud/cloud_policy_client_unittest.cc
@@ -50,6 +50,7 @@ const char kDMToken[] = "fake-dm-token";
 const char kDeviceDMToken[] = "fake-device-dm-token";
 const char kMachineCertificate[] = "fake-machine-certificate";
 const char kEnrollmentCertificate[] = "fake-enrollment-certificate";
+const char kEnrollmentId[] = "fake-enrollment-id";
 const char kRequisition[] = "fake-requisition";
 const char kStateKey[] = "fake-state-key";
 const char kPayload[] = "input_payload";
@@ -180,6 +181,8 @@ class CloudPolicyClientTest : public testing::Test {
     upload_enrollment_certificate_request_.mutable_cert_upload_request()
         ->set_certificate_type(
             em::DeviceCertUploadRequest::ENTERPRISE_ENROLLMENT_CERTIFICATE);
+    upload_enrollment_id_request_.mutable_cert_upload_request()
+        ->set_enrollment_id(kEnrollmentId);
     upload_certificate_response_.mutable_cert_upload_response();
 
     upload_status_request_.mutable_device_status_report_request();
@@ -468,6 +471,7 @@ class CloudPolicyClientTest : public testing::Test {
   em::DeviceManagementRequest unregistration_request_;
   em::DeviceManagementRequest upload_machine_certificate_request_;
   em::DeviceManagementRequest upload_enrollment_certificate_request_;
+  em::DeviceManagementRequest upload_enrollment_id_request_;
   em::DeviceManagementRequest upload_status_request_;
   em::DeviceManagementRequest chrome_desktop_report_request_;
   em::DeviceManagementRequest remote_command_request_;
@@ -977,6 +981,18 @@ TEST_F(CloudPolicyClientTest, UploadCertificateFailure) {
   EXPECT_EQ(DM_STATUS_REQUEST_FAILED, client_->status());
 }
 
+TEST_F(CloudPolicyClientTest, UploadEnterpriseEnrollmentId) {
+  Register();
+
+  ExpectUploadCertificate(upload_enrollment_id_request_);
+  EXPECT_CALL(callback_observer_, OnCallbackComplete(true)).Times(1);
+  CloudPolicyClient::StatusCallback callback =
+      base::BindRepeating(&MockStatusCallbackObserver::OnCallbackComplete,
+                          base::Unretained(&callback_observer_));
+  client_->UploadEnterpriseEnrollmentId(kEnrollmentId, callback);
+  EXPECT_EQ(DM_STATUS_SUCCESS, client_->status());
+}
+
 TEST_F(CloudPolicyClientTest, UploadStatus) {
   Register();
 

diff --git a/components/policy/core/common/cloud/mock_cloud_policy_client.h b/components/policy/core/common/cloud/mock_cloud_policy_client.h
@@ -40,6 +40,8 @@ class MockCloudPolicyClient : public CloudPolicyClient {
                void(const std::string&, const StatusCallback&));
   MOCK_METHOD2(UploadEnterpriseEnrollmentCertificate,
                void(const std::string&, const StatusCallback&));
+  MOCK_METHOD2(UploadEnterpriseEnrollmentId,
+               void(const std::string&, const StatusCallback&));
   MOCK_METHOD3(UploadDeviceStatus,
                void(const enterprise_management::DeviceStatusReportRequest*,
                     const enterprise_management::SessionStatusReportRequest*,