[fido] Ignore alwaysUv for u2f-only requests

u2f-only requests should not try to obtain a PinUvAuthToken even if the
key supports CTAP 2.1 and the alwaysUv flag is on.

Fixed: 1149572
Change-Id: Ib9a132b4b9fe92bc9830d87395097a2440acf2ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2542822
Commit-Queue: Nina Satragno <nsatragno@chromium.org>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/heads/master@{#828418}

content/browser/webauth/authenticator_impl_unittest.cc

@@ -3961,6 +3961,40 @@ TEST_F(PINAuthenticatorImplTest, MakeCredentialAlwaysUv) {
   EXPECT_TRUE(HasUV(result.response));
 }
 
+TEST_F(PINAuthenticatorImplTest, MakeCredentialAlwaysUvU2fOnly) {
+  // Test that even if an authenticator is reporting alwaysUv = 1, cryptotoken
+  // requests don't try getting a PinUvAuthToken.
+  NavigateAndCommit(GURL(kCryptotokenOrigin));
+
+  for (bool internal_uv : {true, false}) {
+    SCOPED_TRACE(::testing::Message() << "internal_uv=" << internal_uv);
+    device::VirtualCtap2Device::Config config;
+    config.internal_uv_support = internal_uv;
+    config.u2f_support = internal_uv;
+    config.always_uv = true;
+    config.pin_support = true;
+    virtual_device_factory_->mutable_state()->fingerprints_enrolled = true;
+    virtual_device_factory_->SetCtap2Config(config);
+
+    PublicKeyCredentialCreationOptionsPtr options = make_credential_options(
+        device::UserVerificationRequirement::kDiscouraged);
+    if (internal_uv) {
+      MakeCredentialResult result =
+          AuthenticatorMakeCredential(std::move(options));
+      EXPECT_EQ(result.status, AuthenticatorStatus::SUCCESS);
+      EXPECT_FALSE(HasUV(result.response));
+    } else {
+      MakeCredentialResult result =
+          AuthenticatorMakeCredentialAndWaitForTimeout(std::move(options));
+      EXPECT_EQ(result.status, AuthenticatorStatus::NOT_ALLOWED_ERROR);
+    }
+
+    // Look at the permissions to verify that a PinUvAuthToken was not obtained.
+    EXPECT_EQ(
+        virtual_device_factory_->mutable_state()->pin_uv_token_permissions, 0);
+  }
+}
+
 TEST_F(PINAuthenticatorImplTest, MakeCredentialMinPINLengthNewPIN) {
   // Test that an authenticator advertising a min PIN length other than the
   // default makes it all the way to CollectPIN when setting a new PIN.
@@ -4174,6 +4208,41 @@ TEST_F(PINAuthenticatorImplTest, GetAssertionAlwaysUv) {
   EXPECT_TRUE(HasUV(result.response));
 }
 
+TEST_F(PINAuthenticatorImplTest, GetAssertionAlwaysUvU2fOnly) {
+  // Test that even if an authenticator is reporting alwaysUv = 1, cryptotoken
+  // requests don't try getting a PinUvAuthToken.
+  NavigateAndCommit(GURL(kCryptotokenOrigin));
+  PublicKeyCredentialRequestOptionsPtr options =
+      get_credential_options(device::UserVerificationRequirement::kDiscouraged);
+  ASSERT_TRUE(virtual_device_factory_->mutable_state()->InjectRegistration(
+      options->allow_credentials[0].id(), kTestRelyingPartyId));
+
+  for (bool internal_uv : {true, false}) {
+    SCOPED_TRACE(::testing::Message() << "internal_uv=" << internal_uv);
+    device::VirtualCtap2Device::Config config;
+    config.internal_uv_support = internal_uv;
+    config.u2f_support = internal_uv;
+    config.always_uv = true;
+    config.pin_support = true;
+    virtual_device_factory_->mutable_state()->fingerprints_enrolled = true;
+    virtual_device_factory_->SetCtap2Config(config);
+
+    if (internal_uv) {
+      GetAssertionResult result = AuthenticatorGetAssertion(options.Clone());
+      EXPECT_EQ(result.status, AuthenticatorStatus::SUCCESS);
+      EXPECT_FALSE(HasUV(result.response));
+    } else {
+      GetAssertionResult result =
+          AuthenticatorGetAssertionAndWaitForTimeout(options.Clone());
+      EXPECT_EQ(result.status, AuthenticatorStatus::NOT_ALLOWED_ERROR);
+    }
+
+    // Look at the permissions to verify that a PinUvAuthToken was not obtained.
+    EXPECT_EQ(
+        virtual_device_factory_->mutable_state()->pin_uv_token_permissions, 0);
+  }
+}
+
 TEST_F(PINAuthenticatorImplTest, MakeCredentialNoSupportedAlgorithm) {
   NavigateAndCommit(GURL(kTestOrigin1));
 

device/fido/get_assertion_request_handler.cc

@@ -269,7 +269,8 @@ CtapGetAssertionRequest SpecializeRequestForAuthenticator(
     specialized_request.large_blob_read = false;
     specialized_request.large_blob_write.reset();
   }
-  if (authenticator.Options() && authenticator.Options()->always_uv) {
+  if (!request.is_u2f_only && authenticator.Options() &&
+      authenticator.Options()->always_uv) {
     specialized_request.user_verification =
         UserVerificationRequirement::kRequired;
   }

device/fido/make_credential_request_handler.cc

@@ -850,10 +850,12 @@ void MakeCredentialRequestHandler::SpecializeRequestForAuthenticator(
       break;
   }
 
-  request->user_verification = request->resident_key_required ||
-                                       (auth_options && auth_options->always_uv)
-                                   ? UserVerificationRequirement::kRequired
-                                   : options_.user_verification;
+  if (!request->is_u2f_only && (request->resident_key_required ||
+                                (auth_options && auth_options->always_uv))) {
+    request->user_verification = UserVerificationRequirement::kRequired;
+  } else {
+    request->user_verification = options_.user_verification;
+  }
 
   if (options_.cred_protect_request &&
       authenticator->SupportsCredProtectExtension()) {

device/fido/virtual_ctap2_device.cc

@@ -483,8 +483,8 @@ VirtualCtap2Device::VirtualCtap2Device(scoped_refptr<State> state,
   Init({ProtocolVersion::kCtap2});
   std::vector<ProtocolVersion> versions = {ProtocolVersion::kCtap2};
   if (config.u2f_support) {
-    // Devices with alwaysUv may disable u2f. Let's be strict here.
-    DCHECK(!config.always_uv);
+    // Devices with alwaysUv may disable u2f if they don't support internal uv.
+    DCHECK(!config.always_uv || config.internal_uv_support);
     versions.emplace_back(ProtocolVersion::kU2f);
     u2f_device_ = std::make_unique<VirtualU2fDevice>(NewReferenceToState());
   }
@@ -689,7 +689,8 @@ FidoDevice::CancelToken VirtualCtap2Device::DeviceTransact(
   auto cmd_type = command[0];
   // The CTAP2 commands start at one, so a "command" of zero indicates that this
   // is a U2F message.
-  if (cmd_type == 0 && config_.u2f_support) {
+  if (cmd_type == 0 && config_.u2f_support &&
+      (!config_.always_uv || mutable_state()->fingerprints_enrolled)) {
     u2f_device_->DeviceTransact(std::move(command), std::move(cb));
     return 0;
   }