[webauthn] Send wink command to u2f devices.

This patch adds code to explicitly send a wink command to U2F devices
whenever an operation requires a test of user presence. This needs to be
sent for older devices that will not blink on their own.

Bug: 990456
Change-Id: Ia44d0e139f4946742514eb6bf3484d13aa501b5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1746630
Commit-Queue: Nina Satragno <nsatragno@chromium.org>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/heads/master@{#686600}

device/fido/fido_device.cc

@@ -18,6 +18,10 @@ namespace device {
 FidoDevice::FidoDevice() = default;
 FidoDevice::~FidoDevice() = default;
 
+void FidoDevice::TryWink(base::OnceClosure callback) {
+  std::move(callback).Run();
+}
+
 base::string16 FidoDevice::GetDisplayName() const {
   const auto id = GetId();
   return base::string16(id.begin(), id.end());

device/fido/fido_device.h

@@ -66,6 +66,11 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoDevice {
   // call (i.e. hairpin) |callback|.
   virtual CancelToken DeviceTransact(std::vector<uint8_t> command,
                                      DeviceCallback callback) = 0;
+  // Attempt to make the device "wink", i.e. grab the attention of the user
+  // usually by flashing a light. |callback| is run after a successful wink or
+  // if the device does not support winking, in which case it may run
+  // immediately.
+  virtual void TryWink(base::OnceClosure callback);
   // Cancel attempts to cancel an enqueued request. If the request is currently
   // active it will be aborted if possible, which is expected to cause it to
   // complete with |kCtap2ErrKeepAliveCancel|. If the request is still enqueued

device/fido/get_assertion_task_unittest.cc

@@ -74,6 +74,7 @@ TEST_F(FidoGetAssertionTaskTest, TestGetAssertionSuccess) {
 
 TEST_F(FidoGetAssertionTaskTest, TestU2fSignSuccess) {
   auto device = MockFidoDevice::MakeU2f();
+  device->ExpectWinkedAtLeastOnce();
   device->ExpectRequestAndRespondWith(
       test_data::kU2fSignCommandApdu,
       test_data::kApduEncodedNoErrorSignResponse);
@@ -166,6 +167,7 @@ TEST_F(FidoGetAssertionTaskTest, TestU2fSignRequestWithEmptyAllowedList) {
                                          test_data::kClientDataJson);
 
   auto device = MockFidoDevice::MakeU2f();
+  device->ExpectWinkedAtLeastOnce();
   device->ExpectRequestAndRespondWith(
       test_data::kU2fFakeRegisterCommand,
       test_data::kApduEncodedNoErrorSignResponse);
@@ -236,11 +238,13 @@ TEST_F(FidoGetAssertionTaskTest, TestU2fFallbackForAppIdExtension) {
                                       error);
   // After falling back to U2F the request will use the alternative app_param,
   // which will be rejected.
+  device->ExpectWinkedAtLeastOnce();
   device->ExpectRequestAndRespondWith(
       test_data::kU2fSignCommandApduWithAlternativeApplicationParameter,
       test_data::kU2fWrongDataApduResponse);
   // After the rejection, the U2F sign request with the primary application
   // parameter should be tried.
+  device->ExpectWinkedAtLeastOnce();
   device->ExpectRequestAndRespondWith(
       test_data::kU2fSignCommandApdu,
       test_data::kApduEncodedNoErrorSignResponse);

device/fido/hid/fido_hid_device.cc

@@ -22,6 +22,7 @@ namespace device {
 
 // U2F devices only provide a single report so specify a report ID of 0 here.
 static constexpr uint8_t kReportId = 0x00;
+static constexpr uint8_t kWinkCapability = 0x01;
 
 FidoHidDevice::FidoHidDevice(device::mojom::HidDeviceInfoPtr device_info,
                              device::mojom::HidManager* hid_manager)
@@ -42,8 +43,11 @@ FidoDevice::CancelToken FidoHidDevice::DeviceTransact(
     std::vector<uint8_t> command,
     DeviceCallback callback) {
   const CancelToken token = next_cancel_token_++;
-  pending_transactions_.emplace_back(std::move(command), std::move(callback),
-                                     token);
+  const auto command_type = supported_protocol() == ProtocolVersion::kCtap2
+                                ? FidoHidDeviceCommand::kCbor
+                                : FidoHidDeviceCommand::kMsg;
+  pending_transactions_.emplace_back(command_type, std::move(command),
+                                     std::move(callback), token);
   Transition();
   return token;
 }
@@ -110,18 +114,28 @@ void FidoHidDevice::Transition(base::Optional<State> next_state) {
                              weak_factory_.GetWeakPtr()));
       break;
     case State::kReady: {
+      DCHECK(!pending_transactions_.empty());
+
+      // Only try to wink if device claims support.
+      if (pending_transactions_.front().command_type ==
+              FidoHidDeviceCommand::kWink &&
+          !(capabilities_ & kWinkCapability)) {
+        DeviceCallback pending_cb =
+            std::move(pending_transactions_.front().callback);
+        pending_transactions_.pop_front();
+        std::move(pending_cb).Run(base::nullopt);
+        break;
+      }
+
       state_ = State::kBusy;
       busy_state_ = BusyState::kWriting;
-      DCHECK(!pending_transactions_.empty());
       ArmTimeout();
 
       // Write message to the device.
       current_token_ = pending_transactions_.front().token;
-      const auto command_type = supported_protocol() == ProtocolVersion::kCtap2
-                                    ? FidoHidDeviceCommand::kCbor
-                                    : FidoHidDeviceCommand::kMsg;
       auto maybe_message(FidoHidMessage::Create(
-          channel_id_, command_type, output_report_size_,
+          channel_id_, pending_transactions_.front().command_type,
+          output_report_size_,
           std::move(pending_transactions_.front().command)));
       DCHECK(maybe_message);
       WriteMessage(std::move(*maybe_message));
@@ -146,10 +160,12 @@ void FidoHidDevice::Transition(base::Optional<State> next_state) {
 }
 
 FidoHidDevice::PendingTransaction::PendingTransaction(
+    FidoHidDeviceCommand command_type,
     std::vector<uint8_t> in_command,
     DeviceCallback in_callback,
     CancelToken in_token)
-    : command(std::move(in_command)),
+    : command_type(command_type),
+      command(std::move(in_command)),
       callback(std::move(in_callback)),
       token(in_token) {}
 
@@ -205,7 +221,7 @@ void FidoHidDevice::OnInitWriteComplete(std::vector<uint8_t> nonce,
 
 // ParseInitReply parses a potential reply to a U2FHID_INIT message. If the
 // reply matches the given nonce then the assigned channel ID is returned.
-static base::Optional<uint32_t> ParseInitReply(
+base::Optional<uint32_t> FidoHidDevice::ParseInitReply(
     const std::vector<uint8_t>& nonce,
     const std::vector<uint8_t>& buf) {
   auto message = FidoHidMessage::CreateFromSerializedData(buf);
@@ -232,6 +248,8 @@ static base::Optional<uint32_t> ParseInitReply(
     return base::nullopt;
   }
 
+  capabilities_ = payload[16];
+
   return static_cast<uint32_t>(payload[8]) << 24 |
          static_cast<uint32_t>(payload[9]) << 16 |
          static_cast<uint32_t>(payload[10]) << 8 |
@@ -408,7 +426,8 @@ void FidoHidDevice::MessageReceived(FidoHidMessage message) {
 
   const auto cmd = message.cmd();
   auto response = message.GetMessagePayload();
-  if (cmd != FidoHidDeviceCommand::kMsg && cmd != FidoHidDeviceCommand::kCbor) {
+  if (cmd != FidoHidDeviceCommand::kMsg && cmd != FidoHidDeviceCommand::kCbor &&
+      cmd != FidoHidDeviceCommand::kWink) {
     if (cmd != FidoHidDeviceCommand::kError || response.size() != 1) {
       FIDO_LOG(ERROR) << "Unknown HID message received: "
                       << static_cast<int>(cmd) << " "
@@ -458,6 +477,19 @@ void FidoHidDevice::MessageReceived(FidoHidMessage message) {
   }
 }
 
+void FidoHidDevice::TryWink(base::OnceClosure callback) {
+  const CancelToken token = next_cancel_token_++;
+  pending_transactions_.emplace_back(
+      FidoHidDeviceCommand::kWink, std::vector<uint8_t>(),
+      base::BindOnce(
+          [](base::OnceClosure cb, base::Optional<std::vector<uint8_t>> data) {
+            std::move(cb).Run();
+          },
+          std::move(callback)),
+      token);
+  Transition();
+}
+
 void FidoHidDevice::ArmTimeout() {
   DCHECK(timeout_callback_.IsCancelled());
   timeout_callback_.Reset(

device/fido/hid/fido_hid_device.h

@@ -38,6 +38,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoHidDevice : public FidoDevice {
   // FidoDevice:
   CancelToken DeviceTransact(std::vector<uint8_t> command,
                              DeviceCallback callback) final;
+  void TryWink(base::OnceClosure callback) final;
   void Cancel(CancelToken token) final;
   std::string GetId() const final;
   FidoTransportProtocol DeviceTransport() const final;
@@ -75,11 +76,13 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoHidDevice : public FidoDevice {
   };
 
   struct COMPONENT_EXPORT(DEVICE_FIDO) PendingTransaction {
-    PendingTransaction(std::vector<uint8_t> command,
+    PendingTransaction(FidoHidDeviceCommand command_type,
+                       std::vector<uint8_t> command,
                        DeviceCallback callback,
                        CancelToken token);
     ~PendingTransaction();
 
+    FidoHidDeviceCommand command_type;
     std::vector<uint8_t> command;
     DeviceCallback callback;
     CancelToken token;
@@ -94,6 +97,8 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoHidDevice : public FidoDevice {
   // Ask device to allocate a unique channel id for this connection.
   void OnAllocateChannel(std::vector<uint8_t> nonce,
                          base::Optional<FidoHidMessage> message);
+  base::Optional<uint32_t> ParseInitReply(const std::vector<uint8_t>& nonce,
+                                          const std::vector<uint8_t>& buf);
   void OnPotentialInitReply(std::vector<uint8_t> nonce,
                             bool success,
                             uint8_t report_id,
@@ -117,6 +122,8 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoHidDevice : public FidoDevice {
 
   base::WeakPtr<FidoDevice> GetWeakPtr() override;
 
+  uint8_t capabilities_ = 0;
+
   // |output_report_size_| is the size of the packets that will be sent to the
   // device. (For HID devices, these are called reports.)
   const uint8_t output_report_size_;

device/fido/hid/fido_hid_device_unittest.cc

@@ -39,6 +39,9 @@ constexpr uint8_t kU2fMockResponseMessage[] = {
     0x5f, 0x44, 0x41, 0x54, 0x41, 0x90, 0x00,
 };
 
+// HID_WINK(0x08), followed by payload length(0).
+constexpr uint8_t kU2fWinkResponseMessage[] = {0x08, 0x00};
+
 // APDU encoded success response with data "MOCK_DATA" followed by a SW_NO_ERROR
 // APDU response code(9000).
 constexpr uint8_t kU2fMockResponseData[] = {0x4d, 0x4f, 0x43, 0x4b, 0x5f, 0x44,
@@ -70,10 +73,12 @@ constexpr uint8_t kMockCancelResponse[] = {
 // Returns HID_INIT request to send to device with mock connection.
 std::vector<uint8_t> CreateMockInitResponse(
     base::span<const uint8_t> nonce,
-    base::span<const uint8_t> channel_id) {
+    base::span<const uint8_t> channel_id,
+    base::span<const uint8_t> payload = base::span<const uint8_t>()) {
   auto init_response = fido_parsing_utils::Materialize(kInitResponsePrefix);
   fido_parsing_utils::Append(&init_response, nonce);
   fido_parsing_utils::Append(&init_response, channel_id);
+  fido_parsing_utils::Append(&init_response, payload);
   init_response.resize(64);
   return init_response;
 }
@@ -222,10 +227,12 @@ TEST_F(FidoHidDeviceTest, TestDeviceError) {
 
   // Add pending transactions manually and ensure they are processed.
   TestDeviceCallbackReceiver receiver_1;
-  device->pending_transactions_.emplace_back(GetMockDeviceRequest(),
+  device->pending_transactions_.emplace_back(FidoHidDeviceCommand::kMsg,
+                                             GetMockDeviceRequest(),
                                              receiver_1.callback(), 0);
   TestDeviceCallbackReceiver receiver_2;
-  device->pending_transactions_.emplace_back(GetMockDeviceRequest(),
+  device->pending_transactions_.emplace_back(FidoHidDeviceCommand::kMsg,
+                                             GetMockDeviceRequest(),
                                              receiver_2.callback(), 0);
   TestDeviceCallbackReceiver receiver_3;
   device->DeviceTransact(GetMockDeviceRequest(), receiver_3.callback());
@@ -726,4 +733,104 @@ TEST_F(FidoHidDeviceTest, TestDeviceMessageError) {
   EXPECT_TRUE(get_info_callback.was_called());
 }
 
+// Test that the wink command does not get sent if the device does not support
+// it.
+TEST_F(FidoHidDeviceTest, TestWinkNotSupported) {
+  constexpr uint8_t kChannelId[] = {0x01, 0x02, 0x03, 0x04};
+  constexpr uint8_t kWinkNotSupportedPayload[] = {0x00, 0x00, 0x00, 0x00, 0x00};
+
+  auto hid_device = TestHidDevice();
+
+  // Replace device HID connection with custom client connection bound to mock
+  // server-side mojo connection.
+  device::mojom::HidConnectionPtr connection_client;
+  MockFidoHidConnection mock_connection(
+      hid_device.Clone(), mojo::MakeRequest(&connection_client),
+      fido_parsing_utils::Materialize(kChannelId));
+
+  // Initial write for establishing a channel ID.
+  mock_connection.ExpectWriteHidInit();
+
+  EXPECT_CALL(mock_connection, ReadPtr(_))
+      // Respond to HID_INIT indicating the device does not support winking.
+      .WillOnce(Invoke([&](device::mojom::HidConnection::ReadCallback* cb) {
+        std::move(*cb).Run(
+            true, 0,
+            CreateMockInitResponse(mock_connection.nonce(),
+                                   mock_connection.connection_channel_id(),
+                                   kWinkNotSupportedPayload));
+      }));
+
+  // Add device and set mock connection to fake hid manager.
+  fake_hid_manager_->AddDeviceAndSetConnection(std::move(hid_device),
+                                               std::move(connection_client));
+  FidoDeviceEnumerateCallbackReceiver receiver(hid_manager_.get());
+  hid_manager_->GetDevices(receiver.callback());
+  receiver.WaitForCallback();
+
+  std::vector<std::unique_ptr<FidoHidDevice>> u2f_devices =
+      receiver.TakeReturnedDevicesFiltered();
+  ASSERT_EQ(1u, u2f_devices.size());
+  auto& device = u2f_devices.front();
+
+  device::test::TestCallbackReceiver<> callback_receiver;
+  device->TryWink(callback_receiver.callback());
+  scoped_task_environment_.FastForwardUntilNoTasksRemain();
+  EXPECT_TRUE(callback_receiver.was_called());
+}
+
+// Test that the wink command is sent to a device that supports it.
+TEST_F(FidoHidDeviceTest, TestSuccessfulWink) {
+  constexpr uint8_t kChannelId[] = {0x01, 0x02, 0x03, 0x04};
+  constexpr uint8_t kWinkSupportedPayload[] = {0x00, 0x00, 0x00, 0x00, 0x01};
+
+  auto hid_device = TestHidDevice();
+
+  // Replace device HID connection with custom client connection bound to mock
+  // server-side mojo connection.
+  device::mojom::HidConnectionPtr connection_client;
+  MockFidoHidConnection mock_connection(
+      hid_device.Clone(), mojo::MakeRequest(&connection_client),
+      fido_parsing_utils::Materialize(kChannelId));
+
+  // Initial write for establishing a channel ID.
+  mock_connection.ExpectWriteHidInit();
+
+  mock_connection.ExpectHidWriteWithCommand(FidoHidDeviceCommand::kWink);
+
+  EXPECT_CALL(mock_connection, ReadPtr(_))
+      // Respond to HID_INIT indicating the device supports winking.
+      .WillOnce(Invoke([&](device::mojom::HidConnection::ReadCallback* cb) {
+        std::move(*cb).Run(
+            true, 0,
+            CreateMockInitResponse(mock_connection.nonce(),
+                                   mock_connection.connection_channel_id(),
+                                   kWinkSupportedPayload));
+      }))
+      // Response to HID_WINK.
+      .WillOnce(Invoke([&](device::mojom::HidConnection::ReadCallback* cb) {
+        std::move(*cb).Run(true, 0,
+                           CreateMockResponseWithChannelId(
+                               mock_connection.connection_channel_id(),
+                               kU2fWinkResponseMessage));
+      }));
+
+  // Add device and set mock connection to fake hid manager.
+  fake_hid_manager_->AddDeviceAndSetConnection(std::move(hid_device),
+                                               std::move(connection_client));
+  FidoDeviceEnumerateCallbackReceiver receiver(hid_manager_.get());
+  hid_manager_->GetDevices(receiver.callback());
+  receiver.WaitForCallback();
+
+  std::vector<std::unique_ptr<FidoHidDevice>> u2f_devices =
+      receiver.TakeReturnedDevicesFiltered();
+  ASSERT_EQ(1u, u2f_devices.size());
+  auto& device = u2f_devices.front();
+
+  device::test::TestCallbackReceiver<> callback_receiver;
+  device->TryWink(callback_receiver.callback());
+  scoped_task_environment_.FastForwardUntilNoTasksRemain();
+  EXPECT_TRUE(callback_receiver.was_called());
+}
+
 }  // namespace device

device/fido/make_credential_task_unittest.cc

@@ -102,6 +102,7 @@ TEST_F(FidoMakeCredentialTaskTest, TestRegisterSuccessWithFake) {
 
 TEST_F(FidoMakeCredentialTaskTest, FallbackToU2fRegisterSuccess) {
   auto device = MockFidoDevice::MakeU2f();
+  device->ExpectWinkedAtLeastOnce();
   device->ExpectRequestAndRespondWith(
       test_data::kU2fRegisterCommandApdu,
       test_data::kApduEncodedNoErrorRegisterResponse);
@@ -123,6 +124,7 @@ TEST_F(FidoMakeCredentialTaskTest, DefaultToU2fWhenClientPinSet) {
   device_info.options = std::move(options);
 
   auto device = MockFidoDevice::MakeCtap(std::move(device_info));
+  device->ExpectWinkedAtLeastOnce();
   device->ExpectRequestAndRespondWith(
       test_data::kU2fRegisterCommandApdu,
       test_data::kApduEncodedNoErrorRegisterResponse);
@@ -168,6 +170,7 @@ TEST_F(FidoMakeCredentialTaskTest, TestU2fOnly) {
   // request, because the task is instantiated in U2F-only mode.
   auto device = MockFidoDevice::MakeCtap();
 
+  device->ExpectWinkedAtLeastOnce();
   device->ExpectRequestAndRespondWith(
       test_data::kU2fRegisterCommandApdu,
       test_data::kApduEncodedNoErrorRegisterResponse);

device/fido/mock_fido_device.cc

@@ -48,6 +48,7 @@ MockFidoDevice::MakeU2fWithGetInfoExpectation() {
   auto device = std::make_unique<MockFidoDevice>();
   device->StubGetId();
   device->StubGetDisplayName();
+  device->ExpectWinkedAtLeastOnce();
   device->ExpectCtap2CommandAndRespondWith(
       CtapRequestCommand::kAuthenticatorGetInfo, base::nullopt);
   return device;
@@ -108,6 +109,10 @@ FidoDevice::CancelToken MockFidoDevice::DeviceTransact(
   return DeviceTransactPtr(command, cb);
 }
 
+void MockFidoDevice::TryWink(base::OnceClosure cb) {
+  TryWinkRef(cb);
+}
+
 FidoTransportProtocol MockFidoDevice::DeviceTransport() const {
   return transport_protocol_;
 }
@@ -116,6 +121,12 @@ base::WeakPtr<FidoDevice> MockFidoDevice::GetWeakPtr() {
   return weak_factory_.GetWeakPtr();
 }
 
+void MockFidoDevice::ExpectWinkedAtLeastOnce() {
+  EXPECT_CALL(*this, TryWinkRef(::testing::_))
+      .Times(::testing::AtLeast(1))
+      .WillRepeatedly([](base::OnceClosure& cb) { std::move(cb).Run(); });
+}
+
 void MockFidoDevice::StubGetId() {
   // Use a counter to keep the device ID unique.
   static size_t i = 0;

device/fido/mock_fido_device.h

@@ -60,6 +60,11 @@ class MockFidoDevice : public ::testing::StrictMock<FidoDevice> {
                  base::Optional<AuthenticatorGetInfoResponse> device_info);
   ~MockFidoDevice() override;
 
+  // TODO(crbug.com/729950): Remove these workarounds once support for move-only
+  // types is added to GMock.
+  MOCK_METHOD1(TryWinkRef, void(base::OnceClosure& cb));
+  void TryWink(base::OnceClosure cb) override;
+
   // GMock cannot mock a method taking a move-only type.
   // TODO(crbug.com/729950): Remove these workarounds once support for move-only
   // types is added to GMock.

device/fido/u2f_register_operation.cc

@@ -37,9 +37,9 @@ void U2fRegisterOperation::Start() {
   if (exclude_list && !exclude_list->empty()) {
     // First try signing with the excluded credentials to see whether this
     // device should be excluded.
-    TrySign();
+    WinkAndTrySign();
   } else {
-    TryRegistration();
+    WinkAndTryRegistration();
   }
 }
 
@@ -47,6 +47,11 @@ void U2fRegisterOperation::Cancel() {
   canceled_ = true;
 }
 
+void U2fRegisterOperation::WinkAndTrySign() {
+  device()->TryWink(base::BindOnce(&U2fRegisterOperation::TrySign,
+                                   weak_factory_.GetWeakPtr()));
+}
+
 void U2fRegisterOperation::TrySign() {
   base::Optional<std::vector<uint8_t>> sign_command;
   if (probing_alternative_rp_id_) {
@@ -98,7 +103,7 @@ void U2fRegisterOperation::OnCheckForExcludedKeyHandle(
       // Duplicate registration found. Waiting for user touch.
       base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
           FROM_HERE,
-          base::BindOnce(&U2fRegisterOperation::TrySign,
+          base::BindOnce(&U2fRegisterOperation::WinkAndTrySign,
                          weak_factory_.GetWeakPtr()),
           kU2fRetryDelay);
       break;
@@ -116,11 +121,11 @@ void U2fRegisterOperation::OnCheckForExcludedKeyHandle(
         current_key_handle_index_ = 0;
       }
       if (current_key_handle_index_ < request().exclude_list->size()) {
-        TrySign();
+        WinkAndTrySign();
       } else {
         // Reached the end of exclude list with no duplicate credential.
         // Proceed with registration.
-        TryRegistration();
+        WinkAndTryRegistration();
       }
       break;
 
@@ -134,6 +139,11 @@ void U2fRegisterOperation::OnCheckForExcludedKeyHandle(
   }
 }
 
+void U2fRegisterOperation::WinkAndTryRegistration() {
+  device()->TryWink(base::BindOnce(&U2fRegisterOperation::TryRegistration,
+                                   weak_factory_.GetWeakPtr()));
+}
+
 void U2fRegisterOperation::TryRegistration() {
   DispatchDeviceRequest(
       ConvertToU2fRegisterCommand(request()),
@@ -176,7 +186,7 @@ void U2fRegisterOperation::OnRegisterResponseReceived(
       // Waiting for user touch, retry after delay.
       base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
           FROM_HERE,
-          base::BindOnce(&U2fRegisterOperation::TryRegistration,
+          base::BindOnce(&U2fRegisterOperation::WinkAndTryRegistration,
                          weak_factory_.GetWeakPtr()),
           kU2fRetryDelay);
       break;

device/fido/u2f_register_operation.h

@@ -44,9 +44,11 @@ class COMPONENT_EXPORT(DEVICE_FIDO) U2fRegisterOperation
   using ExcludeListIterator =
       std::vector<PublicKeyCredentialDescriptor>::const_iterator;
 
+  void WinkAndTrySign();
   void TrySign();
   void OnCheckForExcludedKeyHandle(
       base::Optional<std::vector<uint8_t>> device_response);
+  void WinkAndTryRegistration();
   void TryRegistration();
   void OnRegisterResponseReceived(
       base::Optional<std::vector<uint8_t>> device_response);

device/fido/u2f_register_operation_unittest.cc

@@ -74,6 +74,7 @@ TEST_F(U2fRegisterOperationTest, TestRegisterSuccess) {
   auto request = CreateRegisterRequest();
   auto device = std::make_unique<MockFidoDevice>();
   EXPECT_CALL(*device, GetId()).WillRepeatedly(testing::Return("device"));
+  device->ExpectWinkedAtLeastOnce();
   device->ExpectRequestAndRespondWith(
       test_data::kU2fRegisterCommandApdu,
       test_data::kApduEncodedNoErrorRegisterResponse);
@@ -114,6 +115,7 @@ TEST_F(U2fRegisterOperationTest, TestDelayedSuccess) {
 
   auto device = std::make_unique<MockFidoDevice>();
   EXPECT_CALL(*device, GetId()).WillRepeatedly(testing::Return("device"));
+  device->ExpectWinkedAtLeastOnce();
 
   // Device error out once waiting for user presence before retrying.
   ::testing::InSequence s;
@@ -152,6 +154,7 @@ TEST_F(U2fRegisterOperationTest, TestRegistrationWithExclusionList) {
 
   auto device = std::make_unique<MockFidoDevice>();
   EXPECT_CALL(*device, GetId()).WillRepeatedly(::testing::Return("device"));
+  device->ExpectWinkedAtLeastOnce();
   // DeviceTransact() will be called three times including two check only sign-
   // in calls and one registration call. For the first two calls, device will
   // invoke MockFidoDevice::WrongData/WrongLength as the authenticator did not
@@ -200,11 +203,11 @@ TEST_F(U2fRegisterOperationTest, TestRegistrationWithDuplicateHandle) {
 
   auto device = std::make_unique<MockFidoDevice>();
   EXPECT_CALL(*device, GetId()).WillRepeatedly(::testing::Return("device"));
-  // For three keys in exclude list, the first two keys will invoke
-  // MockFidoDevice::WrongData and the final duplicate key handle will invoke
-  // MockFidoDevice::NoErrorSign. Once duplicate key handle is found, bogus
-  // registration is called to confirm user presence. This invokes
-  // MockFidoDevice::NoErrorRegister.
+  device->ExpectWinkedAtLeastOnce();
+  // For three keys in exclude list, the first two keys will return
+  // SW_WRONG_DATA and the final duplicate key handle will invoke
+  // SW_NO_ERROR. This means user presence has already been collected, so the
+  // request is concluded with Ctap2ErrCredentialExcluded.
   ::testing::InSequence s;
   device->ExpectRequestAndRespondWith(
       test_data::kU2fSignCommandApduWithKeyAlpha,
@@ -240,6 +243,7 @@ TEST_F(U2fRegisterOperationTest, TestIndividualAttestation) {
 
     auto device = std::make_unique<MockFidoDevice>();
     EXPECT_CALL(*device, GetId()).WillRepeatedly(::testing::Return("device"));
+    device->ExpectWinkedAtLeastOnce();
 
     device->ExpectRequestAndRespondWith(
         individual_attestation

device/fido/u2f_sign_operation.cc

@@ -34,12 +34,12 @@ void U2fSignOperation::Start() {
       // authenticator (at least) crashes if we try the wrong AppID first.
       app_param_type_ = ApplicationParameterType::kAlternative;
     }
-    TrySign();
+    WinkAndTrySign();
   } else {
     // In order to make U2F authenticators blink on sign request with an empty
     // allow list, we send fake enrollment to the device and error out when the
     // user has provided presence.
-    TryFakeEnrollment();
+    WinkAndTryFakeEnrollment();
   }
 }
 
@@ -47,6 +47,11 @@ void U2fSignOperation::Cancel() {
   canceled_ = true;
 }
 
+void U2fSignOperation::WinkAndTrySign() {
+  device()->TryWink(
+      base::BindOnce(&U2fSignOperation::TrySign, weak_factory_.GetWeakPtr()));
+}
+
 void U2fSignOperation::TrySign() {
   DispatchDeviceRequest(
       ConvertToU2fSignCommand(request(), app_param_type_, key_handle()),
@@ -107,13 +112,13 @@ void U2fSignOperation::OnSignResponseReceived(
         // |application_parameter_| failed, but there is also
         // the primary value to try.
         app_param_type_ = ApplicationParameterType::kPrimary;
-        TrySign();
+        WinkAndTrySign();
       } else if (++current_key_handle_index_ < request().allow_list.size()) {
         // Key is not for this device. Try signing with the next key.
         if (request().alternative_application_parameter.has_value()) {
           app_param_type_ = ApplicationParameterType::kAlternative;
         }
-        TrySign();
+        WinkAndTrySign();
       } else {
         // No provided key was accepted by this device. Send registration
         // (i.e. fake enroll) request to device.
@@ -125,7 +130,7 @@ void U2fSignOperation::OnSignResponseReceived(
       // Waiting for user touch. Retry after 200 milliseconds delay.
       base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
           FROM_HERE,
-          base::BindOnce(&U2fSignOperation::TrySign,
+          base::BindOnce(&U2fSignOperation::WinkAndTrySign,
                          weak_factory_.GetWeakPtr()),
           kU2fRetryDelay);
       break;
@@ -138,6 +143,11 @@ void U2fSignOperation::OnSignResponseReceived(
   }
 }
 
+void U2fSignOperation::WinkAndTryFakeEnrollment() {
+  device()->TryWink(base::BindOnce(&U2fSignOperation::TryFakeEnrollment,
+                                   weak_factory_.GetWeakPtr()));
+}
+
 void U2fSignOperation::TryFakeEnrollment() {
   DispatchDeviceRequest(
       ConstructBogusU2fRegistrationCommand(),

device/fido/u2f_sign_operation.h

@@ -41,9 +41,11 @@ class COMPONENT_EXPORT(DEVICE_FIDO) U2fSignOperation
   void Cancel() override;
 
  private:
+  void WinkAndTrySign();
   void TrySign();
   void OnSignResponseReceived(
       base::Optional<std::vector<uint8_t>> device_response);
+  void WinkAndTryFakeEnrollment();
   void TryFakeEnrollment();
   void OnEnrollmentResponseReceived(
       base::Optional<std::vector<uint8_t>> device_response);

device/fido/u2f_sign_operation_unittest.cc

@@ -60,6 +60,7 @@ TEST_F(U2fSignOperationTest, SignSuccess) {
 
   auto device = std::make_unique<MockFidoDevice>();
   EXPECT_CALL(*device, GetId()).WillRepeatedly(testing::Return("device"));
+  device->ExpectWinkedAtLeastOnce();
   InSequence s;
   device->ExpectRequestAndRespondWith(
       test_data::kU2fSignCommandApdu,
@@ -128,6 +129,7 @@ TEST_F(U2fSignOperationTest, DelayedSuccess) {
   // responding successfully.
   auto device = std::make_unique<MockFidoDevice>();
   EXPECT_CALL(*device, GetId()).WillRepeatedly(::testing::Return("device"));
+  device->ExpectWinkedAtLeastOnce();
 
   InSequence s;
   device->ExpectRequestAndRespondWith(
@@ -160,6 +162,7 @@ TEST_F(U2fSignOperationTest, MultipleHandles) {
 
   auto device = std::make_unique<MockFidoDevice>();
   EXPECT_CALL(*device, GetId()).WillRepeatedly(::testing::Return("device"));
+  device->ExpectWinkedAtLeastOnce();
   InSequence s;
   // Wrong key would respond with SW_WRONG_DATA.
   device->ExpectRequestAndRespondWith(
@@ -193,6 +196,7 @@ TEST_F(U2fSignOperationTest, MultipleHandlesLengthError) {
 
   auto device = std::make_unique<MockFidoDevice>();
   EXPECT_CALL(*device, GetId()).WillRepeatedly(::testing::Return("device"));
+  device->ExpectWinkedAtLeastOnce();
   InSequence s;
 
   // Wrong key would respond with the key handle length.
@@ -224,6 +228,7 @@ TEST_F(U2fSignOperationTest, FakeEnroll) {
        fido_parsing_utils::Materialize(test_data::kKeyHandleBeta)});
 
   auto device = std::make_unique<MockFidoDevice>();
+  device->ExpectWinkedAtLeastOnce();
   InSequence s;
   device->ExpectRequestAndRespondWith(
       test_data::kU2fSignCommandApduWithKeyAlpha,
@@ -255,6 +260,7 @@ TEST_F(U2fSignOperationTest, DelayedFakeEnrollment) {
   // enrollment.
   auto device = std::make_unique<MockFidoDevice>();
   EXPECT_CALL(*device, GetId()).WillRepeatedly(::testing::Return("device0"));
+  device->ExpectWinkedAtLeastOnce();
   InSequence s;
   device->ExpectRequestAndRespondWith(test_data::kU2fSignCommandApdu,
                                       test_data::kU2fWrongDataApduResponse);
@@ -286,6 +292,7 @@ TEST_F(U2fSignOperationTest, FakeEnrollErroringOut) {
   // to prevent the test from crashing or timing out.
   auto device = std::make_unique<MockFidoDevice>();
   EXPECT_CALL(*device, GetId()).WillRepeatedly(::testing::Return("device0"));
+  device->ExpectWinkedAtLeastOnce();
   InSequence s;
   device->ExpectRequestAndRespondWith(test_data::kU2fSignCommandApdu,
                                       test_data::kU2fWrongDataApduResponse);
@@ -310,6 +317,7 @@ TEST_F(U2fSignOperationTest, SignWithCorruptedResponse) {
 
   auto device = std::make_unique<MockFidoDevice>();
   EXPECT_CALL(*device, GetId()).WillRepeatedly(::testing::Return("device"));
+  device->ExpectWinkedAtLeastOnce();
   InSequence s;
   device->ExpectRequestAndRespondWith(test_data::kU2fSignCommandApdu,
                                       test_data::kTestCorruptedU2fSignResponse);
@@ -334,6 +342,7 @@ TEST_F(U2fSignOperationTest, AlternativeApplicationParameter) {
 
   auto device = std::make_unique<MockFidoDevice>();
   EXPECT_CALL(*device, GetId()).WillRepeatedly(::testing::Return("device"));
+  device->ExpectWinkedAtLeastOnce();
   InSequence s;
   // The first request will use the alternative app_param.
   device->ExpectRequestAndRespondWith(
@@ -368,6 +377,7 @@ TEST_F(U2fSignOperationTest, AlternativeApplicationParameterRejection) {
 
   auto device = std::make_unique<MockFidoDevice>();
   EXPECT_CALL(*device, GetId()).WillRepeatedly(::testing::Return("device"));
+  device->ExpectWinkedAtLeastOnce();
   InSequence s;
   // The first request will use the alternative app_param, which will be
   // rejected.

device/fido/virtual_fido_device.cc

@@ -236,6 +236,10 @@ VirtualFidoDevice::RegistrationData* VirtualFidoDevice::FindRegistrationData(
   return &it->second;
 }
 
+void VirtualFidoDevice::TryWink(base::OnceClosure cb) {
+  std::move(cb).Run();
+}
+
 std::string VirtualFidoDevice::GetId() const {
   // Use our heap address to get a unique-ish number. (0xffe1 is a prime).
   return "VirtualFidoDevice-" + std::to_string((size_t)this % 0xffe1);

device/fido/virtual_fido_device.h

@@ -226,6 +226,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) VirtualFidoDevice : public FidoDevice {
       base::span<const uint8_t, kRpIdHashLength> application_parameter);
 
   // FidoDevice:
+  void TryWink(base::OnceClosure cb) override;
   std::string GetId() const override;
   FidoTransportProtocol DeviceTransport() const override;