webauthnn: make caBLE Platform calls take arguments struct.

The MakeCredential and GetAssertion calls take a large number of
arguments. This change makes them structs instead. This will be useful
in the next change when these arguments need to be held pending a
notification on Android.

BUG=1002262

Change-Id: Ia03c2f786e36243902749183e6d3307a45c0717a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2500275
Commit-Queue: Adam Langley <agl@chromium.org>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/heads/master@{#822731}

chrome/android/features/cablev2_authenticator/native/cablev2_authenticator_android.cc

@@ -175,46 +175,37 @@ class AndroidPlatform : public device::cablev2::authenticator::Platform {
   }
 
   // Platform:
-  void MakeCredential(
-      const std::string& origin,
-      const std::string& rp_id,
-      base::span<const uint8_t> challenge,
-      base::span<const uint8_t> user_id,
-      base::span<const int> algorithms,
-      base::span<const std::vector<uint8_t>> excluded_cred_ids,
-      bool resident_key_required,
-      device::cablev2::authenticator::Platform::MakeCredentialCallback callback)
-      override {
+  void MakeCredential(std::unique_ptr<MakeCredentialParams> params) override {
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
     GlobalData& global_data = GetGlobalData();
     DCHECK(!global_data.pending_make_credential_callback);
-    global_data.pending_make_credential_callback = std::move(callback);
+    global_data.pending_make_credential_callback = std::move(params->callback);
 
     Java_CableAuthenticator_makeCredential(
-        env_, cable_authenticator_, ConvertUTF8ToJavaString(env_, origin),
-        ConvertUTF8ToJavaString(env_, rp_id), ToJavaByteArray(env_, challenge),
-        ToJavaByteArray(env_, user_id), ToJavaIntArray(env_, algorithms),
-        ToJavaArrayOfByteArray(env_, excluded_cred_ids), resident_key_required);
+        env_, cable_authenticator_,
+        ConvertUTF8ToJavaString(env_, params->origin),
+        ConvertUTF8ToJavaString(env_, params->rp_id),
+        ToJavaByteArray(env_, params->challenge),
+        ToJavaByteArray(env_, params->user_id),
+        ToJavaIntArray(env_, params->algorithms),
+        ToJavaArrayOfByteArray(env_, params->excluded_cred_ids),
+        params->resident_key_required);
   }
 
-  void GetAssertion(
-      const std::string& origin,
-      const std::string& rp_id,
-      base::span<const uint8_t> challenge,
-      base::span<const std::vector<uint8_t>> allowed_cred_ids,
-      device::cablev2::authenticator::Platform::GetAssertionCallback callback)
-      override {
+  void GetAssertion(std::unique_ptr<GetAssertionParams> params) override {
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
     GlobalData& global_data = GetGlobalData();
     DCHECK(!global_data.pending_get_assertion_callback);
-    global_data.pending_get_assertion_callback = std::move(callback);
+    global_data.pending_get_assertion_callback = std::move(params->callback);
 
     Java_CableAuthenticator_getAssertion(
-        env_, cable_authenticator_, ConvertUTF8ToJavaString(env_, origin),
-        ConvertUTF8ToJavaString(env_, rp_id), ToJavaByteArray(env_, challenge),
-        ToJavaArrayOfByteArray(env_, allowed_cred_ids));
+        env_, cable_authenticator_,
+        ConvertUTF8ToJavaString(env_, params->origin),
+        ConvertUTF8ToJavaString(env_, params->rp_id),
+        ToJavaByteArray(env_, params->challenge),
+        ToJavaArrayOfByteArray(env_, params->allowed_cred_ids));
   }
 
   std::unique_ptr<BLEAdvert> SendBLEAdvert(

device/fido/cable/v2_authenticator.cc

@@ -549,7 +549,15 @@ class CTAP2Processor : public Transaction {
           return base::nullopt;
         }
 
-        std::vector<int> algorithms;
+        auto params = std::make_unique<Platform::MakeCredentialParams>();
+        params->origin = *make_cred_request.origin;
+        params->rp_id = *make_cred_request.rp_id;
+        params->challenge = *make_cred_request.challenge;
+        params->user_id = *make_cred_request.user_id;
+        params->callback =
+            base::BindOnce(&CTAP2Processor::OnMakeCredentialResponse,
+                           weak_factory_.GetWeakPtr());
+
         if (!device::cbor_extract::ForEachPublicKeyEntry(
                 *make_cred_request.cred_params, cbor::Value("alg"),
                 base::BindRepeating(
@@ -567,11 +575,10 @@ class CTAP2Processor : public Transaction {
                       out->push_back(static_cast<int>(alg));
                       return true;
                     },
-                    base::Unretained(&algorithms)))) {
+                    base::Unretained(&params->algorithms)))) {
           return base::nullopt;
         }
 
