Use specific error messages where possible.

Since the current implementation doesn't support non-U2F authenticators,
all non-U2F scenarios can be immediately denied with NotSupportedError.

This CL also moves handling for empty allowCredentials (part of the
aforementioned errors) out of u2f_sign, since u2f_sign should never be
called with empty allowCredentials.

Bug: 819256
Change-Id: I991949919fbe7a5e1e9c123ee750d80c3d25e9c3
Reviewed-on: https://chromium-review.googlesource.com/1006482
Commit-Queue: Kim Paulhamus <kpaulhamus@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550346}

components/password_manager/content/common/credential_manager_mojom_traits.cc

@@ -88,9 +88,17 @@ bool EnumTraits<password_manager::mojom::CredentialManagerError,
           password_manager::CredentialManagerError::PASSWORDSTOREUNAVAILABLE;
       return true;
     case password_manager::mojom::CredentialManagerError::NOT_ALLOWED:
-    case password_manager::mojom::CredentialManagerError::NOT_SUPPORTED:
+    case password_manager::mojom::CredentialManagerError::
+        AUTHENTICATOR_CRITERIA_UNSUPPORTED:
+    case password_manager::mojom::CredentialManagerError::ALGORITHM_UNSUPPORTED:
+    case password_manager::mojom::CredentialManagerError::
+        EMPTY_ALLOW_CREDENTIALS:
+    case password_manager::mojom::CredentialManagerError::
+        USER_VERIFICATION_UNSUPPORTED:
     case password_manager::mojom::CredentialManagerError::INVALID_DOMAIN:
-    case password_manager::mojom::CredentialManagerError::INVALID_STATE:
+    case password_manager::mojom::CredentialManagerError::CREDENTIAL_EXCLUDED:
+    case password_manager::mojom::CredentialManagerError::
+        CREDENTIAL_NOT_RECOGNIZED:
     case password_manager::mojom::CredentialManagerError::NOT_IMPLEMENTED:
     case password_manager::mojom::CredentialManagerError::NOT_FOCUSED:
     case password_manager::mojom::CredentialManagerError::UNKNOWN:

content/browser/webauth/authenticator_impl.cc

@@ -462,17 +462,17 @@ void AuthenticatorImpl::MakeCredential(
       !AreOptionsSupportedByU2fAuthenticators(options)) {
     InvokeCallbackAndCleanup(
         std::move(callback),
-        webauth::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR, nullptr);
+        webauth::mojom::AuthenticatorStatus::AUTHENTICATOR_CRITERIA_UNSUPPORTED,
+        nullptr);
     return;
   }
 
-  // TODO(crbug.com/819256): Improve messages for "Not Supported" errors.
   if (!base::FeatureList::IsEnabled(features::kWebAuthCtap2) &&
       !IsAlgorithmSupportedByU2fAuthenticators(
           options->public_key_parameters)) {
     InvokeCallbackAndCleanup(
         std::move(callback),
-        webauth::mojom::AuthenticatorStatus::NOT_SUPPORTED_ERROR, nullptr);
+        webauth::mojom::AuthenticatorStatus::ALGORITHM_UNSUPPORTED, nullptr);
     return;
   }
 
@@ -580,7 +580,8 @@ void AuthenticatorImpl::GetAssertion(
           webauth::mojom::UserVerificationRequirement::REQUIRED) {
     InvokeCallbackAndCleanup(
         std::move(callback),
-        webauth::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR, nullptr);
+        webauth::mojom::AuthenticatorStatus::USER_VERIFICATION_UNSUPPORTED,
+        nullptr);
     return;
   }
 
@@ -601,13 +602,32 @@ void AuthenticatorImpl::GetAssertion(
     echo_appid_extension_ = true;
   }
 
-  DCHECK(get_assertion_response_callback_.is_null());
-  get_assertion_response_callback_ = std::move(callback);
-
-  // Pass along valid keys from allow_list, if any.
+  // Pass along valid keys from allow_list.
   std::vector<std::vector<uint8_t>> handles =
       FilterCredentialList(std::move(options->allow_credentials));
 
+  // There are two different descriptions of what should happen when
+  // "allowCredentials" is empty.
+  // a) WebAuthN 6.2.3 step 6[1] implies "NotAllowedError".
+  // b) CTAP step 7.2 step 2[2] says the device should error out with
+  // "CTAP2_ERR_OPTION_NOT_SUPPORTED". This also resolves to "NotAllowedError".
+  // The behavior in both cases is consistent with the current implementation.
+  // TODO(crbug.com/831712): When CTAP2 authenticators are supported, this check
+  // should be enforced by handlers in fido/device on a per-device basis.
+
+  // [1] https://w3c.github.io/webauthn/#authenticatorgetassertion
+  // [2]
+  // https://fidoalliance.org/specs/fido-v2.0-ps-20170927/fido-client-to-authenticator-protocol-v2.0-ps-20170927.html
+  if (handles.empty()) {
+    InvokeCallbackAndCleanup(
+        std::move(callback),
+        webauth::mojom::AuthenticatorStatus::EMPTY_ALLOW_CREDENTIALS, nullptr);
+    return;
+  }
+
+  DCHECK(get_assertion_response_callback_.is_null());
+  get_assertion_response_callback_ = std::move(callback);
+
   timer_->Start(
       FROM_HERE, options->adjusted_timeout,
       base::Bind(&AuthenticatorImpl::OnTimeout, base::Unretained(this)));
