Disable resident key credential creation and usage.

Prevents create() calls where requireResidentKey = true.

Also disables resident credentials in getAssertion calls entirely until
they can be fully and properly supported. (Previously, if an empty list
was passed to getAssertion() and resident credentials for
that RP existed, then a random single credential would be passed back.)


Bug: 896404
Change-Id: Ic6efcb99891825f929efba616ff5cf5788236a7d
Reviewed-on: https://chromium-review.googlesource.com/c/1287089
Commit-Queue: Kim Paulhamus <kpaulhamus@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600858}

components/password_manager/content/common/credential_manager_mojom_traits.cc

@@ -95,6 +95,7 @@ bool EnumTraits<blink::mojom::CredentialManagerError,
     case blink::mojom::CredentialManagerError::CREDENTIAL_NOT_RECOGNIZED:
     case blink::mojom::CredentialManagerError::NOT_IMPLEMENTED:
     case blink::mojom::CredentialManagerError::NOT_FOCUSED:
+    case blink::mojom::CredentialManagerError::RESIDENT_CREDENTIALS_UNSUPPORTED:
     case blink::mojom::CredentialManagerError::UNKNOWN:
       *output = password_manager::CredentialManagerError::UNKNOWN;
       return true;

content/browser/webauth/authenticator_impl.cc

@@ -583,6 +583,16 @@ void AuthenticatorImpl::MakeCredential(
     return;
   }
 
+  if (options->authenticator_selection &&
+      options->authenticator_selection->require_resident_key) {
+    // Disallow the creation of resident credentials.
+    InvokeCallbackAndCleanup(
+        std::move(callback),
+        blink::mojom::AuthenticatorStatus::RESIDENT_CREDENTIALS_UNSUPPORTED,
+        nullptr, Focus::kDontCheck);
+    return;
+  }
+
   DCHECK(make_credential_response_callback_.is_null());
   make_credential_response_callback_ = std::move(callback);
 
@@ -676,6 +686,15 @@ void AuthenticatorImpl::GetAssertion(
     return;
   }
 
+  if (options->allow_credentials.empty()) {
+    // Chrome currently does not support any resident keys.
+    InvokeCallbackAndCleanup(
+        std::move(callback),
+        blink::mojom::AuthenticatorStatus::RESIDENT_CREDENTIALS_UNSUPPORTED,
+        nullptr);
+    return;
+  }
+
   if (options->appid) {
     alternative_application_parameter_ =
         ProcessAppIdExtension(*options->appid, caller_origin);

content/browser/webauth/authenticator_impl_unittest.cc

@@ -527,8 +527,8 @@ TEST_F(AuthenticatorImplTest, MakeCredentialUserVerification) {
   EXPECT_EQ(AuthenticatorStatus::NOT_ALLOWED_ERROR, callback_receiver.status());
 }
 
-// Test that MakeCredential request times out with NOT_ALLOWED_ERROR if resident
-// key is requested for U2F devices on create().
+// Test that MakeCredential request returns if resident
+// key is requested on create().
 TEST_F(AuthenticatorImplTest, MakeCredentialResidentKey) {
   SimulateNavigation(GURL(kTestOrigin1));
   device::test::ScopedVirtualFidoDevice scoped_virtual_device;
@@ -547,7 +547,10 @@ TEST_F(AuthenticatorImplTest, MakeCredentialResidentKey) {
   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::RESIDENT_CREDENTIALS_UNSUPPORTED,
+            callback_receiver.status());
+
+  // TODO add CTAP device
 }
 
 // Test that MakeCredential request times out with NOT_ALLOWED_ERROR if a
@@ -932,7 +935,7 @@ TEST_F(AuthenticatorImplTest, GetAssertionWithEmptyAllowCredentials) {
   base::RunLoop().RunUntilIdle();
   task_runner->FastForwardBy(base::TimeDelta::FromMinutes(1));
   callback_receiver.WaitForCallback();
-  EXPECT_EQ(AuthenticatorStatus::CREDENTIAL_NOT_RECOGNIZED,
+  EXPECT_EQ(AuthenticatorStatus::RESIDENT_CREDENTIALS_UNSUPPORTED,
             callback_receiver.status());
 }
 

content/browser/webauth/webauth_browsertest.cc

@@ -67,9 +67,9 @@ 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 kResidentCredentialsErrorMessage[] =
+    "webauth: NotSupportedError: Resident credentials or empty "
+    "'allowCredentials' lists are not supported at this time.";
 
 constexpr char kRelyingPartySecurityErrorMessage[] =
     "webauth: SecurityError: The relying party ID 'localhost' is not a "
@@ -736,7 +736,7 @@ IN_PROC_BROWSER_TEST_F(WebAuthJavascriptClientBrowserTest,
         shell()->web_contents()->GetMainFrame(),
         BuildCreateCallWithParameters(parameters), &result));
 
-    ASSERT_EQ(kTimeoutErrorMessage, result);
+    ASSERT_EQ(kResidentCredentialsErrorMessage, result);
   }
 }
 
