zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.89k stars 6.64k forks source link

Bluetooth: Host: advertising sets linked with the same identity may use different RPAs #64625

Closed kapi-no closed 8 months ago

kapi-no commented 1 year ago

Describe the bug

Advertising sets linked with the same Bluetooth identity may use different RPAs, which breaks the assumption introduced by the following PR:

https://github.com/zephyrproject-rtos/zephyr/pull/55449

This misbehaviour happens under certain conditions and when the developer uses the rpa_expired callback API:

/**
 * @brief The RPA validity of the advertising set has expired.
 *
 * This callback notifies the application that the RPA validity of
 * the advertising set has expired. The user can use this callback
 * to synchronize the advertising payload update with the RPA rotation.
 *
 * @param adv  The advertising set object.
 *
 * @return true to rotate the current RPA, or false to use it for the
 *         next rotation period.
 */
bool (*rpa_expired)(struct bt_le_ext_adv *adv);

Let's assume that the application defines two advertising sets that are linked to the same Bluetooth identity. Both sets start to advertise with the RPA X. At some point, the RPA is timed out and the rpa_expired callback fires for both advertising sets. If the first advertising set chooses to withhold the RPA rotation by returning false in the rpa_expired callback and the second set progresses with the RPA rotation by returning true in the rpa_expired callback, then the first set will keep using the RPA X in the subsequent RPA rotation period and the second set will start to advertise with a new RPA Y. In this case, two advertising sets belonging to the same Bluetooth identity will use two different RPAs: RPA X and RPA Y.

To Reproduce

Steps to reproduce the behavior:

  1. Configure the Bluetooth sample to use the Advertising Extension (CONFIG_BT_EXT_ADV) and Privacy (CONFIG_BT_PRIVACY). Make sure to set a proper Kconfig option to have the possibility to register at least two advertising sets (CONFIG_BT_EXT_ADV_MAX_ADV_SET=2).

    @alwa-nordic EDIT since https://github.com/zephyrproject-rtos/zephyr/pull/64733: Set CONFIG_BT_RPA_SHARING=y.

  2. In the application code, create two advertising sets using the bt_le_ext_adv_create API and register the rpa_expired callback for both of them.
  3. Set two different payloads using the bt_le_ext_adv_set_data API to easily distinguish two advertising sets on the scanning device.
  4. Always return false In the rpa_expired callback implementation for the first advertising set.
  5. Always return true In the rpa_expired callback implementation for the second advertising set.
  6. Start the advertising for both sets using the bt_le_ext_adv_start API.
  7. Build and flash the application.
  8. See that both advertising sets use the same RPA during the first rotation.
  9. Wait for the RPA rotation timeout.
  10. See that the first advertising set uses the RPA from step 9 and the second advertising set uses a new one.

Expected behavior

After RPA timeout, both advertising sets should either stay with the same RPA or migrate to the new one based on the return value of the rpa_expired API.

Impact

The feature for sharing the same RPA between the advertising sets does not work reliably.

Environment (please complete the following information):

alwa-nordic commented 1 year ago

After RPA timeout, both advertising sets should either stay with the same RPA or migrate to the new one based on the return value of the rpa_expired API.

Which of those behaviors makes the most sense to you, @kapi-no? It seems both options have drawbacks. I see you implemented the rpa_expired hook originally. Maybe we need to rethink our API in some other way when we take into account both @niym-ot's need to synchronize RPAs and your need to delay rotations?

kapi-no commented 1 year ago

For my use case, I will probably disable the RPA sharing feature once we agree on a proper way to make this feature configurable in PR #64733.

However, in the RPA sharing mode, we need to devise a policy on how to treat the return value from the rpa_expired callback and document it in the callback API documentation.

My suggestion is to withhold the RPA rotation if any of the advertising sets in the RPA sharing mode returns false. The advertising set typically withholds the RPA rotation for the given rotation period in case of failure to update its advertising payload that contains random data or unique identifiers. In such a case, the advertising payload that did not get any updates leaks privacy (it acts as if the RPA rotation did not happen), so the RPA rotation doesn't provide any value.

Following this return value policy (strong false & weak true), to perform RPA rotation all advertising sets need to return true in the rpa_expired callback.

If there is need at some point to also have the inverse return value policy (i. e. weak false & strong true), we can add a Kconfig option that allows users to configure the policy according to their needs and requirements.

I think it would be best to merge my PR #64733 before we start to align the implementation for the RPA sharing mode. We need to align the rpa_expired callback documentation to indicate how the return value is interpreted in two cases: RPA sharing and RPA unique modes.

@niym-ot, please let me know if you see any blockers with my proposal when it comes to your use case.

alwa-nordic commented 1 year ago

Would it make sense to align the behavior to what HCI specifies for Controller-based privacy?

kapi-no commented 1 year ago

@alwa-nordic, do you have any specifics in mind?

The Bluetooth Host needs to be notified about the RPA-expired event as it needs to synchronize RPA rotation with advertising payload update. I think we cannot rely on the automatic RPA rotations at the Bluetooth Controller level.

alwa-nordic commented 1 year ago

I mean that whatever the behavior in Controller-based privacy is, I think it is wise to replicate that behavior in the host by default.

I think Controller-based privacy will assign a separate RPA to each advertiser set. I.e. The non-shared mode should be default.

Any comments on this, anyone?

kapi-no commented 1 year ago

Oh, I see. So the optional configuration to make the RPA sharing is done in the following PR:

https://github.com/zephyrproject-rtos/zephyr/pull/64733

The current PR disables the RPA sharing by default and you can enable it using a dedicated Kconfig symbol

CONFIG_BT_RPA_SHARING=y

The discussion here aims to address the issue with the rpa_expired callback API in case the RPA sharing is enabled (CONFIG_BT_RPA_SHARING=y). In the soon to be default mode (CONFIG_BT_RPA_SHARING=n), the issue does not appear.

alwa-nordic commented 1 year ago

@kapi-no Now that your problem is solved in a different way, do you intend to work on this issue? If not, please unassign yourself :)

kapi-no commented 1 year ago

Sure, I will unassign myself as this issue does not impact my project anymore. However, the issue still exists and should not be closed.

@alwa-nordic, thank you for updating the issue description. Now this issue only appears when the CONFIG_BT_RPA_SHARING is enabled.

github-actions[bot] commented 9 months ago

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

jori-nordic commented 9 months ago

@niym-ot It seems you implemented the feature. Could you try to analyze the issue too?

niym-ot commented 9 months ago

@niym-ot It seems you implemented the feature. Could you try to analyze the issue too? Sure @jori-nordic

niym-ot commented 9 months ago

@jori-nordic

What's the proposal here ? Current behavior (As I understand from the use case mentioned above)

When one of the adv_set's rpa_expired cb is returned as false ( with CONFIG_BT_RPA_SHARING=y), adv_sets advertise with different RPA's.

Proposal ??

If any of the adv_set's rpa_expired cb returns false, all adv_set needs to continue with the old RPA

Please suggest

jori-nordic commented 9 months ago

If any of the adv_set's rpa_expired cb returns false, all adv_set needs to continue with the old RPA

That would be fine with me. Logging a debug message in that case would also be helpful.