-        std::vector<std::vector<uint8_t>> excluded_credential_ids;
         if (make_cred_request.excluded_credentials &&
             !device::cbor_extract::ForEachPublicKeyEntry(
                 *make_cred_request.excluded_credentials, cbor::Value("id"),
@@ -584,19 +591,13 @@ class CTAP2Processor : public Transaction {
                       out->push_back(value.GetBytestring());
                       return true;
                     },
-                    base::Unretained(&excluded_credential_ids)))) {
+                    base::Unretained(&params->excluded_cred_ids)))) {
           return base::nullopt;
         }
 
         // TODO: plumb the rk flag through once GmsCore supports resident
         // keys. This will require support for optional maps in |Extract|.
-        platform_->MakeCredential(
-            *make_cred_request.origin, *make_cred_request.rp_id,
-            *make_cred_request.challenge, *make_cred_request.user_id,
-            algorithms, excluded_credential_ids,
-            /*resident_key_required=*/false,
-            base::BindOnce(&CTAP2Processor::OnMakeCredentialResponse,
-                           weak_factory_.GetWeakPtr()));
+        platform_->MakeCredential(std::move(params));
         return std::vector<uint8_t>();
       }
 
@@ -606,6 +607,7 @@ class CTAP2Processor : public Transaction {
           FIDO_LOG(ERROR) << "Invalid makeCredential payload";
           return base::nullopt;
         }
+
         GetAssertionRequest get_assertion_request;
         if (!device::cbor_extract::Extract<GetAssertionRequest>(
                 &get_assertion_request, kGetAssertionParseSteps,
@@ -614,7 +616,14 @@ class CTAP2Processor : public Transaction {
           return base::nullopt;
         }
 
-        std::vector<std::vector<uint8_t>> allowed_credential_ids;
+        auto params = std::make_unique<Platform::GetAssertionParams>();
+        params->origin = *get_assertion_request.origin;
+        params->rp_id = *get_assertion_request.rp_id;
+        params->challenge = *get_assertion_request.challenge;
+        params->callback =
+            base::BindOnce(&CTAP2Processor::OnGetAssertionResponse,
+                           weak_factory_.GetWeakPtr());
+
         if (get_assertion_request.allowed_credentials &&
             !device::cbor_extract::ForEachPublicKeyEntry(
                 *get_assertion_request.allowed_credentials, cbor::Value("id"),
@@ -627,16 +636,11 @@ class CTAP2Processor : public Transaction {
                       out->push_back(value.GetBytestring());
                       return true;
                     },
-                    base::Unretained(&allowed_credential_ids)))) {
+                    base::Unretained(&params->allowed_cred_ids)))) {
           return base::nullopt;
         }
 
-        platform_->GetAssertion(
-            *get_assertion_request.origin, *get_assertion_request.rp_id,
-            *get_assertion_request.challenge, allowed_credential_ids,
-            base::BindOnce(&CTAP2Processor::OnGetAssertionResponse,
-                           weak_factory_.GetWeakPtr()));
-
+        platform_->GetAssertion(std::move(params));
         return std::vector<uint8_t>();
       }
 
@@ -819,6 +823,10 @@ class PairingDataGenerator {
 }  // namespace
 
 Platform::BLEAdvert::~BLEAdvert() = default;
+Platform::MakeCredentialParams::MakeCredentialParams() = default;
+Platform::MakeCredentialParams::~MakeCredentialParams() = default;
+Platform::GetAssertionParams::GetAssertionParams() = default;
+Platform::GetAssertionParams::~GetAssertionParams() = default;
 Platform::~Platform() = default;
 Transport::~Transport() = default;
 Transaction::~Transaction() = default;

device/fido/cable/v2_authenticator.h

@@ -10,7 +10,7 @@
 
 #include <stdint.h>
 
-#include "base/callback_forward.h"
+#include "base/callback.h"
 #include "base/containers/span.h"
 #include "base/optional.h"
 #include "device/fido/cable/v2_constants.h"
