fido: add U2F-only mode for cryptotoken requests

This adds an |is_u2f_only_| field to CtapMakeCredentialRequest to make
authenticators aware whether the current request originates from the
cryptotoken (U2F) extensions. If it does, request handling changes as follows:
 - For Windows API requests, U2F-only mode is enabled.
 - FidoDeviceAuthenticators will only communicate via U2F, not CTAP.

This is done to force the authenticator return a U2F-format attestation
statement, rather than one in an (incompatible) CTAP2 format such as
"packed".

GetAssertionTask already implements a U2F fallback for all requests with an App
ID extension (and CTAP need not be strictly avoided as is the case with
MakeCredential); thus no separate boolean field is required for
CtapGetAssertionRequest.

Bug: 898718
Change-Id: I8f17655d8df530546b62e2237c73aaa997483060
Reviewed-on: https://chromium-review.googlesource.com/c/1338952
Commit-Queue: Kim Paulhamus <kpaulhamus@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609618}

content/browser/webauth/authenticator_impl.cc

@@ -666,10 +666,12 @@ void AuthenticatorImpl::MakeCredential(
                 options->authenticator_selection)
           : device::AuthenticatorSelectionCriteria();
 
+  auto ctap_request = CreateCtapMakeCredentialRequest(
+      client_data_json_, options, individual_attestation);
+  ctap_request.set_is_u2f_only(OriginIsCryptoTokenExtension(caller_origin));
+
   request_ = std::make_unique<device::MakeCredentialRequestHandler>(
-      connector_, transports_,
-      CreateCtapMakeCredentialRequest(client_data_json_, options,
-                                      individual_attestation),
+      connector_, transports_, std::move(ctap_request),
       std::move(authenticator_selection_criteria),
       base::BindOnce(&AuthenticatorImpl::OnRegisterResponse,
                      weak_factory_.GetWeakPtr()));
@@ -777,7 +779,6 @@ void AuthenticatorImpl::GetAssertion(
   auto ctap_request =
       CreateCtapGetAssertionRequest(client_data_json_, std::move(options),
                                     alternative_application_parameter_);
-
   auto opt_platform_authenticator_info =
       CreatePlatformAuthenticatorIfAvailableAndCheckIfCredentialExists(
           ctap_request);

content/browser/webauth/authenticator_impl_unittest.cc

@@ -769,6 +769,46 @@ TEST_F(AuthenticatorImplTest, CryptotokenBypass) {
   }
 }
 
