Default to U2F register when client pin is set

According to the CTAP spec, CTAP2 authenticators will always error out
when MakeCredential request with an empty "pinAuth" parameter is
received to a device that has client pin already set. This prevents any
users with CTAP2 devices with client pin set to use Chrome as a client.

Until client pin command is implemented on Chrome, use U2F register
request when possible when we know that the device has client pin set.

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

device/fido/fido_test_data.h

@@ -1045,6 +1045,22 @@ constexpr uint8_t kTestGetInfoResponseCrossPlatformDevice[] = {
     0x19, 0x04, 0xB0, 0x06, 0x81, 0x01,
 };
 
+// AuthenticatorGetInfo request with all configurations equal to that of
+// kTestAuthenticatorGetInfoResponse except clientPin option is set to true.
+constexpr uint8_t kTestGetInfoResponseWithClientPinSet[] = {
+    0x00, 0xA6, 0x01, 0x82, 0x68, 0x46, 0x49, 0x44, 0x4F, 0x5F, 0x32, 0x5F,
+    0x30, 0x66, 0x55, 0x32, 0x46, 0x5F, 0x56, 0x32, 0x02, 0x82, 0x63, 0x75,
+    0x76, 0x6D, 0x6B, 0x68, 0x6D, 0x61, 0x63, 0x2D, 0x73, 0x65, 0x63, 0x72,
+    0x65, 0x74, 0x03, 0x50, 0xF8, 0xA0, 0x11, 0xF3, 0x8C, 0x0A, 0x4D, 0x15,
+    0x80, 0x06, 0x17, 0x11, 0x1F, 0x9E, 0xDC, 0x7D, 0x04, 0xA5, 0x62, 0x72,
+    0x6B, 0xF5, 0x62, 0x75, 0x70, 0xF5, 0x62, 0x75, 0x76, 0xF5, 0x64, 0x70,
+    0x6C, 0x61, 0x74, 0xF5,
+    // clientPin : true
+    0x69, 0x63, 0x6C, 0x69, 0x65, 0x6E, 0x75,
+    // End of clientPin setting.
+    0x50, 0x69, 0x6E, 0xF4, 0x05, 0x19, 0x04, 0xB0, 0x06, 0x81, 0x01,
+};
+
 // A Sample well formed response to CTAP MakeCredential request.
 constexpr uint8_t kTestMakeCredentialResponse[] = {
     // Success status byte

device/fido/make_credential_task.cc

@@ -14,6 +14,33 @@
 
 namespace device {
 
+namespace {
+
+// Checks whether the incoming MakeCredential request has ClientPin option that
+// is compatible with the Chrome's CTAP2 implementation. According to the CTAP
+// spec, CTAP2 authenticators that have client pin set will always error out on
+// MakeCredential request when "pinAuth" parameter in the request is not set. As
+// ClientPin is not supported on Chrome yet, this check allows Chrome to avoid
+// such failures if possible by defaulting to U2F request when user verification
+// is not required and the device supports U2F protocol.
+// TODO(hongjunchoi): Remove this once ClientPin command is implemented.
+// See: https://crbug.com/870892
+bool IsClientPinOptionCompatible(const FidoDevice* device,
+                                 const CtapMakeCredentialRequest& request) {
+  if (request.user_verification_required())
+    return true;
+
+  DCHECK(device && device->device_info());
+  bool client_pin_set =
+      device->device_info()->options().client_pin_availability() ==
+      AuthenticatorSupportedOptions::ClientPinAvailability::kSupportedAndPinSet;
+  bool supports_u2f = base::ContainsKey(device->device_info()->versions(),
+                                        ProtocolVersion::kU2f);
+  return !client_pin_set || !supports_u2f;
+}
+
+}  // namespace
+
 MakeCredentialTask::MakeCredentialTask(
     FidoDevice* device,
     CtapMakeCredentialRequest request_parameter,
@@ -27,7 +54,8 @@ MakeCredentialTask::~MakeCredentialTask() = default;
 
 void MakeCredentialTask::StartTask() {
   if (base::FeatureList::IsEnabled(kNewCtap2Device) &&
-      device()->supported_protocol() == ProtocolVersion::kCtap) {
+      device()->supported_protocol() == ProtocolVersion::kCtap &&
+      IsClientPinOptionCompatible(device(), request_parameter_)) {
     MakeCredential();
   } else {
     U2fRegister();

device/fido/make_credential_task_unittest.cc

@@ -31,6 +31,10 @@ namespace device {
 
 namespace {
 
+constexpr std::array<uint8_t, kAaguidLength> kTestDeviceAaguid = {
+    {0xF8, 0xA0, 0x11, 0xF3, 0x8C, 0x0A, 0x4D, 0x15, 0x80, 0x06, 0x17, 0x11,
+     0x1F, 0x9E, 0xDC, 0x7D}};
+
 using TestMakeCredentialTaskCallback =
     ::device::test::StatusAndValueCallbackReceiver<
         CtapDeviceResponseCode,
@@ -131,5 +135,58 @@ TEST_F(FidoMakeCredentialTaskTest, TestDefaultU2fRegisterOperationWithoutFlag) {
             make_credential_callback_receiver().status());
 }
 
+TEST_F(FidoMakeCredentialTaskTest, DefaultToU2fWhenClientPinSet) {
+  AuthenticatorGetInfoResponse device_info(
+      {ProtocolVersion::kCtap, ProtocolVersion::kU2f},
+      fido_parsing_utils::Materialize(kTestDeviceAaguid));
+  AuthenticatorSupportedOptions options;
+  options.SetClientPinAvailability(
+      AuthenticatorSupportedOptions::ClientPinAvailability::
+          kSupportedAndPinSet);
+  device_info.SetOptions(std::move(options));
+
+  auto device = MockFidoDevice::MakeCtap(std::move(device_info));
+  device->ExpectRequestAndRespondWith(
+      test_data::kU2fRegisterCommandApdu,
+      test_data::kApduEncodedNoErrorRegisterResponse);
+
+  const auto task = CreateMakeCredentialTask(device.get());
+  make_credential_callback_receiver().WaitForCallback();
+  EXPECT_EQ(CtapDeviceResponseCode::kSuccess,
+            make_credential_callback_receiver().status());
+  EXPECT_TRUE(make_credential_callback_receiver().value());
+}
+
+TEST_F(FidoMakeCredentialTaskTest, EnforceClientPinWhenUserVerificationSet) {
+  AuthenticatorGetInfoResponse device_info(
+      {ProtocolVersion::kCtap, ProtocolVersion::kU2f},
+      fido_parsing_utils::Materialize(kTestDeviceAaguid));
+  AuthenticatorSupportedOptions options;
+  options.SetClientPinAvailability(
+      AuthenticatorSupportedOptions::ClientPinAvailability::
+          kSupportedAndPinSet);
+  device_info.SetOptions(std::move(options));
+
+  auto device = MockFidoDevice::MakeCtap(std::move(device_info));
+  device->ExpectCtap2CommandAndRespondWith(
+      CtapRequestCommand::kAuthenticatorMakeCredential, base::nullopt);
+
+  PublicKeyCredentialRpEntity rp(test_data::kRelyingPartyId);
+  PublicKeyCredentialUserEntity user(
+      fido_parsing_utils::Materialize(test_data::kUserId));
+  auto request = CtapMakeCredentialRequest(
+      test_data::kClientDataHash, std::move(rp), std::move(user),
+      PublicKeyCredentialParams(
+          std::vector<PublicKeyCredentialParams::CredentialInfo>(1)));
+  request.SetUserVerificationRequired(true);
+  const auto task = std::make_unique<MakeCredentialTask>(
+      device.get(), std::move(request), callback_receiver_.callback());
+
+  make_credential_callback_receiver().WaitForCallback();
+  EXPECT_EQ(CtapDeviceResponseCode::kCtap2ErrOther,
+            make_credential_callback_receiver().status());
+  EXPECT_FALSE(make_credential_callback_receiver().value());
+}
+
 }  // namespace
 }  // namespace device