@@ -807,7 +807,7 @@ IN_PROC_BROWSER_TEST_F(WebAuthJavascriptClientBrowserTest,
   ASSERT_TRUE(content::ExecuteScriptAndExtractString(
       shell()->web_contents()->GetMainFrame(),
       BuildGetCallWithParameters(parameters), &result));
-  ASSERT_EQ(kInvalidStateErrorMessage, result);
+  ASSERT_EQ(kResidentCredentialsErrorMessage, result);
 }
 
 // WebAuthBrowserBleDisabledTest

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

@@ -41,6 +41,13 @@ promise_test(async t => {
     navigator.credentials.create({ publicKey : customMakeCredOptions}));
 }, "navigator.credentials.create() with empty rpId returns SecurityError");
 
+promise_test(async t => {
+  var customMakeCredOptions = deepCopy(MAKE_CREDENTIAL_OPTIONS);
+  customMakeCredOptions.authenticatorSelection.requireResidentKey = true;
+  return promise_rejects(t, "NotSupportedError",
+    navigator.credentials.create({ publicKey : customMakeCredOptions}));
+}, "navigator.credentials.create() with resident keys is disabled");
+
 promise_test(t => {
   return navigator.credentials.test.clearAuthenticators();
 }, "Clean up testing environment.");

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, "InvalidStateError",
+  return promise_rejects(t, "NotSupportedError",
     navigator.credentials.get({ publicKey : customGetAssertionOptions}));
-}, "navigator.credentials.get() with empty allowCredentials returns InvalidStateError if the user has provided user presence");
+}, "navigator.credentials.get() with empty allowCredentials is disabled");
 
 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, "InvalidStateError",
+  return promise_rejects(t, "NotSupportedError",
     navigator.credentials.get({ publicKey : customGetAssertionOptions}));
-}, "navigator.credentials.get() with empty allowCredentials returns InvalidStateError if the user has provided user presence");
+}, "navigator.credentials.get() with empty allowCredentials returns NotSupportedError");
 
 promise_test(t => {
   return navigator.credentials.test.clearAuthenticators();

third_party/blink/public/platform/modules/credentialmanager/credential_manager.mojom

@@ -30,6 +30,7 @@ enum CredentialManagerError {
   CREDENTIAL_NOT_RECOGNIZED,
   NOT_IMPLEMENTED,
   NOT_FOCUSED,
+  RESIDENT_CREDENTIALS_UNSUPPORTED,
   ANDROID_ALGORITHM_UNSUPPORTED,
   ANDROID_EMPTY_ALLOW_CREDENTIALS,
   ANDROID_NOT_SUPPORTED_ERROR,

third_party/blink/public/platform/modules/webauthn/authenticator.mojom

@@ -21,6 +21,7 @@ enum AuthenticatorStatus {
   CREDENTIAL_NOT_RECOGNIZED,
   NOT_IMPLEMENTED,
   NOT_FOCUSED,
+  RESIDENT_CREDENTIALS_UNSUPPORTED,
   USER_VERIFICATION_UNSUPPORTED,
   ALGORITHM_UNSUPPORTED,
   EMPTY_ALLOW_CREDENTIALS,

third_party/blink/renderer/modules/credentialmanager/credential_manager_type_converters.cc

@@ -130,6 +130,9 @@ TypeConverter<CredentialManagerError, AuthenticatorStatus>::Convert(
       return CredentialManagerError::NOT_IMPLEMENTED;
     case blink::mojom::blink::AuthenticatorStatus::NOT_FOCUSED:
       return CredentialManagerError::NOT_FOCUSED;
+    case blink::mojom::blink::AuthenticatorStatus::
+        RESIDENT_CREDENTIALS_UNSUPPORTED:
+      return CredentialManagerError::RESIDENT_CREDENTIALS_UNSUPPORTED;
     case blink::mojom::blink::AuthenticatorStatus::ALGORITHM_UNSUPPORTED:
       return CredentialManagerError::ANDROID_ALGORITHM_UNSUPPORTED;
     case blink::mojom::blink::AuthenticatorStatus::EMPTY_ALLOW_CREDENTIALS:

third_party/blink/renderer/modules/credentialmanager/credentials_container.cc

@@ -241,6 +241,11 @@ DOMException* CredentialManagerErrorToDOMException(
       return DOMException::Create(DOMExceptionCode::kNotAllowedError,
                                   "The operation is not allowed at this time "
                                   "because the page does not have focus.");
+    case CredentialManagerError::RESIDENT_CREDENTIALS_UNSUPPORTED:
+      return DOMException::Create(DOMExceptionCode::kNotSupportedError,
+                                  "Resident credentials or empty "
+                                  "'allowCredentials' lists are not supported "
+                                  "at this time.");
     case CredentialManagerError::ANDROID_ALGORITHM_UNSUPPORTED:
       return DOMException::Create(DOMExceptionCode::kNotSupportedError,
                                   "None of the algorithms specified in "