device/fido: mark U2F sign requests sent while probing exclude list

Since the U2F Zero authenticator cannot deal with check-only sign
requests, exclude lists are probed with UP-requiring sign requests
instead. Since these requests are indinstinguishable from actual sign
requests, they are problematic for authenticators that want to show the
request type to the user (e.g. soft tokens and authenticators with a
display).

This change aims to make these workaround sign requests stand out while
preserving compatibility with authenticators behaving like the U2F Zero.
This is achieved by setting the challenge of the sign requests sent
while probing an exclude list to the same fixed bogus challenge used to
collect a touch on certain errors.

R=agl@chromium.org

Bug: 1000641
Change-Id: I7ae43a0ecc2380714ce4752a84c7b6cd4572cdb6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1831857
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Adam Langley <agl@chromium.org>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/heads/master@{#701648}

AUTHORS

@@ -274,6 +274,7 @@ Evan Wallace <evan.exe@gmail.com>
 Evangelos Foutras <evangelos@foutrelis.com>
 Evgeniy Dushistov <dushistov@gmail.com>
 Evgeny Agafonchikov <evgeny.agafonchikov@akvelon.com>
+Fabian Henneke <fabian.henneke@gmail.com>
 Fabien Tassin <fta@sofaraway.org>
 Felix H. Dahlke <fhd@ubercode.de>
 Fengrong Fang <fr.fang@samsung.com>

device/fido/fido_test_data.h

@@ -163,6 +163,27 @@ constexpr uint8_t kU2fSignCommandApduWithKeyAlpha[] = {
     0x00, 0x00,
 };
 
+constexpr uint8_t kU2fSignCommandApduWithKeyAlphaAndBogusChallenge[] = {
+    // CLA, INS, P1, P2 APDU instruction parameters
+    0x00, 0x02, 0x03, 0x00,
+    // Data Length (3 bytes in big endian order)
+    0x00, 0x00, 0x42,
+    // Bogus challenge parameter
+    0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42,
+    0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42,
+    0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42,
+    // Application parameter
+    0x11, 0x94, 0x22, 0x8D, 0xA8, 0xFD, 0xBD, 0xEE, 0xFD, 0x26, 0x1B, 0xD7,
+    0xB6, 0x59, 0x5C, 0xFD, 0x70, 0xA5, 0x0D, 0x70, 0xC6, 0x40, 0x7B, 0xCF,
+    0x01, 0x3D, 0xE9, 0x6D, 0x4E, 0xFB, 0x17, 0xDE,
+    // Dummy key handle length
+    0x01,
+    // Key handle
+    0xEA,
+    // Max response length
+    0x00, 0x00,
+};
+
 constexpr uint8_t kU2fSignCommandApduWithKeyBeta[] = {
     // CLA, INS, P1, P2 APDU instruction parameters
     0x00, 0x02, 0x03, 0x00,
@@ -184,6 +205,27 @@ constexpr uint8_t kU2fSignCommandApduWithKeyBeta[] = {
     0x00, 0x00,
 };
 
+constexpr uint8_t kU2fSignCommandApduWithKeyBetaAndBogusChallenge[] = {
+    // CLA, INS, P1, P2 APDU instruction parameters
+    0x00, 0x02, 0x03, 0x00,
+    // Data Length (3 bytes in big endian order)
+    0x00, 0x00, 0x42,
+    // Bogus challenge parameter
+    0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42,
+    0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42,
+    0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42,
+    // Application parameter
+    0x11, 0x94, 0x22, 0x8D, 0xA8, 0xFD, 0xBD, 0xEE, 0xFD, 0x26, 0x1B, 0xD7,
+    0xB6, 0x59, 0x5C, 0xFD, 0x70, 0xA5, 0x0D, 0x70, 0xC6, 0x40, 0x7B, 0xCF,
+    0x01, 0x3D, 0xE9, 0x6D, 0x4E, 0xFB, 0x17, 0xDE,
+    // Dummy key handle length
+    0x01,
+    // Key handle
+    0xEB,
+    // Max response length
+    0x00, 0x00,
+};
+
 constexpr uint8_t kU2fSignCommandApduWithKeyGamma[] = {
     // CLA, INS, P1, P2 APDU instruction parameters
     0x00, 0x02, 0x03, 0x00,
@@ -205,6 +247,27 @@ constexpr uint8_t kU2fSignCommandApduWithKeyGamma[] = {
     0x00, 0x00,
 };
 
+constexpr uint8_t kU2fSignCommandApduWithKeyGammaAndBogusChallenge[] = {
+    // CLA, INS, P1, P2 APDU instruction parameters
+    0x00, 0x02, 0x03, 0x00,
+    // Data Length (3 bytes in big endian order)
+    0x00, 0x00, 0x42,
+    // Bogus challenge parameter
+    0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42,
+    0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42,
+    0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42,
+    // Application parameter
+    0x11, 0x94, 0x22, 0x8D, 0xA8, 0xFD, 0xBD, 0xEE, 0xFD, 0x26, 0x1B, 0xD7,
+    0xB6, 0x59, 0x5C, 0xFD, 0x70, 0xA5, 0x0D, 0x70, 0xC6, 0x40, 0x7B, 0xCF,
+    0x01, 0x3D, 0xE9, 0x6D, 0x4E, 0xFB, 0x17, 0xDE,
+    // Dummy key handle length
+    0x01,
+    // Key handle
+    0xEC,
+    // Max response length
+    0x00, 0x00,
+};
+
 constexpr uint8_t kU2fSignCommandApdu[] = {
     // CLA, INS, P1, P2 APDU instruction parameters
     0x00, 0x02, 0x03, 0x00,

device/fido/u2f_command_constructor.cc

@@ -53,12 +53,12 @@ base::Optional<std::vector<uint8_t>> ConvertToU2fRegisterCommand(
       request.client_data_hash, is_invidual_attestation);
 }
 
-base::Optional<std::vector<uint8_t>> ConvertToU2fSignCommand(
+base::Optional<std::vector<uint8_t>> ConvertToU2fSignCommandWithBogusChallenge(
     const CtapMakeCredentialRequest& request,
     base::span<const uint8_t> key_handle) {
   return ConstructU2fSignCommand(
       fido_parsing_utils::CreateSHA256Hash(request.rp.id),
-      request.client_data_hash, key_handle);
+      kBogusChallenge, key_handle);
 }
 
 base::Optional<std::vector<uint8_t>> ConvertToU2fSignCommand(

device/fido/u2f_command_constructor.h

@@ -37,9 +37,10 @@ COMPONENT_EXPORT(DEVICE_FIDO)
 base::Optional<std::vector<uint8_t>> ConvertToU2fRegisterCommand(
     const CtapMakeCredentialRequest& request);
 
-// Extracts APDU encoded U2F sign command from CtapMakeCredentialRequest.
+// Turns a CtapMakeCredentialRequest into an APDU encoded U2F sign command
+// for the same RP and key handle, but a bogus challenge.
 COMPONENT_EXPORT(DEVICE_FIDO)
-base::Optional<std::vector<uint8_t>> ConvertToU2fSignCommand(
+base::Optional<std::vector<uint8_t>> ConvertToU2fSignCommandWithBogusChallenge(
     const CtapMakeCredentialRequest& request,
     base::span<const uint8_t> key_handle);
 

device/fido/u2f_register_operation.cc

@@ -57,9 +57,11 @@ void U2fRegisterOperation::TrySign() {
   if (probing_alternative_rp_id_) {
     CtapMakeCredentialRequest sign_request(request());
     sign_request.rp.id = *request().app_id;
-    sign_command = ConvertToU2fSignCommand(sign_request, excluded_key_handle());
+    sign_command = ConvertToU2fSignCommandWithBogusChallenge(
+      sign_request, excluded_key_handle());
   } else {
-    sign_command = ConvertToU2fSignCommand(request(), excluded_key_handle());
+    sign_command = ConvertToU2fSignCommandWithBogusChallenge(
+      request(), excluded_key_handle());
   }
 
   DispatchDeviceRequest(

device/fido/u2f_register_operation_unittest.cc

@@ -155,17 +155,19 @@ TEST_F(U2fRegisterOperationTest, TestRegistrationWithExclusionList) {
   auto device = std::make_unique<MockFidoDevice>();
   EXPECT_CALL(*device, GetId()).WillRepeatedly(::testing::Return("device"));
   device->ExpectWinkedAtLeastOnce();
-  // DeviceTransact() will be called three times including two check only sign-
-  // in calls and one registration call. For the first two calls, device will
-  // invoke MockFidoDevice::WrongData/WrongLength as the authenticator did not
-  // create the two key handles provided in the exclude list. At the third call,
-  // MockFidoDevice::NoErrorRegister will be invoked after registration.
+  // DeviceTransact() will be called three times including two sign-in calls
+  // with bogus challenges and one registration call. For the first two calls,
+  // device will invoke MockFidoDevice::WrongData/WrongLength as the
+  // authenticator did not create the two key handles provided in the exclude
+  // list. At the third call, MockFidoDevice::NoErrorRegister will be invoked
+  // after registration.
   ::testing::InSequence s;
   device->ExpectRequestAndRespondWith(
-      test_data::kU2fSignCommandApduWithKeyAlpha,
+      test_data::kU2fSignCommandApduWithKeyAlphaAndBogusChallenge,
       test_data::kU2fWrongDataApduResponse);
-  device->ExpectRequestAndRespondWith(test_data::kU2fSignCommandApduWithKeyBeta,
-                                      test_data::kU2fWrongLengthApduResponse);
+  device->ExpectRequestAndRespondWith(
+      test_data::kU2fSignCommandApduWithKeyBetaAndBogusChallenge,
+      test_data::kU2fWrongLengthApduResponse);
   device->ExpectRequestAndRespondWith(
       test_data::kU2fRegisterCommandApdu,
       test_data::kApduEncodedNoErrorRegisterResponse);
@@ -210,12 +212,15 @@ TEST_F(U2fRegisterOperationTest, TestRegistrationWithDuplicateHandle) {
   // request is concluded with Ctap2ErrCredentialExcluded.
   ::testing::InSequence s;
   device->ExpectRequestAndRespondWith(
-      test_data::kU2fSignCommandApduWithKeyAlpha,
+      test_data::kU2fSignCommandApduWithKeyAlphaAndBogusChallenge,
+      test_data::kU2fWrongDataApduResponse);
+  device->ExpectRequestAndRespondWith(
+      test_data::kU2fSignCommandApduWithKeyBetaAndBogusChallenge,
       test_data::kU2fWrongDataApduResponse);
-  device->ExpectRequestAndRespondWith(test_data::kU2fSignCommandApduWithKeyBeta,
-                                      test_data::kU2fWrongDataApduResponse);
+  // The signature in the response is intentionally incorrect since nothing
+  // should depend on it being correct.
   device->ExpectRequestAndRespondWith(
-      test_data::kU2fSignCommandApduWithKeyGamma,
+      test_data::kU2fSignCommandApduWithKeyGammaAndBogusChallenge,
       test_data::kApduEncodedNoErrorSignResponse);
 
   auto u2f_register = std::make_unique<U2fRegisterOperation>(