Skip to content

Commit

Permalink
[CrOS MultiDevice] Handle removed/disabled BluetoothAdapter on request.
Browse files Browse the repository at this point in the history
SecureChannel previously had no checks for when the BluetoothAdapter is
disabled or removed.

This CL adds an additional check within SecureChannelImpl that will
cancel a connection request early if the BluetoothAdapter is either
disabled or removed, as well as notify the client of the failure
reason.

Bug: 854366
Change-Id: I021435611c8a018da3e77a9eeb9c001311adada4
Reviewed-on: https://chromium-review.googlesource.com/1194675
Reviewed-by: Ryan Hansberry <[email protected]>
Reviewed-by: Daniel Cheng <[email protected]>
Reviewed-by: Kyle Horimoto <[email protected]>
Commit-Queue: Kyle Qian <[email protected]>
Cr-Commit-Position: refs/heads/master@{#589399}
  • Loading branch information
Kyle Qian authored and Commit Bot committed Sep 7, 2018
1 parent dd1a032 commit 3e7cdbb
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,13 @@ enum ConnectionAttemptFailureReason {
REMOTE_DEVICE_INVALID_PSK,

// Timeouts occurred trying to contact the remote device.
TIMEOUT_FINDING_DEVICE
TIMEOUT_FINDING_DEVICE,

// The local Bluetooth adapter is disabled (turned off).
ADAPTER_DISABLED,

// The local Bluetooth adapter is not present.
ADAPTER_NOT_PRESENT
};

enum ConnectionCreationDetail {
Expand Down
36 changes: 34 additions & 2 deletions chromeos/services/secure_channel/secure_channel_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,16 @@ SecureChannelImpl::ConnectionRequestWaitingForDisconnection::

SecureChannelImpl::SecureChannelImpl(
scoped_refptr<device::BluetoothAdapter> bluetooth_adapter)
: timer_factory_(TimerFactoryImpl::Factory::Get()->BuildInstance()),
: bluetooth_adapter_(std::move(bluetooth_adapter)),
timer_factory_(TimerFactoryImpl::Factory::Get()->BuildInstance()),
remote_device_cache_(
cryptauth::RemoteDeviceCache::Factory::Get()->BuildInstance()),
ble_service_data_helper_(
BleServiceDataHelperImpl::Factory::Get()->BuildInstance(
remote_device_cache_.get())),
ble_connection_manager_(
BleConnectionManagerImpl::Factory::Get()->BuildInstance(
bluetooth_adapter,
bluetooth_adapter_,
ble_service_data_helper_.get(),
timer_factory_.get())),
pending_connection_manager_(
Expand Down Expand Up @@ -207,6 +208,17 @@ void SecureChannelImpl::ProcessConnectionRequest(
return;
}

// Check 4: Medium-specific verification.
switch (connection_medium) {
case ConnectionMedium::kBluetoothLowEnergy:
// Is the local Bluetooth adapter disabled or not present? If either,
// notify client and return early.
if (CheckIfBluetoothAdapterDisabledOrNotPresent(
api_fn_name, client_connection_parameters.get()))
return;
break;
}

// At this point, the request has been deemed valid.
ConnectionAttemptDetails connection_attempt_details(
device_to_connect.GetDeviceId(), local_device.GetDeviceId(),
Expand Down Expand Up @@ -320,6 +332,26 @@ bool SecureChannelImpl::CheckForInvalidInputDevice(
return true;
}

bool SecureChannelImpl::CheckIfBluetoothAdapterDisabledOrNotPresent(
ApiFunctionName api_fn_name,
ClientConnectionParameters* client_connection_parameters) {
if (!bluetooth_adapter_->IsPresent()) {
RejectRequestForReason(
api_fn_name, mojom::ConnectionAttemptFailureReason::ADAPTER_NOT_PRESENT,
client_connection_parameters);
return true;
}

if (!bluetooth_adapter_->IsPowered()) {
RejectRequestForReason(
api_fn_name, mojom::ConnectionAttemptFailureReason::ADAPTER_DISABLED,
client_connection_parameters);
return true;
}

return false;
}

base::Optional<SecureChannelImpl::InvalidRemoteDeviceReason>
SecureChannelImpl::AddDeviceToCacheIfPossible(
ApiFunctionName api_fn_name,
Expand Down
10 changes: 9 additions & 1 deletion chromeos/services/secure_channel/secure_channel_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ class SecureChannelImpl : public mojom::SecureChannel,
~SecureChannelImpl() override;

private:
SecureChannelImpl(scoped_refptr<device::BluetoothAdapter> bluetooth_adapter);
explicit SecureChannelImpl(
scoped_refptr<device::BluetoothAdapter> bluetooth_adapter);

enum class InvalidRemoteDeviceReason { kInvalidPublicKey, kInvalidPsk };

Expand Down Expand Up @@ -136,13 +137,20 @@ class SecureChannelImpl : public mojom::SecureChannel,
ClientConnectionParameters* client_connection_parameters,
bool is_local_device);

// Checks if |bluetooth_adapter_| is disabled or not present and rejects the
// connection request if so. Returns whether the request was rejected.
bool CheckIfBluetoothAdapterDisabledOrNotPresent(
ApiFunctionName api_fn_name,
ClientConnectionParameters* client_connection_parameters);

// Validates |device| and adds it to the |remote_device_cache_| if it is
// valid. If it is not valid, the reason is provided as a return type, and the
// device is not added to the cache.
base::Optional<InvalidRemoteDeviceReason> AddDeviceToCacheIfPossible(
ApiFunctionName api_fn_name,
const cryptauth::RemoteDevice& device);

scoped_refptr<device::BluetoothAdapter> bluetooth_adapter_;
std::unique_ptr<TimerFactory> timer_factory_;
std::unique_ptr<cryptauth::RemoteDeviceCache> remote_device_cache_;
std::unique_ptr<BleServiceDataHelper> ble_service_data_helper_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,14 @@ class SecureChannelServiceTest : public testing::Test {
void SetUp() override {
mock_adapter_ =
base::MakeRefCounted<testing::NiceMock<device::MockBluetoothAdapter>>();
is_adapter_powered_ = true;
is_adapter_present_ = true;
ON_CALL(*mock_adapter_, IsPresent())
.WillByDefault(
Invoke(this, &SecureChannelServiceTest::is_adapter_present));
ON_CALL(*mock_adapter_, IsPowered())
.WillByDefault(
Invoke(this, &SecureChannelServiceTest::is_adapter_powered));
device::BluetoothAdapterFactory::SetAdapterForTesting(mock_adapter_);

test_task_runner_ = base::MakeRefCounted<base::TestSimpleTaskRunner>();
Expand Down Expand Up @@ -584,6 +592,12 @@ class SecureChannelServiceTest : public testing::Test {

const cryptauth::RemoteDeviceList& test_devices() { return test_devices_; }

bool is_adapter_present() { return is_adapter_present_; }
void set_is_adapter_present(bool present) { is_adapter_present_ = present; }

bool is_adapter_powered() { return is_adapter_powered_; }
void set_is_adapter_powered(bool powered) { is_adapter_powered_ = powered; }

private:
void AttemptConnectionAndVerifyPendingConnection(
const cryptauth::RemoteDevice& device_to_connect,
Expand Down Expand Up @@ -853,6 +867,9 @@ class SecureChannelServiceTest : public testing::Test {
std::unique_ptr<service_manager::TestConnectorFactory> connector_factory_;
std::unique_ptr<service_manager::Connector> connector_;

bool is_adapter_powered_;
bool is_adapter_present_;

mojom::SecureChannelPtr secure_channel_ptr_;

DISALLOW_COPY_AND_ASSIGN(SecureChannelServiceTest);
Expand Down Expand Up @@ -948,6 +965,48 @@ TEST_F(SecureChannelServiceTest, InitiateConnection_MissingLocalDevicePsk) {
mojom::ConnectionAttemptFailureReason::LOCAL_DEVICE_INVALID_PSK);
}

TEST_F(SecureChannelServiceTest,
ListenForConnection_BluetoothAdapterNotPresent) {
FinishInitialization();

set_is_adapter_present(false);

CallListenForConnectionFromDeviceAndVerifyRejection(
test_devices()[0], test_devices()[1], "feature", ConnectionPriority::kLow,
mojom::ConnectionAttemptFailureReason::ADAPTER_NOT_PRESENT);
}

TEST_F(SecureChannelServiceTest,
InitiateConnection_BluetoothAdapterNotPresent) {
FinishInitialization();

set_is_adapter_present(false);

CallInitiateConnectionToDeviceAndVerifyRejection(
test_devices()[0], test_devices()[1], "feature", ConnectionPriority::kLow,
mojom::ConnectionAttemptFailureReason::ADAPTER_NOT_PRESENT);
}

TEST_F(SecureChannelServiceTest, ListenForConnection_BluetoothAdapterDisabled) {
FinishInitialization();

set_is_adapter_powered(false);

CallListenForConnectionFromDeviceAndVerifyRejection(
test_devices()[0], test_devices()[1], "feature", ConnectionPriority::kLow,
mojom::ConnectionAttemptFailureReason::ADAPTER_DISABLED);
}

TEST_F(SecureChannelServiceTest, InitiateConnection_BluetoothAdapterDisabled) {
FinishInitialization();

set_is_adapter_powered(false);

CallInitiateConnectionToDeviceAndVerifyRejection(
test_devices()[0], test_devices()[1], "feature", ConnectionPriority::kLow,
mojom::ConnectionAttemptFailureReason::ADAPTER_DISABLED);
}

TEST_F(SecureChannelServiceTest, CallsQueuedBeforeInitializationComplete) {
CallInitiateConnectionToDeviceAndVerifyInitializationNotComplete(
test_devices()[4], test_devices()[5], "feature",
Expand Down

0 comments on commit 3e7cdbb

Please sign in to comment.