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

Bluetooth: GATT: CCC and CF values written by privacy-disabled peer before bonding may be lost #54770

Closed
MarekPieta opened this issue Feb 13, 2023 · 0 comments · Fixed by #54781
Assignees
Labels
area: Bluetooth Host bug The issue is a bug, or the PR is fixing a bug priority: high High impact/importance bug Release Blocker Use this label for justified release blockers
Milestone

Comments

@MarekPieta
Copy link
Collaborator

Describe the bug
Values of CCC and CF may not be stored in non-volatile memory if a connected peer writes the values before it's bonded. If the peer then bonds and reboots without disconnection, the values written over GATT will be lost (they would not be stored in non-volatile memory). On the other hand if the same peer disconnects after bonding an before the reboot, these values are written to non-volatile memory.

To Reproduce

Used nRF Desktop application (applications/nrf_desktop) from nRF Connect SDK (https://github.com/nrfconnect/sdk-nrf).

Steps to reproduce the behavior:

  1. Flash the nRF52840 DK with configuration that works as HID keyboard
    west build -b nrf52840dk_nrf52840 --pristine -- -DCONF_FILE=prj_keyboard.conf
    west flash --erase
  2. Add logs when the following functions are called in bluetooth/host/gatt.c: cf_write (CF value GATT write), cf_set (CF value loaded from non-volatile memory).
  3. Connect and bond with a Bluetooth Central.
    Make sure that the connected Central uses GATT Caching and triggers the CF GATT write only before bonding (otherwise the issue would not replicate; I replicated the problem with a Windows 10 Bluetooth host).
  4. Reboot Zephyr device while the Bluetooth connection is still maintained.
  5. Notice that CF value written for the bonded peer is lost (it's not loaded from settings).

If the peer is disconnected before the reboot, the value is stored during the disconnection (that works around the issue).

Expected behavior
CF and CCC values should be stored between connections for bonded peers.

Impact
Implementation of Zephyr's Bluetooth host is not consistent with the Bluetooth Core specification.

The Bluetooth Core specification (v. 5.2, vol. 3, part G, section 3.3.3.3):
The Client Characteristic Configuration descriptor value shall be persistent across connections for bonded devices.

The Bluetooth Core specification (v. 5.2, vol. 3, part G, section 7.2):

A Client Supported Features characteristic value shall exist for each connected client. For clients with a trusted relationship, the characteristic value shall be persistent across connections. For clients without a trusted relationship the characteristic value shall be set to the default value at each connection.

Environment:
Replicated on nRF Connnect SDK (https://github.com/nrfconnect/sdk-nrf; commit hash: 5be84b48937b0c42fb2b8d643ef452a25a693f17).

Additional context
Similar issue was already spotted some time ago:
#39989

Seems that the introduced fix does not cover all of the edge cases:
#40006
The fix relied on storing the CF and CCC on identity_resolved callback which may actually not be triggered e.g. if the connected peer does not support privacy.

I would suggest to trigger an additional write to storage of the mentioned values also on pairing_complete from bt_conn_auth_info_cb to prevent the issue.

@MarekPieta MarekPieta added bug The issue is a bug, or the PR is fixing a bug area: Bluetooth Host labels Feb 13, 2023
@MarekPieta MarekPieta added this to the v3.3.0 milestone Feb 13, 2023
@carlescufi carlescufi added the priority: high High impact/importance bug label Feb 13, 2023
@stephanosio stephanosio added the Release Blocker Use this label for justified release blockers label Feb 13, 2023
jori-nordic added a commit to jori-nordic/zephyr that referenced this issue Feb 13, 2023
On bond establishment: save the CF and CCC data that have been written
before the peer was bonded.

On identity resolved: update the CF data to use the peer's identity address
instead of its private address (same as is currently done for the CCC).

Fixes zephyrproject-rtos#54770.

Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
jori-nordic added a commit to jori-nordic/zephyr that referenced this issue Feb 13, 2023
On bond establishment: save the CF and CCC data that have been written
before the peer was bonded.

On identity resolved: update the CF data to use the peer's identity address
instead of its private address (same as is currently done for the CCC).

Fixes zephyrproject-rtos#54770.

Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
jori-nordic added a commit to jori-nordic/zephyr that referenced this issue Feb 13, 2023
On bond establishment: save the CF and CCC data that have been written
before the peer was bonded.

On identity resolved: update the CF data to use the peer's identity address
instead of its private address (same as is currently done for the CCC).

Fixes zephyrproject-rtos#54770.

Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
jori-nordic added a commit to jori-nordic/zephyr that referenced this issue Feb 13, 2023
On bond establishment: save the CF and CCC data that have been written
before the peer was bonded.

On identity resolved: update the CF data to use the peer's identity address
instead of its private address (same as is currently done for the CCC).

Fixes zephyrproject-rtos#54770.

Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
stephanosio pushed a commit that referenced this issue Feb 15, 2023
On bond establishment: save the CF and CCC data that have been written
before the peer was bonded.

On identity resolved: update the CF data to use the peer's identity address
instead of its private address (same as is currently done for the CCC).

Fixes #54770.

Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
dipanjan115 pushed a commit to dipanjan115/zephyr that referenced this issue Mar 8, 2023
… bonding

On bond establishment: save the CF and CCC data that have been written
before the peer was bonded.

On identity resolved: update the CF data to use the peer's identity address
instead of its private address (same as is currently done for the CCC).

Fixes zephyrproject-rtos#54770.

Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
(cherry picked from a0614b6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Bluetooth Host bug The issue is a bug, or the PR is fixing a bug priority: high High impact/importance bug Release Blocker Use this label for justified release blockers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants