Skip to content

Commit

Permalink
webauthn: “cable” is now “hybrid”
Browse files Browse the repository at this point in the history
FIDO has decided that caBLE is hybrid CTAP. This CL changes the name in
many places, although doesn't do a bulk rename of files because that
would cause a merging problem with other, pending CLs.

The name "cable" is still recognised as a transport string and is now an
alias for "hybrid".

Change-Id: Ibd2d38fcc0e19535302b5ba3691a73980993de01
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3828784
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1034949}
  • Loading branch information
Adam Langley authored and Chromium LUCI CQ committed Aug 15, 2022
1 parent 0a29bc7 commit cbafdd4
Show file tree
Hide file tree
Showing 31 changed files with 121 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,7 @@ class AuthenticatorDialogViewTest : public DialogBrowserTest {
transport_availability;
transport_availability.available_transports = {
AuthenticatorTransport::kUsbHumanInterfaceDevice,
AuthenticatorTransport::kInternal,
AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy};
AuthenticatorTransport::kInternal, AuthenticatorTransport::kHybrid};

std::array<uint8_t, device::kP256X962Length> public_key = {0};
AuthenticatorRequestDialogModel::PairedPhone phone("Phone", 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class AuthenticatorDialogTest : public DialogBrowserTest {
transport_availability.available_transports = {
AuthenticatorTransport::kUsbHumanInterfaceDevice,
AuthenticatorTransport::kInternal,
AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy,
AuthenticatorTransport::kHybrid,
AuthenticatorTransport::kAndroidAccessory,
};
if (name == "cable_server_link_activate") {
Expand Down
10 changes: 5 additions & 5 deletions chrome/browser/webauthn/authenticator_request_dialog_model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ constexpr int GetMessageIdForTransportDescription(
return IDS_WEBAUTHN_TRANSPORT_USB;
case AuthenticatorTransport::kInternal:
return IDS_WEBAUTHN_TRANSPORT_INTERNAL;
case AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy:
case AuthenticatorTransport::kHybrid:
return IDS_WEBAUTHN_TRANSPORT_CABLE;
case AuthenticatorTransport::kAndroidAccessory:
return IDS_WEBAUTHN_TRANSPORT_AOA;
Expand All @@ -79,7 +79,7 @@ constexpr int GetMessageIdForTransportShortDescription(
return IDS_WEBAUTHN_TRANSPORT_POPUP_USB;
case AuthenticatorTransport::kInternal:
return IDS_WEBAUTHN_TRANSPORT_POPUP_INTERNAL;
case AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy:
case AuthenticatorTransport::kHybrid:
return IDS_WEBAUTHN_TRANSPORT_POPUP_CABLE;
case AuthenticatorTransport::kAndroidAccessory:
return IDS_WEBAUTHN_TRANSPORT_POPUP_AOA;
Expand All @@ -105,7 +105,7 @@ constexpr const gfx::VectorIcon* GetTransportIcon(
return &vector_icons::kUsbIcon;
case AuthenticatorTransport::kInternal:
return &kLaptopIcon;
case AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy:
case AuthenticatorTransport::kHybrid:
return &kSmartphoneIcon;
case AuthenticatorTransport::kAndroidAccessory:
return &kUsbCableIcon;
Expand Down Expand Up @@ -897,7 +897,7 @@ void AuthenticatorRequestDialogModel::StartGuidedFlowForTransport(
case AuthenticatorTransport::kInternal:
StartPlatformAuthenticatorFlow();
break;
case AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy:
case AuthenticatorTransport::kHybrid:
EnsureBleAdapterIsPoweredAndContinueWithStep(Step::kCableActivate);
break;
case AuthenticatorTransport::kAndroidAccessory:
Expand Down Expand Up @@ -1052,7 +1052,7 @@ void AuthenticatorRequestDialogModel::PopulateMechanisms(
AuthenticatorTransport::kInternal,
};

const auto kCable = AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy;
const auto kCable = AuthenticatorTransport::kHybrid;
bool include_add_phone_option = false;

if (cable_ui_type_) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const base::flat_set<AuthenticatorTransport> kAllTransports = {
AuthenticatorTransport::kUsbHumanInterfaceDevice,
AuthenticatorTransport::kNearFieldCommunication,
AuthenticatorTransport::kInternal,
AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy,
AuthenticatorTransport::kHybrid,
};

const base::flat_set<AuthenticatorTransport> kAllTransportsWithoutCable = {
Expand Down Expand Up @@ -154,7 +154,7 @@ TEST_F(AuthenticatorRequestDialogModelTest, Mechanisms) {
const auto ga = RequestType::kGetAssertion;
const auto usb = AuthenticatorTransport::kUsbHumanInterfaceDevice;
const auto internal = AuthenticatorTransport::kInternal;
const auto cable = AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy;
const auto cable = AuthenticatorTransport::kHybrid;
const auto aoa = AuthenticatorTransport::kAndroidAccessory;
const auto v1 = TransportAvailabilityParam::kHasCableV1Extension;
const auto v2 = TransportAvailabilityParam::kHasCableV2Extension;
Expand Down Expand Up @@ -335,8 +335,7 @@ TEST_F(AuthenticatorRequestDialogModelTest, WinCancel) {
AuthenticatorRequestDialogModel::TransportAvailabilityInfo tai;
tai.has_win_native_api_authenticator = true;
tai.win_native_api_authenticator_id = "ID";
tai.available_transports.insert(
device::FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy);
tai.available_transports.insert(device::FidoTransportProtocol::kHybrid);

AuthenticatorRequestDialogModel model(/*web_contents=*/nullptr);
model.set_cable_transport_info(absl::nullopt, {}, base::DoNothing(),
Expand Down Expand Up @@ -431,8 +430,7 @@ TEST_F(AuthenticatorRequestDialogModelTest, Cable2ndFactorFlows) {
transports_info.is_ble_powered = test.ble_power == BLEPower::ON;
transports_info.can_power_on_ble_adapter = true;
transports_info.request_type = test.request_type;
transports_info.available_transports = {
AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy};
transports_info.available_transports = {AuthenticatorTransport::kHybrid};
transports_info.is_off_the_record_context =
test.profile == Profile::INCOGNITO;

Expand Down Expand Up @@ -533,8 +531,7 @@ TEST_F(AuthenticatorRequestDialogModelTest, BleAdapterAlreadyPowered) {
AuthenticatorTransport transport;
Step expected_final_step;
} kTestCases[] = {
{AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy,
Step::kCableActivate},
{AuthenticatorTransport::kHybrid, Step::kCableActivate},
};

for (const auto test_case : kTestCases) {
Expand Down Expand Up @@ -562,8 +559,7 @@ TEST_F(AuthenticatorRequestDialogModelTest, BleAdapterNeedToBeManuallyPowered) {
AuthenticatorTransport transport;
Step expected_final_step;
} kTestCases[] = {
{AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy,
Step::kCableActivate},
{AuthenticatorTransport::kHybrid, Step::kCableActivate},
};

for (const auto test_case : kTestCases) {
Expand Down Expand Up @@ -606,8 +602,7 @@ TEST_F(AuthenticatorRequestDialogModelTest,
AuthenticatorTransport transport;
Step expected_final_step;
} kTestCases[] = {
{AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy,
Step::kCableActivate},
{AuthenticatorTransport::kHybrid, Step::kCableActivate},
};

for (const auto test_case : kTestCases) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -477,8 +477,7 @@ TEST_F(ChromeAuthenticatorRequestDelegateWindowsBehaviorTest,
AuthenticatorRequestDialogModel::TransportAvailabilityInfo tai;
tai.has_win_native_api_authenticator = true;
tai.win_native_api_authenticator_id = "ID";
tai.available_transports.insert(
device::FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy);
tai.available_transports.insert(device::FidoTransportProtocol::kHybrid);

CreateObjectsUnderTest();
delegate_->dialog_model()->set_cable_transport_info(
Expand Down
8 changes: 3 additions & 5 deletions chrome/browser/webauthn/chrome_webauthn_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -475,13 +475,12 @@ class WebAuthnCableSecondFactor : public WebAuthnBrowserTest {

std::vector<std::unique_ptr<device::FidoDiscoveryBase>> Create(
device::FidoTransportProtocol transport) override {
if (transport !=
device::FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy) {
if (transport != device::FidoTransportProtocol::kHybrid) {
return {};
}

auto discovery = std::make_unique<PendingDiscovery>(
device::FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy);
device::FidoTransportProtocol::kHybrid);
add_authenticator_callback_ = discovery->GetAddAuthenticatorCallback();
return SingleDiscovery(std::move(discovery));
}
Expand Down Expand Up @@ -655,8 +654,7 @@ class WebAuthnCableSecondFactor : public WebAuthnBrowserTest {
ChromeAuthenticatorRequestDelegate* delegate,
device::FidoRequestHandlerBase::TransportAvailabilityInfo* tai)
override {
tai->available_transports.insert(
device::FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy);
tai->available_transports.insert(device::FidoTransportProtocol::kHybrid);
tai->is_ble_powered = true;
}

Expand Down
8 changes: 3 additions & 5 deletions content/browser/webauth/authenticator_common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -293,13 +293,12 @@ base::flat_set<device::FidoTransportProtocol> GetWebAuthnTransports(
transports.insert(device::FidoTransportProtocol::kInternal);
}

transports.insert(
device::FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy);
transports.insert(device::FidoTransportProtocol::kHybrid);

// 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.
// non-kHybrid transports in that case.
#if !BUILDFLAG(IS_WIN)
// In order for AOA to be active the |AuthenticatorRequestClientDelegate|
// must still configure a |UsbDeviceManager|.
Expand Down Expand Up @@ -1316,8 +1315,7 @@ void AuthenticatorCommon::OnRegisterResponse(
is_transport_used_internal =
*transport == device::FidoTransportProtocol::kInternal;
is_transport_used_cable =
*transport ==
device::FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy;
*transport == device::FidoTransportProtocol::kHybrid;
}

const auto attestation =
Expand Down
13 changes: 5 additions & 8 deletions content/browser/webauth/authenticator_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1465,11 +1465,10 @@ TEST_F(AuthenticatorImplTest, NoSilentAuthenticationForCable) {

if (is_cable_device) {
virtual_device_factory_->SetTransport(
device::FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy);
device::FidoTransportProtocol::kHybrid);
for (auto& cred : options->allow_credentials) {
cred.transports.clear();
cred.transports.emplace(
device::FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy);
cred.transports.emplace(device::FidoTransportProtocol::kHybrid);
}
}

Expand Down Expand Up @@ -3418,7 +3417,7 @@ TEST_F(AuthenticatorContentBrowserClientTest,
GetTestPublicKeyCredentialRequestOptions();
std::vector<uint8_t> id(32u, 1u);
base::flat_set<device::FidoTransportProtocol> transports{
device::FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy};
device::FidoTransportProtocol::kHybrid};
options->allow_credentials.clear();
options->allow_credentials.emplace_back(device::CredentialType::kPublicKey,
std::move(id), std::move(transports));
Expand Down Expand Up @@ -8501,9 +8500,7 @@ class AuthenticatorCableV2Test

std::vector<std::unique_ptr<device::FidoDiscoveryBase>> Create(
device::FidoTransportProtocol transport) override {
if (transport !=
device::FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy ||
!discovery_) {
if (transport != device::FidoTransportProtocol::kHybrid || !discovery_) {
return {};
}

Expand Down Expand Up @@ -8995,7 +8992,7 @@ TEST_F(AuthenticatorCableV2AuthenticatorTest, GetAssertion) {
PublicKeyCredentialRequestOptionsPtr options =
GetTestPublicKeyCredentialRequestOptions();
options->allow_credentials[0].transports.insert(
device::FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy);
device::FidoTransportProtocol::kHybrid);
ASSERT_TRUE(virtual_device_.mutable_state()->InjectRegistration(
options->allow_credentials[0].id, options->relying_party_id));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,7 @@ TEST(AuthenticatorMojomTraitsTest, SerializeCredentialDescriptors) {
FidoTransportProtocol::kUsbHumanInterfaceDevice);
success_cases[2].transports.emplace(
FidoTransportProtocol::kNearFieldCommunication);
success_cases[2].transports.emplace(
FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy);
success_cases[2].transports.emplace(FidoTransportProtocol::kHybrid);
success_cases[2].transports.emplace(
FidoTransportProtocol::kBluetoothLowEnergy);

Expand Down
38 changes: 38 additions & 0 deletions content/browser/webauth/webauth_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1091,6 +1091,44 @@ IN_PROC_BROWSER_TEST_F(WebAuthJavascriptClientBrowserTest,
ASSERT_EQ(kNotAllowedErrorMessage, result);
}

IN_PROC_BROWSER_TEST_F(WebAuthJavascriptClientBrowserTest, HybridRecognised) {
// Ensure that both "cable" and "hybrid" are recognised as the same transport.
auto* virtual_device_factory = InjectVirtualFidoDeviceFactory();
virtual_device_factory->SetTransport(device::FidoTransportProtocol::kHybrid);
virtual_device_factory->SetSupportedProtocol(device::ProtocolVersion::kCtap2);
static const uint8_t kCredentialId[] = {1};
ASSERT_TRUE(virtual_device_factory->mutable_state()->InjectRegistration(
kCredentialId, "www.acme.com"));

GetParameters parameters;
for (const char* const transport_str : {"hybrid", "cable", "usb"}) {
SCOPED_TRACE(transport_str);
const bool should_fail = (strcmp(transport_str, "usb") == 0);

parameters.allow_credentials =
"[{"
" type: 'public-key',"
" id: new Uint8Array([1]),"
" transports: ['" +
std::string(transport_str) +
"'],"
"}]";
if (should_fail) {
parameters.timeout = kShortTimeout;
}
std::string result = EvalJs(shell()->web_contents()->GetPrimaryMainFrame(),
BuildGetCallWithParameters(parameters),
EXECUTE_SCRIPT_USE_MANUAL_REPLY)
.ExtractString();

if (should_fail) {
ASSERT_EQ(kNotAllowedErrorMessage, result);
} else {
ASSERT_EQ("webauth: OK", result);
}
}
}

// Tests that when navigator.credentials.get() is called with an empty
// allowCredentials list, we get a NotSupportedError.
IN_PROC_BROWSER_TEST_F(WebAuthJavascriptClientBrowserTest,
Expand Down
5 changes: 2 additions & 3 deletions device/fido/ble_adapter_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,8 @@ class FakeFidoRequestHandlerBase : public FidoRequestHandlerBase {
public:
FakeFidoRequestHandlerBase(MockObserver* observer,
FidoDiscoveryFactory* fido_discovery_factory)
: FidoRequestHandlerBase(
fido_discovery_factory,
{FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy}) {
: FidoRequestHandlerBase(fido_discovery_factory,
{FidoTransportProtocol::kHybrid}) {
set_observer(observer);
Start();
}
Expand Down
2 changes: 1 addition & 1 deletion device/fido/cable/fido_cable_device.cc
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ std::string FidoCableDevice::GetId() const {
}

FidoTransportProtocol FidoCableDevice::DeviceTransport() const {
return FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy;
return FidoTransportProtocol::kHybrid;
}

FidoDevice::CancelToken FidoCableDevice::DeviceTransact(
Expand Down
3 changes: 1 addition & 2 deletions device/fido/cable/fido_cable_discovery.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,7 @@ FidoCableDiscovery::ObservedDeviceData::~ObservedDeviceData() = default;

FidoCableDiscovery::FidoCableDiscovery(
std::vector<CableDiscoveryData> discovery_data)
: FidoDeviceDiscovery(
FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy),
: FidoDeviceDiscovery(FidoTransportProtocol::kHybrid),
discovery_data_(std::move(discovery_data)) {
// Windows currently does not support multiple EIDs, thus we ignore any extra
// discovery data.
Expand Down
2 changes: 1 addition & 1 deletion device/fido/cable/fido_tunnel_device.cc
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ std::string FidoTunnelDevice::GetId() const {

FidoTransportProtocol FidoTunnelDevice::DeviceTransport() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy;
return FidoTransportProtocol::kHybrid;
}

base::WeakPtr<FidoDevice> FidoTunnelDevice::GetWeakPtr() {
Expand Down
3 changes: 1 addition & 2 deletions device/fido/cable/v2_discovery.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ Discovery::Discovery(
pairing_callback,
absl::optional<base::RepeatingCallback<void(size_t)>>
invalidated_pairing_callback)
: FidoDeviceDiscovery(
FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy),
: FidoDeviceDiscovery(FidoTransportProtocol::kHybrid),
request_type_(request_type),
network_context_(network_context),
qr_keys_(KeysFromQRGeneratorKey(qr_generator_key)),
Expand Down
6 changes: 3 additions & 3 deletions device/fido/fake_fido_discovery.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ FakeFidoDiscovery* FakeFidoDiscoveryFactory::ForgeNextNfcDiscovery(

FakeFidoDiscovery* FakeFidoDiscoveryFactory::ForgeNextCableDiscovery(
FakeFidoDiscovery::StartMode mode) {
next_cable_discovery_ = std::make_unique<FakeFidoDiscovery>(
FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy, mode);
next_cable_discovery_ =
std::make_unique<FakeFidoDiscovery>(FidoTransportProtocol::kHybrid, mode);
return next_cable_discovery_.get();
}

Expand All @@ -89,7 +89,7 @@ FakeFidoDiscoveryFactory::Create(FidoTransportProtocol transport) {
case FidoTransportProtocol::kBluetoothLowEnergy:
case FidoTransportProtocol::kAndroidAccessory:
return {};
case FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy:
case FidoTransportProtocol::kHybrid:
return SingleDiscovery(std::move(next_cable_discovery_));
case FidoTransportProtocol::kInternal:
return SingleDiscovery(std::move(next_platform_discovery_));
Expand Down
2 changes: 1 addition & 1 deletion device/fido/fido_device.cc
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ void FidoDevice::SetDeviceInfo(AuthenticatorGetInfoResponse device_info) {
bool FidoDevice::NoSilentRequests() const {
// caBLE devices do not support silent requests.
const auto transport = DeviceTransport();
return transport == FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy ||
return transport == FidoTransportProtocol::kHybrid ||
transport == FidoTransportProtocol::kAndroidAccessory;
}

Expand Down
2 changes: 1 addition & 1 deletion device/fido/fido_discovery_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ std::vector<std::unique_ptr<FidoDiscoveryBase>> FidoDiscoveryFactory::Create(
std::make_unique<FidoHidDiscovery>(hid_ignore_list_));
case FidoTransportProtocol::kBluetoothLowEnergy:
return {};
case FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy:
case FidoTransportProtocol::kHybrid:
#if BUILDFLAG(IS_MAC)
if (!base::IsProcessSelfResponsible()) {
// On recent macOS a process must have listed Bluetooth metadata in
Expand Down

0 comments on commit cbafdd4

Please sign in to comment.