Device enterprise registration with a certificate.

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

Index: components/policy/core/common/cloud/cloud_policy_client.cc
diff --git a/components/policy/core/common/cloud/cloud_policy_client.cc b/components/policy/core/common/cloud/cloud_policy_client.cc
index eaee73ca991b8d4d96dff2cd4e8f6bbaae3befd4..3c49113dbe4d07c38e3b0bb48bd5e3fb85c51f05 100644
--- a/components/policy/core/common/cloud/cloud_policy_client.cc
+++ b/components/policy/core/common/cloud/cloud_policy_client.cc
@@ -53,7 +53,8 @@ CloudPolicyClient::CloudPolicyClient(
     const std::string& machine_model,
     const std::string& verification_key_hash,
     DeviceManagementService* service,
-    scoped_refptr<net::URLRequestContextGetter> request_context)
+    scoped_refptr<net::URLRequestContextGetter> request_context,
+    SigningService* signing_service)
     : machine_id_(machine_id),
       machine_model_(machine_model),
       verification_key_hash_(verification_key_hash),
@@ -64,10 +65,25 @@ CloudPolicyClient::CloudPolicyClient(
       invalidation_version_(0),
       fetched_invalidation_version_(0),
       service_(service),                  // Can be null for unit tests.
+      signing_service_(signing_service),
       status_(DM_STATUS_SUCCESS),
       request_context_(request_context) {
 }
 
+CloudPolicyClient::CloudPolicyClient(

[achuithb, 2016/08/23 18:40:28]

Get rid of this

[The one and only Dr. Crash, 2016/08/24 05:53:44]

Done.

+    const std::string& machine_id,
+    const std::string& machine_model,
+    const std::string& verification_key_hash,
+    DeviceManagementService* service,
+    scoped_refptr<net::URLRequestContextGetter> request_context) :
+    CloudPolicyClient(machine_id,
+                      machine_model,
+                      verification_key_hash,
+                      service,
+                      request_context,
+                      nullptr /* signing_service */) {
+}
+
 CloudPolicyClient::~CloudPolicyClient() {
   base::STLDeleteValues(&responses_);
 }
@@ -87,6 +103,17 @@ void CloudPolicyClient::SetupRegistration(const std::string& dm_token,
   NotifyRegistrationStateChanged();
 }
 
+void CloudPolicyClient::SetClientId(const std::string& client_id) {
+  if (client_id.empty()) {

[achuithb, 2016/08/23 18:40:28]

use ternary operator instead

[The one and only Dr. Crash, 2016/08/24 05:53:44]

Done.

+    // Generate a new client ID. This is intentionally done on each new
+    // registration request in order to preserve privacy. Reusing IDs would
+    // mean the server could track clients by their registration attempts.
+    client_id_ = base::GenerateGUID();
+  } else {
+    client_id_ = client_id;
+  }
+}
+
 void CloudPolicyClient::Register(em::DeviceRegisterRequest::Type type,
                                  em::DeviceRegisterRequest::Flavor flavor,
                                  const std::string& auth_token,
@@ -97,14 +124,7 @@ void CloudPolicyClient::Register(em::DeviceRegisterRequest::Type type,
   DCHECK(!auth_token.empty());
   DCHECK(!is_registered());
 
-  if (client_id.empty()) {
-    // Generate a new client ID. This is intentionally done on each new
-    // registration request in order to preserve privacy. Reusing IDs would mean
-    // the server could track clients by their registration attempts.
-    client_id_ = base::GenerateGUID();
-  } else {
-    client_id_ = client_id;
-  }
+  SetClientId(client_id);
 
   policy_fetch_request_job_.reset(
       service_->CreateJob(DeviceManagementRequestJob::TYPE_REGISTRATION,
@@ -135,6 +155,69 @@ void CloudPolicyClient::Register(em::DeviceRegisterRequest::Type type,
                  base::Unretained(this)));
 }
 
+void CloudPolicyClient::RegisterWithCertificate(
+    em::DeviceRegisterRequest::Type type,
+    em::DeviceRegisterRequest::Flavor flavor,
+    const std::string& pem_certificate_chain,
+    const std::string& client_id,
+    const std::string& requisition,
+    const std::string& current_state_key) {
+  DCHECK(signing_service_);
+  DCHECK(service_);
+  DCHECK(!pem_certificate_chain.empty());
+  DCHECK(!is_registered());
+
+  SetClientId(client_id);
+
+  em::CertificateBasedDeviceRegistrationData data;
+  data.set_certificate_type(em::CertificateBasedDeviceRegistrationData::
+      ENTERPRISE_ENROLLMENT_CERTIFICATE);
+  data.set_device_certificate(pem_certificate_chain);
+
+  em::DeviceRegisterRequest* request = data.mutable_device_register_request();
+  if (!client_id.empty())
+    request->set_reregister(true);
+  request->set_type(type);
+  if (!machine_id_.empty())
+    request->set_machine_id(machine_id_);
+  if (!machine_model_.empty())
+    request->set_machine_model(machine_model_);
+  if (!requisition.empty())
+    request->set_requisition(requisition);
+  if (!current_state_key.empty())
+    request->set_server_backed_state_key(current_state_key);
+  request->set_flavor(flavor);
+
+  signing_service_->SignData(data.SerializeAsString(),
+      base::Bind(&CloudPolicyClient::OnRegisterWithCertificateRequestSigned,
+                 base::Unretained(this)));

[achuithb, 2016/08/23 18:40:28]

Why base::Unretained? Couldn't you use a weak ptr instead? Here and elsewhere

[The one and only Dr. Crash, 2016/08/24 05:53:44]

Again, that's me matching the style of the file.

[achuithb, 2016/08/24 06:05:36]

Could you please switch to using weak_ptr_factory? base::Unretained can cause
access to freed memory during shutdown which is a security problem.

[The one and only Dr. Crash, 2016/08/24 08:19:16]

Sure.

+}
+
+void CloudPolicyClient::OnRegisterWithCertificateRequestSigned(bool success,
+    em::SignedData signed_data) {
+  if (!success) {
+    em::DeviceManagementResponse response;

[achuithb, 2016/08/23 18:40:28]

const

[The one and only Dr. Crash, 2016/08/24 05:53:44]

Done.

+    OnRegisterCompleted(DM_STATUS_CANNOT_SIGN_REQUEST, 0, response);
+    return;
+  }
+  policy_fetch_request_job_.reset(

[achuithb, 2016/08/23 18:40:28]

newline before this

[The one and only Dr. Crash, 2016/08/24 05:53:44]

Done.

+      service_->CreateJob(
+          DeviceManagementRequestJob::TYPE_CERT_BASED_REGISTRATION,
+          GetRequestContext()));
+  policy_fetch_request_job_->SetClientID(client_id_);
+  em::SignedData* signed_request = policy_fetch_request_job_->GetRequest()->
+      mutable_cert_based_register_request()->mutable_signed_request();
+  signed_request->set_data(signed_data.data());
+  signed_request->set_signature(signed_data.signature());
+  signed_request->set_extra_data_bytes(signed_data.extra_data_bytes());
+  policy_fetch_request_job_->SetRetryCallback(
+      base::Bind(&CloudPolicyClient::OnRetryRegister,
+                 base::Unretained(this)));
+  policy_fetch_request_job_->Start(
+      base::Bind(&CloudPolicyClient::OnRegisterCompleted,
+                 base::Unretained(this)));
+}
+
 void CloudPolicyClient::SetInvalidationInfo(int64_t version,
                                             const std::string& payload) {
   invalidation_version_ = version;