use AttestationClient to create certificate request.

We are deprecating attestation methods by CryptohomeClient.

This is the minimum change for chromium-side attestation flow to support
ECC key type.

As for implementation details, it is chosen to let the AttestationFlow
object to hold a constant key type instead of being flexible when
calling GetCertificate. The reasons are 1., we don't have the needs to
specify key type with a single attestation flow instance, and it
requires the key type passing through almost the entire attestation
flow. It implied more code change, but also implies impact to
readability because we already use "key type" terminology to refers to
the type of EMK/EUK in this class; more presence of overloaded "key
type" is not easy for readers.

BUG=b:158955123
TEST=unit_tests.

Change-Id: Iab3cdd0b943219765ec7b99626cba53c382ce6d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2474933
Commit-Queue: Leo Lai <cylai@google.com>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820615}

chromeos/attestation/attestation_flow.cc

@@ -26,6 +26,22 @@ namespace attestation {
 
 namespace {
 
+base::Optional<::attestation::CertificateProfile> ProfileToAttestationProtoEnum(
+    AttestationCertificateProfile p) {
+  switch (p) {
+    case PROFILE_ENTERPRISE_MACHINE_CERTIFICATE:
+      return ::attestation::CertificateProfile::ENTERPRISE_MACHINE_CERTIFICATE;
+    case PROFILE_ENTERPRISE_USER_CERTIFICATE:
+      return ::attestation::CertificateProfile::ENTERPRISE_USER_CERTIFICATE;
+    case PROFILE_CONTENT_PROTECTION_CERTIFICATE:
+      return ::attestation::CertificateProfile::CONTENT_PROTECTION_CERTIFICATE;
+    case PROFILE_ENTERPRISE_ENROLLMENT_CERTIFICATE:
+      return ::attestation::CertificateProfile::
+          ENTERPRISE_ENROLLMENT_CERTIFICATE;
+  }
+  return {};
+}
+
 // A reasonable timeout that gives enough time for attestation to be ready,
 // yet does not make the caller wait too long.
 constexpr uint16_t kReadyTimeoutInSeconds = 60;
@@ -67,15 +83,25 @@ AttestationKeyType AttestationFlow::GetKeyTypeForProfile(
 
 AttestationFlow::AttestationFlow(cryptohome::AsyncMethodCaller* async_caller,
                                  CryptohomeClient* cryptohome_client,
-                                 std::unique_ptr<ServerProxy> server_proxy)
+                                 std::unique_ptr<ServerProxy> server_proxy,
+                                 ::attestation::KeyType crypto_key_type)
     : async_caller_(async_caller),
       cryptohome_client_(cryptohome_client),
       attestation_client_(AttestationClient::Get()),
       server_proxy_(std::move(server_proxy)),
+      crypto_key_type_(crypto_key_type),
       ready_timeout_(base::TimeDelta::FromSeconds(kReadyTimeoutInSeconds)),
       retry_delay_(
           base::TimeDelta::FromMilliseconds(kRetryDelayInMilliseconds)) {}
 
+AttestationFlow::AttestationFlow(cryptohome::AsyncMethodCaller* async_caller,
+                                 CryptohomeClient* cryptohome_client,
+                                 std::unique_ptr<ServerProxy> server_proxy)
+    : AttestationFlow(async_caller,
+                      cryptohome_client,
+                      std::move(server_proxy),
+                      ::attestation::KEY_TYPE_RSA) {}
+
 AttestationFlow::~AttestationFlow() = default;
 
 void AttestationFlow::GetCertificate(
@@ -220,12 +246,25 @@ void AttestationFlow::StartCertificateRequest(
   AttestationKeyType key_type = GetKeyTypeForProfile(certificate_profile);
   if (generate_new_key) {
     // Get the attestation service to create a Privacy CA certificate request.
-    async_caller_->AsyncTpmAttestationCreateCertRequest(
-        server_proxy_->GetType(), certificate_profile,
-        cryptohome::Identification(account_id), request_origin,
-        base::BindOnce(&AttestationFlow::SendCertificateRequestToPCA,
-                       weak_factory_.GetWeakPtr(), key_type, account_id,
-                       key_name, std::move(callback)));
+    const base::Optional<::attestation::CertificateProfile>
+        attestation_profile =
+            ProfileToAttestationProtoEnum(certificate_profile);
+    if (!attestation_profile) {
+      LOG(DFATAL) << "Attestation: Unrecognized profile type: "
+                  << certificate_profile;
+      return;
+    }
+
+    ::attestation::CreateCertificateRequestRequest request;
+    request.set_username(cryptohome::Identification(account_id).id());
+    request.set_certificate_profile(*attestation_profile);
+    request.set_request_origin(request_origin);
+    request.set_key_type(crypto_key_type_);
+
+    attestation_client_->CreateCertificateRequest(
+        request, base::BindOnce(&AttestationFlow::SendCertificateRequestToPCA,
+                                weak_factory_.GetWeakPtr(), key_type,
+                                account_id, key_name, std::move(callback)));
     return;
   }
 
@@ -264,23 +303,25 @@ void AttestationFlow::OnKeyExistCheckComplete(
                           key_name, std::move(callback), true);
 }
 
-void AttestationFlow::SendCertificateRequestToPCA(AttestationKeyType key_type,
+void AttestationFlow::SendCertificateRequestToPCA(
+    AttestationKeyType key_type,
     const AccountId& account_id,
     const std::string& key_name,
     CertificateCallback callback,
-                                                  bool success,
-                                                  const std::string& data) {
-  if (!success) {
-    LOG(ERROR) << "Attestation: Failed to create certificate request.";
+    const ::attestation::CreateCertificateRequestReply& reply) {
+  if (reply.status() != ::attestation::STATUS_SUCCESS) {
+    LOG(ERROR) << "Attestation: Failed to create certificate request. Status: "
+               << reply.status();
     std::move(callback).Run(ATTESTATION_UNSPECIFIED_FAILURE, "");
     return;
   }
 
   // Send the request to the Privacy CA.
   server_proxy_->SendCertificateRequest(
-      data, base::BindOnce(&AttestationFlow::SendCertificateResponseToDaemon,
-                           weak_factory_.GetWeakPtr(), key_type, account_id,
-                           key_name, std::move(callback)));
+      reply.pca_request(),
+      base::BindOnce(&AttestationFlow::SendCertificateResponseToDaemon,
+                     weak_factory_.GetWeakPtr(), key_type, account_id, key_name,
+                     std::move(callback)));
 }
 
 void AttestationFlow::SendCertificateResponseToDaemon(

chromeos/attestation/attestation_flow.h

@@ -74,6 +74,11 @@ class COMPONENT_EXPORT(CHROMEOS_ATTESTATION) AttestationFlow {
   AttestationFlow(cryptohome::AsyncMethodCaller* async_caller,
                   CryptohomeClient* cryptohome_client,
                   std::unique_ptr<ServerProxy> server_proxy);
+  AttestationFlow(cryptohome::AsyncMethodCaller* async_caller,
+                  CryptohomeClient* cryptohome_client,
+                  std::unique_ptr<ServerProxy> server_proxy,
+                  ::attestation::KeyType crypto_key_type);
+
   virtual ~AttestationFlow();
 
   // Sets the timeout for attestation to be ready.
@@ -241,14 +246,13 @@ class COMPONENT_EXPORT(CHROMEOS_ATTESTATION) AttestationFlow {
   //   account_id - Identifies the active user.
   //   key_name - The name of the key for which a certificate is requested.
   //   callback - Called when the operation completes.
-  //   success - The status of request creation.
-  //   data - The request data for the Privacy CA.
-  void SendCertificateRequestToPCA(AttestationKeyType key_type,
+  //   reply - the result returned by |AttestationClient|.
+  void SendCertificateRequestToPCA(
+      AttestationKeyType key_type,
       const AccountId& account_id,
       const std::string& key_name,
       CertificateCallback callback,
-                                   bool success,
-                                   const std::string& data);
+      const ::attestation::CreateCertificateRequestReply& reply);
 
   // Called when the Privacy CA responds to a certificate request.  The response
   // is asynchronously forwarded as-is to the attestation daemon in order to
@@ -295,6 +299,9 @@ class COMPONENT_EXPORT(CHROMEOS_ATTESTATION) AttestationFlow {
   AttestationClient* attestation_client_;
   std::unique_ptr<ServerProxy> server_proxy_;
 
+  // The key type that asks attestation service to create with.
+  const ::attestation::KeyType crypto_key_type_;
+
   base::TimeDelta ready_timeout_;
   base::TimeDelta retry_delay_;
 

chromeos/dbus/attestation/attestation_client.h

@@ -92,10 +92,24 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS_ATTESTATION) AttestationClient {
         ::attestation::AttestationStatus status) = 0;
 
     // Allowlists |request| so the certificate requests that comes in afterwards
-    // will get a fake certificate. if any alias of |request| has been
+    // will get a fake certificate. If any alias of |request| has been
     // allowlisted this functions performs no-ops.
     virtual void AllowlistCertificateRequest(
         const ::attestation::GetCertificateRequest& request) = 0;
+
+    // Allowlists the request that has |username|, |request_origin|, |profile|,
+    // and |key_type|, so the certificate requests that comes in afterwards will
+    // be successfully created.
+    virtual void AllowlistLegacyCreateCertificateRequest(
+        const std::string& username,
+        const std::string& request_origin,
+        ::attestation::CertificateProfile profile,
+        ::attestation::KeyType key_type) = 0;
+
+    // Gets the mutable |CreateCertificateRequestReply| that is returned when
+    // queried.
+    virtual ::attestation::CreateCertificateRequestReply*
+    mutable_certificate_request_reply() = 0;
   };
 
   // Not copyable or movable.

chromeos/dbus/attestation/fake_attestation_client.cc

@@ -49,6 +49,11 @@ FakeAttestationClient::FakeAttestationClient() = default;
 
 FakeAttestationClient::~FakeAttestationClient() = default;
 
+::attestation::CreateCertificateRequestReply*
+FakeAttestationClient::mutable_certificate_request_reply() {
+  return &certificate_request_reply_;
+}
+
 void FakeAttestationClient::GetKeyInfo(
     const ::attestation::GetKeyInfoRequest& request,
     GetKeyInfoCallback callback) {
@@ -153,7 +158,18 @@ void FakeAttestationClient::FinishEnroll(
 void FakeAttestationClient::CreateCertificateRequest(
     const ::attestation::CreateCertificateRequestRequest& request,
     CreateCertificateRequestCallback callback) {
-  NOTIMPLEMENTED();
+  for (const auto& req : allowlisted_create_requests_) {
+    if (req.username() == request.username() &&
+        req.request_origin() == request.request_origin() &&
+        req.certificate_profile() == request.certificate_profile() &&
+        req.key_type() == request.key_type()) {
+      PostProtoResponse(std::move(callback), certificate_request_reply_);
+      return;
+    }
+  }
+  ::attestation::CreateCertificateRequestReply failed_reply;
+  failed_reply.set_status(::attestation::STATUS_UNEXPECTED_DEVICE_ERROR);
+  PostProtoResponse(std::move(callback), failed_reply);
 }
 
 void FakeAttestationClient::FinishCertificateRequest(
@@ -267,6 +283,19 @@ void FakeAttestationClient::AllowlistCertificateRequest(
   certificate_indices_.push_back(kCertificateNotAssigned);
 }
 
+void FakeAttestationClient::AllowlistLegacyCreateCertificateRequest(
+    const std::string& username,
+    const std::string& request_origin,
+    ::attestation::CertificateProfile profile,
+    ::attestation::KeyType key_type) {
+  ::attestation::CreateCertificateRequestRequest request;
+  request.set_username(username);
+  request.set_request_origin(request_origin);
+  request.set_certificate_profile(profile);
+  request.set_key_type(key_type);
+  allowlisted_create_requests_.push_back(request);
+}
+
 AttestationClient::TestInterface* FakeAttestationClient::GetTestInterface() {
   return this;
 }

chromeos/dbus/attestation/fake_attestation_client.h

@@ -99,6 +99,13 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS_ATTESTATION) FakeAttestationClient
       ::attestation::AttestationStatus status) override;
   void AllowlistCertificateRequest(
       const ::attestation::GetCertificateRequest& request) override;
+  void AllowlistLegacyCreateCertificateRequest(
+      const std::string& username,
+      const std::string& request_origin,
+      ::attestation::CertificateProfile profile,
+      ::attestation::KeyType key_type) override;
+  ::attestation::CreateCertificateRequestReply*
+  mutable_certificate_request_reply() override;
 
   AttestationClient::TestInterface* GetTestInterface() override;
 
@@ -110,8 +117,15 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS_ATTESTATION) FakeAttestationClient
 
   bool is_enrolled_ = false;
 
+  ::attestation::CreateCertificateRequestReply certificate_request_reply_;
+
   // Maintains the allowlisted certificate requests.
   std::vector<::attestation::GetCertificateRequest> allowlisted_requests_;
+
+  // Maintains the allowlisted legacy create-certificate requests.
+  std::vector<::attestation::CreateCertificateRequestRequest>
+      allowlisted_create_requests_;
+
   // Maintains the numbers assigned to the allowlisted requests.
   std::vector<int> certificate_indices_;
   // The count of certificates that has been issued.