@@ -666,7 +686,7 @@ void AuthenticatorImpl::OnRegisterResponse(
       // |exclude_credentials|.
       InvokeCallbackAndCleanup(
           std::move(make_credential_response_callback_),
-          webauth::mojom::AuthenticatorStatus::INVALID_STATE, nullptr);
+          webauth::mojom::AuthenticatorStatus::CREDENTIAL_EXCLUDED, nullptr);
       return;
     case device::FidoReturnCode::kAuthenticatorResponseInvalid:
       // The response from the authenticator was corrupted.
@@ -759,7 +779,8 @@ void AuthenticatorImpl::OnSignResponse(
       // No authenticators contained the credential.
       InvokeCallbackAndCleanup(
           std::move(get_assertion_response_callback_),
-          webauth::mojom::AuthenticatorStatus::INVALID_STATE, nullptr);
+          webauth::mojom::AuthenticatorStatus::CREDENTIAL_NOT_RECOGNIZED,
+          nullptr);
       return;
     case device::FidoReturnCode::kAuthenticatorResponseInvalid:
       // The response from the authenticator was corrupted.

content/browser/webauth/authenticator_impl_unittest.cc

@@ -383,11 +383,11 @@ TEST_F(AuthenticatorImplTest, MakeCredentialOriginAndRpIds) {
     TestMakeCredentialCallback cb;
     authenticator->MakeCredential(std::move(options), cb.callback());
     cb.WaitForCallback();
-    EXPECT_EQ(AuthenticatorStatus::NOT_SUPPORTED_ERROR, cb.status());
+    EXPECT_EQ(AuthenticatorStatus::ALGORITHM_UNSUPPORTED, cb.status());
   }
 }
 
