webauthn: enable discoverable credentials over caBLEv2

The Android implementation of caBLEv2 does not yet support discoverable
credentials. Because of this, Chromium on the desktop assumes that no
caBLEv2 implementation supports them. But Android isn't the only
implementation now.

Thus this change enables discoverable credentials over caBLEv2 on the
desktop. Because it's too late to get a new error string on Android for
M100, I plan on reverting this change prior to the M100 branch. If
someone really wants to try this in M100, the flag is in chrome://flags.

On Android, we need to handle this case better. Thus this change also:

  1) Adds a specific error for discoverable credential requests.
  2) Advertises CTAP 2.1 support so that clients will send an
     authenticatorSelection command, rather than a dummy makeCredential,
     if they want to make a caBLE device “flash”. That is caught and
     also results in a specific error.

BUG=1002262

Change-Id: I2da176ac57f985ec6dd2853cef38862efffe1597
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3433592
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Auto-Submit: Adam Langley <agl@chromium.org>
Commit-Queue: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/main@{#969697}

chrome/android/features/cablev2_authenticator/java/src/org/chromium/chrome/browser/webauth/authenticator/CableAuthenticatorUI.java

@@ -84,6 +84,8 @@ public class CableAuthenticatorUI extends Fragment implements OnClickListener {
     private static final int ERROR_UNEXPECTED_EOF = 100;
     private static final int ERROR_NO_SCREENLOCK = 110;
     private static final int ERROR_NO_BLUETOOTH_PERMISSION = 111;
+    private static final int ERROR_AUTHENTICATOR_SELECTION_RECEIVED = 114;
+    private static final int ERROR_DISCOVERABLE_CREDENTIALS_REQUEST = 115;
 
     private enum Mode {
         QR, // QR code scanned by external app.
@@ -746,6 +748,12 @@ public class CableAuthenticatorUI extends Fragment implements OnClickListener {
                 settingsButtonVisible = true;
                 break;
 
+                // TODO: create a dedicated error message for these cases, which
+                // result when the client sends a discoverable-credentials
+                // request that Android cannot handle.
+                // case ERROR_AUTHENTICATOR_SELECTION_RECEIVED:
+                // case ERROR_DISCOVERABLE_CREDENTIALS_REQUEST:
+
             default:
                 TextView errorCodeTextView = (TextView) mErrorView.findViewById(R.id.error_code);
                 errorCodeTextView.setText(

chrome/android/features/cablev2_authenticator/native/cablev2_authenticator_android.cc

@@ -205,8 +205,9 @@ enum class CableV2MobileResult {
   kInvalidQR = 9,
   kInvalidServerLink = 10,
   kEOFWhileProcessing = 11,
+  kDiscoverableCredentialsRejected = 12,
 
-  kMaxValue = 11,
+  kMaxValue = 12,
 };
 
 // JavaByteArrayToSpan returns a span that aliases |data|. Be aware that the
@@ -501,6 +502,10 @@ class AndroidPlatform : public device::cablev2::authenticator::Platform {
         case Error::UNKNOWN_COMMAND:
           result = CableV2MobileResult::kUnknownCommand;
           break;
+        case Error::AUTHENTICATOR_SELECTION_RECEIVED:
+        case Error::DISCOVERABLE_CREDENTIALS_REQUEST:
+          result = CableV2MobileResult::kDiscoverableCredentialsRejected;
+          break;
         case Error::INTERNAL_ERROR:
         case Error::SERVER_LINK_WRONG_LENGTH:
         case Error::SERVER_LINK_NOT_ON_CURVE:

content/browser/webauth/authenticator_common.cc

@@ -380,26 +380,21 @@ base::flat_set<device::FidoTransportProtocol> GetWebAuthnTransports(
     transports.insert(device::FidoTransportProtocol::kInternal);
   }
 
-  // caBLE devices don't yet support discoverable credentials and so we
-  // shouldn't offer them for such requests unless forced by a feature flag.
-  if (!uses_discoverable_creds ||
-      base::FeatureList::IsEnabled(device::kWebAuthCableDisco)) {
-    if (base::FeatureList::IsEnabled(features::kWebAuthCable)) {
-      transports.insert(
-          device::FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy);
-    }
-
-    // kAndroidAccessory doesn't work on Windows because of USB stack issues.
-    // Note: even if this value were inserted it wouldn't take effect on Windows
-    // versions with a native API because FidoRequestHandlerBase filters out
-    // non-kCloudAssistedBluetoothLowEnergy transports in that case.
-#if !BUILDFLAG(IS_WIN)
-    // In order for AOA to be active the |AuthenticatorRequestClientDelegate|
-    // must still configure a |UsbDeviceManager|.
-    transports.insert(device::FidoTransportProtocol::kAndroidAccessory);
-#endif
+  if (base::FeatureList::IsEnabled(features::kWebAuthCable)) {
+    transports.insert(
+        device::FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy);
   }
 
+  // kAndroidAccessory doesn't work on Windows because of USB stack issues.
+  // Note: even if this value were inserted it wouldn't take effect on Windows
+  // versions with a native API because FidoRequestHandlerBase filters out
+  // non-kCloudAssistedBluetoothLowEnergy transports in that case.
+#if !BUILDFLAG(IS_WIN)
+  // In order for AOA to be active the |AuthenticatorRequestClientDelegate|
+  // must still configure a |UsbDeviceManager|.
+  transports.insert(device::FidoTransportProtocol::kAndroidAccessory);
+#endif
+
   return transports;
 }
 

content/browser/webauth/authenticator_impl_unittest.cc

@@ -7749,42 +7749,6 @@ TEST_F(ResidentKeyAuthenticatorImplTest, ConditionalUI) {
   EXPECT_TRUE(test_client_.is_conditional);
 }
 
-TEST_F(ResidentKeyAuthenticatorImplWithFlagsTest,
-       NoDiscoverableCredentialsViaCable) {
-  // caBLE devices never support discoverable credentials currently and we
-  // shouldn't offer them for such requests.
-
-  NavigateAndCommit(GURL(kTestOrigin1));
-
-  device::VirtualCtap2Device::Config config;
-  config.resident_key_support = true;
-  config.internal_uv_support = true;
-  virtual_device_factory_->SetCtap2Config(config);
-  virtual_device_factory_->mutable_state()->fingerprints_enrolled = true;
-
-  ASSERT_TRUE(virtual_device_factory_->mutable_state()->InjectResidentKey(
-      /*credential_id=*/{{4, 3, 2, 1}}, kTestRelyingPartyId,
-      /*user_id=*/{{1, 2, 3, 4}}, absl::nullopt, absl::nullopt));
-
-  for (const auto transport :
-       {device::FidoTransportProtocol::kUsbHumanInterfaceDevice,
-        device::FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy}) {
-    virtual_device_factory_->SetTransport(transport);
-    PublicKeyCredentialRequestOptionsPtr options =
-        GetTestPublicKeyCredentialRequestOptions();
-    options->allow_credentials.clear();
-
-    if (transport == device::FidoTransportProtocol::kUsbHumanInterfaceDevice) {
-      EXPECT_EQ(AuthenticatorGetAssertion(std::move(options)).status,
-                AuthenticatorStatus::SUCCESS);
-    } else {
-      EXPECT_EQ(
-          AuthenticatorGetAssertionAndWaitForTimeout(std::move(options)).status,
-          AuthenticatorStatus::NOT_ALLOWED_ERROR);
-    }
-  }
-}
-
 class InternalAuthenticatorImplTest : public AuthenticatorTestBase {
  protected:
   InternalAuthenticatorImplTest() = default;
@@ -7957,16 +7921,18 @@ TEST_F(TouchIdAuthenticatorImplTest, IsUVPAA) {
 }
 #endif  // BUILDFLAG(IS_MAC)
 
-class CableV2AuthenticatorImplTest : public AuthenticatorImplTest {
+// AuthenticatorCableV2Test tests features of the caBLEv2 transport and
+// protocol.
+class AuthenticatorCableV2Test : public AuthenticatorImplTest {
  public:
-  CableV2AuthenticatorImplTest()
+  AuthenticatorCableV2Test()
       : network_context_(device::cablev2::NewMockTunnelServer(
-            base::BindRepeating(&CableV2AuthenticatorImplTest::OnContact,
+            base::BindRepeating(&AuthenticatorCableV2Test::OnContact,
                                 base::Unretained(this)))),
         virtual_device_(new VirtualFidoDevice::State, DeviceConfig()),
-        browser_client_(base::BindRepeating(
-            &CableV2AuthenticatorImplTest::MaybeContactPhones,
-            base::Unretained(this))) {
+        browser_client_(
+            base::BindRepeating(&AuthenticatorCableV2Test::MaybeContactPhones,
+                                base::Unretained(this))) {
     scoped_feature_list_.InitWithFeatures(
         {features::kWebAuthCable, device::kWebAuthPhoneSupport},
         /*disabled_features=*/{});
@@ -8000,7 +7966,7 @@ class CableV2AuthenticatorImplTest : public AuthenticatorImplTest {
 
   base::RepeatingCallback<void(device::cablev2::PairingEvent)>
   GetPairingCallback() {
-    return base::BindRepeating(&CableV2AuthenticatorImplTest::OnPairingEvent,
+    return base::BindRepeating(&AuthenticatorCableV2Test::OnPairingEvent,
                                base::Unretained(this));
   }
 
@@ -8026,6 +7992,13 @@ class CableV2AuthenticatorImplTest : public AuthenticatorImplTest {
     std::unique_ptr<device::cablev2::Discovery> discovery_;
   };
 
+  class TestAuthenticationDelegate : public WebAuthenticationDelegate {
+   public:
+    bool SupportsResidentKeys(RenderFrameHost*) override { return true; }
+
+    bool IsFocused(WebContents* web_contents) override { return true; }
+  };
+
   class ContactWhenReadyAuthenticatorRequestDelegate
       : public AuthenticatorRequestClientDelegate {
    public:
@@ -8056,8 +8029,13 @@ class CableV2AuthenticatorImplTest : public AuthenticatorImplTest {
           callback_);
     }
 
+    WebAuthenticationDelegate* GetWebAuthenticationDelegate() override {
+      return &authentication_delegate_;
+    }
+
    private:
     base::RepeatingClosure callback_;
+    TestAuthenticationDelegate authentication_delegate_;
   };
 
   // MaybeContactPhones is called when OnTransportAvailabilityEnumerated is
@@ -8132,7 +8110,7 @@ class CableV2AuthenticatorImplTest : public AuthenticatorImplTest {
   base::test::ScopedFeatureList scoped_feature_list_;
 };
 
-TEST_F(CableV2AuthenticatorImplTest, QRBasedWithNoPairing) {
+TEST_F(AuthenticatorCableV2Test, QRBasedWithNoPairing) {
   auto discovery = std::make_unique<device::cablev2::Discovery>(
       device::FidoRequestType::kGetAssertion, network_context_.get(),
       qr_generator_key_, std::move(ble_advert_events_),
@@ -8148,7 +8126,8 @@ TEST_F(CableV2AuthenticatorImplTest, QRBasedWithNoPairing) {
   std::unique_ptr<device::cablev2::authenticator::Transaction> transaction =
       device::cablev2::authenticator::TransactFromQRCode(
           device::cablev2::authenticator::NewMockPlatform(
-              std::move(ble_advert_callback_), &virtual_device_),
+              std::move(ble_advert_callback_), &virtual_device_,
+              /*observer=*/nullptr),
           network_context_.get(), root_secret_, "Test Authenticator",
           zero_qr_secret_, peer_identity_x962_,
           /*contact_id=*/absl::nullopt,
@@ -8158,43 +8137,7 @@ TEST_F(CableV2AuthenticatorImplTest, QRBasedWithNoPairing) {
   EXPECT_EQ(pairings_.size(), 0u);
 }
 
-TEST_F(CableV2AuthenticatorImplTest, QRBasedWithNoPairingGetAssertion) {
-  // Similar to `QRBasedWithNoPairing`, but exercise the GetAssertion code
-  // instead.
-  auto discovery = std::make_unique<device::cablev2::Discovery>(
-      device::FidoRequestType::kGetAssertion, network_context_.get(),
-      qr_generator_key_, std::move(ble_advert_events_),
-      /*pairings=*/std::vector<std::unique_ptr<device::cablev2::Pairing>>(),
-      /*contact_device_stream=*/nullptr,
-      /*extension_contents=*/std::vector<device::CableDiscoveryData>(),
-      GetPairingCallback());
-
-  AuthenticatorEnvironmentImpl::GetInstance()
-      ->ReplaceDefaultDiscoveryFactoryForTesting(
-          std::make_unique<DiscoveryFactory>(std::move(discovery)));
-
-  std::unique_ptr<device::cablev2::authenticator::Transaction> transaction =
-      device::cablev2::authenticator::TransactFromQRCode(
-          device::cablev2::authenticator::NewMockPlatform(
-              std::move(ble_advert_callback_), &virtual_device_),
-          network_context_.get(), root_secret_, "Test Authenticator",
-          zero_qr_secret_, peer_identity_x962_,
-          /*contact_id=*/absl::nullopt,
-          /*use_new_crypter_construction=*/false);
-
-  PublicKeyCredentialRequestOptionsPtr options =
-      GetTestPublicKeyCredentialRequestOptions();
-  options->allow_credentials[0].transports.insert(
-      device::FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy);
-  ASSERT_TRUE(virtual_device_.mutable_state()->InjectRegistration(
-      options->allow_credentials[0].id, options->relying_party_id));
-
-  EXPECT_EQ(AuthenticatorGetAssertion(std::move(options)).status,
-            AuthenticatorStatus::SUCCESS);
-  EXPECT_EQ(pairings_.size(), 0u);
-}
-
-TEST_F(CableV2AuthenticatorImplTest, QRBasedNewCrypterConstruction) {
+TEST_F(AuthenticatorCableV2Test, QRBasedNewCrypterConstruction) {
   // The new Crypter construction should be transparently supported by the
   // client code.
   auto discovery = std::make_unique<device::cablev2::Discovery>(
@@ -8212,7 +8155,8 @@ TEST_F(CableV2AuthenticatorImplTest, QRBasedNewCrypterConstruction) {
   std::unique_ptr<device::cablev2::authenticator::Transaction> transaction =
       device::cablev2::authenticator::TransactFromQRCode(
           device::cablev2::authenticator::NewMockPlatform(
-              std::move(ble_advert_callback_), &virtual_device_),
+              std::move(ble_advert_callback_), &virtual_device_,
+              /*observer=*/nullptr),
           network_context_.get(), root_secret_, "Test Authenticator",
           zero_qr_secret_, peer_identity_x962_,
           /*contact_id=*/absl::nullopt,
@@ -8222,7 +8166,7 @@ TEST_F(CableV2AuthenticatorImplTest, QRBasedNewCrypterConstruction) {
   EXPECT_EQ(pairings_.size(), 0u);
 }
 
-TEST_F(CableV2AuthenticatorImplTest, PairingBased) {
+TEST_F(AuthenticatorCableV2Test, PairingBased) {
   // First do unpaired exchange to get pairing data.
   auto discovery = std::make_unique<device::cablev2::Discovery>(
       device::FidoRequestType::kGetAssertion, network_context_.get(),
@@ -8240,7 +8184,8 @@ TEST_F(CableV2AuthenticatorImplTest, PairingBased) {
   std::unique_ptr<device::cablev2::authenticator::Transaction> transaction =
       device::cablev2::authenticator::TransactFromQRCode(
           device::cablev2::authenticator::NewMockPlatform(
-              std::move(ble_advert_callback_), &virtual_device_),
+              std::move(ble_advert_callback_), &virtual_device_,
+              /*observer=*/nullptr),
           network_context_.get(), root_secret_, "Test Authenticator",
           zero_qr_secret_, peer_identity_x962_, contact_id,
           /*use_new_crypter_construction=*/false);
@@ -8294,7 +8239,8 @@ TEST_F(CableV2AuthenticatorImplTest, PairingBased) {
         CHECK_EQ(request_type_hint, expected_request_type_string);
         transaction = device::cablev2::authenticator::TransactFromFCM(
             device::cablev2::authenticator::NewMockPlatform(
-                std::move(ble_advert_callback_), &virtual_device_),
+                std::move(ble_advert_callback_), &virtual_device_,
+                /*observer=*/nullptr),
             network_context_.get(), root_secret_, routing_id, tunnel_id,
             pairing_id, client_nonce, contact_id);
       });
@@ -8320,7 +8266,7 @@ static std::unique_ptr<device::cablev2::Pairing> DummyPairing() {
   return ret;
 }
 
-TEST_F(CableV2AuthenticatorImplTest, ContactIDDisabled) {
+TEST_F(AuthenticatorCableV2Test, ContactIDDisabled) {
   std::vector<std::unique_ptr<device::cablev2::Pairing>> pairings;
   pairings.emplace_back(DummyPairing());
   // Passing |nullopt| as the callback here causes all contact IDs to be
@@ -8355,6 +8301,90 @@ TEST_F(CableV2AuthenticatorImplTest, ContactIDDisabled) {
   ASSERT_EQ(pairings_.size(), 0u);
 }
 
+// AuthenticatorCableV2AuthenticatorTest tests aspects of the authenticator
+// implementation, rather than of the underlying caBLEv2 transport.
+class AuthenticatorCableV2AuthenticatorTest
+    : public AuthenticatorCableV2Test,
+      public device::cablev2::authenticator::Observer {
+ public:
+  void SetUp() override {
+    AuthenticatorCableV2Test::SetUp();
+
+    auto discovery = std::make_unique<device::cablev2::Discovery>(
+        device::FidoRequestType::kGetAssertion, network_context_.get(),
+        qr_generator_key_, std::move(ble_advert_events_),
+        /*pairings=*/std::vector<std::unique_ptr<device::cablev2::Pairing>>(),
+        /*contact_device_stream=*/nullptr,
+        /*extension_contents=*/std::vector<device::CableDiscoveryData>(),
+        GetPairingCallback());
+
+    AuthenticatorEnvironmentImpl::GetInstance()
+        ->ReplaceDefaultDiscoveryFactoryForTesting(
+            std::make_unique<DiscoveryFactory>(std::move(discovery)));
+
+    transaction_ = device::cablev2::authenticator::TransactFromQRCode(
+        device::cablev2::authenticator::NewMockPlatform(
+            std::move(ble_advert_callback_), &virtual_device_, this),
+        network_context_.get(), root_secret_, "Test Authenticator",
+        zero_qr_secret_, peer_identity_x962_,
+        /*contact_id=*/absl::nullopt,
+        /*use_new_crypter_construction=*/false);
+  }
+
+ protected:
+  // device::cablev2::authenticator::Observer
+  void OnStatus(device::cablev2::authenticator::Platform::Status) override {}
+  void OnCompleted(
+      absl::optional<device::cablev2::authenticator::Platform::Error> error)
+      override {
+    did_complete_ = true;
+    error_ = error;
+  }
+
+  std::unique_ptr<device::cablev2::authenticator::Transaction> transaction_;
+  bool did_complete_ = false;
+  absl::optional<device::cablev2::authenticator::Platform::Error> error_;
+};
+
+TEST_F(AuthenticatorCableV2AuthenticatorTest, GetAssertion) {
+  PublicKeyCredentialRequestOptionsPtr options =
+      GetTestPublicKeyCredentialRequestOptions();
+  options->allow_credentials[0].transports.insert(
+      device::FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy);
+  ASSERT_TRUE(virtual_device_.mutable_state()->InjectRegistration(
+      options->allow_credentials[0].id, options->relying_party_id));
+
+  EXPECT_EQ(AuthenticatorGetAssertion(std::move(options)).status,
+            AuthenticatorStatus::SUCCESS);
+}
+
+TEST_F(AuthenticatorCableV2AuthenticatorTest, MakeDiscoverableCredential) {
+  auto options = GetTestPublicKeyCredentialCreationOptions();
+  options->authenticator_selection->resident_key =
+      device::ResidentKeyRequirement::kRequired;
+  EXPECT_EQ(
+      AuthenticatorMakeCredentialAndWaitForTimeout(std::move(options)).status,
+      AuthenticatorStatus::NOT_ALLOWED_ERROR);
+
+  ASSERT_TRUE(did_complete_);
+  ASSERT_TRUE(error_.has_value());
+  EXPECT_EQ(*error_, device::cablev2::authenticator::Platform::Error::
+                         AUTHENTICATOR_SELECTION_RECEIVED);
+}
+
+TEST_F(AuthenticatorCableV2AuthenticatorTest, EmptyAllowList) {
+  auto options = GetTestPublicKeyCredentialRequestOptions();
+  options->allow_credentials.clear();
+  EXPECT_EQ(
+      AuthenticatorGetAssertionAndWaitForTimeout(std::move(options)).status,
+      AuthenticatorStatus::NOT_ALLOWED_ERROR);
+
+  ASSERT_TRUE(did_complete_);
+  ASSERT_TRUE(error_.has_value());
+  EXPECT_EQ(*error_, device::cablev2::authenticator::Platform::Error::
+                         DISCOVERABLE_CREDENTIALS_REQUEST);
+}
+
 // AuthenticatorImplWithRequestProxyTest tests behavior with an installed
 // TestWebAuthenticationRequestProxy that takes over WebAuthn request handling.
 class AuthenticatorImplWithRequestProxyTest : public AuthenticatorImplTest {

device/fido/cable/v2_authenticator.cc

@@ -31,6 +31,7 @@
 #include "net/cookies/site_for_cookies.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
 #include "services/network/public/mojom/network_context.mojom.h"
+#include "third_party/abseil-cpp/absl/types/variant.h"
 #include "third_party/blink/public/mojom/webauthn/authenticator.mojom.h"
 #include "third_party/boringssl/src/include/openssl/aes.h"
 #include "third_party/boringssl/src/include/openssl/ec_key.h"
@@ -192,6 +193,7 @@ std::vector<uint8_t> BuildGetInfoResponse() {
   std::array<uint8_t, device::kAaguidLength> aaguid{};
   std::vector<cbor::Value> versions;
   versions.emplace_back("FIDO_2_0");
+  versions.emplace_back("FIDO_2_1");
 
   cbor::Value::MapValue options;
   // This code is only invoked if a screen-lock (i.e. user verification) is
@@ -527,25 +529,27 @@ class CTAP2Processor : public Transaction {
     }
 
     std::vector<uint8_t>& msg = absl::get<std::vector<uint8_t>>(update);
-    absl::optional<std::vector<uint8_t>> response = ProcessCTAPMessage(msg);
-    if (!response) {
-      // TODO(agl): expose more error information from |ProcessCTAPMessage|.
-      platform_->OnCompleted(Platform::Error::INVALID_CTAP);
+    const absl::variant<std::vector<uint8_t>, Platform::Error> result =
+        ProcessCTAPMessage(msg);
+    if (const auto* error = absl::get_if<Platform::Error>(&result)) {
+      platform_->OnCompleted(*error);
       return;
     }
 
-    if (response->empty()) {
+    const std::vector<uint8_t>& response =
+        absl::get<std::vector<uint8_t>>(result);
+    if (response.empty()) {
       // Response is pending.
       return;
     }
 
-    transport_->Write(std::move(*response));
+    transport_->Write(std::move(response));
   }
 
-  absl::optional<std::vector<uint8_t>> ProcessCTAPMessage(
+  absl::variant<std::vector<uint8_t>, Platform::Error> ProcessCTAPMessage(
       base::span<const uint8_t> message_bytes) {
     if (message_bytes.empty()) {
-      return absl::nullopt;
+      return Platform::Error::INVALID_CTAP;
     }
     const auto command = message_bytes[0];
     const auto cbor_bytes = message_bytes.subspan(1);
@@ -556,7 +560,7 @@ class CTAP2Processor : public Transaction {
       if (!payload) {
         FIDO_LOG(ERROR) << "CBOR decoding failed for "
                         << base::HexEncode(cbor_bytes);
-        return absl::nullopt;
+        return Platform::Error::INVALID_CTAP;
       }
       FIDO_LOG(DEBUG) << "<- (" << base::HexEncode(&command, 1) << ") "
                       << cbor::DiagnosticWriter::Write(*payload);
@@ -570,24 +574,24 @@ class CTAP2Processor : public Transaction {
           device::CtapRequestCommand::kAuthenticatorGetInfo): {
         if (payload) {
           FIDO_LOG(ERROR) << "getInfo command incorrectly contained payload";
-          return absl::nullopt;
+          return Platform::Error::INVALID_CTAP;
         }
 
         absl::optional<std::vector<uint8_t>> response = BuildGetInfoResponse();
         if (!response) {
-          return absl::nullopt;
+          return Platform::Error::INTERNAL_ERROR;
         }
         response->insert(
             response->begin(),
             static_cast<uint8_t>(CtapDeviceResponseCode::kSuccess));
-        return response;
+        return *response;
       }
 
       case static_cast<uint8_t>(
           device::CtapRequestCommand::kAuthenticatorMakeCredential): {
         if (!payload || !payload->is_map()) {
           FIDO_LOG(ERROR) << "Invalid makeCredential payload";
-          return absl::nullopt;
+          return Platform::Error::INVALID_CTAP;
         }
 
         MakeCredRequest make_cred_request;
@@ -595,7 +599,7 @@ class CTAP2Processor : public Transaction {
                 &make_cred_request, kMakeCredParseSteps, payload->GetMap())) {
           FIDO_LOG(ERROR) << "Failed to parse makeCredential request: "
                           << base::HexEncode(cbor_bytes);
-          return absl::nullopt;
+          return Platform::Error::INVALID_CTAP;
         }
 
         auto params = blink::mojom::PublicKeyCredentialCreationOptions::New();
@@ -609,16 +613,21 @@ class CTAP2Processor : public Transaction {
         params->user.name = *make_cred_request.user_name;
         params->user.display_name = *make_cred_request.user_display_name;
 
+        const bool rk =
+            make_cred_request.resident_key && *make_cred_request.resident_key;
+        if (rk && !base::FeatureList::IsEnabled(device::kWebAuthCableDisco)) {
+          return Platform::Error::DISCOVERABLE_CREDENTIALS_REQUEST;
+        }
+
         params->authenticator_selection.emplace(
             device::AuthenticatorAttachment::kPlatform,
-            (make_cred_request.resident_key && *make_cred_request.resident_key)
-                ? device::ResidentKeyRequirement::kRequired
-                : device::ResidentKeyRequirement::kDiscouraged,
+            rk ? device::ResidentKeyRequirement::kRequired
+               : device::ResidentKeyRequirement::kDiscouraged,
             device::UserVerificationRequirement::kRequired);
 
         if (!CopyCredIds(make_cred_request.excluded_credentials,
                          &params->exclude_credentials)) {
-          return absl::nullopt;
+          return Platform::Error::INTERNAL_ERROR;
         }
 
         if (!device::cbor_extract::ForEachPublicKeyEntry(
@@ -645,7 +654,7 @@ class CTAP2Processor : public Transaction {
                       return true;
                     },
                     base::Unretained(&params->public_key_parameters)))) {
-          return absl::nullopt;
+          return Platform::Error::INVALID_CTAP;
         }
 
         transaction_received_ = true;
@@ -660,7 +669,7 @@ class CTAP2Processor : public Transaction {
           device::CtapRequestCommand::kAuthenticatorGetAssertion): {
         if (!payload || !payload->is_map()) {
           FIDO_LOG(ERROR) << "Invalid makeCredential payload";
-          return absl::nullopt;
+          return Platform::Error::INVALID_CTAP;
         }
 
         GetAssertionRequest get_assertion_request;
@@ -668,7 +677,13 @@ class CTAP2Processor : public Transaction {
                 &get_assertion_request, kGetAssertionParseSteps,
                 payload->GetMap())) {
           FIDO_LOG(ERROR) << "Failed to parse getAssertion request";
-          return absl::nullopt;
+          return Platform::Error::INVALID_CTAP;
+        }
+
+        if ((!get_assertion_request.allowed_credentials ||
+             get_assertion_request.allowed_credentials->empty()) &&
+            !base::FeatureList::IsEnabled(device::kWebAuthCableDisco)) {
+          return Platform::Error::DISCOVERABLE_CREDENTIALS_REQUEST;
         }
 
         auto params = blink::mojom::PublicKeyCredentialRequestOptions::New();
@@ -680,7 +695,7 @@ class CTAP2Processor : public Transaction {
 
         if (!CopyCredIds(get_assertion_request.allowed_credentials,
                          &params->allow_credentials)) {
-          return absl::nullopt;
+          return Platform::Error::INTERNAL_ERROR;
         }
 
         transaction_received_ = true;
@@ -692,10 +707,19 @@ class CTAP2Processor : public Transaction {
         return std::vector<uint8_t>();
       }
 
+      case static_cast<uint8_t>(
+          device::CtapRequestCommand::kAuthenticatorSelection): {
+        if (payload) {
+          FIDO_LOG(ERROR) << "Invalid authenticatorSelection payload";
+          return Platform::Error::INVALID_CTAP;
+        }
+        return Platform::Error::AUTHENTICATOR_SELECTION_RECEIVED;
+      }
+
       default:
         FIDO_LOG(ERROR) << "Received unknown command "
                         << static_cast<unsigned>(command);
-        return absl::nullopt;
+        return Platform::Error::INVALID_CTAP;
     }
   }
 

device/fido/cable/v2_authenticator.h

@@ -65,6 +65,8 @@ class Platform {
     NO_BLUETOOTH_PERMISSION = 111,
     QR_URI_ERROR = 112,
     EOF_WHILE_PROCESSING = 113,
+    AUTHENTICATOR_SELECTION_RECEIVED = 114,
+    DISCOVERABLE_CREDENTIALS_REQUEST = 115,
   };
 
   using MakeCredentialCallback =

device/fido/cable/v2_test_util.cc

@@ -31,8 +31,7 @@
 #include "third_party/blink/public/mojom/webauthn/authenticator.mojom.h"
 #include "url/gurl.h"
 
-namespace device {
-namespace cablev2 {
+namespace device::cablev2 {
 namespace {
 
 // TestNetworkContext intercepts WebSocket creation calls and simulates a
@@ -357,9 +356,11 @@ class DummyBLEAdvert
 class TestPlatform : public authenticator::Platform {
  public:
   TestPlatform(Discovery::AdvertEventStream::Callback ble_advert_callback,
-               device::VirtualCtap2Device* ctap2_device)
+               device::VirtualCtap2Device* ctap2_device,
+               authenticator::Observer* observer)
       : ble_advert_callback_(ble_advert_callback),
-        ctap2_device_(ctap2_device) {}
+        ctap2_device_(ctap2_device),
+        observer_(observer) {}
 
   void MakeCredential(
       blink::mojom::PublicKeyCredentialCreationOptionsPtr params,
@@ -400,8 +401,17 @@ class TestPlatform : public authenticator::Platform {
                        weak_factory_.GetWeakPtr(), std::move(callback)));
   }
 
-  void OnStatus(Status status) override {}
-  void OnCompleted(absl::optional<Error> maybe_error) override {}
+  void OnStatus(Status status) override {
+    if (observer_) {
+      observer_->OnStatus(status);
+    }
+  }
+
+  void OnCompleted(absl::optional<Error> maybe_error) override {
+    if (observer_) {
+      observer_->OnCompleted(maybe_error);
+    }
+  }
 
   std::unique_ptr<authenticator::Platform::BLEAdvert> SendBLEAdvert(
       base::span<const uint8_t, kAdvertSize> payload) override {
@@ -512,6 +522,7 @@ class TestPlatform : public authenticator::Platform {
 
   Discovery::AdvertEventStream::Callback ble_advert_callback_;
   const raw_ptr<device::VirtualCtap2Device> ctap2_device_;
+  authenticator::Observer* const observer_;
   base::WeakPtrFactory<TestPlatform> weak_factory_{this};
 };
 
@@ -526,11 +537,11 @@ namespace authenticator {
 
 std::unique_ptr<authenticator::Platform> NewMockPlatform(
     Discovery::AdvertEventStream::Callback ble_advert_callback,
-    device::VirtualCtap2Device* ctap2_device) {
-  return std::make_unique<TestPlatform>(ble_advert_callback, ctap2_device);
+    device::VirtualCtap2Device* ctap2_device,
+    authenticator::Observer* observer) {
+  return std::make_unique<TestPlatform>(ble_advert_callback, ctap2_device,
+                                        observer);
 }
 
 }  // namespace authenticator
-
-}  // namespace cablev2
-}  // namespace device
+}  // namespace device::cablev2

device/fido/cable/v2_test_util.h

@@ -9,6 +9,7 @@
 
 #include "base/callback_forward.h"
 #include "base/containers/span.h"
+#include "device/fido/cable/v2_authenticator.h"
 #include "device/fido/cable/v2_constants.h"
 #include "device/fido/cable/v2_discovery.h"
 #include "services/network/public/mojom/network_context.mojom-forward.h"
@@ -38,14 +39,26 @@ std::unique_ptr<network::mojom::NetworkContext> NewMockTunnelServer(
 
 namespace authenticator {
 
-class Platform;
+// Observer is an interface that can be implemented by tests that wish to see
+// certain platform events.
+class Observer {
+ public:
+  // See `Platform::OnStatus`.
+  virtual void OnStatus(Platform::Status) = 0;
+
+  // See `Platform::OnCompleted`.
+  virtual void OnCompleted(absl::optional<Platform::Error>) = 0;
+};
 
 // NewMockPlatform returns a |Platform| that implements the makeCredential
 // operation by forwarding it to |ctap2_device|. Transmitted BLE adverts are
-// forwarded to |ble_advert_callback|.
+// forwarded to |ble_advert_callback|. |observer| may be |nullptr| but, if not,
+// then corresponding calls to the mock `Platform` are forwarded to the
+// observer.
 std::unique_ptr<Platform> NewMockPlatform(
     Discovery::AdvertEventStream::Callback ble_advert_callback,
-    device::VirtualCtap2Device* ctap2_device);
+    device::VirtualCtap2Device* ctap2_device,
+    Observer* observer);
 
 }  // namespace authenticator
 

tools/metrics/histograms/enums.xml

@@ -93558,6 +93558,7 @@ others/histograms.xml -->
   <int value="9" label="Invalid QR"/>
   <int value="10" label="Invalid server link data"/>
   <int value="11" label="EOF while processing a request"/>
+  <int value="12" label="Discoverable credentials request rejected"/>
 </enum>
 
 <enum name="WebAuthenticationCableV2TunnelEvent">