@@ -39,6 +39,26 @@ class Platform {
       base::OnceCallback<void(uint32_t status,
                               base::span<const uint8_t> client_data_json,
                               base::span<const uint8_t> attestation_obj)>;
+
+  struct MakeCredentialParams {
+    MakeCredentialParams();
+    ~MakeCredentialParams();
+    MakeCredentialParams(const MakeCredentialParams&) = delete;
+    MakeCredentialParams& operator=(const MakeCredentialParams&) = delete;
+    MakeCredentialParams(MakeCredentialParams&&) = delete;
+
+    std::string origin;
+    std::string rp_id;
+    std::vector<uint8_t> challenge;
+    std::vector<uint8_t> user_id;
+    std::vector<int> algorithms;
+    std::vector<std::vector<uint8_t>> excluded_cred_ids;
+    bool resident_key_required = false;
+    MakeCredentialCallback callback;
+  };
+
+  virtual void MakeCredential(std::unique_ptr<MakeCredentialParams> params) = 0;
+
   using GetAssertionCallback =
       base::OnceCallback<void(uint32_t status,
                               base::span<const uint8_t> client_data_json,
@@ -46,22 +66,21 @@ class Platform {
                               base::span<const uint8_t> auth_data,
                               base::span<const uint8_t> sig)>;
 
-  virtual void MakeCredential(
-      const std::string& origin,
-      const std::string& rp_id,
-      base::span<const uint8_t> challenge,
-      base::span<const uint8_t> user_id,
-      base::span<const int> algorithms,
-      base::span<const std::vector<uint8_t>> excluded_cred_ids,
-      bool resident_key_required,
-      MakeCredentialCallback callback) = 0;
-
-  virtual void GetAssertion(
-      const std::string& origin,
-      const std::string& rp_id,
-      base::span<const uint8_t> challenge,
-      base::span<const std::vector<uint8_t>> allowed_cred_ids,
-      GetAssertionCallback callback) = 0;
+  struct GetAssertionParams {
+    GetAssertionParams();
+    ~GetAssertionParams();
+    GetAssertionParams(const GetAssertionParams&) = delete;
+    GetAssertionParams& operator=(const GetAssertionParams&) = delete;
+    GetAssertionParams(GetAssertionParams&&) = delete;
+
+    std::string origin;
+    std::string rp_id;
+    std::vector<uint8_t> challenge;
+    std::vector<std::vector<uint8_t>> allowed_cred_ids;
+    GetAssertionCallback callback;
+  };
+
+  virtual void GetAssertion(std::unique_ptr<GetAssertionParams> params) = 0;
 
   virtual std::unique_ptr<BLEAdvert> SendBLEAdvert(
       base::span<const uint8_t, kAdvertSize> payload) = 0;

device/fido/cable/v2_test_util.cc

@@ -337,35 +337,29 @@ class TestPlatform : public authenticator::Platform {
   TestPlatform(Discovery* discovery, device::VirtualCtap2Device* ctap2_device)
       : discovery_(discovery), ctap2_device_(ctap2_device) {}
 
-  void MakeCredential(const std::string& origin,
-                      const std::string& rp_id,
-                      base::span<const uint8_t> challenge,
-                      base::span<const uint8_t> user_id,
-                      base::span<const int> algorithms,
-                      base::span<const std::vector<uint8_t>> excluded_cred_ids,
-                      bool resident_key_required,
-                      MakeCredentialCallback callback) override {
+  void MakeCredential(std::unique_ptr<MakeCredentialParams> params) override {
     std::string challenge_b64;
     base::Base64UrlEncode(
-        base::StringPiece(reinterpret_cast<const char*>(challenge.data()),
-                          challenge.size()),
+        base::StringPiece(
+            reinterpret_cast<const char*>(params->challenge.data()),
+            params->challenge.size()),
         base::Base64UrlEncodePolicy::OMIT_PADDING, &challenge_b64);
 
     std::string client_data_json = base::StringPrintf(
         R"({"type": "webauthn.create", "challenge": "%s", "origin": "%s",
               "androidPackageName": "com.chrome.unittest"})",
-        challenge_b64.c_str(), origin.c_str());
+        challenge_b64.c_str(), params->origin.c_str());
     std::vector<device::PublicKeyCredentialParams::CredentialInfo> cred_infos;
-    for (const auto& algo : algorithms) {
+    for (const auto& algo : params->algorithms) {
       device::PublicKeyCredentialParams::CredentialInfo cred_info;
       cred_info.algorithm = algo;
       cred_infos.push_back(cred_info);
     }
 
     device::CtapMakeCredentialRequest request(
-        client_data_json, device::PublicKeyCredentialRpEntity(rp_id),
+        client_data_json, device::PublicKeyCredentialRpEntity(params->rp_id),
         device::PublicKeyCredentialUserEntity(
-            device::fido_parsing_utils::Materialize(user_id),
+            device::fido_parsing_utils::Materialize(params->user_id),
             /*name=*/base::nullopt, /*display_name=*/base::nullopt,
             /*icon_url=*/base::nullopt),
         device::PublicKeyCredentialParams(std::move(cred_infos)));
@@ -377,14 +371,10 @@ class TestPlatform : public authenticator::Platform {
         ToCTAP2Command(std::move(request_cbor)),
         base::BindOnce(&TestPlatform::OnMakeCredentialResult,
                        weak_factory_.GetWeakPtr(), std::move(client_data_json),
-                       std::move(callback)));
+                       std::move(params->callback)));
   }
 
-  void GetAssertion(const std::string& origin,
-                    const std::string& rp_id,
-                    base::span<const uint8_t> challenge,
-                    base::span<const std::vector<uint8_t>> allowed_cred_ids,
-                    GetAssertionCallback callback) override {
+  void GetAssertion(std::unique_ptr<GetAssertionParams> params) override {
     NOTREACHED();
   }