[webauthn] Fallback to PIN for non UV authenticators

Some older authenticators support internal user verification but not UV
token or getting UV retries. Before this patch, if internal user
verification failed for one of these authenticators we would either drop
the request or show an error to the user. With this change, when an
authenticator supports falling back to PIN, chrome will show the PIN
dialog and alert the user.

Some authenticators will immediately fail when sent a uv = required
request if their internal uv is locked. To mitigate this behaviour, a
touch is requested from devices that return a response too quickly.

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

content/browser/webauth/authenticator_impl_unittest.cc

@@ -3863,6 +3863,35 @@ TEST_F(InternalUVAuthenticatorImplTest, MakeCredential) {
   }
 }
 
+// Test falling back to PIN for devices that support internal user verification
+// but not uv token.
+TEST_F(InternalUVAuthenticatorImplTest, MakeCredentialFallBackToPin) {
+  mojo::Remote<blink::mojom::Authenticator> authenticator =
+      ConnectToAuthenticator();
+
+  device::VirtualCtap2Device::Config config;
+  config.internal_uv_support = true;
+  config.pin_support = true;
+  config.user_verification_succeeds = false;
+  virtual_device_factory_->mutable_state()->pin = kTestPIN;
+  virtual_device_factory_->mutable_state()->pin_retries =
+      device::kMaxPinRetries;
+  virtual_device_factory_->mutable_state()->fingerprints_enrolled = true;
+  virtual_device_factory_->SetCtap2Config(config);
+
+  auto options =
+      make_credential_options(device::UserVerificationRequirement::kRequired);
+
+  TestMakeCredentialCallback callback_receiver;
+  authenticator->MakeCredential(std::move(options),
+                                callback_receiver.callback());
+  callback_receiver.WaitForCallback();
+
+  EXPECT_EQ(AuthenticatorStatus::SUCCESS, callback_receiver.status());
+  EXPECT_TRUE(HasUV(callback_receiver));
+  EXPECT_TRUE(test_client_.collected_pin());
+}
+
 TEST_F(InternalUVAuthenticatorImplTest, MakeCredentialCryptotoken) {
   auto task_runner = base::MakeRefCounted<base::TestMockTimeTaskRunner>(
       base::Time::Now(), base::TimeTicks::Now());
@@ -3928,6 +3957,38 @@ TEST_F(InternalUVAuthenticatorImplTest, GetAssertion) {
   }
 }
 