-// Test that service returns NOT_SUPPORTED_ERROR if no parameters contain
+// Test that service returns ALGORITHM_UNSUPPORTED if no parameters contain
 // a supported algorithm.
 TEST_F(AuthenticatorImplTest, MakeCredentialNoSupportedAlgorithm) {
   SimulateNavigation(GURL(kTestOrigin1));
@@ -400,11 +400,11 @@ TEST_F(AuthenticatorImplTest, MakeCredentialNoSupportedAlgorithm) {
   TestMakeCredentialCallback cb;
   authenticator->MakeCredential(std::move(options), cb.callback());
   cb.WaitForCallback();
-  EXPECT_EQ(AuthenticatorStatus::NOT_SUPPORTED_ERROR, cb.status());
+  EXPECT_EQ(AuthenticatorStatus::ALGORITHM_UNSUPPORTED, cb.status());
 }
 
-// Test that service returns NOT_ALLOWED_ERROR if user verification is
-// REQUIRED for get().
+// Test that service returns USER_VERIFICATION_UNSUPPORTED if user verification
+// is REQUIRED for get().
 TEST_F(AuthenticatorImplTest, GetAssertionUserVerification) {
   SimulateNavigation(GURL(kTestOrigin1));
   AuthenticatorPtr authenticator = ConnectToAuthenticator();
@@ -416,11 +416,11 @@ TEST_F(AuthenticatorImplTest, GetAssertionUserVerification) {
   TestGetAssertionCallback cb;
   authenticator->GetAssertion(std::move(options), cb.callback());
   cb.WaitForCallback();
-  EXPECT_EQ(AuthenticatorStatus::NOT_ALLOWED_ERROR, cb.status());
+  EXPECT_EQ(AuthenticatorStatus::USER_VERIFICATION_UNSUPPORTED, cb.status());
 }
 
-// Test that service returns NOT_ALLOWED_ERROR if user verification is
-// REQUIRED for create().
+// Test that service returns AUTHENTICATOR_CRITERIA_UNSUPPORTED if user
+// verification is REQUIRED for create().
 TEST_F(AuthenticatorImplTest, MakeCredentialUserVerification) {
   SimulateNavigation(GURL(kTestOrigin1));
   AuthenticatorPtr authenticator = ConnectToAuthenticator();
@@ -433,11 +433,12 @@ TEST_F(AuthenticatorImplTest, MakeCredentialUserVerification) {
   TestMakeCredentialCallback cb;
   authenticator->MakeCredential(std::move(options), cb.callback());
   cb.WaitForCallback();
-  EXPECT_EQ(AuthenticatorStatus::NOT_ALLOWED_ERROR, cb.status());
+  EXPECT_EQ(AuthenticatorStatus::AUTHENTICATOR_CRITERIA_UNSUPPORTED,
+            cb.status());
 }
 
-// Test that service returns NOT_ALLOWED_ERROR if resident key is
-// requested for create().
+// Test that service returns AUTHENTICATOR_CRITERIA_UNSUPPORTED if resident key
+// is requested for create().
 TEST_F(AuthenticatorImplTest, MakeCredentialResidentKey) {
   SimulateNavigation(GURL(kTestOrigin1));
   AuthenticatorPtr authenticator = ConnectToAuthenticator();
@@ -449,11 +450,12 @@ TEST_F(AuthenticatorImplTest, MakeCredentialResidentKey) {
   TestMakeCredentialCallback cb;
   authenticator->MakeCredential(std::move(options), cb.callback());
   cb.WaitForCallback();
-  EXPECT_EQ(AuthenticatorStatus::NOT_ALLOWED_ERROR, cb.status());
+  EXPECT_EQ(AuthenticatorStatus::AUTHENTICATOR_CRITERIA_UNSUPPORTED,
+            cb.status());
 }
 
-// Test that service returns NOT_ALLOWED_ERROR if a platform authenticator is
-// requested for U2F.
+// Test that service returns AUTHENTICATOR_CRITERIA_UNSUPPORTED if a platform
+// authenticator is requested for U2F.
 TEST_F(AuthenticatorImplTest, MakeCredentialPlatformAuthenticator) {
   SimulateNavigation(GURL(kTestOrigin1));
   AuthenticatorPtr authenticator = ConnectToAuthenticator();
@@ -466,7 +468,8 @@ TEST_F(AuthenticatorImplTest, MakeCredentialPlatformAuthenticator) {
   TestMakeCredentialCallback cb;
   authenticator->MakeCredential(std::move(options), cb.callback());
   cb.WaitForCallback();
-  EXPECT_EQ(AuthenticatorStatus::NOT_ALLOWED_ERROR, cb.status());
+  EXPECT_EQ(AuthenticatorStatus::AUTHENTICATOR_CRITERIA_UNSUPPORTED,
+            cb.status());
 }
 
 // Parses its arguments as JSON and expects that all the keys in the first are
@@ -612,7 +615,7 @@ TEST_F(AuthenticatorImplTest, AppIdExtension) {
     SCOPED_TRACE(std::string(test_case.origin) + " " +
                  std::string(test_case.claimed_authority));
 
-    EXPECT_EQ(AuthenticatorStatus::INVALID_STATE,
+    EXPECT_EQ(AuthenticatorStatus::CREDENTIAL_NOT_RECOGNIZED,
               TryAuthenticationWithAppId(test_case.origin,
                                          test_case.claimed_authority));
   }
@@ -702,7 +705,7 @@ TEST_F(AuthenticatorImplTest, OversizedCredentialId) {
     if (should_be_valid) {
       EXPECT_EQ(AuthenticatorStatus::SUCCESS, cb.status());
     } else {
-      EXPECT_EQ(AuthenticatorStatus::INVALID_STATE, cb.status());
+      EXPECT_EQ(AuthenticatorStatus::CREDENTIAL_NOT_RECOGNIZED, cb.status());
     }
   }
 }
@@ -773,6 +776,23 @@ TEST_F(AuthenticatorImplTest, TestU2fDeviceDoesNotSupportGetAssertion) {
   EXPECT_EQ(AuthenticatorStatus::NOT_ALLOWED_ERROR, cb.status());
 }
 
+TEST_F(AuthenticatorImplTest, GetAssertionWithEmptyAllowCredentials) {
+  device::test::ScopedVirtualFidoDevice scoped_virtual_device;
+  TestServiceManagerContext service_manager_context;
+
+  SimulateNavigation(GURL(kTestOrigin1));
+  AuthenticatorPtr authenticator = ConnectToAuthenticator();
+  PublicKeyCredentialRequestOptionsPtr options =
+      GetTestPublicKeyCredentialRequestOptions();
+  options->allow_credentials.clear();
+
+  TestGetAssertionCallback cb;
+  authenticator->GetAssertion(std::move(options), cb.callback());
+  cb.WaitForCallback();
+
+  EXPECT_EQ(AuthenticatorStatus::EMPTY_ALLOW_CREDENTIALS, cb.status());
+}
+
 enum class IndividualAttestation {
   REQUESTED,
   NOT_REQUESTED,

content/browser/webauth/webauth_browsertest.cc

@@ -57,16 +57,33 @@ using TestGetCallbackReceiver = ::device::test::StatusAndValueCallbackReceiver<
     AuthenticatorStatus,
     GetAssertionAuthenticatorResponsePtr>;
 
-constexpr char kNotAllowedErrorMessage[] =
-    "NotAllowedError: The operation either timed out or was not allowed. See: "
-    "https://w3c.github.io/webauthn/#sec-assertion-privacy.";
-
 constexpr char kRelyingPartySecurityErrorMessage[] =
     "SecurityError: The relying party ID 'localhost' is not a registrable "
     "domain suffix of, nor equal to 'https://www.example.com";
 
-constexpr char kNotSupportedErrorMessage[] =
-    "NotSupportedError: Parameters for this operation are not supported.";
+constexpr char kAlgorithmUnsupportedErrorMessage[] =
+    "NotSupportedError: None of the algorithms specified in "
+    "`pubKeyCredParams` are compatible with "
+    "CTAP1/U2F authenticators, and CTAP2 "
+    "authenticators are not yet supported.";
+
+constexpr char kAuthenticatorCriteriaErrorMessage[] =
+    "NotSupportedError: The specified `authenticatorSelection` "
+    "criteria cannot be fulfilled by CTAP1/U2F "
+    "authenticators, and CTAP2 authenticators "
+    "are not yet supported.";
+
+constexpr char kUserVerificationErrorMessage[] =
+    "NotSupportedError: The specified `userVerification` "
+    "requirement cannot be fulfilled by "
+    "CTAP1/U2F authenticators, and CTAP2 "
+    "authenticators are not yet supported.";
+
+constexpr char kEmptyAllowCredentialsErrorMessage[] =
+    "NotSupportedError: The `allowCredentials` list cannot be left "
+    "empty for CTAP1/U2F authenticators, and "
+    "support for CTAP2 authenticators is not yet "
+    "implemented.";
 
 // Templates to be used with base::ReplaceStringPlaceholders. Can be
 // modified to include up to 9 replacements. The default values for
@@ -121,11 +138,26 @@ constexpr char kGetPublicKeyTemplate[] =
     "  rp: 'example.com',"
     "  timeout: 60000,"
     "  userVerification: '$1',"
-    "  allowCredentials: [{ type: 'public-key',"
-    "     id: new TextEncoder().encode('allowedCredential'),"
-    "     transports: ['usb', 'nfc', 'ble']}] }"
+    "  $2}"
     "}).catch(c => window.domAutomationController.send(c.toString()));";
 
+// Default values for kGetPublicKeyTemplate.
+struct GetParameters {
+  const char* user_verification = kPreferredVerification;
+  const char* allow_credentials =
+      "allowCredentials: [{ type: 'public-key',"
+      "     id: new TextEncoder().encode('allowedCredential'),"
+      "     transports: ['usb', 'nfc', 'ble']}]";
+};
+
+std::string BuildGetCallWithParameters(const GetParameters& parameters) {
+  std::vector<std::string> substititions;
+  substititions.push_back(parameters.user_verification);
+  substititions.push_back(parameters.allow_credentials);
+  return base::ReplaceStringPlaceholders(kGetPublicKeyTemplate, substititions,
+                                         nullptr);
+}
+
 // Helper class that executes the given |closure| the very last moment before
 // the next navigation commits in a given WebContents.
 class ClosureExecutorBeforeNavigationCommit
@@ -500,7 +532,7 @@ class WebAuthJavascriptClientBrowserTest : public WebAuthBrowserTestBase {
 };
 
 // Tests that when navigator.credentials.create() is called with user
-// verification required we get a NotAllowedError.
+// verification required we get a NotSupportedError.
 IN_PROC_BROWSER_TEST_F(WebAuthJavascriptClientBrowserTest,
                        CreatePublicKeyCredentialWithUserVerification) {
   CreateParameters parameters;
@@ -509,11 +541,11 @@ IN_PROC_BROWSER_TEST_F(WebAuthJavascriptClientBrowserTest,
   ASSERT_TRUE(content::ExecuteScriptAndExtractString(
       shell()->web_contents()->GetMainFrame(),
       BuildCreateCallWithParameters(parameters), &result));
-  ASSERT_EQ(kNotAllowedErrorMessage, result);
+  ASSERT_EQ(kAuthenticatorCriteriaErrorMessage, result);
 }
 
 // Tests that when navigator.credentials.create() is called with resident key
-// required, we get a NotAllowedError.
+// required, we get a NotSupportedError.
 IN_PROC_BROWSER_TEST_F(WebAuthJavascriptClientBrowserTest,
                        CreatePublicKeyCredentialWithResidentKeyRequired) {
   CreateParameters parameters;
@@ -523,20 +555,7 @@ IN_PROC_BROWSER_TEST_F(WebAuthJavascriptClientBrowserTest,
       shell()->web_contents()->GetMainFrame(),
       BuildCreateCallWithParameters(parameters), &result));
 
-  ASSERT_EQ(kNotAllowedErrorMessage, result);
-}
-
-// Tests that when navigator.credentials.get() is called with user verification
-// required, we get a NotAllowedError.
-IN_PROC_BROWSER_TEST_F(WebAuthJavascriptClientBrowserTest,
-                       GetPublicKeyCredentialUserVerification) {
-  const std::string kScript = base::ReplaceStringPlaceholders(
-      kGetPublicKeyTemplate, {"required"}, nullptr);
-
-  std::string result;
-  ASSERT_TRUE(content::ExecuteScriptAndExtractString(
-      shell()->web_contents()->GetMainFrame(), kScript, &result));
-  ASSERT_EQ(kNotAllowedErrorMessage, result);
+  ASSERT_EQ(kAuthenticatorCriteriaErrorMessage, result);
 }
 
 // Tests that when navigator.credentials.create() is called with an invalid
@@ -565,11 +584,11 @@ IN_PROC_BROWSER_TEST_F(WebAuthJavascriptClientBrowserTest,
       shell()->web_contents()->GetMainFrame(),
       BuildCreateCallWithParameters(parameters), &result));
 
-  ASSERT_EQ(kNotSupportedErrorMessage, result);
+  ASSERT_EQ(kAlgorithmUnsupportedErrorMessage, result);
 }
 
 // Tests that when navigator.credentials.create() is called with a
-// platform authenticator requested, we get a NotAllowedError.
+// platform authenticator requested, we get a NotSupportedError.
 IN_PROC_BROWSER_TEST_F(WebAuthJavascriptClientBrowserTest,
                        CreatePublicKeyCredentialPlatformAuthenticator) {
   CreateParameters parameters;
@@ -579,7 +598,33 @@ IN_PROC_BROWSER_TEST_F(WebAuthJavascriptClientBrowserTest,
       shell()->web_contents()->GetMainFrame(),
       BuildCreateCallWithParameters(parameters), &result));
 
-  ASSERT_EQ(kNotAllowedErrorMessage, result);
+  ASSERT_EQ(kAuthenticatorCriteriaErrorMessage, result);
+}
+
+// Tests that when navigator.credentials.get() is called with user verification
+// required, we get a NotSupportedError.
+IN_PROC_BROWSER_TEST_F(WebAuthJavascriptClientBrowserTest,
+                       GetPublicKeyCredentialUserVerification) {
+  GetParameters parameters;
+  parameters.user_verification = "required";
+  std::string result;
+  ASSERT_TRUE(content::ExecuteScriptAndExtractString(
+      shell()->web_contents()->GetMainFrame(),
+      BuildGetCallWithParameters(parameters), &result));
+  ASSERT_EQ(kUserVerificationErrorMessage, result);
+}
+
+// Tests that when navigator.credentials.get() is called with an empty
+// allowCredentials list, we get a NotSupportedError.
+IN_PROC_BROWSER_TEST_F(WebAuthJavascriptClientBrowserTest,
+                       GetPublicKeyCredentialEmptyAllowCredentialsList) {
+  GetParameters parameters;
+  parameters.allow_credentials = "";
+  std::string result;
+  ASSERT_TRUE(content::ExecuteScriptAndExtractString(
+      shell()->web_contents()->GetMainFrame(),
+      BuildGetCallWithParameters(parameters), &result));
+  ASSERT_EQ(kEmptyAllowCredentialsErrorMessage, result);
 }
 
 // WebAuthBrowserBleDisabledTest

device/fido/u2f_sign.cc

@@ -44,36 +44,18 @@ U2fSign::U2fSign(service_manager::Connector* connector,
                  std::move(registered_keys)),
       alt_application_parameter_(std::move(alt_application_parameter)),
       completion_callback_(std::move(completion_callback)),
-      weak_factory_(this) {}
+      weak_factory_(this) {
+  // U2F devices require at least one key handle.
+  // TODO(crbug.com/831712): When CTAP2 authenticators are supported, this check
+  // should be enforced by handlers in fido/device on a per-device basis.
+  DCHECK(registered_keys_.size() > 0);
+}
 
 U2fSign::~U2fSign() = default;
 
 void U2fSign::TryDevice() {
   DCHECK(current_device_);
 
-  // There are two different descriptions of what should happen when
-  // "allowCredentials" is empty.
-  // a) WebAuthN 6.2.3 step 6[1] implies "NotAllowedError". The current
-  // implementation returns this in response to receiving
-  // CONDITIONS_NOT_SATISFIED from TrySign.
-  // b) CTAP step 7.2 step 2[2] says the device should error out with
-  // "CTAP2_ERR_OPTION_NOT_SUPPORTED". This also resolves to "NotAllowedError".
-  // The behavior in both cases is consistent with the current implementation.
-  // When CTAP2 authenticators are supported, this check should be enforced by
-  // handlers in fido/device on a per-device basis.
-
-  // [1] https://w3c.github.io/webauthn/#authenticatorgetassertion
-  // [2]
-  // https://fidoalliance.org/specs/fido-v2.0-ps-20170927/fido-client-to-authenticator-protocol-v2.0-ps-20170927.html
-  if (registered_keys_.size() == 0) {
-    // Send registration (Fake enroll) if no keys were provided.
-    InitiateDeviceTransaction(
-        U2fRequest::GetBogusRegisterCommand(),
-        base::BindOnce(&U2fSign::OnTryDevice, weak_factory_.GetWeakPtr(),
-                       registered_keys_.cend(),
-                       ApplicationParameterType::kPrimary));
-    return;
-  }
   // Try signing current device with the first registered key.
   auto it = registered_keys_.cbegin();
   InitiateDeviceTransaction(
@@ -150,6 +132,10 @@ void U2fSign::OnTryDevice(std::vector<std::vector<uint8_t>>::const_iterator it,
       } else {
         // No provided key was accepted by this device. Send registration
         // (Fake enroll) request to device.
+        // We do this to prevent user confusion. Otherwise, if the device
+        // doesn't blink, the user might think it's broken rather than that
+        // it's not registered. Once the user consents to use the device,
+        // the relying party can inform them that it hasn't been registered.
         InitiateDeviceTransaction(
             U2fRequest::GetBogusRegisterCommand(),
             base::BindOnce(&U2fSign::OnTryDevice, weak_factory_.GetWeakPtr(),

third_party/WebKit/LayoutTests/http/tests/credentialmanager/credentialscontainer-create-basics.html

@@ -26,7 +26,6 @@ promise_test(t => {
         navigator.credentials.create({publicKey : MAKE_CREDENTIAL_OPTIONS}));
 }, "Verify that invalid domain error returned by mock is properly handled.");
 
-
 promise_test(t => {
   var customMakeCredOptions = deepCopy(MAKE_CREDENTIAL_OPTIONS);
   delete customMakeCredOptions.challenge;
@@ -119,17 +118,31 @@ promise_test(t => {
 
 promise_test(t => {
   mockAuthenticator.setAuthenticatorStatus(
-      webauth.mojom.AuthenticatorStatus.NOT_SUPPORTED_ERROR);
+      webauth.mojom.AuthenticatorStatus.AUTHENTICATOR_CRITERIA_UNSUPPORTED);
+  return promise_rejects(t, "NotSupportedError",
+      navigator.credentials.create({ publicKey : MAKE_CREDENTIAL_OPTIONS}));
+}, "Verify that authenticator criteria unsupported error returned by mock is properly handled.");
+
+promise_test(t => {
+  mockAuthenticator.setAuthenticatorStatus(
+      webauth.mojom.AuthenticatorStatus.ALGORITHM_UNSUPPORTED);
   return promise_rejects(t, "NotSupportedError",
       navigator.credentials.create({ publicKey : MAKE_CREDENTIAL_OPTIONS}));
-}, "Verify that not supported error returned by mock is properly handled.");
+}, "Verify that algorithm unsupported error returned by mock is properly handled.");
 
 promise_test(t => {
   mockAuthenticator.setAuthenticatorStatus(
-      webauth.mojom.AuthenticatorStatus.INVALID_STATE);
+      webauth.mojom.AuthenticatorStatus.CREDENTIAL_EXCLUDED);
   return promise_rejects(t, "InvalidStateError",
-    navigator.credentials.create({ publicKey : MAKE_CREDENTIAL_OPTIONS}));
-}, "Verify that InvalidState (duplicate registration) returned by mock is properly handled.");
+      navigator.credentials.create({ publicKey : MAKE_CREDENTIAL_OPTIONS}));
+}, "Verify that credential excluded error returned by mock is properly handled.");
+
+promise_test(t => {
+  mockAuthenticator.setAuthenticatorStatus(
+      webauth.mojom.AuthenticatorStatus.CREDENTIAL_NOT_RECOGNIZED);
+  return promise_rejects(t, "InvalidStateError",
+      navigator.credentials.create({ publicKey : MAKE_CREDENTIAL_OPTIONS}));
+}, "Verify that credential not recognized error returned by mock is properly handled.");
 
 promise_test(_ => {
   mockAuthenticator.reset();
@@ -174,7 +187,7 @@ promise_test(_ => {
 promise_test(t => {
   mockAuthenticator.reset();
   mockAuthenticator.setAuthenticatorStatus(
-      webauth.mojom.AuthenticatorStatus.NOT_SUPPORTED_ERROR);
+      webauth.mojom.AuthenticatorStatus.AUTHENTICATOR_CRITERIA_UNSUPPORTED);
   var customMakeCredOptions = deepCopy(MAKE_CREDENTIAL_OPTIONS);
   return promise_rejects(t, "NotSupportedError",
       navigator.credentials.create({publicKey: customMakeCredOptions}));
@@ -183,7 +196,7 @@ promise_test(t => {
 promise_test(t => {
   mockAuthenticator.reset();
   mockAuthenticator.setAuthenticatorStatus(
-      webauth.mojom.AuthenticatorStatus.NOT_SUPPORTED_ERROR);
+      webauth.mojom.AuthenticatorStatus.AUTHENTICATOR_CRITERIA_UNSUPPORTED);
   var customMakeCredOptions = deepCopy(MAKE_CREDENTIAL_OPTIONS);
   customMakeCredOptions.authenticatorSelection.userVerification = 'required';
   return promise_rejects(t, "NotSupportedError",
@@ -200,14 +213,14 @@ promise_test(_ => {
   });
 }, "navigator.credentials.create() with userVerification discouraged");
 
-promise_test(_ => {
+promise_test(t => {
   mockAuthenticator.reset();
-  mockAuthenticator.setDefaultsForSuccessfulMakeCredential();
+  mockAuthenticator.setAuthenticatorStatus(
+      webauth.mojom.AuthenticatorStatus.AUTHENTICATOR_CRITERIA_UNSUPPORTED);
   var customMakeCredOptions = deepCopy(MAKE_CREDENTIAL_OPTIONS);
   customMakeCredOptions.authenticatorSelection.authenticatorAttachment = 'platform';
-  return navigator.credentials.create({publicKey: customMakeCredOptions}).then(r => {
-      assertValidMakeCredentialResponse(r);
-  });
+  return promise_rejects(t, "NotSupportedError",
+      navigator.credentials.create({publicKey: customMakeCredOptions}));
 }, "navigator.credentials.create() with platform authenticatorAttachment");
 
 promise_test(_ => {

third_party/WebKit/LayoutTests/http/tests/credentialmanager/credentialscontainer-get-basics.html

@@ -145,13 +145,23 @@ promise_test(t => {
 
 promise_test(t => {
   mockAuthenticator.setAuthenticatorStatus(
-      webauth.mojom.AuthenticatorStatus.NOT_SUPPORTED_ERROR);
+      webauth.mojom.AuthenticatorStatus.USER_VERIFICATION_UNSUPPORTED);
   return promise_rejects(t, "NotSupportedError",
       navigator.credentials.get({ publicKey : GET_CREDENTIAL_OPTIONS}));
-}, "Verify that not supported error returned by mock is properly handled.");
+}, "Verify that user verification unsupported error returned by mock is properly handled.");
+
+promise_test(t => {
+  mockAuthenticator.setAuthenticatorStatus(
+      webauth.mojom.AuthenticatorStatus.EMPTY_ALLOW_CREDENTIALS);
+  var customGetCredentialOptions = deepCopy(GET_CREDENTIAL_OPTIONS);
+  delete customGetCredentialOptions.allowCredentials;
+  return promise_rejects(t, "NotSupportedError",
+      navigator.credentials.get({ publicKey : customGetCredentialOptions}));
+}, "Verify that empty allow credentials error returned by mock is properly handled.");
+
 
 promise_test(function(t) {
-  var customGetCredentialOptions = deepCopy(MAKE_CREDENTIAL_OPTIONS);
+  var customGetCredentialOptions = deepCopy(GET_CREDENTIAL_OPTIONS);
   delete customGetCredentialOptions.challenge;
   return promise_rejects(t, new TypeError(),
       navigator.credentials.get({publicKey: customGetCredentialOptions}));
@@ -160,8 +170,9 @@ promise_test(function(t) {
 promise_test(_ => {
   mockAuthenticator.reset();
   mockAuthenticator.setDefaultsForSuccessfulGetAssertion();
-  var customGetCredentialOptions = deepCopy(MAKE_CREDENTIAL_OPTIONS);
+  var customGetCredentialOptions = deepCopy(GET_CREDENTIAL_OPTIONS);
   delete customGetCredentialOptions.rpId;
+  console.log(customGetCredentialOptions);
   return navigator.credentials.get({publicKey: customGetCredentialOptions}).then(r => {
       assertValidGetCredentialResponse(r);
   });
@@ -170,7 +181,7 @@ promise_test(_ => {
 promise_test(_ => {
   mockAuthenticator.reset();
   mockAuthenticator.setDefaultsForSuccessfulGetAssertion();
-  var customGetCredentialOptions = deepCopy(MAKE_CREDENTIAL_OPTIONS);
+  var customGetCredentialOptions = deepCopy(GET_CREDENTIAL_OPTIONS);
   delete customGetCredentialOptions.userVerification;
   return navigator.credentials.get({publicKey: customGetCredentialOptions}).then(r => {
       assertValidGetCredentialResponse(r);

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

@@ -114,6 +114,16 @@ promise_test(async t => {
     navigator.credentials.get({ publicKey : customGetAssertionOptions}));
 }, "navigator.credentials.get() for unregistered device returns InvalidStateError");
 
+promise_test(async t => {
+  var customGetAssertionOptions = deepCopy(GET_CREDENTIAL_OPTIONS);
+  var someOtherCredential = deepCopy(ACCEPTABLE_CREDENTIAL);
+  someOtherCredential.id = new TextEncoder().encode("someOtherCredential");
+  delete customGetAssertionOptions.allowCredentials;
+
+  return promise_rejects(t, "NotSupportedError",
+    navigator.credentials.get({ publicKey : customGetAssertionOptions}));
+}, "navigator.credentials.get() with empty allowCredentials returns NotSupportedError");
+
 promise_test(t => {
   return navigator.credentials.test.clearAuthenticators();
 }, "Clean up testing environment.");

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

+<!DOCTYPE html>
+<title>Credential Manager: End-to-end tests for get() with a virtual authenticator.</title>
+<script src="../resources/testharness.js"></script>
+<script src="../resources/testharnessreport.js"></script>
+<script src="/gen/layout_test_data/mojo/public/js/mojo_bindings.js"></script>
+<script src="/gen/third_party/blink/public/platform/modules/credentialmanager/credential_manager.mojom.js"></script>
+<script src="/gen/third_party/blink/public/platform/modules/webauth/authenticator.mojom.js"></script>
+<script src="/gen/third_party/blink/public/platform/modules/webauth/virtual_authenticator.mojom.js"></script>
+<script src="resources/test-inputs.js"></script>
+<script src="resources/virtual-navigator-credentials.js"></script>
+<body>
+<script>
+
+if (document.location.host != "subdomain.example.test:8443") {
+  document.location = "https://subdomain.example.test:8443/credentialmanager/credentialscontainer-get-from-nested-frame.html";
+  promise_test(_ => new Promise(_ => {}), "Stall tests on the wrong host.");
+}
+
+promise_test(async _ => {
+  let authenticators = await navigator.credentials.test.authenticators();
+  assert_equals(authenticators.length, 0);
+  let testAuthenticator = await navigator.credentials.test.createAuthenticator();
+  assert_true(await testAuthenticator.generateAndRegisterKey(ACCEPTABLE_CREDENTIAL_ID, "subdomain.example.test"));
+}, "Set up the testing environment.");
+
+promise_test(async t => {
+  let testAuthenticator = await navigator.credentials.test.createAuthenticator();
+  assert_true(await testAuthenticator.generateAndRegisterKey(ACCEPTABLE_CREDENTIAL_ID, "subdomain.example.test"));
+  let keys = await testAuthenticator.registeredKeys();
+  assert_equals(keys.length, 1);
+  var customGetAssertionOptions = deepCopy(GET_CREDENTIAL_OPTIONS);
+  var someOtherCredential = deepCopy(ACCEPTABLE_CREDENTIAL);
+  someOtherCredential.id = new TextEncoder().encode("someOtherCredential");
+  customGetAssertionOptions.allowCredentials = [someOtherCredential];
+
+  return promise_rejects(t, "InvalidStateError",
+    navigator.credentials.get({ publicKey : customGetAssertionOptions}));
+}, "navigator.credentials.get() for unregistered device returns InvalidStateError");
+
+promise_test(async t => {
+  var customGetAssertionOptions = deepCopy(GET_CREDENTIAL_OPTIONS);
+  var someOtherCredential = deepCopy(ACCEPTABLE_CREDENTIAL);
+  someOtherCredential.id = new TextEncoder().encode("someOtherCredential");
+  delete customGetAssertionOptions.allowCredentials;
+
+  return promise_rejects(t, "NotSupportedError",
+    navigator.credentials.get({ publicKey : customGetAssertionOptions}));
+}, "navigator.credentials.get() with empty allowCredentials returns NotSupportedError");
+
+promise_test(t => {
+  return navigator.credentials.test.clearAuthenticators();
+}, "Clean up testing environment.");
+
+</script>

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

@@ -25,9 +25,13 @@ enum CredentialManagerError {
   PENDING_REQUEST,
   PASSWORD_STORE_UNAVAILABLE,
   NOT_ALLOWED,
-  NOT_SUPPORTED,
+  AUTHENTICATOR_CRITERIA_UNSUPPORTED,
+  ALGORITHM_UNSUPPORTED,
+  EMPTY_ALLOW_CREDENTIALS,
+  USER_VERIFICATION_UNSUPPORTED,
   INVALID_DOMAIN,
-  INVALID_STATE,
+  CREDENTIAL_EXCLUDED,
+  CREDENTIAL_NOT_RECOGNIZED,
   NOT_IMPLEMENTED,
   NOT_FOCUSED,
   UNKNOWN

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

@@ -16,9 +16,13 @@ enum AuthenticatorStatus {
   SUCCESS,
   PENDING_REQUEST,
   NOT_ALLOWED_ERROR,
-  NOT_SUPPORTED_ERROR,
+  AUTHENTICATOR_CRITERIA_UNSUPPORTED,
+  ALGORITHM_UNSUPPORTED,
+  EMPTY_ALLOW_CREDENTIALS,
+  USER_VERIFICATION_UNSUPPORTED,
   INVALID_DOMAIN,
-  INVALID_STATE,
+  CREDENTIAL_EXCLUDED,
+  CREDENTIAL_NOT_RECOGNIZED,
   NOT_IMPLEMENTED,
   NOT_FOCUSED,
   UNKNOWN_ERROR,

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

@@ -108,8 +108,16 @@ CredentialManagerError
 TypeConverter<CredentialManagerError, AuthenticatorStatus>::Convert(
     const AuthenticatorStatus& status) {
   switch (status) {
-    case webauth::mojom::blink::AuthenticatorStatus::NOT_SUPPORTED_ERROR:
-      return CredentialManagerError::NOT_SUPPORTED;
+    case webauth::mojom::blink::AuthenticatorStatus::
+        AUTHENTICATOR_CRITERIA_UNSUPPORTED:
+      return CredentialManagerError::AUTHENTICATOR_CRITERIA_UNSUPPORTED;
+    case webauth::mojom::blink::AuthenticatorStatus::ALGORITHM_UNSUPPORTED:
+      return CredentialManagerError::ALGORITHM_UNSUPPORTED;
+    case webauth::mojom::blink::AuthenticatorStatus::EMPTY_ALLOW_CREDENTIALS:
+      return CredentialManagerError::EMPTY_ALLOW_CREDENTIALS;
+    case webauth::mojom::blink::AuthenticatorStatus::
+        USER_VERIFICATION_UNSUPPORTED:
+      return CredentialManagerError::USER_VERIFICATION_UNSUPPORTED;
     case webauth::mojom::blink::AuthenticatorStatus::NOT_ALLOWED_ERROR:
       return CredentialManagerError::NOT_ALLOWED;
     case webauth::mojom::blink::AuthenticatorStatus::UNKNOWN_ERROR:
@@ -118,8 +126,10 @@ TypeConverter<CredentialManagerError, AuthenticatorStatus>::Convert(
       return CredentialManagerError::PENDING_REQUEST;
     case webauth::mojom::blink::AuthenticatorStatus::INVALID_DOMAIN:
       return CredentialManagerError::INVALID_DOMAIN;
-    case webauth::mojom::blink::AuthenticatorStatus::INVALID_STATE:
-      return CredentialManagerError::INVALID_STATE;
+    case webauth::mojom::blink::AuthenticatorStatus::CREDENTIAL_EXCLUDED:
+      return CredentialManagerError::CREDENTIAL_EXCLUDED;
+    case webauth::mojom::blink::AuthenticatorStatus::CREDENTIAL_NOT_RECOGNIZED:
+      return CredentialManagerError::CREDENTIAL_NOT_RECOGNIZED;
     case webauth::mojom::blink::AuthenticatorStatus::NOT_IMPLEMENTED:
       return CredentialManagerError::NOT_IMPLEMENTED;
     case webauth::mojom::blink::AuthenticatorStatus::NOT_FOCUSED:

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

@@ -247,16 +247,42 @@ DOMException* CredentialManagerErrorToDOMException(
           kNotAllowedError,
           "The operation either timed out or was not allowed. See: "
           "https://w3c.github.io/webauthn/#sec-assertion-privacy.");
-    case CredentialManagerError::NOT_SUPPORTED:
-      return DOMException::Create(
-          kNotSupportedError,
-          "Parameters for this operation are not supported.");
+    case CredentialManagerError::AUTHENTICATOR_CRITERIA_UNSUPPORTED:
+      return DOMException::Create(kNotSupportedError,
+                                  "The specified `authenticatorSelection` "
+                                  "criteria cannot be fulfilled by CTAP1/U2F "
+                                  "authenticators, and CTAP2 authenticators "
+                                  "are not yet supported.");
+    case CredentialManagerError::ALGORITHM_UNSUPPORTED:
+      return DOMException::Create(kNotSupportedError,
+                                  "None of the algorithms specified in "
+                                  "`pubKeyCredParams` are compatible with "
+                                  "CTAP1/U2F authenticators, and CTAP2 "
+                                  "authenticators are not yet supported.");
+    case CredentialManagerError::EMPTY_ALLOW_CREDENTIALS:
+      return DOMException::Create(kNotSupportedError,
+                                  "The `allowCredentials` list cannot be left "
+                                  "empty for CTAP1/U2F authenticators, and "
+                                  "support for CTAP2 authenticators is not yet "
+                                  "implemented.");
+    case CredentialManagerError::USER_VERIFICATION_UNSUPPORTED:
+      return DOMException::Create(kNotSupportedError,
+                                  "The specified `userVerification` "
+                                  "requirement cannot be fulfilled by "
+                                  "CTAP1/U2F authenticators, and CTAP2 "
+                                  "authenticators are not yet supported.");
     case CredentialManagerError::INVALID_DOMAIN:
       return DOMException::Create(kSecurityError, "This is an invalid domain.");
-    case CredentialManagerError::INVALID_STATE:
+    case CredentialManagerError::CREDENTIAL_EXCLUDED:
       return DOMException::Create(
           kInvalidStateError,
-          "Attempting to register an already-registered key.");
+          "The user attempted to register an authenticator that contains one "
+          "of the credentials already registered with the relying party.");
+    case CredentialManagerError::CREDENTIAL_NOT_RECOGNIZED:
+      return DOMException::Create(kInvalidStateError,
+                                  "The user attempted to use an authenticator "
+                                  "that recognized none of the provided "
+                                  "credentials.");
     case CredentialManagerError::NOT_IMPLEMENTED:
       return DOMException::Create(kNotSupportedError, "Not implemented");
     case CredentialManagerError::NOT_FOCUSED: