Commit bd177d01 authored by Adam Langley's avatar Adam Langley Committed by Commit Bot

webauthn: privacy tweaks for the caBLE extension.

The caBLE extension to WebAuthn is used to allow a phone to be used as
a security key. However, it involves broadcasting a site-controlled
value over BLE.

This change causes:
  · the caBLE extension to be ignored if a request is made by a
    non-focused tab. (Otherwise the BLE broadcast could happen in a tab
    that isn't in the forefront.)
  · BLE broadcasts to be delayed for 500 milliseconds after the UI is
    triggered. This ensures that the UI has time to show itself and cannot
    be suppressed by rapidly canceling requests etc.

Also note that a prior change[1] caused the caBLE extension to be
ignored unless embedders opt-in as other embedders may not be displaying
UI for this case.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1776792

Change-Id: Id2bf2160448f58ee4c13ab649fc24eb56b45b382
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1779164
Commit-Queue: Adam Langley <agl@chromium.org>
Reviewed-by: default avatarMartin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/heads/master@{#694817}
parent 66a8b746
......@@ -601,7 +601,8 @@ void AuthenticatorCommon::StartGetAssertionRequest() {
std::vector<device::CableDiscoveryData> cable_extension;
if (ctap_get_assertion_request_->cable_extension &&
request_delegate_->ShouldPermitCableExtension(caller_origin_)) {
request_delegate_->ShouldPermitCableExtension(caller_origin_) &&
IsFocused()) {
cable_extension = *ctap_get_assertion_request_->cable_extension;
}
......
......@@ -304,7 +304,14 @@ void FidoCableDiscovery::OnStartDiscoverySessionWithFilter(
std::unique_ptr<BluetoothDiscoverySession> session) {
SetDiscoverySession(std::move(session));
FIDO_LOG(DEBUG) << "Discovery session started.";
StartAdvertisement();
// Advertising is delayed by 500ms to ensure that any UI has a chance to
// appear as we don't want to start broadcasting without the user being
// aware.
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(&FidoCableDiscovery::StartAdvertisement,
weak_factory_.GetWeakPtr()),
base::TimeDelta::FromMilliseconds(500));
}
void FidoCableDiscovery::StartAdvertisement() {
......
......@@ -325,7 +325,8 @@ class FidoCableDiscoveryTest : public ::testing::Test {
return std::make_unique<FakeFidoCableDiscovery>(std::move(discovery_data));
}
base::test::TaskEnvironment task_environment_;
base::test::TaskEnvironment task_environment_{
base::test::TaskEnvironment::TimeSource::MOCK_TIME};
};
// Tests regular successful discovery flow for Cable device.
......@@ -344,7 +345,7 @@ TEST_F(FidoCableDiscoveryTest, TestDiscoveryFindsNewDevice) {
BluetoothAdapterFactory::SetAdapterForTesting(mock_adapter);
cable_discovery->Start();
task_environment_.RunUntilIdle();
task_environment_.FastForwardUntilNoTasksRemain();
}
// Tests successful discovery flow for Apple Cable device.
......@@ -363,7 +364,7 @@ TEST_F(FidoCableDiscoveryTest, TestDiscoveryFindsNewAppleDevice) {
BluetoothAdapterFactory::SetAdapterForTesting(mock_adapter);
cable_discovery->Start();
task_environment_.RunUntilIdle();
task_environment_.FastForwardUntilNoTasksRemain();
}
// Tests a scenario where upon broadcasting advertisement and scanning, client
......@@ -384,7 +385,7 @@ TEST_F(FidoCableDiscoveryTest, TestDiscoveryFindsIncorrectDevice) {
BluetoothAdapterFactory::SetAdapterForTesting(mock_adapter);
cable_discovery->Start();
task_environment_.RunUntilIdle();
task_environment_.FastForwardUntilNoTasksRemain();
}
// Windows currently does not support multiple EIDs, so the following tests are
......@@ -423,7 +424,7 @@ TEST_F(FidoCableDiscoveryTest, TestDiscoveryWithMultipleEids) {
BluetoothAdapterFactory::SetAdapterForTesting(mock_adapter);
cable_discovery->Start();
task_environment_.RunUntilIdle();
task_environment_.FastForwardUntilNoTasksRemain();
}
// Tests a scenario where only one of the two client EID's are advertised
......@@ -456,7 +457,7 @@ TEST_F(FidoCableDiscoveryTest, TestDiscoveryWithPartialAdvertisementSuccess) {
BluetoothAdapterFactory::SetAdapterForTesting(mock_adapter);
cable_discovery->Start();
task_environment_.RunUntilIdle();
task_environment_.FastForwardUntilNoTasksRemain();
}
// Test the scenario when all advertisement for client EID's fails.
......@@ -488,7 +489,7 @@ TEST_F(FidoCableDiscoveryTest, TestDiscoveryWithAdvertisementFailures) {
BluetoothAdapterFactory::SetAdapterForTesting(mock_adapter);
cable_discovery->Start();
task_environment_.RunUntilIdle();
task_environment_.FastForwardUntilNoTasksRemain();
EXPECT_TRUE(cable_discovery->advertisements_.empty());
}
#endif // !defined(OS_WIN)
......@@ -509,7 +510,7 @@ TEST_F(FidoCableDiscoveryTest, TestUnregisterAdvertisementUponDestruction) {
BluetoothAdapterFactory::SetAdapterForTesting(mock_adapter);
cable_discovery->Start();
task_environment_.RunUntilIdle();
task_environment_.FastForwardUntilNoTasksRemain();
EXPECT_EQ(1u, cable_discovery->advertisements_.size());
cable_discovery.reset();
......@@ -548,6 +549,7 @@ TEST_F(FidoCableDiscoveryTest, TestResumeDiscoveryAfterPoweredOn) {
}
mock_adapter->NotifyAdapterPoweredChanged(true);
task_environment_.FastForwardUntilNoTasksRemain();
}
} // namespace device
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment