Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature ble host privacy #13717

Merged
merged 65 commits into from
Oct 15, 2020
Merged

Feature ble host privacy #13717

merged 65 commits into from
Oct 15, 2020

Conversation

pan-
Copy link
Member

@pan- pan- commented Oct 5, 2020

Summary of changes

This PR reimplements Bluetooth privacy for all type of Bluetooth controllers (4.0 to 5.2).
As soon as privacy is enabled, a private address is used to advertise, scan or initiate connection and peers resolvable addresses are resolved if a bond with matching IRK is found in the security database.

This should solve most of the pairing issues observed on Android and iOS where the local device is not able to retrieve a previous bond.

Internally there is two implementations of the address resolution:

  • Host based where the software compute if the address is known or not
  • Controller based where the Bluetooth controller resolves automatically addresses received during advertising or scan.

New configurations flags have been added to select at compile time the implementation to use:

  • ble-gap-host-based-private-address-resolution Forces the system to use host based resolution
  • ble-gap-host-privacy-resolved-cache-size Indicate how many entries must be booked for the host based resolution cache.

This PR is the result of the following PRs merged on the feature branch

Impact of changes

Existing applications should not have anything to change regards to calls to privacy APIs as this PR enables it for all type of controllers and not just resolution when supported by the hardware.

The advertising state was showing up as enabled as soon as the function startAdvertising was called. This is was not correct as it wasn't reflecting the reality of operations. The state is now modified when advertising is actually active. Two new events have been added to Gap::EventHandler to catch the change of advertising state.

Migration actions required

Documentation

None


Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[x] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

privacy-tests.zip

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[x] Tests / results supplied as part of this PR

Reviewers


pan- and others added 30 commits October 5, 2020 11:50
This PAL interface is responsible for generating, resolving private address and maintaining the controller or host resolving list.
It also indicates to upper level if LL resolution is supported or not and allows them to enable it.
It controls private address generation and host or controller address resolution.
Resolution list has been removed from the pal as this is handled by the PrivateAddressController.
A race condition was present if a single event was present in the event queue and the event was generating a new event.
enabling/disabling scanning and advertising operations can be prevented if the previous operations hasn't completed.
Return that the stack is busy and cannot fulfill the request at the moment rather than the state is invalid.
…platform.

The function to get it has been removed as this operation is driven by the security manager.
This also ensure the random static address used by gap is the correct one.
@pan-
Copy link
Member Author

pan- commented Oct 12, 2020

@alexherriott-dmc You should start advertising when the privacy has been enabled. You receive that event through Gap::onPrivacyEnabled (see:

virtual void onPrivacyEnabled()
)

@alexherriott-dmc
Copy link

@pan- thanks for that, that seems to work then. I'm still testing bonding for #13499, it works on the iOS test device. I will let you know if the Android devices also work. Another thing I have noticed, SetPeripheralPrivacyConfiguration(REJECT_NON_RESOLVED_ADDRESS) seems to be behaving differently and not rejecting unbonded devices. Instead it lets them connect and attempt to pair. Before I would see disconnects with reason 22 for encryption failure. Is there something else I need to address there?

@pan-
Copy link
Member Author

pan- commented Oct 13, 2020

@alexherriott-dmc In the case you described, the connection event should not be passed to the application, the system should take care of disconnecting the peer. Is it what you are observing ?
To prevent controllers to accept the connection from unknown peer you need a bt5 controller and you should enable the whitelist filled with paired addresses.
That won't work with bt4 devices because whitelist filtering is made by the controller before the address resolution on the host. So it would reject known devices advertising with a private address.

if (_connect_to_host_resolved_address_state == ConnectionToHostResolvedAddressState::scan) {
return BLE_ERROR_OPERATION_NOT_PERMITTED;
}
#endif BLE_GAP_HOST_BASED_PRIVATE_ADDRESS_RESOLUTION
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#endif //BLE_GAP_HOST_BASED_PRIVATE_ADDRESS_RESOLUTION

The connection event reported by the WB55 is incorrect if controller privacy is not enable and the peer connects with an unknown private resolvable address: The RPA field contains the connection address (it should be empty) and the peer address is all FF while it should be equal to the connection address.
@pan-
Copy link
Member Author

pan- commented Oct 14, 2020

@0xc0170 We consider that PR ready, anything you would like from us ?

@mergify mergify bot added needs: CI and removed needs: review labels Oct 15, 2020
@mbed-ci
Copy link

mbed-ci commented Oct 15, 2020

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_build-ARM ✔️
jenkins-ci/mbed-os-ci_build-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️
jenkins-ci/mbed-os-ci_dynamic-memory-usage ✔️
jenkins-ci/mbed-os-ci_cloud-client-pytest ✔️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants