SecureChannel: Handle missing/stale BeaconSeeds |
||||
Issue descriptionCurrently, SecureChannel assumes that a BLE advertisement can be created (i.e., that both the local device and remote device have valid BeaconSeeds for the current time period). If a user keeps a Chromebook offline for over a month straight, it's possible that that the synced BeaconSeeds are out of date. To complete this task: (1) BleAdvertiserImpl creates advertisements (see [1]). When an advertisement is generated, it possible that the return value for GenerateForegroundAdvertisement() is null, but that case is currently not handled. (2) If the advertisement is indeed null, BleAdvertiserImpl should notify its delegate and not try to advertise. (3) BleConnectionManagerImpl is BleAdvertiserImpl's delegate, and it should in turn invoke the failure callback for the device, passing BleInitiatorFailureType::kCouldNotGenerateAdvertisement as the failure type. (4) BleInitiatorOperation should pass this failure up to BleInitiatorConnectionAttempt, which should notify all PendingBleInitiatorConnectionRequests. PendingBleInitiatorConnectionRequest already handles this case (see [3]). [1] https://cs.chromium.org/chromium/src/chromeos/services/secure_channel/ble_advertiser_impl.cc?q=GenerateForegroundAdvertisement [2] https://cs.chromium.org/chromium/src/chromeos/services/secure_channel/ble_initiator_failure_type.h?q=kCouldNotGenerateAdvertisement [3] https://cs.chromium.org/chromium/src/chromeos/services/secure_channel/pending_ble_initiator_connection_request.cc?q=kCouldNotGenerateAdvertisement
,
Aug 1
,
Aug 14
,
Aug 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/175fdd0bc72c8e32c4b03fc13a19c1dd780c5a2b commit 175fdd0bc72c8e32c4b03fc13a19c1dd780c5a2b Author: Kyle Qian <kyleqian@google.com> Date: Mon Aug 27 23:34:22 2018 [CrOS MultiDevice] Handle missing/stale BeaconSeeds. SecureChannel previously assumed that BleServiceDataHelper::GenerateForegroundAdvertisement() would always successfully return advertisement data. However, it currently returns a nullptr if either the local device public key or the remote device ID is invalid. In particular, the former can happen if a user keeps their Chromebook offline for over a month straight, resulting in stale BeaconSeeds. This CL adds the missing nullptr check within BleAdvertiserImpl, as well as a new associated delegate method, which is implemented within BleConnectionManagerImpl. Bug: 854452 Change-Id: Icc0675950c674a34553fc8ea1f0bc0e70eece123 Reviewed-on: https://chromium-review.googlesource.com/1175078 Commit-Queue: Kyle Qian <kyleqian@google.com> Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Cr-Commit-Position: refs/heads/master@{#586490} [modify] https://crrev.com/175fdd0bc72c8e32c4b03fc13a19c1dd780c5a2b/chromeos/services/secure_channel/ble_advertiser.cc [modify] https://crrev.com/175fdd0bc72c8e32c4b03fc13a19c1dd780c5a2b/chromeos/services/secure_channel/ble_advertiser.h [modify] https://crrev.com/175fdd0bc72c8e32c4b03fc13a19c1dd780c5a2b/chromeos/services/secure_channel/ble_advertiser_impl.cc [modify] https://crrev.com/175fdd0bc72c8e32c4b03fc13a19c1dd780c5a2b/chromeos/services/secure_channel/ble_advertiser_impl.h [modify] https://crrev.com/175fdd0bc72c8e32c4b03fc13a19c1dd780c5a2b/chromeos/services/secure_channel/ble_advertiser_impl_unittest.cc [modify] https://crrev.com/175fdd0bc72c8e32c4b03fc13a19c1dd780c5a2b/chromeos/services/secure_channel/ble_connection_manager_impl.cc [modify] https://crrev.com/175fdd0bc72c8e32c4b03fc13a19c1dd780c5a2b/chromeos/services/secure_channel/ble_connection_manager_impl.h [modify] https://crrev.com/175fdd0bc72c8e32c4b03fc13a19c1dd780c5a2b/chromeos/services/secure_channel/ble_connection_manager_impl_unittest.cc [modify] https://crrev.com/175fdd0bc72c8e32c4b03fc13a19c1dd780c5a2b/chromeos/services/secure_channel/fake_ble_advertiser.cc [modify] https://crrev.com/175fdd0bc72c8e32c4b03fc13a19c1dd780c5a2b/chromeos/services/secure_channel/fake_ble_advertiser.h
,
Aug 27
|
||||
►
Sign in to add a comment |
||||
Comment 1 by khorimoto@chromium.org
, Aug 1