New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 854366 link

Starred by 0 users
Status: Fixed
Owner:
█ kyleqian@google.com
Last visit > 30 days ago
Closed: Sep 7
Cc:
█ jlklein@chromium.org
jessejames@chromium.org
█ lesliewatkins@chromium.org
khorimoto@chromium.org
█ jordynass@chromium.org
█ jhawkins@chromium.org
hansberry@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

SecureChannel: Handle BluetoothAdapter being disabled or removed

Project Member Reported by khorimoto@chromium.org, Jun 19 2018 
Currently, there are no checks within the service for when the BluetoothAdapter is disabled (i.e., turned off by a user) or removed (i.e., disappears due to firmware bug - b/64035404).

Instant Tethering and EasyUnlock automatically shut off when Bluetooth is not enabled, so connection requests originating from them will work "by chance" since they will be canceled. However, AWM and MultiDeviceSetup do not currently have those checks.

Proposed solution:
(1) Add two extra failure reasons (Bluetooth disabled and Bluetooth removed) to secure_channel.mojom.
(2) Pass the BluetoothAdapter to PendingConnectionRequestBase, and if/when the adapter becomes disabled/removed, call OnFinishedWithoutConnection(), passing the appropriate reasons defined in step 1.

[1] https://cs.chromium.org/chromium/src/chromeos/services/secure_channel/public/mojom/secure_channel.mojom?q=ConnectionAttemptFailureReason
[2] https://cs.chromium.org/chromium/src/chromeos/services/secure_channel/pending_connection_request_base.h

Comment 1 by khorimoto@chromium.org, Aug 1

Labels: -M-69 M-70
Moved to M-70.

Comment 2 by khorimoto@chromium.org, Aug 1

Cc: jessejames@chromium.org

Comment 3 by jlklein@chromium.org, Aug 16

Owner: kyleqian@chromium.org
Status: Assigned (was: Available)

Comment 4 by kyleqian@google.com, Aug 23

Owner: kyleqian@google.com

Comment 5 by khorimoto@chromium.org, Aug 23

Status: Started (was: Assigned)

Comment 6 by dgagnon@chromium.org, Sep 5 

Friendly ping to get an update as it is marked as RBS. Thanks

Comment 7 by jlklein@chromium.org, Sep 5

Labels: -Pri-1 -ReleaseBlock-Stable Pri-2
Removing RBS and downgrading priority because AWM and MultiDeviceSetup don't use bluetooth right now at all.
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 7 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3e7cdbb5855fe699f2a1a8dd276031b84cabbd49

commit 3e7cdbb5855fe699f2a1a8dd276031b84cabbd49
Author: Kyle Qian <kyleqian@google.com>
Date: Fri Sep 07 01:18:40 2018

[CrOS MultiDevice] Handle removed/disabled BluetoothAdapter on request.

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 <hansberry@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Kyle Qian <kyleqian@google.com>
Cr-Commit-Position: refs/heads/master@{#589399}
[modify] https://crrev.com/3e7cdbb5855fe699f2a1a8dd276031b84cabbd49/chromeos/services/secure_channel/public/mojom/secure_channel.mojom
[modify] https://crrev.com/3e7cdbb5855fe699f2a1a8dd276031b84cabbd49/chromeos/services/secure_channel/secure_channel_impl.cc
[modify] https://crrev.com/3e7cdbb5855fe699f2a1a8dd276031b84cabbd49/chromeos/services/secure_channel/secure_channel_impl.h
[modify] https://crrev.com/3e7cdbb5855fe699f2a1a8dd276031b84cabbd49/chromeos/services/secure_channel/secure_channel_service_unittest.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 7 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5a076a10e27d5d1904a51394854ba2b5652eb60a

commit 5a076a10e27d5d1904a51394854ba2b5652eb60a
Author: Kyle Qian <kyleqian@google.com>
Date: Fri Sep 07 01:56:15 2018

[CrOS MultiDevice] Handle removed/disabled BluetoothAdapter on pending.

SecureChannel previously had no handlers for when the BluetoothAdapter
is disabled or removed while a connection request is pending.

This CL adds observer methods to PendingConnectionRequestBase that will
fire when device::BluetoothAdapter detects that the adapter has been
disabled or removed, causing the request to be cancelled.

Bug:  854366 
Change-Id: Ice45c373f42a6e9065d8c3564988904e02e7fa9e
Reviewed-on: https://chromium-review.googlesource.com/1195099
Commit-Queue: Kyle Qian <kyleqian@google.com>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589407}
[modify] https://crrev.com/5a076a10e27d5d1904a51394854ba2b5652eb60a/chromeos/services/secure_channel/BUILD.gn
[add] https://crrev.com/5a076a10e27d5d1904a51394854ba2b5652eb60a/chromeos/services/secure_channel/pending_ble_connection_request_base.h
[add] https://crrev.com/5a076a10e27d5d1904a51394854ba2b5652eb60a/chromeos/services/secure_channel/pending_ble_connection_request_base_unittest.cc
[modify] https://crrev.com/5a076a10e27d5d1904a51394854ba2b5652eb60a/chromeos/services/secure_channel/pending_ble_initiator_connection_request.cc
[modify] https://crrev.com/5a076a10e27d5d1904a51394854ba2b5652eb60a/chromeos/services/secure_channel/pending_ble_initiator_connection_request.h
[modify] https://crrev.com/5a076a10e27d5d1904a51394854ba2b5652eb60a/chromeos/services/secure_channel/pending_ble_initiator_connection_request_unittest.cc
[modify] https://crrev.com/5a076a10e27d5d1904a51394854ba2b5652eb60a/chromeos/services/secure_channel/pending_ble_listener_connection_request.cc
[modify] https://crrev.com/5a076a10e27d5d1904a51394854ba2b5652eb60a/chromeos/services/secure_channel/pending_ble_listener_connection_request.h
[modify] https://crrev.com/5a076a10e27d5d1904a51394854ba2b5652eb60a/chromeos/services/secure_channel/pending_ble_listener_connection_request_unittest.cc
[modify] https://crrev.com/5a076a10e27d5d1904a51394854ba2b5652eb60a/chromeos/services/secure_channel/pending_connection_manager_impl.cc
[modify] https://crrev.com/5a076a10e27d5d1904a51394854ba2b5652eb60a/chromeos/services/secure_channel/pending_connection_manager_impl.h
[modify] https://crrev.com/5a076a10e27d5d1904a51394854ba2b5652eb60a/chromeos/services/secure_channel/pending_connection_manager_impl_unittest.cc
[modify] https://crrev.com/5a076a10e27d5d1904a51394854ba2b5652eb60a/chromeos/services/secure_channel/secure_channel_impl.cc
[modify] https://crrev.com/5a076a10e27d5d1904a51394854ba2b5652eb60a/chromeos/services/secure_channel/secure_channel_service_unittest.cc

Comment 10 by jlklein@chromium.org, Sep 7

Status: Fixed (was: Started)
►

Sign in to add a comment