+// Test falling back to PIN for devices that support internal user verification
+// but not uv token.
+TEST_F(InternalUVAuthenticatorImplTest, GetAssertionFallbackToPIN) {
+  mojo::Remote<blink::mojom::Authenticator> authenticator =
+      ConnectToAuthenticator();
+
+  device::VirtualCtap2Device::Config config;
+  config.internal_uv_support = true;
+  config.pin_support = true;
+  config.user_verification_succeeds = false;
+  virtual_device_factory_->mutable_state()->pin = kTestPIN;
+  virtual_device_factory_->mutable_state()->pin_retries =
+      device::kMaxPinRetries;
+  virtual_device_factory_->mutable_state()->fingerprints_enrolled = true;
+  virtual_device_factory_->SetCtap2Config(config);
+
+  ASSERT_TRUE(virtual_device_factory_->mutable_state()->InjectRegistration(
+      get_credential_options()->allow_credentials[0].id(),
+      kTestRelyingPartyId));
+
+  auto options =
+      get_credential_options(device::UserVerificationRequirement::kRequired);
+
+  TestGetAssertionCallback callback_receiver;
+  authenticator->GetAssertion(std::move(options), callback_receiver.callback());
+  callback_receiver.WaitForCallback();
+
+  EXPECT_EQ(AuthenticatorStatus::SUCCESS, callback_receiver.status());
+  EXPECT_TRUE(HasUV(callback_receiver));
+  EXPECT_TRUE(test_client_.collected_pin());
+}
+
 TEST_F(InternalUVAuthenticatorImplTest, GetAssertionCryptotoken) {
   mojo::Remote<blink::mojom::Authenticator> authenticator =
       ConnectToAuthenticator();

device/fido/fido_request_handler_base.cc

@@ -12,7 +12,10 @@
 #include "base/logging.h"
 #include "base/strings/string_piece.h"
 #include "base/threading/sequenced_task_runner_handle.h"
+#include "base/time/time.h"
+#include "base/timer/elapsed_timer.h"
 #include "build/build_config.h"
+#include "components/device_event_log/device_event_log.h"
 #include "device/fido/ble_adapter_manager.h"
 #include "device/fido/fido_authenticator.h"
 #include "device/fido/fido_discovery_factory.h"
@@ -21,8 +24,23 @@
 #include "device/fido/win/authenticator.h"
 #endif
 
+namespace {
+// Authenticators that return a response in less than this time are likely to
+// have done so without interaction from the user.
+static const base::TimeDelta kMinExpectedAuthenticatorResponseTime =
+    base::TimeDelta::FromMilliseconds(300);
+}  // namespace
+
 namespace device {
 
+// FidoRequestHandlerBase::AuthenticatorState ---------------------------------
+
+FidoRequestHandlerBase::AuthenticatorState::AuthenticatorState(
+    FidoAuthenticator* authenticator)
+    : authenticator(authenticator) {}
+
+FidoRequestHandlerBase::AuthenticatorState::~AuthenticatorState() = default;
+
 // FidoRequestHandlerBase::TransportAvailabilityInfo --------------------------
 
 FidoRequestHandlerBase::TransportAvailabilityInfo::TransportAvailabilityInfo() =
@@ -154,7 +172,7 @@ void FidoRequestHandlerBase::StartAuthenticatorRequest(
   if (authenticator_it == active_authenticators_.end())
     return;
 
-  InitializeAuthenticatorAndDispatchRequest(authenticator_it->second);
+  InitializeAuthenticatorAndDispatchRequest(authenticator_it->second.get());
 }
 
 void FidoRequestHandlerBase::CancelActiveAuthenticators(
@@ -164,7 +182,7 @@ void FidoRequestHandlerBase::CancelActiveAuthenticators(
     DCHECK(!task_it->first.empty());
     if (task_it->first != exclude_device_id) {
       DCHECK(task_it->second);
-      task_it->second->Cancel();
+      task_it->second->authenticator->Cancel();
 
       // Note that the pointer being erased is non-owning. The actual
       // FidoAuthenticator instance is owned by its discovery (which in turn is
@@ -229,6 +247,20 @@ void FidoRequestHandlerBase::Start() {
     discovery->Start();
 }
 
+bool FidoRequestHandlerBase::AuthenticatorMayHaveReturnedImmediately(
+    const std::string& authenticator_id) {
+  auto it = active_authenticators_.find(authenticator_id);
+  if (it == active_authenticators_.end())
+    return false;
+
+  if (!it->second->timer)
+    return true;
+
+  FIDO_LOG(DEBUG) << "Authenticator returned in "
+                  << it->second->timer->Elapsed();
+  return it->second->timer->Elapsed() < kMinExpectedAuthenticatorResponseTime;
+}
+
 void FidoRequestHandlerBase::AuthenticatorRemoved(
     FidoDiscoveryBase* discovery,
     FidoAuthenticator* authenticator) {
@@ -271,7 +303,8 @@ void FidoRequestHandlerBase::AuthenticatorPairingModeChanged(
 
   if (observer_) {
     observer_->FidoAuthenticatorPairingModeChanged(
-        device_id, is_in_pairing_mode, it->second->GetDisplayName());
+        device_id, is_in_pairing_mode,
+        it->second->authenticator->GetDisplayName());
   }
 }
 
@@ -296,7 +329,11 @@ void FidoRequestHandlerBase::AuthenticatorAdded(
     FidoAuthenticator* authenticator) {
   DCHECK(authenticator &&
          !base::Contains(active_authenticators(), authenticator->GetId()));
-  active_authenticators_.emplace(authenticator->GetId(), authenticator);
+  auto authenticator_state =
+      std::make_unique<AuthenticatorState>(authenticator);
+  auto* weak_authenticator_state = authenticator_state.get();
+  active_authenticators_.emplace(authenticator->GetId(),
+                                 std::move(authenticator_state));
 
   // If |observer_| exists, dispatching request to |authenticator| is
   // delegated to |observer_|. Else, dispatch request to |authenticator|
@@ -318,7 +355,7 @@ void FidoRequestHandlerBase::AuthenticatorAdded(
         FROM_HERE,
         base::BindOnce(
             &FidoRequestHandlerBase::InitializeAuthenticatorAndDispatchRequest,
-            GetWeakPtr(), authenticator));
+            GetWeakPtr(), weak_authenticator_state));
   } else {
     VLOG(2) << "Embedder controls the dispatch.";
   }
@@ -347,10 +384,11 @@ void FidoRequestHandlerBase::NotifyObserverTransportAvailability() {
 }
 
 void FidoRequestHandlerBase::InitializeAuthenticatorAndDispatchRequest(
-    FidoAuthenticator* authenticator) {
-  authenticator->InitializeAuthenticator(
-      base::BindOnce(&FidoRequestHandlerBase::DispatchRequest,
-                     weak_factory_.GetWeakPtr(), authenticator));
+    AuthenticatorState* authenticator_state) {
+  authenticator_state->timer = std::make_unique<base::ElapsedTimer>();
+  authenticator_state->authenticator->InitializeAuthenticator(base::BindOnce(
+      &FidoRequestHandlerBase::DispatchRequest, weak_factory_.GetWeakPtr(),
+      authenticator_state->authenticator));
 }
 
 void FidoRequestHandlerBase::ConstructBleAdapterPowerManager() {

device/fido/fido_request_handler_base.h

@@ -25,6 +25,10 @@
 #include "device/fido/fido_discovery_base.h"
 #include "device/fido/fido_transport_protocol.h"
 
+namespace base {
+class ElapsedTimer;
+}
+
 namespace device {
 
 class BleAdapterManager;
@@ -39,8 +43,6 @@ class FidoDiscoveryFactory;
 class COMPONENT_EXPORT(DEVICE_FIDO) FidoRequestHandlerBase
     : public FidoDiscoveryBase::Observer {
  public:
-  using AuthenticatorMap =
-      std::map<std::string, FidoAuthenticator*, std::less<>>;
   using RequestCallback = base::RepeatingCallback<void(const std::string&)>;
   using BlePairingCallback =
       base::RepeatingCallback<void(std::string authenticator_id,
@@ -50,6 +52,17 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoRequestHandlerBase
 
   enum class RequestType { kMakeCredential, kGetAssertion };
 
+  struct AuthenticatorState {
+    explicit AuthenticatorState(FidoAuthenticator* authenticator);
+    ~AuthenticatorState();
+
+    FidoAuthenticator* authenticator;
+    std::unique_ptr<base::ElapsedTimer> timer;
+  };
+
+  using AuthenticatorMap =
+      std::map<std::string, std::unique_ptr<AuthenticatorState>, std::less<>>;
+
   // Encapsulates data required to initiate WebAuthN UX dialog. Once all
   // components of TransportAvailabilityInfo is set,
   // AuthenticatorRequestClientDelegate should be notified.
@@ -233,6 +246,13 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoRequestHandlerBase
 
   void Start();
 
+  // Returns |true| if a short enough time has elapsed since the request was
+  // dispatched that an authenticator may be suspected to have returned a
+  // response without user interaction.
+  // Must be called after |DispatchRequest| is called.
+  bool AuthenticatorMayHaveReturnedImmediately(
+      const std::string& authenticator_id);
+
   AuthenticatorMap& active_authenticators() { return active_authenticators_; }
   std::vector<std::unique_ptr<FidoDiscoveryBase>>& discoveries() {
     return discoveries_;
@@ -273,7 +293,8 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoRequestHandlerBase
   // DispatchRequest(). InitializeAuthenticator() sends a GetInfo command
   // to FidoDeviceAuthenticator instances in order to determine their protocol
   // versions before a request can be dispatched.
-  void InitializeAuthenticatorAndDispatchRequest(FidoAuthenticator*);
+  void InitializeAuthenticatorAndDispatchRequest(
+      AuthenticatorState* authenticator_state);
   void ConstructBleAdapterPowerManager();
 
   AuthenticatorMap active_authenticators_;

device/fido/get_assertion_handler_unittest.cc

@@ -845,7 +845,7 @@ TEST(GetAssertionRequestHandlerWinTest, TestWinUsbDiscovery) {
     EXPECT_EQ(handler->AuthenticatorsForTesting().size(), 1u);
     EXPECT_EQ(handler->AuthenticatorsForTesting()
                   .begin()
-                  ->second->IsWinNativeApiAuthenticator(),
+                  ->second->authenticator->IsWinNativeApiAuthenticator(),
               enable_api);
   }
 }

device/fido/get_assertion_request_handler.cc

@@ -398,6 +398,22 @@ void GetAssertionRequestHandler::HandleResponse(
          authenticator->WillNeedPINToGetAssertion(request_, observer()) !=
              PINDisposition::kUsePIN);
 
+  if ((status == CtapDeviceResponseCode::kCtap2ErrPinRequired ||
+       status == CtapDeviceResponseCode::kCtap2ErrOperationDenied) &&
+      authenticator->WillNeedPINToGetAssertion(request_, observer()) ==
+          PINDisposition::kUsePINForFallback) {
+    // Some authenticators will return this error immediately without user
+    // interaction when internal UV is locked.
+    if (AuthenticatorMayHaveReturnedImmediately(authenticator->GetId())) {
+      authenticator->GetTouch(base::BindOnce(
+          &GetAssertionRequestHandler::StartPINFallbackForInternalUv,
+          weak_factory_.GetWeakPtr(), authenticator));
+      return;
+    }
+    StartPINFallbackForInternalUv(authenticator);
+    return;
+  }
+
   const base::Optional<GetAssertionStatus> maybe_result =
       ConvertDeviceResponseCode(status);
   if (!maybe_result) {

device/fido/make_credential_request_handler.cc

@@ -421,6 +421,22 @@ void MakeCredentialRequestHandler::HandleResponse(
          will_need_pin == MakeCredentialPINDisposition::kNoPIN ||
          will_need_pin == MakeCredentialPINDisposition::kUsePINForFallback);
 
+  if ((status == CtapDeviceResponseCode::kCtap2ErrPinAuthInvalid ||
+       status == CtapDeviceResponseCode::kCtap2ErrPinRequired) &&
+      authenticator->WillNeedPINToMakeCredential(request_, observer()) ==
+          MakeCredentialPINDisposition::kUsePINForFallback) {
+    // Some authenticators will return this error immediately without user
+    // interaction when internal UV is locked.
+    if (AuthenticatorMayHaveReturnedImmediately(authenticator->GetId())) {
+      authenticator->GetTouch(base::BindOnce(
+          &MakeCredentialRequestHandler::StartPINFallbackForInternalUv,
+          weak_factory_.GetWeakPtr(), authenticator));
+      return;
+    }
+    StartPINFallbackForInternalUv(authenticator);
+    return;
+  }
+
   const base::Optional<MakeCredentialStatus> maybe_result =
       ConvertDeviceResponseCode(status);
   if (!maybe_result) {