device/fido: fix an issue in authenticator error response handling

1) Change FidoRequestHandler to not terminate a WebAuthn request when a
request handler replies with the CTAP2_ERR_INVALID_CREDENTIAL or
CTAP2_ERR_CREDENTIAL_NOT_VALID CTAP2 error codes. These error codes do
not indicate that the user has interacted with the authenticator and
we therefore must not resolve the  WebAuthN request promise upon
receiving such an error.

2) Remove kCtap2ErrCredentialNotValid since
CTAP2_ERR_CREDENTIAL_NOT_VALID has recently been dropped from the CTAP2
spec.

3) Change references to kCtap2ErrCredentialNotValid in U2F code to
kCtap2ErrNoCredentials in order to not change behavior of that code with
regards to request canceling.

Change-Id: Ied8dd2c8b4af939d6b922c0007520b90d3a92388
Reviewed-on: https://chromium-review.googlesource.com/1185220
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Jun Choi <hongjunchoi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585575}

device/fido/fido_constants.h

@@ -102,7 +102,6 @@ enum class CtapDeviceResponseCode : uint8_t {
   kCtap2ErrTooManyElements = 0x17,
   kCtap2ErrExtensionNotSupported = 0x18,
   kCtap2ErrCredentialExcluded = 0x19,
-  kCtap2ErrCredentialNotValid = 0x20,
   kCtap2ErrProcesssing = 0x21,
   kCtap2ErrInvalidCredential = 0x22,
   kCtap2ErrUserActionPending = 0x23,
@@ -156,7 +155,6 @@ constexpr std::array<CtapDeviceResponseCode, 51> GetCtapResponseCodeList() {
           CtapDeviceResponseCode::kCtap2ErrTooManyElements,
           CtapDeviceResponseCode::kCtap2ErrExtensionNotSupported,
           CtapDeviceResponseCode::kCtap2ErrCredentialExcluded,
-          CtapDeviceResponseCode::kCtap2ErrCredentialNotValid,
           CtapDeviceResponseCode::kCtap2ErrProcesssing,
           CtapDeviceResponseCode::kCtap2ErrInvalidCredential,
           CtapDeviceResponseCode::kCtap2ErrUserActionPending,

device/fido/fido_request_handler.h

@@ -94,8 +94,6 @@ class FidoRequestHandler : public FidoRequestHandlerBase {
       // authenticator.
       case CtapDeviceResponseCode::kCtap2ErrCredentialExcluded:
         return FidoReturnCode::kUserConsentButCredentialExcluded;
-      case CtapDeviceResponseCode::kCtap2ErrInvalidCredential:
-      case CtapDeviceResponseCode::kCtap2ErrCredentialNotValid:
       case CtapDeviceResponseCode::kCtap2ErrNoCredentials:
         return FidoReturnCode::kUserConsentButCredentialNotRecognized;
 
@@ -115,6 +113,17 @@ class FidoRequestHandler : public FidoRequestHandlerBase {
                          FidoReturnCode::kUserConsentDenied)
                    : base::nullopt;
 
+      // This error is returned by some authenticators (e.g. the "Yubico FIDO
+      // 2" CTAP2 USB keys) during GetAssertion **before the user interacted
+      // with the device**. The authenticator does this to avoid blinking (and
+      // possibly asking the user for their PIN) for requests it knows
+      // beforehand it cannot handle.
+      //
+      // Ignore this error to avoid canceling the request without user
+      // interaction.
+      case CtapDeviceResponseCode::kCtap2ErrInvalidCredential:
+        return base::nullopt;
+
       // For all other errors, the authenticator will be dropped, and other
       // authenticators may continue.
       default:

device/fido/get_assertion_task_unittest.cc

@@ -208,7 +208,7 @@ TEST_F(FidoGetAssertionTaskTest, TestU2fSignRequestWithEmptyAllowedList) {
       get_assertion_callback_receiver().callback());
 
   get_assertion_callback_receiver().WaitForCallback();
-  EXPECT_EQ(CtapDeviceResponseCode::kCtap2ErrCredentialNotValid,
+  EXPECT_EQ(CtapDeviceResponseCode::kCtap2ErrNoCredentials,
             get_assertion_callback_receiver().status());
   EXPECT_FALSE(get_assertion_callback_receiver().value());
 }

device/fido/u2f_sign_operation.cc

@@ -84,8 +84,7 @@ void U2fSignOperation::OnSignResponseReceived(
     case apdu::ApduResponse::Status::SW_NO_ERROR: {
       if (is_fake_enrollment) {
         std::move(callback())
-            .Run(CtapDeviceResponseCode::kCtap2ErrCredentialNotValid,
-                 base::nullopt);
+            .Run(CtapDeviceResponseCode::kCtap2ErrNoCredentials, base::nullopt);
       } else {
         auto application_parameter =
             application_parameter_type == ApplicationParameterType::kPrimary

device/fido/u2f_sign_operation_unittest.cc

@@ -222,7 +222,7 @@ TEST_F(U2fSignOperationTest, FakeEnroll) {
   u2f_sign->Start();
   sign_callback_receiver().WaitForCallback();
 
-  EXPECT_EQ(CtapDeviceResponseCode::kCtap2ErrCredentialNotValid,
+  EXPECT_EQ(CtapDeviceResponseCode::kCtap2ErrNoCredentials,
             sign_callback_receiver().status());
   EXPECT_FALSE(sign_callback_receiver().value());
 }
@@ -253,7 +253,7 @@ TEST_F(U2fSignOperationTest, DelayedFakeEnrollment) {
   u2f_sign->Start();
   sign_callback_receiver().WaitForCallback();
 
-  EXPECT_EQ(CtapDeviceResponseCode::kCtap2ErrCredentialNotValid,
+  EXPECT_EQ(CtapDeviceResponseCode::kCtap2ErrNoCredentials,
             sign_callback_receiver().status());
   EXPECT_FALSE(sign_callback_receiver().value());
 }