fido: make GetAssertionRequestHandler use AuthTokenRequester

This replaces the bits of GetAssertionRequestHandler that request a
PIN/UV Auth Token by calling out to AuthTokenRequester instead. See
CL:2469445 for the equivalent CL for MakeCredential.

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

device/fido/fido_authenticator.cc

@@ -73,18 +73,18 @@ void FidoAuthenticator::ChangePIN(const std::string& old_pin,
   NOTREACHED();
 }
 
-FidoAuthenticator::MakeCredentialPINUVDisposition
+FidoAuthenticator::PINUVDisposition
 FidoAuthenticator::PINUVDispositionForMakeCredential(
     const CtapMakeCredentialRequest& request,
     const FidoRequestHandlerBase::Observer* observer) {
-  return MakeCredentialPINUVDisposition::kNoUV;
+  return PINUVDisposition::kNoUV;
 }
 
-FidoAuthenticator::GetAssertionPINDisposition
-FidoAuthenticator::WillNeedPINToGetAssertion(
+FidoAuthenticator::PINUVDisposition
+FidoAuthenticator::PINUVDispositionForGetAssertion(
     const CtapGetAssertionRequest& request,
     const FidoRequestHandlerBase::Observer* observer) {
-  return GetAssertionPINDisposition::kNoPIN;
+  return PINUVDisposition::kNoUV;
 }
 
 void FidoAuthenticator::GetCredentialsMetadata(

device/fido/fido_authenticator.h

@@ -146,9 +146,9 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoAuthenticator {
                          const std::string& new_pin,
                          SetPINCallback callback);
 
-  // MakeCredentialPINUVDisposition enumerates the possible options for
-  // obtaining user verification when making a credential.
-  enum class MakeCredentialPINUVDisposition {
+  // PINUVDisposition enumerates the possible options for obtaining user
+  // verification when making a CTAP2 request.
+  enum class PINUVDisposition {
     // No UV (neither clientPIN nor internal) is needed to make this
     // credential.
     kNoUV,
@@ -165,31 +165,17 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoAuthenticator {
     // The request cannot be satisfied by this authenticator.
     kUnsatisfiable,
   };
-  // PINUVDispositionForMakeCredential returns whether and how user verification
+
+  // PINUVDisposition returns whether and how user verification
   // should be obtained in order to serve the given request on this
   // authenticator.
-  virtual MakeCredentialPINUVDisposition PINUVDispositionForMakeCredential(
+  virtual PINUVDisposition PINUVDispositionForMakeCredential(
       const CtapMakeCredentialRequest& request,
       const FidoRequestHandlerBase::Observer* observer);
 
-  // GetAssertionPINDisposition enumerates the possible interactions between
-  // a user-verification level and the PIN support of an authenticator when
-  // getting an assertion.
-  enum class GetAssertionPINDisposition {
-    // kNoPIN means that a PIN will not be needed for this assertion.
-    kNoPIN,
-    // kUsePIN means that a PIN must be gathered and used for this assertion.
-    kUsePIN,
-    // kUsePINForFallback means that a PIN may be used for fallback if internal
-    // user verification fails.
-    kUsePINForFallback,
-    // kUnsatisfiable means that the request cannot be satisfied by this
-    // authenticator.
-    kUnsatisfiable,
-  };
   // WillNeedPINToGetAssertion returns whether a PIN prompt will be needed to
   // serve the given request on this authenticator.
-  virtual GetAssertionPINDisposition WillNeedPINToGetAssertion(
+  virtual PINUVDisposition PINUVDispositionForGetAssertion(
       const CtapGetAssertionRequest& request,
       const FidoRequestHandlerBase::Observer* observer);
 

device/fido/fido_device_authenticator.cc

@@ -349,7 +349,7 @@ void FidoDeviceAuthenticator::OnHaveEphemeralKeyForChangePIN(
       std::move(callback), base::BindOnce(&pin::EmptyResponse::Parse));
 }
 
-FidoAuthenticator::MakeCredentialPINUVDisposition
+FidoAuthenticator::PINUVDisposition
 FidoDeviceAuthenticator::PINUVDispositionForMakeCredential(
     const CtapMakeCredentialRequest& request,
     const FidoRequestHandlerBase::Observer* observer) {
@@ -363,9 +363,6 @@ FidoDeviceAuthenticator::PINUVDispositionForMakeCredential(
       Options()->user_verification_availability ==
       UserVerificationAvailability::kSupportedAndConfigured;
 
-  const bool can_get_token =
-      (can_collect_pin && pin_supported) || CanGetUvToken();
-
   // CTAP 2.0 requires a PIN for credential creation once a PIN has been set.
   // Thus, if fallback to U2F isn't possible, a PIN will be needed if set.
   const bool u2f_fallback_possible =
@@ -373,73 +370,74 @@ FidoDeviceAuthenticator::PINUVDispositionForMakeCredential(
       device()->device_info()->versions.contains(ProtocolVersion::kU2f) &&
       IsConvertibleToU2fRegisterCommand(request) &&
       !ShouldPreferCTAP2EvenIfItNeedsAPIN(request);
-  const bool uv_required =
-      request.user_verification == UserVerificationRequirement::kRequired ||
-      (pin_configured && !u2f_fallback_possible);
-  const bool uv_preferred =
-      request.user_verification == UserVerificationRequirement::kPreferred;
-
-  if (!uv_required && !(uv_preferred && (pin_configured || uv_configured))) {
-    return MakeCredentialPINUVDisposition::kNoUV;
+
+  const UserVerificationRequirement uv_requirement =
+      (pin_configured && !u2f_fallback_possible)
+          ? UserVerificationRequirement::kRequired
+          : request.user_verification;
+
+  if (uv_requirement == UserVerificationRequirement::kDiscouraged ||
+      (uv_requirement == UserVerificationRequirement::kPreferred &&
+       ((!pin_configured || !can_collect_pin) && !uv_configured))) {
+    return PINUVDisposition::kNoUV;
   }
 
   // Authenticators with built-in UV that don't support UV token should try
   // sending the request as-is with uv=true first.
   if (uv_configured && !CanGetUvToken()) {
     return (can_collect_pin && pin_supported)
-               ? MakeCredentialPINUVDisposition::kNoTokenInternalUVPINFallback
-               : MakeCredentialPINUVDisposition::kNoTokenInternalUV;
+               ? PINUVDisposition::kNoTokenInternalUVPINFallback
+               : PINUVDisposition::kNoTokenInternalUV;
   }
 
+  const bool can_get_token =
+      (can_collect_pin && pin_supported) || CanGetUvToken();
+
   if (can_get_token) {
-    return MakeCredentialPINUVDisposition::kGetToken;
+    return PINUVDisposition::kGetToken;
   }
 
-  return MakeCredentialPINUVDisposition::kUnsatisfiable;
+  return PINUVDisposition::kUnsatisfiable;
 }
 
-FidoAuthenticator::GetAssertionPINDisposition
-FidoDeviceAuthenticator::WillNeedPINToGetAssertion(
+FidoAuthenticator::PINUVDisposition
+FidoDeviceAuthenticator::PINUVDispositionForGetAssertion(
     const CtapGetAssertionRequest& request,
     const FidoRequestHandlerBase::Observer* observer) {
-  const bool can_use_pin = (Options()->client_pin_availability ==
-                            AuthenticatorSupportedOptions::
-                                ClientPinAvailability::kSupportedAndPinSet) &&
-                           // The PIN is effectively unavailable if there's no
-                           // UI support for collecting it.
-                           observer && observer->SupportsPIN();
-
-  // Authenticators with built-in UV can use that.
-  if (Options()->user_verification_availability ==
-      UserVerificationAvailability::kSupportedAndConfigured) {
-    return can_use_pin ? GetAssertionPINDisposition::kUsePINForFallback
-                       : GetAssertionPINDisposition::kNoPIN;
-  }
+  // TODO(crbug.com/1149405): GetAssertion requests don't allow in-line UV
+  // enrollment. Perhaps we should change this and align with MakeCredential
+  // behavior.
+  const bool can_collect_pin = observer && observer->SupportsPIN();
+  const bool pin_configured = Options()->client_pin_availability ==
+                              ClientPinAvailability::kSupportedAndPinSet;
+
+  const bool uv_configured =
+      Options()->user_verification_availability ==
+      UserVerificationAvailability::kSupportedAndConfigured;
 
-  const bool resident_key_request = request.allow_list.empty();
+  const UserVerificationRequirement uv_requirement =
+      request.allow_list.empty() ? UserVerificationRequirement::kRequired
+                                 : request.user_verification;
 
-  if (resident_key_request) {
-    if (can_use_pin) {
-      return GetAssertionPINDisposition::kUsePIN;
-    }
-    return GetAssertionPINDisposition::kUnsatisfiable;
+  if (uv_requirement == UserVerificationRequirement::kDiscouraged ||
+      (uv_requirement == UserVerificationRequirement::kPreferred &&
+       ((!pin_configured || !can_collect_pin) && !uv_configured))) {
+    return PINUVDisposition::kNoUV;
   }
 
-  // If UV is required then the PIN must be used if set, or else this request
-  // cannot be satisfied.
-  if (request.user_verification == UserVerificationRequirement::kRequired) {
-    if (can_use_pin) {
-      return GetAssertionPINDisposition::kUsePIN;
-    }
-    return GetAssertionPINDisposition::kUnsatisfiable;
+  // Authenticators with built-in UV that don't support UV token should try
+  // sending the request as-is with uv=true first.
+  if (uv_configured && !CanGetUvToken()) {
+    return (can_collect_pin && pin_configured)
+               ? PINUVDisposition::kNoTokenInternalUVPINFallback
+               : PINUVDisposition::kNoTokenInternalUV;
   }
 
-  // If UV is preferred and a PIN is set, use it.
-  if (request.user_verification == UserVerificationRequirement::kPreferred &&
-      can_use_pin) {
-    return GetAssertionPINDisposition::kUsePIN;
+  if ((can_collect_pin && pin_configured) || CanGetUvToken()) {
+    return PINUVDisposition::kGetToken;
   }
-  return GetAssertionPINDisposition::kNoPIN;
+
+  return PINUVDisposition::kUnsatisfiable;
 }
 
 void FidoDeviceAuthenticator::GetCredentialsMetadata(

device/fido/fido_device_authenticator.h

@@ -67,13 +67,13 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoDeviceAuthenticator
   void ChangePIN(const std::string& old_pin,
                  const std::string& new_pin,
                  SetPINCallback callback) override;
-  MakeCredentialPINUVDisposition PINUVDispositionForMakeCredential(
+  PINUVDisposition PINUVDispositionForMakeCredential(
       const CtapMakeCredentialRequest& request,
       const FidoRequestHandlerBase::Observer* observer) override;
 
   // WillNeedPINToGetAssertion returns whether a PIN prompt will be needed to
   // serve the given request on this authenticator.
-  GetAssertionPINDisposition WillNeedPINToGetAssertion(
+  PINUVDisposition PINUVDispositionForGetAssertion(
       const CtapGetAssertionRequest& request,
       const FidoRequestHandlerBase::Observer* observer) override;
 

device/fido/get_assertion_request_handler.h

@@ -6,6 +6,7 @@
 #define DEVICE_FIDO_GET_ASSERTION_REQUEST_HANDLER_H_
 
 #include <memory>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -13,6 +14,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
+#include "device/fido/auth_token_requester.h"
 #include "device/fido/authenticator_get_assertion_response.h"
 #include "device/fido/ctap_get_assertion_request.h"
 #include "device/fido/fido_constants.h"
@@ -29,7 +31,6 @@ class FidoAuthenticator;
 class FidoDiscoveryFactory;
 
 namespace pin {
-struct RetriesResponse;
 class TokenResponse;
 }  // namespace pin
 
@@ -51,7 +52,8 @@ enum class GetAssertionStatus {
 };
 
 class COMPONENT_EXPORT(DEVICE_FIDO) GetAssertionRequestHandler
-    : public FidoRequestHandlerBase {
+    : public FidoRequestHandlerBase,
+      public AuthTokenRequester::Delegate {
  public:
   using CompletionCallback = base::OnceCallback<void(
       GetAssertionStatus,
@@ -70,10 +72,8 @@ class COMPONENT_EXPORT(DEVICE_FIDO) GetAssertionRequestHandler
  private:
   enum class State {
     kWaitingForTouch,
-    kWaitingForSecondTouch,
-    kGettingRetries,
-    kWaitingForPIN,
-    kRequestWithPIN,
+    kWaitingForToken,
+    kWaitingForResponseWithToken,
     kReadingMultipleResponses,
     kFinished,
   };
@@ -89,6 +89,24 @@ class COMPONENT_EXPORT(DEVICE_FIDO) GetAssertionRequestHandler
   void AuthenticatorRemoved(FidoDiscoveryBase* discovery,
                             FidoAuthenticator* authenticator) override;
 
+  // AuthTokenRequester::Delegate:
+  void AuthenticatorSelectedForPINUVAuthToken(
+      FidoAuthenticator* authenticator) override;
+  void CollectNewPIN(uint32_t min_pin_length,
+                     ProvidePINCallback provide_pin_cb) override;
+  void CollectExistingPIN(int attempts,
+                          uint32_t min_pin_length,
+                          ProvidePINCallback provide_pin_cb) override;
+  void PromptForInternalUVRetry(int attempts) override;
+  void InternalUVLockedForAuthToken() override;
+  void HavePINUVAuthTokenResultForAuthenticator(
+      FidoAuthenticator* authenticator,
+      AuthTokenRequester::Result result,
+      base::Optional<pin::TokenResponse> response) override;
+
+  void ObtainPINUVAuthToken(FidoAuthenticator* authenticator,
+                            std::set<pin::Permissions> permissions,
+                            bool skip_pin_touch);
   void HandleResponse(
       FidoAuthenticator* authenticator,
       CtapGetAssertionRequest request,
@@ -100,22 +118,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) GetAssertionRequestHandler
       CtapGetAssertionRequest request,
       CtapDeviceResponseCode response_code,
       base::Optional<AuthenticatorGetAssertionResponse> response);
-  void CollectPINThenSendRequest(FidoAuthenticator* authenticator);
-  void StartPINFallbackForInternalUv(FidoAuthenticator* authenticator);
   void TerminateUnsatisfiableRequestPostTouch(FidoAuthenticator* authenticator);
-  void OnPinRetriesResponse(CtapDeviceResponseCode status,
-                            base::Optional<pin::RetriesResponse> response);
-  void OnHavePIN(std::string pin);
-  void OnHavePINToken(CtapDeviceResponseCode status,
-                      base::Optional<pin::TokenResponse> response);
-  void OnStartUvTokenOrFallback(FidoAuthenticator* authenticator,
-                                CtapDeviceResponseCode status,
-                                base::Optional<pin::RetriesResponse> response);
-  void OnUvRetriesResponse(CtapDeviceResponseCode status,
-                           base::Optional<pin::RetriesResponse> response);
-  void OnHaveUvToken(FidoAuthenticator* authenticator,
-                     CtapDeviceResponseCode status,
-                     base::Optional<pin::TokenResponse> response);
   void DispatchRequestWithToken(pin::TokenResponse token);
   void OnGetAssertionSuccess(FidoAuthenticator* authenticator,
                              CtapGetAssertionRequest request);
@@ -132,23 +135,32 @@ class COMPONENT_EXPORT(DEVICE_FIDO) GetAssertionRequestHandler
   CtapGetAssertionRequest request_;
   CtapGetAssertionOptions options_;
   base::Optional<pin::TokenResponse> pin_token_;
+
   // If true, and if at the time the request is dispatched to the first
   // authenticator no other authenticators are available, the request handler
   // will skip the initial touch that is usually required to select a PIN
   // protected authenticator.
   bool allow_skipping_pin_touch_;
-  // authenticator_ points to the authenticator that will be used for this
-  // operation. It's only set after the user touches an authenticator to select
-  // it, after which point that authenticator will be used exclusively through
-  // requesting PIN etc. The object is owned by the underlying discovery object
-  // and this pointer is cleared if it's removed during processing.
-  FidoAuthenticator* authenticator_ = nullptr;
+
+  // selected_authenticator_for_pin_uv_auth_token_ points to the authenticator
+  // that was tapped by the user while requesting a pinUvAuthToken from
+  // connected authenticators. The object is owned by the underlying discovery
+  // object and this pointer is cleared if it's removed during processing.
+  FidoAuthenticator* selected_authenticator_for_pin_uv_auth_token_ = nullptr;
+
   // responses_ holds the set of responses while they are incrementally read
   // from the device. Only used when more than one response is returned.
   std::vector<AuthenticatorGetAssertionResponse> responses_;
+
   // remaining_responses_ contains the number of responses that remain to be
   // read when multiple responses are returned.
   size_t remaining_responses_ = 0;
+
+  // auth_token_requester_map_ holds active AuthTokenRequesters for
+  // authenticators that need a pinUvAuthToken to service the request.
+  std::map<FidoAuthenticator*, std::unique_ptr<AuthTokenRequester>>
+      auth_token_requester_map_;
+
   SEQUENCE_CHECKER(my_sequence_checker_);
   base::WeakPtrFactory<GetAssertionRequestHandler> weak_factory_{this};
 

device/fido/make_credential_request_handler.cc

@@ -27,8 +27,7 @@
 
 namespace device {
 
-using MakeCredentialPINUVDisposition =
-    FidoAuthenticator::MakeCredentialPINUVDisposition;
+using PINUVDisposition = FidoAuthenticator::PINUVDisposition;
 using BioEnrollmentAvailability =
     AuthenticatorSupportedOptions::BioEnrollmentAvailability;
 
@@ -130,7 +129,7 @@ MakeCredentialStatus IsCandidateAuthenticatorPostTouch(
   }
 
   if (authenticator->PINUVDispositionForMakeCredential(request, observer) ==
-      MakeCredentialPINUVDisposition::kUnsatisfiable) {
+      PINUVDisposition::kUnsatisfiable) {
     return MakeCredentialStatus::kAuthenticatorMissingUserVerification;
   }
 
@@ -428,14 +427,14 @@ void MakeCredentialRequestHandler::DispatchRequest(
   auto uv_disposition = authenticator->PINUVDispositionForMakeCredential(
       *request.get(), observer());
   switch (uv_disposition) {
-    case MakeCredentialPINUVDisposition::kNoUV:
-    case MakeCredentialPINUVDisposition::kNoTokenInternalUV:
-    case MakeCredentialPINUVDisposition::kNoTokenInternalUVPINFallback:
+    case PINUVDisposition::kNoUV:
+    case PINUVDisposition::kNoTokenInternalUV:
+    case PINUVDisposition::kNoTokenInternalUVPINFallback:
       break;
-    case MakeCredentialPINUVDisposition::kGetToken:
+    case PINUVDisposition::kGetToken:
       ObtainPINUVAuthToken(authenticator, skip_pin_touch);
       return;
-    case MakeCredentialPINUVDisposition::kUnsatisfiable:
+    case PINUVDisposition::kUnsatisfiable:
       // |IsCandidateAuthenticatorPostTouch| should have handled this case.
       NOTREACHED();
       return;
@@ -529,8 +528,8 @@ void MakeCredentialRequestHandler::HavePINUVAuthTokenResultForAuthenticator(
                       << authenticator->GetId();
       return;
     case AuthTokenRequester::Result::kPostTouchAuthenticatorInternalUVLock:
-      HandleInternalUvLocked(authenticator);
-      return;
+      error = MakeCredentialStatus::kAuthenticatorMissingUserVerification;
+      break;
     case AuthTokenRequester::Result::kPostTouchAuthenticatorResponseInvalid:
       error = MakeCredentialStatus::kAuthenticatorResponseInvalid;
       break;
@@ -641,7 +640,7 @@ void MakeCredentialRequestHandler::HandleResponse(
       (status == CtapDeviceResponseCode::kCtap2ErrPinAuthInvalid ||
        status == CtapDeviceResponseCode::kCtap2ErrPinRequired) &&
       authenticator->PINUVDispositionForMakeCredential(*request, observer()) ==
-          MakeCredentialPINUVDisposition::kNoTokenInternalUVPINFallback) {
+          PINUVDisposition::kNoTokenInternalUVPINFallback) {
     // Authenticators without uvToken support will return this error immediately
     // without user interaction when internal UV is locked.
     const base::TimeDelta response_time = request_timer.Elapsed();
@@ -718,15 +717,6 @@ void MakeCredentialRequestHandler::HandleResponse(
       .Run(MakeCredentialStatus::kSuccess, std::move(*response), authenticator);
 }
 
-void MakeCredentialRequestHandler::HandleInternalUvLocked(
-    FidoAuthenticator* authenticator) {
-  state_ = State::kFinished;
-  CancelActiveAuthenticators(authenticator->GetId());
-  std::move(completion_callback_)
-      .Run(MakeCredentialStatus::kAuthenticatorMissingUserVerification,
-           base::nullopt, nullptr);
-}
-
 void MakeCredentialRequestHandler::HandleInapplicableAuthenticator(
     FidoAuthenticator* authenticator,
     std::unique_ptr<CtapMakeCredentialRequest> request) {

device/fido/make_credential_request_handler.h

@@ -178,7 +178,6 @@ class COMPONENT_EXPORT(DEVICE_FIDO) MakeCredentialRequestHandler
       base::ElapsedTimer request_timer,
       CtapDeviceResponseCode response_code,
       base::Optional<AuthenticatorMakeCredentialResponse> response);
-  void HandleInternalUvLocked(FidoAuthenticator* authenticator);
   void HandleInapplicableAuthenticator(
       FidoAuthenticator* authenticator,
       std::unique_ptr<CtapMakeCredentialRequest> request);