+// Requests originating from cryptotoken should only target U2F devices.
+TEST_F(AuthenticatorImplTest, CryptoTokenU2fOnly) {
+  EnableFeature(device::kWebAuthProxyCryptotoken);
+  constexpr char kCryptotokenOrigin[] =
+      "chrome-extension://kmendfapggjehodndflmmgagdbamhnfd";
+  TestServiceManagerContext smc;
+  SimulateNavigation(GURL(kTestOrigin1));
+  auto task_runner = base::MakeRefCounted<base::TestMockTimeTaskRunner>(
+      base::Time::Now(), base::TimeTicks::Now());
+  auto authenticator = ConstructAuthenticatorWithTimer(task_runner);
+  url::AddStandardScheme("chrome-extension", url::SCHEME_WITH_HOST);
+
+  // TODO(martinkr): ScopedVirtualFidoDevice does not offer devices that
+  // support both U2F and CTAP yet; we should test those.
+  for (const bool u2f_authenticator : {true, false}) {
+    SCOPED_TRACE(u2f_authenticator ? "U2F" : "CTAP");
+    OverrideLastCommittedOrigin(main_rfh(),
+                                url::Origin::Create(GURL(kCryptotokenOrigin)));
+
+    device::test::ScopedVirtualFidoDevice scoped_virtual_device;
+    scoped_virtual_device.SetSupportedProtocol(
+        u2f_authenticator ? device::ProtocolVersion::kU2f
+                          : device::ProtocolVersion::kCtap);
+
+    PublicKeyCredentialCreationOptionsPtr options =
+        GetTestPublicKeyCredentialCreationOptions();
+    TestMakeCredentialCallback callback_receiver;
+    authenticator->MakeCredential(std::move(options),
+                                  callback_receiver.callback());
+
+    base::RunLoop().RunUntilIdle();
+    task_runner->FastForwardBy(base::TimeDelta::FromMinutes(1));
+    callback_receiver.WaitForCallback();
+
+    EXPECT_EQ((u2f_authenticator ? AuthenticatorStatus::SUCCESS
+                                 : AuthenticatorStatus::NOT_ALLOWED_ERROR),
+              callback_receiver.status());
+  }
+}
+
 // Verify that a credential registered with U2F can be used via webauthn.
 TEST_F(AuthenticatorImplTest, AppIdExtension) {
   SimulateNavigation(GURL(kTestOrigin1));

device/fido/ctap_make_credential_request.h

@@ -85,6 +85,9 @@ class COMPONENT_EXPORT(DEVICE_FIDO) CtapMakeCredentialRequest {
     return pin_auth_;
   }
 
+  void set_is_u2f_only(bool is_u2f_only) { is_u2f_only_ = is_u2f_only; }
+  bool is_u2f_only() { return is_u2f_only_; }
+
  private:
   std::string client_data_json_;
   std::array<uint8_t, kClientDataHashLength> client_data_hash_;
@@ -101,6 +104,10 @@ class COMPONENT_EXPORT(DEVICE_FIDO) CtapMakeCredentialRequest {
   // asserted to CTAP2 authenticators.
   bool hmac_secret_ = false;
 
+  // If true, instruct the request handler only to dispatch this request via
+  // U2F.
+  bool is_u2f_only_ = false;
+
   base::Optional<std::vector<PublicKeyCredentialDescriptor>> exclude_list_;
   base::Optional<std::vector<uint8_t>> pin_auth_;
   base::Optional<uint8_t> pin_protocol_;

device/fido/make_credential_task.cc

@@ -25,10 +25,11 @@ namespace {
 // is not required and the device supports U2F protocol.
 // TODO(hongjunchoi): Remove this once ClientPin command is implemented.
 // See: https://crbug.com/870892
-bool IsClientPinOptionCompatible(const FidoDevice* device,
-                                 const CtapMakeCredentialRequest& request) {
+bool ShouldUseU2fBecauseCtapRequiresClientPin(
+    const FidoDevice* device,
+    const CtapMakeCredentialRequest& request) {
   if (request.user_verification() == UserVerificationRequirement::kRequired)
-    return true;
+    return false;
 
   DCHECK(device && device->device_info());
   bool client_pin_set =
@@ -36,7 +37,7 @@ bool IsClientPinOptionCompatible(const FidoDevice* device,
       AuthenticatorSupportedOptions::ClientPinAvailability::kSupportedAndPinSet;
   bool supports_u2f = base::ContainsKey(device->device_info()->versions(),
                                         ProtocolVersion::kU2f);
-  return !client_pin_set || !supports_u2f;
+  return client_pin_set && supports_u2f;
 }
 
 }  // namespace
@@ -55,7 +56,8 @@ MakeCredentialTask::~MakeCredentialTask() = default;
 void MakeCredentialTask::StartTask() {
   if (base::FeatureList::IsEnabled(kNewCtap2Device) &&
       device()->supported_protocol() == ProtocolVersion::kCtap &&
-      IsClientPinOptionCompatible(device(), request_parameter_)) {
+      !request_parameter_.is_u2f_only() &&
+      !ShouldUseU2fBecauseCtapRequiresClientPin(device(), request_parameter_)) {
     MakeCredential();
   } else {
     device()->set_supported_protocol(ProtocolVersion::kU2f);

device/fido/make_credential_task_unittest.cc

@@ -187,5 +187,32 @@ TEST_F(FidoMakeCredentialTaskTest, EnforceClientPinWhenUserVerificationSet) {
   EXPECT_FALSE(make_credential_callback_receiver().value());
 }
 
+TEST_F(FidoMakeCredentialTaskTest, TestU2fOnly) {
+  // Regardless of the device's supported protocol, it should receive a U2F
+  // request, because the task is instantiated in U2F-only mode.
+  auto device = MockFidoDevice::MakeCtap();
+
+  device->ExpectRequestAndRespondWith(
+      test_data::kU2fRegisterCommandApdu,
+      test_data::kApduEncodedNoErrorRegisterResponse);
+
+  PublicKeyCredentialRpEntity rp(test_data::kRelyingPartyId);
+  PublicKeyCredentialUserEntity user(
+      fido_parsing_utils::Materialize(test_data::kUserId));
+  auto request = CtapMakeCredentialRequest(
+      test_data::kClientDataJson, std::move(rp), std::move(user),
+      PublicKeyCredentialParams(
+          std::vector<PublicKeyCredentialParams::CredentialInfo>(1)));
+  request.set_is_u2f_only(true);
+  const auto task = std::make_unique<MakeCredentialTask>(
+      device.get(), std::move(request), callback_receiver_.callback());
+  make_credential_callback_receiver().WaitForCallback();
+
+  EXPECT_EQ(CtapDeviceResponseCode::kSuccess,
+            make_credential_callback_receiver().status());
+  EXPECT_TRUE(make_credential_callback_receiver().value());
+  EXPECT_EQ(device->supported_protocol(), ProtocolVersion::kU2f);
+}
+
 }  // namespace
 }  // namespace device

device/fido/win/authenticator.cc

@@ -191,7 +191,7 @@ void WinWebAuthnApiAuthenticator::MakeCredentialBlocking(
       WEBAUTHN_CREDENTIALS{
           0, nullptr},  // Ignored because pExcludeCredentialList is set.
       WEBAUTHN_EXTENSIONS{extensions.size(), extensions.data()},
-      use_u2f_only_
+      request.is_u2f_only()
           ? WEBAUTHN_AUTHENTICATOR_ATTACHMENT_CROSS_PLATFORM_U2F_V2
           : ToWinAuthenticatorAttachment(request.authenticator_attachment()),
       request.resident_key_required(),
@@ -279,6 +279,15 @@ void WinWebAuthnApiAuthenticator::GetAssertionBlocking(
   static BOOL kUseAppIdTrue = TRUE;  // const
 
   base::string16 rp_id16 = base::UTF8ToUTF16(request.rp_id());
+  base::Optional<base::string16> opt_app_id16 = base::nullopt;
+  // TODO(martinkr): alternative_application_parameter() is already hashed,
+  // so this doesn't work. We need to make it store the full AppID.
+  if (request.alternative_application_parameter()) {
+    opt_app_id16 = base::UTF8ToUTF16(base::StringPiece(
+        reinterpret_cast<const char*>(
+            request.alternative_application_parameter()->data()),
+        request.alternative_application_parameter()->size()));
+  }
 
   WEBAUTHN_CLIENT_DATA client_data{
       WEBAUTHN_CLIENT_DATA_CURRENT_VERSION, request.client_data_json().size(),
@@ -299,12 +308,11 @@ void WinWebAuthnApiAuthenticator::GetAssertionBlocking(
           0, nullptr},  // Ignored because pAllowCredentialList is set.
       WEBAUTHN_EXTENSIONS{0, nullptr},
       // Note that attachment is effectively restricted via |allow_list|.
-      use_u2f_only_ ? WEBAUTHN_AUTHENTICATOR_ATTACHMENT_CROSS_PLATFORM_U2F_V2
-                    : WEBAUTHN_AUTHENTICATOR_ATTACHMENT_ANY,
+      WEBAUTHN_AUTHENTICATOR_ATTACHMENT_ANY,
       ToWinUserVerificationRequirement(request.user_verification()),
-      0,                                          // flags
-      use_u2f_only_ ? rp_id16.c_str() : nullptr,  // pwszU2fAppId
-      use_u2f_only_ ? &kUseAppIdTrue : nullptr,   // pbU2fAppId
+      0,                                               // flags
+      opt_app_id16 ? opt_app_id16->c_str() : nullptr,  // pwszU2fAppId
+      opt_app_id16 ? &kUseAppIdTrue : nullptr,         // pbU2fAppId
       &cancellation_id_, &allow_credential_list,
   };
 

device/fido/win/authenticator.h

@@ -32,19 +32,6 @@ class COMPONENT_EXPORT(DEVICE_FIDO) WinWebAuthnApiAuthenticator
   WinWebAuthnApiAuthenticator(WinWebAuthnApi* win_api, HWND current_window);
   ~WinWebAuthnApiAuthenticator() override;
 
-  // Forces the Windows WebAuthn API not to communicate with CTAP2 devices for
-  // this request. Dual-protocol devices will use U2F.
-  //
-  // If enabled, the RP ID field of the request will be interpreted and used as
-  // an App ID instead.
-  //
-  // This method must not be called after any of the FidoAuthenticator
-  // interface methods have been invoked.
-  void enable_u2f_only_mode() {
-    DCHECK(!thread_);
-    use_u2f_only_ = true;
-  }
-
   // FidoAuthenticator
   void InitializeAuthenticator(base::OnceClosure callback) override;
   void MakeCredential(CtapMakeCredentialRequest request,
@@ -82,7 +69,6 @@ class COMPONENT_EXPORT(DEVICE_FIDO) WinWebAuthnApiAuthenticator
   WinWebAuthnApi* win_api_;
   std::unique_ptr<base::Thread> thread_;
   HWND current_window_;
-  bool use_u2f_only_ = false;
 
   GUID cancellation_id_ = {};
   base::AtomicFlag operation_cancelled_;