Make U2F devices blink on empty allow list

Currently when empty allow list is passed on as a parameter to
GetAssertion request, all requests to U2F devices are dropped. This is
in accordance with current version of the CTAP spec, as empty allow list
implies resident key credentials and U2F devices do not support resident
keys.

However, this behavior causes requests to U2F devices to be dropped
without U2F devices blinking, which causes user confusion. In order to
minimize user confusion, sent fake registration call to U2F devices for
GetAssertion request with empty allow list.

Bug: 860062
Change-Id: I8e62d01e46cd90f393035149254286b58a932e78
Reviewed-on: https://chromium-review.googlesource.com/1132456
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575363}

content/browser/webauth/authenticator_impl_unittest.cc

@@ -877,7 +877,8 @@ TEST_F(AuthenticatorImplTest, GetAssertionWithEmptyAllowCredentials) {
   base::RunLoop().RunUntilIdle();
   task_runner->FastForwardBy(base::TimeDelta::FromMinutes(1));
   callback_receiver.WaitForCallback();
-  EXPECT_EQ(AuthenticatorStatus::NOT_ALLOWED_ERROR, callback_receiver.status());
+  EXPECT_EQ(AuthenticatorStatus::CREDENTIAL_NOT_RECOGNIZED,
+            callback_receiver.status());
 }
 
 TEST_F(AuthenticatorImplTest, MakeCredentialAlreadyRegistered) {

content/browser/webauth/webauth_browsertest.cc

@@ -67,6 +67,10 @@ constexpr char kTimeoutErrorMessage[] =
     "webauth: NotAllowedError: The operation either timed out or was not "
     "allowed. See: https://w3c.github.io/webauthn/#sec-assertion-privacy.";
 
+constexpr char kInvalidStateErrorMessage[] =
+    "webauth: InvalidStateError: The user attempted to use an authenticator "
+    "that recognized none of the provided credentials.";
+
 constexpr char kRelyingPartySecurityErrorMessage[] =
     "webauth: SecurityError: The relying party ID 'localhost' is not a "
     "registrable domain suffix of, nor equal to 'https://www.acme.com";
@@ -803,7 +807,7 @@ IN_PROC_BROWSER_TEST_F(WebAuthJavascriptClientBrowserTest,
   ASSERT_TRUE(content::ExecuteScriptAndExtractString(
       shell()->web_contents()->GetMainFrame(),
       BuildGetCallWithParameters(parameters), &result));
-  ASSERT_EQ(kTimeoutErrorMessage, result);
+  ASSERT_EQ(kInvalidStateErrorMessage, result);
 }
 
 // WebAuthBrowserBleDisabledTest

device/fido/get_assertion_task.cc

@@ -71,8 +71,10 @@ void GetAssertionTask::GetAssertion() {
 }
 
 void GetAssertionTask::U2fSign() {
+  DCHECK(!device()->device_info());
   device()->set_supported_protocol(ProtocolVersion::kU2f);
-  if (!IsConvertibleToU2fSignCommand(request_)) {
+
+  if (!CheckUserVerificationCompatible()) {
     std::move(callback_).Run(CtapDeviceResponseCode::kCtap2ErrOther,
                              base::nullopt);
     return;

device/fido/get_assertion_task_unittest.cc

@@ -341,6 +341,9 @@ TEST_F(FidoGetAssertionTaskTest, TestU2fSignRequestWithEmptyAllowedList) {
   auto device = std::make_unique<MockFidoDevice>();
   device->ExpectCtap2CommandAndRespondWith(
       CtapRequestCommand::kAuthenticatorGetInfo, base::nullopt);
+  device->ExpectRequestAndRespondWith(
+      test_data::kU2fFakeRegisterCommand,
+      test_data::kApduEncodedNoErrorSignResponse);
 
   auto task = std::make_unique<GetAssertionTask>(
       device.get(), std::move(request),
@@ -348,7 +351,7 @@ TEST_F(FidoGetAssertionTaskTest, TestU2fSignRequestWithEmptyAllowedList) {
 
   get_assertion_callback_receiver().WaitForCallback();
   EXPECT_EQ(device->supported_protocol(), ProtocolVersion::kU2f);
-  EXPECT_EQ(CtapDeviceResponseCode::kCtap2ErrOther,
+  EXPECT_EQ(CtapDeviceResponseCode::kCtap2ErrCredentialNotValid,
             get_assertion_callback_receiver().status());
   EXPECT_FALSE(get_assertion_callback_receiver().value());
 }

device/fido/u2f_sign_operation.cc

@@ -27,15 +27,30 @@ U2fSignOperation::U2fSignOperation(FidoDevice* device,
 U2fSignOperation::~U2fSignOperation() = default;
 
 void U2fSignOperation::Start() {
-  // Non-empty allow list in |request_| is checked from above
-  // IsConvertibleToU2fSignCommand().
-  auto it = request().allow_list()->cbegin();
+  const auto& allow_list = request().allow_list();
+  if (allow_list && !allow_list->empty()) {
+    const auto it = allow_list->cbegin();
+    DispatchDeviceRequest(
+        ConvertToU2fSignCommand(request(), ApplicationParameterType::kPrimary,
+                                it->id(), true /* is_check_only */),
+        base::BindOnce(&U2fSignOperation::OnCheckForKeyHandlePresence,
+                       weak_factory_.GetWeakPtr(),
+                       ApplicationParameterType::kPrimary, it));
+  } 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 if the
+    // user has provided user presence.
+    SendFakeEnrollment();
+  }
+}
+
+void U2fSignOperation::SendFakeEnrollment() {
   DispatchDeviceRequest(
-      ConvertToU2fSignCommand(request(), ApplicationParameterType::kPrimary,
-                              it->id(), true /* is_check_only */),
-      base::BindOnce(&U2fSignOperation::OnCheckForKeyHandlePresence,
-                     weak_factory_.GetWeakPtr(),
-                     ApplicationParameterType::kPrimary, it));
+      ConstructBogusU2fRegistrationCommand(),
+      base::BindOnce(&U2fSignOperation::OnSignResponseReceived,
+                     weak_factory_.GetWeakPtr(), true /* is_fake_enrollment */,
+                     ApplicationParameterType::kPrimary,
+                     std::vector<uint8_t>()));
 }
 
 void U2fSignOperation::RetrySign(
@@ -160,13 +175,9 @@ void U2fSignOperation::OnCheckForKeyHandlePresence(
       } else {
         // No provided key was accepted by this device. Send registration
         // (Fake enroll) request to device.
-        DispatchDeviceRequest(
-            ConstructBogusU2fRegistrationCommand(),
-            base::BindOnce(
-                &U2fSignOperation::OnSignResponseReceived,
-                weak_factory_.GetWeakPtr(), true /* is_fake_enrollment */,
-                ApplicationParameterType::kPrimary, std::vector<uint8_t>()));
+        SendFakeEnrollment();
       }
+
       break;
     }
     default:

device/fido/u2f_sign_operation.h

@@ -46,6 +46,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) U2fSignOperation
   using AllowedListIterator =
       std::vector<PublicKeyCredentialDescriptor>::const_iterator;
 
+  void SendFakeEnrollment();
   void RetrySign(bool is_fake_enrollment,
                  ApplicationParameterType application_parameter_type,
                  const std::vector<uint8_t>& key_handle);

third_party/WebKit/LayoutTests/TestExpectations

@@ -4396,8 +4396,6 @@ crbug.com/826936 external/wpt/webauthn/getcredential-extensions.https.html [ Pas
 crbug.com/826936 external/wpt/webauthn/getcredential-passing.https.html [ Pass Timeout ]
 crbug.com/826936 external/wpt/webauthn/getcredential-timeout.https.html [ Pass Timeout ]
 crbug.com/826936 external/wpt/webauthn/createcredential-pubkeycredparams.https.html [ Pass Timeout ]
-crbug.com/826936 http/tests/credentialmanager/credentialscontainer-get-from-nested-frame.html [ Pass Timeout ]
-crbug.com/826936 http/tests/credentialmanager/credentialscontainer-get-with-virtual-authenticator.html [ Pass Timeout ]
 
 # WebRTC with Unified Plan
 crbug.com/828866 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/RTCPeerConnection-setRemoteDescription-tracks.https.html [ Timeout ]

third_party/WebKit/LayoutTests/http/tests/credentialmanager/credentialscontainer-get-from-nested-frame.html

@@ -120,9 +120,9 @@ promise_test(async t => {
   someOtherCredential.id = new TextEncoder().encode("someOtherCredential");
   delete customGetAssertionOptions.allowCredentials;
 
-  return promise_rejects(t, "NotSupportedError",
+  return promise_rejects(t, "InvalidStateError",
     navigator.credentials.get({ publicKey : customGetAssertionOptions}));
-}, "navigator.credentials.get() with empty allowCredentials returns NotSupportedError");
+}, "navigator.credentials.get() with empty allowCredentials returns InvalidStateError if the user has provided user presence");
 
 promise_test(t => {
   return navigator.credentials.test.clearAuthenticators();

third_party/WebKit/LayoutTests/http/tests/credentialmanager/credentialscontainer-get-with-virtual-authenticator.html

@@ -43,9 +43,9 @@ promise_test(async t => {
   someOtherCredential.id = new TextEncoder().encode("someOtherCredential");
   delete customGetAssertionOptions.allowCredentials;
 
-  return promise_rejects(t, "NotSupportedError",
+  return promise_rejects(t, "InvalidStateError",
     navigator.credentials.get({ publicKey : customGetAssertionOptions}));
-}, "navigator.credentials.get() with empty allowCredentials returns NotSupportedError");
+}, "navigator.credentials.get() with empty allowCredentials returns InvalidStateError if the user has provided user presence");
 
 promise_test(t => {
   return navigator.credentials.test.clearAuthenticators();