Implement MakeCredential operation for softkey

Add MakeCredential operation to VirtualCtap2Device. VirtualCtap2Device
only supports ES256 algorithm and provides packed attestation statement
during registration.

Bug: 829413
Change-Id: I70782952bdbb2f9f749bb27756a84a6bd37af46d
Reviewed-on: https://chromium-review.googlesource.com/1115900
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572032}

device/fido/BUILD.gn

@@ -110,6 +110,8 @@ component("fido") {
     "u2f_register_operation.h",
     "u2f_sign_operation.cc",
     "u2f_sign_operation.h",
+    "virtual_ctap2_device.cc",
+    "virtual_ctap2_device.h",
     "virtual_fido_device.cc",
     "virtual_fido_device.h",
     "virtual_u2f_device.cc",

device/fido/authenticator_get_info_response.cc

@@ -8,6 +8,7 @@
 
 #include "components/cbor/cbor_values.h"
 #include "components/cbor/cbor_writer.h"
+#include "device/fido/fido_parsing_utils.h"
 
 namespace device {
 
@@ -26,8 +27,9 @@ cbor::CBORValue::ArrayValue ToArrayValue(const Container& container) {
 
 AuthenticatorGetInfoResponse::AuthenticatorGetInfoResponse(
     base::flat_set<ProtocolVersion> versions,
-    std::vector<uint8_t> aaguid)
-    : versions_(std::move(versions)), aaguid_(std::move(aaguid)) {}
+    base::span<const uint8_t, kAaguidLength> aaguid)
+    : versions_(std::move(versions)),
+      aaguid_(fido_parsing_utils::Materialize(aaguid)) {}
 
 AuthenticatorGetInfoResponse::AuthenticatorGetInfoResponse(
     AuthenticatorGetInfoResponse&& that) = default;

device/fido/authenticator_get_info_response.h

@@ -26,7 +26,7 @@ namespace device {
 class COMPONENT_EXPORT(DEVICE_FIDO) AuthenticatorGetInfoResponse {
  public:
   AuthenticatorGetInfoResponse(base::flat_set<ProtocolVersion> versions,
-                               std::vector<uint8_t> aaguid);
+                               base::span<const uint8_t, kAaguidLength> aaguid);
   AuthenticatorGetInfoResponse(AuthenticatorGetInfoResponse&& that);
   AuthenticatorGetInfoResponse& operator=(AuthenticatorGetInfoResponse&& other);
   ~AuthenticatorGetInfoResponse();
@@ -40,7 +40,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) AuthenticatorGetInfoResponse {
       AuthenticatorSupportedOptions options);
 
   const base::flat_set<ProtocolVersion>& versions() const { return versions_; }
-  const std::vector<uint8_t>& aaguid() const { return aaguid_; }
+  const std::array<uint8_t, kAaguidLength>& aaguid() const { return aaguid_; }
   const base::Optional<uint32_t>& max_msg_size() const { return max_msg_size_; }
   const base::Optional<std::vector<uint8_t>>& pin_protocol() const {
     return pin_protocols_;
@@ -52,7 +52,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) AuthenticatorGetInfoResponse {
 
  private:
   base::flat_set<ProtocolVersion> versions_;
-  std::vector<uint8_t> aaguid_;
+  std::array<uint8_t, kAaguidLength> aaguid_;
   base::Optional<uint32_t> max_msg_size_;
   base::Optional<std::vector<uint8_t>> pin_protocols_;
   base::Optional<std::vector<std::string>> extensions_;

device/fido/ctap_make_credential_request.h

@@ -70,6 +70,9 @@ class COMPONENT_EXPORT(DEVICE_FIDO) CtapMakeCredentialRequest {
   exclude_list() const {
     return exclude_list_;
   }
+  const base::Optional<std::vector<uint8_t>>& pin_auth() const {
+    return pin_auth_;
+  }
 
  private:
   std::array<uint8_t, kClientDataHashLength> client_data_hash_;

device/fido/ctap_response_unittest.cc

@@ -198,9 +198,9 @@ constexpr uint8_t kAuthDataCBOR[] = {
     // and test_data::kTestECPublicKeyCOSE.
     0x58, 0xC4};
 
-constexpr uint8_t kTestDeviceAaguid[] = {0xF8, 0xA0, 0x11, 0xF3, 0x8C, 0x0A,
-                                         0x4D, 0x15, 0x80, 0x06, 0x17, 0x11,
-                                         0x1F, 0x9E, 0xDC, 0x7D};
+constexpr std::array<uint8_t, kAaguidLength> kTestDeviceAaguid = {
+    {0xF8, 0xA0, 0x11, 0xF3, 0x8C, 0x0A, 0x4D, 0x15, 0x80, 0x06, 0x17, 0x11,
+     0x1F, 0x9E, 0xDC, 0x7D}};
 
 std::vector<uint8_t> GetTestAttestedCredentialDataBytes() {
   // Combine kTestAttestedCredentialDataPrefix and kTestECPublicKeyCOSE.

device/fido/make_credential_task_unittest.cc

@@ -20,6 +20,7 @@
 #include "device/fido/make_credential_task.h"
 #include "device/fido/mock_fido_device.h"
 #include "device/fido/test_callback_receiver.h"
+#include "device/fido/virtual_ctap2_device.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -107,6 +108,21 @@ TEST_F(FidoMakeCredentialTaskTest, MakeCredentialSuccess) {
   EXPECT_TRUE(device->device_info());
 }
 
+TEST_F(FidoMakeCredentialTaskTest, TestRegisterSuccessWithFake) {
+  auto device = std::make_unique<VirtualCtap2Device>();
+  const auto task = CreateMakeCredentialTask(device.get());
+  make_credential_callback_receiver().WaitForCallback();
+
+  EXPECT_EQ(CtapDeviceResponseCode::kSuccess,
+            make_credential_callback_receiver().status());
+
+  // We don't verify the response from the fake, but do a quick sanity check.
+  ASSERT_TRUE(make_credential_callback_receiver().value());
+  EXPECT_EQ(
+      32u,
+      make_credential_callback_receiver().value()->raw_credential_id().size());
+}
+
 TEST_F(FidoMakeCredentialTaskTest, MakeCredentialWithIncorrectRpIdHash) {
   auto device = std::make_unique<MockFidoDevice>();
 

device/fido/virtual_ctap2_device.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "device/fido/virtual_ctap2_device.h"
+
+#include <array>
+#include <string>
+#include <utility>
+
+#include "base/bind.h"
+#include "base/numerics/safe_conversions.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "components/cbor/cbor_writer.h"
+#include "crypto/ec_private_key.h"
+#include "device/fido/authenticator_make_credential_response.h"
+#include "device/fido/ctap_make_credential_request.h"
+#include "device/fido/ec_public_key.h"
+#include "device/fido/fido_constants.h"
+#include "device/fido/fido_parsing_utils.h"
+#include "device/fido/opaque_attestation_statement.h"
+
+namespace device {
+
+namespace {
+
+constexpr std::array<uint8_t, kAaguidLength> kDeviceAaguid = {
+    {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x01, 0x02, 0x03, 0x04,
+     0x05, 0x06, 0x07, 0x08}};
+
+std::vector<uint8_t> ConstructResponse(CtapDeviceResponseCode response_code,
+                                       base::span<const uint8_t> data) {
+  std::vector<uint8_t> response{base::strict_cast<uint8_t>(response_code)};
+  fido_parsing_utils::Append(&response, data);
+  return response;
+}
+
+void ReturnCtap2Response(
+    FidoDevice::DeviceCallback cb,
+    CtapDeviceResponseCode response_code,
+    base::Optional<base::span<const uint8_t>> data = base::nullopt) {
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE,
+      base::BindOnce(std::move(cb),
+                     ConstructResponse(response_code,
+                                       data.value_or(std::vector<uint8_t>{}))));
+}
+
+bool AreMakeCredentialOptionsValid(const AuthenticatorSupportedOptions& options,
+                                   const CtapMakeCredentialRequest& request) {
+  if (request.resident_key_supported() && !options.supports_resident_key())
+    return false;
+
+  return !request.user_verification_required() ||
+         options.user_verification_availability() ==
+             AuthenticatorSupportedOptions::UserVerificationAvailability::
+                 kSupportedAndConfigured;
+}
+
+// Checks that whether the received MakeCredential request includes EA256
+// algorithm in publicKeyCredParam.
+bool AreMakeCredentialParamsValid(const CtapMakeCredentialRequest& request) {
+  const auto& params =
+      request.public_key_credential_params().public_key_credential_params();
+  return std::any_of(
+      params.begin(), params.end(), [](const auto& credential_info) {
+        return credential_info.algorithm ==
+               base::strict_cast<int>(CoseAlgorithmIdentifier::kCoseEs256);
+      });
+}
+
+std::unique_ptr<ECPublicKey> ConstructECPublicKey(
+    std::string public_key_string) {
+  DCHECK_EQ(64u, public_key_string.size());
+
+  const auto public_key_x_coordinate =
+      base::as_bytes(base::make_span(public_key_string)).first(32);
+  const auto public_key_y_coordinate =
+      base::as_bytes(base::make_span(public_key_string)).last(32);
+  return std::make_unique<ECPublicKey>(
+      fido_parsing_utils::kEs256,
+      fido_parsing_utils::Materialize(public_key_x_coordinate),
+      fido_parsing_utils::Materialize(public_key_y_coordinate));
+}
+
+std::vector<uint8_t> ConstructSignatureBuffer(
+    const AuthenticatorData& authenticator_data,
+    base::span<const uint8_t, kClientDataHashLength> client_data_hash) {
+  std::vector<uint8_t> signature_buffer;
+  fido_parsing_utils::Append(&signature_buffer,
+                             authenticator_data.SerializeToByteArray());
+  fido_parsing_utils::Append(&signature_buffer, client_data_hash);
+  return signature_buffer;
+}
+
+std::vector<uint8_t> ConstructMakeCredentialResponse(
+    base::span<const uint8_t> attestation_certificate,
+    base::span<const uint8_t> signature,
+    AuthenticatorData authenticator_data) {
+  cbor::CBORValue::MapValue attestation_map;
+  attestation_map.emplace("alg", -7);
+  attestation_map.emplace("sig", fido_parsing_utils::Materialize(signature));
+
+  cbor::CBORValue::ArrayValue certificate_chain;
+  certificate_chain.emplace_back(
+      fido_parsing_utils::Materialize(attestation_certificate));
+  attestation_map.emplace("x5c", std::move(certificate_chain));
+  AuthenticatorMakeCredentialResponse make_credential_response(
+      AttestationObject(
+          std::move(authenticator_data),
+          std::make_unique<OpaqueAttestationStatement>(
+              "packed", cbor::CBORValue(std::move(attestation_map)))));
+  return GetSerializedCtapDeviceResponse(make_credential_response);
+}
+
+}  // namespace
+
+VirtualCtap2Device::VirtualCtap2Device()
+    : VirtualFidoDevice(),
+      device_info_(AuthenticatorGetInfoResponse({ProtocolVersion::kCtap},
+                                                kDeviceAaguid)),
+      weak_factory_(this) {}
+
+VirtualCtap2Device::VirtualCtap2Device(scoped_refptr<State> state)
+    : VirtualFidoDevice(std::move(state)),
+      device_info_(AuthenticatorGetInfoResponse({ProtocolVersion::kCtap},
+                                                kDeviceAaguid)),
+      weak_factory_(this) {}
+
+VirtualCtap2Device::~VirtualCtap2Device() = default;
+
+// As all operations for VirtualCtap2Device are synchronous and we do not wait
+// for user touch, Cancel command is no-op.
+void VirtualCtap2Device::Cancel() {}
+
+void VirtualCtap2Device::DeviceTransact(std::vector<uint8_t> command,
+                                        DeviceCallback cb) {
+  if (command.empty()) {
+    ReturnCtap2Response(std::move(cb), CtapDeviceResponseCode::kCtap2ErrOther);
+    return;
+  }
+
+  auto cmd_type = command[0];
+  const auto request_bytes = base::make_span(command).subspan(1);
+  CtapDeviceResponseCode response_code = CtapDeviceResponseCode::kCtap2ErrOther;
+  std::vector<uint8_t> response_data;
+
+  switch (static_cast<CtapRequestCommand>(cmd_type)) {
+    case CtapRequestCommand::kAuthenticatorGetInfo:
+      if (!request_bytes.empty()) {
+        ReturnCtap2Response(std::move(cb),
+                            CtapDeviceResponseCode::kCtap2ErrOther);
+        return;
+      }
+
+      response_code = OnAuthenticatorGetInfo(&response_data);
+      break;
+    case CtapRequestCommand::kAuthenticatorMakeCredential:
+      response_code = OnMakeCredential(request_bytes, &response_data);
+      break;
+    default:
+      break;
+  }
+
+  // Call |callback| via the |MessageLoop| because |AuthenticatorImpl| doesn't
+  // support callback hairpinning.
+  ReturnCtap2Response(std::move(cb), response_code, std::move(response_data));
+}
+
+base::WeakPtr<FidoDevice> VirtualCtap2Device::GetWeakPtr() {
+  return weak_factory_.GetWeakPtr();
+}
+
+void VirtualCtap2Device::SetAuthenticatorSupportedOptions(
+    AuthenticatorSupportedOptions options) {
+  device_info_.SetOptions(std::move(options));
+}
+
+CtapDeviceResponseCode VirtualCtap2Device::OnMakeCredential(
+    base::span<const uint8_t> request_bytes,
+    std::vector<uint8_t>* response) {
+  auto request = ParseCtapMakeCredentialRequest(request_bytes);
+  if (!request)
+    return CtapDeviceResponseCode::kCtap2ErrOther;
+
+  if (!AreMakeCredentialOptionsValid(device_info_.options(), *request) ||
+      !AreMakeCredentialParamsValid(*request)) {
+    return CtapDeviceResponseCode::kCtap2ErrOther;
+  }
+
+  // Client pin is not supported.
+  if (request->pin_auth())
+    return CtapDeviceResponseCode::kCtap2ErrPinInvalid;
+
+  // Check for already registered credentials.
+  const auto rp_id_hash =
+      fido_parsing_utils::CreateSHA256Hash(request->rp().rp_id());
+  if (request->exclude_list()) {
+    for (const auto& excluded_credential : *request->exclude_list()) {
+      if (FindRegistrationData(excluded_credential.id(), rp_id_hash))
+        return CtapDeviceResponseCode::kCtap2ErrCredentialExcluded;
+    }
+  }
+
+  // Create key to register.
+  // Note: Non-deterministic, you need to mock this out if you rely on
+  // deterministic behavior.
+  auto private_key = crypto::ECPrivateKey::Create();
+  std::string public_key;
+  bool status = private_key->ExportRawPublicKey(&public_key);
+  DCHECK(status);
+
+  // Our key handles are simple hashes of the public key.
+  auto hash = fido_parsing_utils::CreateSHA256Hash(public_key);
+  std::vector<uint8_t> key_handle(hash.begin(), hash.end());
+
+  AttestedCredentialData attested_credential_data(
+      kDeviceAaguid, {{0, crypto::kSHA256Length}}, key_handle,
+      ConstructECPublicKey(public_key));
+
+  auto authenticator_data = ConstructAuthenticatorData(
+      rp_id_hash, std::move(attested_credential_data));
+  auto sign_buffer =
+      ConstructSignatureBuffer(authenticator_data, request->client_data_hash());
+
+  // Sign with attestation key.
+  // Note: Non-deterministic, you need to mock this out if you rely on
+  // deterministic behavior.
+  std::vector<uint8_t> sig;
+  std::unique_ptr<crypto::ECPrivateKey> attestation_private_key =
+      crypto::ECPrivateKey::CreateFromPrivateKeyInfo(GetAttestationKey());
+  status = Sign(attestation_private_key.get(), std::move(sign_buffer), &sig);
+  DCHECK(status);
+
+  auto attestation_cert = GenerateAttestationCertificate(
+      false /* individual_attestation_requested */);
+  if (!attestation_cert)
+    return CtapDeviceResponseCode::kCtap2ErrOther;
+
+  *response = ConstructMakeCredentialResponse(std::move(*attestation_cert), sig,
+                                              std::move(authenticator_data));
+
+  StoreNewKey(rp_id_hash, key_handle, std::move(private_key));
+  return CtapDeviceResponseCode::kSuccess;
+}
+
+CtapDeviceResponseCode VirtualCtap2Device::OnAuthenticatorGetInfo(
+    std::vector<uint8_t>* response) const {
+  *response = EncodeToCBOR(device_info_);
+  return CtapDeviceResponseCode::kSuccess;
+}
+
+AuthenticatorData VirtualCtap2Device::ConstructAuthenticatorData(
+    base::span<const uint8_t, kRpIdHashLength> rp_id_hash,
+    base::Optional<AttestedCredentialData> attested_credential_data) {
+  uint8_t flag =
+      base::strict_cast<uint8_t>(AuthenticatorData::Flag::kTestOfUserPresence);
+  std::array<uint8_t, kSignCounterLength> signature_counter;
+
+  // Constructing AuthenticatorData for registration operation.
+  if (attested_credential_data) {
+    flag |= base::strict_cast<uint8_t>(AuthenticatorData::Flag::kAttestation);
+    signature_counter = {{0x00, 0x00, 0x00, 0x01}};
+    // Constructing AuthenticatorData for sign operation.
+  } else {
+    auto* registration_data = FindRegistrationData(
+        attested_credential_data->credential_id(), rp_id_hash);
+    signature_counter[0] = (registration_data->counter >> 24) & 0xff;
+    signature_counter[1] = (registration_data->counter >> 16) & 0xff;
+    signature_counter[2] = (registration_data->counter >> 8) & 0xff;
+    signature_counter[3] = (registration_data->counter) & 0xff;
+  }
+
+  return AuthenticatorData(rp_id_hash, flag, signature_counter,
+                           std::move(attested_credential_data));
+}
+
+}  // namespace device

device/fido/virtual_ctap2_device.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef DEVICE_FIDO_VIRTUAL_CTAP2_DEVICE_H_
+#define DEVICE_FIDO_VIRTUAL_CTAP2_DEVICE_H_
+
+#include <stdint.h>
+
+#include <memory>
+#include <vector>
+
+#include "base/component_export.h"
+#include "base/containers/span.h"
+#include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
+#include "base/optional.h"
+#include "device/fido/attested_credential_data.h"
+#include "device/fido/authenticator_data.h"
+#include "device/fido/authenticator_supported_options.h"
+#include "device/fido/fido_constants.h"
+#include "device/fido/virtual_fido_device.h"
+
+namespace device {
+
+class COMPONENT_EXPORT(DEVICE_FIDO) VirtualCtap2Device
+    : public VirtualFidoDevice {
+ public:
+  VirtualCtap2Device();
+  explicit VirtualCtap2Device(scoped_refptr<State> state);
+  ~VirtualCtap2Device() override;
+
+  // FidoDevice:
+  void Cancel() override;
+  void DeviceTransact(std::vector<uint8_t> command, DeviceCallback cb) override;
+  base::WeakPtr<FidoDevice> GetWeakPtr() override;
+
+  void SetAuthenticatorSupportedOptions(AuthenticatorSupportedOptions options);
+
+ private:
+  CtapDeviceResponseCode OnMakeCredential(base::span<const uint8_t> request,
+                                          std::vector<uint8_t>* response);
+
+  CtapDeviceResponseCode OnAuthenticatorGetInfo(
+      std::vector<uint8_t>* response) const;
+
+  AuthenticatorData ConstructAuthenticatorData(
+      base::span<const uint8_t, kRpIdHashLength> rp_id_hash,
+      base::Optional<AttestedCredentialData> attested_credential_data);
+
+  AuthenticatorGetInfoResponse device_info_;
+  base::WeakPtrFactory<FidoDevice> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(VirtualCtap2Device);
+};
+
+}  // namespace device
+
+#endif  // DEVICE_FIDO_VIRTUAL_CTAP2_DEVICE_H_

device/fido/virtual_fido_device.cc

@@ -7,7 +7,6 @@
 #include <tuple>
 #include <utility>
 
-#include "crypto/ec_private_key.h"
 #include "crypto/ec_signature_creator.h"
 #include "device/fido/fido_parsing_utils.h"
 
@@ -122,6 +121,34 @@ VirtualFidoDevice::GenerateAttestationCertificate(
   return std::vector<uint8_t>(attestation_cert.begin(), attestation_cert.end());
 }
 
+void VirtualFidoDevice::StoreNewKey(
+    base::span<const uint8_t, kRpIdHashLength> application_parameter,
+    base::span<const uint8_t> key_handle,
+    std::unique_ptr<crypto::ECPrivateKey> private_key) {
+  // Store the registration. Because the key handle is the hashed public key we
+  // just generated, no way this should already be registered.
+  bool did_insert = false;
+  std::tie(std::ignore, did_insert) = mutable_state()->registrations.emplace(
+      fido_parsing_utils::Materialize(key_handle),
+      RegistrationData(std::move(private_key), application_parameter, 1));
+  DCHECK(did_insert);
+}
+
+VirtualFidoDevice::RegistrationData* VirtualFidoDevice::FindRegistrationData(
+    base::span<const uint8_t> key_handle,
+    base::span<const uint8_t, kRpIdHashLength> application_parameter) {
+  // Check if this is our key_handle and it's for this appId.
+  auto it = mutable_state()->registrations.find(key_handle);
+  if (it == mutable_state()->registrations.end())
+    return nullptr;
+
+  if (application_parameter !=
+      base::make_span(it->second.application_parameter))
+    return nullptr;
+
+  return &(it->second);
+}
+
 void VirtualFidoDevice::TryWink(WinkCallback cb) {
   std::move(cb).Run();
 }

device/fido/virtual_fido_device.h

@@ -18,6 +18,7 @@
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/optional.h"
+#include "crypto/ec_private_key.h"
 #include "device/fido/fido_constants.h"
 #include "device/fido/fido_device.h"
 #include "device/fido/fido_parsing_utils.h"
@@ -120,6 +121,15 @@ class COMPONENT_EXPORT(DEVICE_FIDO) VirtualFidoDevice : public FidoDevice {
   base::Optional<std::vector<uint8_t>> GenerateAttestationCertificate(
       bool individual_attestation_requested) const;
 
+  void StoreNewKey(
+      base::span<const uint8_t, kRpIdHashLength> application_parameter,
+      base::span<const uint8_t> key_handle,
+      std::unique_ptr<crypto::ECPrivateKey> private_key);
+
+  RegistrationData* FindRegistrationData(
+      base::span<const uint8_t> key_handle,
+      base::span<const uint8_t, kRpIdHashLength> application_parameter);
+
   // FidoDevice:
   void TryWink(WinkCallback cb) override;
   std::string GetId() const override;

device/fido/virtual_u2f_device.cc

@@ -176,14 +176,7 @@ base::Optional<std::vector<uint8_t>> VirtualU2fDevice::DoRegister(
   Append(&response, *attestation_cert);
   Append(&response, sig);
 
-  // Store the registration. Because the key handle is the hashed public key we
-  // just generated, no way this should already be registered.
-  bool did_insert = false;
-  std::tie(std::ignore, did_insert) = mutable_state()->registrations.emplace(
-      std::move(key_handle),
-      RegistrationData(std::move(private_key), application_parameter, 1));
-  DCHECK(did_insert);
-
+  StoreNewKey(application_parameter, key_handle, std::move(private_key));
   return apdu::ApduResponse(std::move(response),
                             apdu::ApduResponse::Status::SW_NO_ERROR)
       .GetEncodedResponse();
@@ -204,42 +197,29 @@ base::Optional<std::vector<uint8_t>> VirtualU2fDevice::DoSign(
     mutable_state()->simulate_press_callback.Run();
   }
 
-  if (data.size() < 32 + 32 + 1) {
+  if (data.size() < 32 + 32 + 1)
     return ErrorStatus(apdu::ApduResponse::Status::SW_WRONG_LENGTH);
-  }
 
   auto challenge_param = data.first<32>();
   auto application_parameter = data.subspan<32, 32>();
   size_t key_handle_length = data[64];
-  if (data.size() != 32 + 32 + 1 + key_handle_length) {
+  if (data.size() != 32 + 32 + 1 + key_handle_length)
     return ErrorStatus(apdu::ApduResponse::Status::SW_WRONG_LENGTH);
-  }
-  auto key_handle = data.last(key_handle_length);
 
-  // Check if this is our key_handle and it's for this appId.
-  auto it = mutable_state()->registrations.find(key_handle);
-  if (it == mutable_state()->registrations.end()) {
-    return ErrorStatus(apdu::ApduResponse::Status::SW_WRONG_DATA);
-  }
-
-  base::span<const uint8_t> registered_app_id_hash =
-      base::make_span(it->second.application_parameter);
-  if (application_parameter != registered_app_id_hash) {
-    // It's important this error looks identical to the previous one, as
-    // tokens should not reveal the existence of keyHandles to unrelated appIds.
+  auto key_handle = data.last(key_handle_length);
+  auto* registration = FindRegistrationData(key_handle, application_parameter);
+  if (!registration)
     return ErrorStatus(apdu::ApduResponse::Status::SW_WRONG_DATA);
-  }
 
-  auto& registration = it->second;
-  ++registration.counter;
+  ++registration->counter;
 
   // First create the part of the response that gets signed over.
   std::vector<uint8_t> response;
   response.push_back(0x01);  // Always pretend we got a touch.
-  response.push_back(registration.counter >> 24);
-  response.push_back(registration.counter >> 16);
-  response.push_back(registration.counter >> 8);
-  response.push_back(registration.counter);
+  response.push_back(registration->counter >> 24);
+  response.push_back(registration->counter >> 16);
+  response.push_back(registration->counter >> 8);
+  response.push_back(registration->counter);
 
   std::vector<uint8_t> sign_buffer;
   sign_buffer.reserve(application_parameter.size() + response.size() +
@@ -250,7 +230,7 @@ base::Optional<std::vector<uint8_t>> VirtualU2fDevice::DoSign(
 
   // Sign with credential key.
   std::vector<uint8_t> sig;
-  bool status = Sign(registration.private_key.get(), sign_buffer, &sig);
+  bool status = Sign(registration->private_key.get(), sign_buffer, &sig);
   DCHECK(status);
 
   // Add signature for full response.