bluetooth: host: smp: Don't try to add peer to Resolving List when Privacy feature is disabled #93821
Conversation
zephyrbot
added
area: Bluetooth
area: Bluetooth Host
zephyrbot
requested review from
alwa-nordic,
cvinayak,
hermabe,
jhedberg,
rugeGerritsen,
sjanc and
Thalley
|
Shouldn't ID key be disabled in distribution list instead of this (ie IRK should not be allowed in first place)? Otherwise peer may use RPAs and thus will never be able to reconnect? |
PavelVPV
force-pushed
the
dont_invalidate_bonding_if_privacy_disable
branch
from
24a34d9 to
91ae0f2
Compare
You seem to be right. I've updated the code to reject IRK and address distribution of privacy is disabled. |
If the Privacy feature is disabled, drop the IdKey flag in the Initiator Key Distribution/Generation field in the Pairing Response. It was observed that when built without the Privacy feature and bonding with a Central device that supports Privacy, the Central device will send the IRK and BD_ADDR through the Identity Information and Identity Address Information commands. This causes the Host to attempt to add the Peer IRK and Peer Identity Address to the Controller's Resolving List. However, this operation will fail because the Privacy feature is disabled. Additionally, the Host will attempt to check for conflicting IRKs. If such a conflict exists, it will either reject the pairing or remove the conflicting IRK based on its configuration. This behavior is unnecessary since the Privacy feature is disabled and IRKs are not used. Signed-off-by: Pavel Vasilyev <[email protected]>
PavelVPV
force-pushed
the
dont_invalidate_bonding_if_privacy_disable
branch
from
91ae0f2 to
9182d23
Compare
|
| #if defined(CONFIG_BT_PRIVACY) | ||
| static uint8_t smp_ident_info(struct bt_smp *smp, struct net_buf *buf) |
There was a problem hiding this comment.
I don't think we have any specific guidelines for this, but I do have a slight preference of using IS_ENABLED(CONFIG_BT_PRIVACY) inside smp_ident_info to handle the return value, instead of guarding the entire function.
Ditto for smp_ident_addr_info
There was a problem hiding this comment.
I just followed the same pattern as other command handlers have in the file.
| @@ -71,7 +71,7 @@ LOG_MODULE_REGISTER(bt_smp); | |||
| #define LINK_DIST 0 | |||
| #endif | |||
|
|
|||
| #define RECV_KEYS (BT_SMP_DIST_ENC_KEY | BT_SMP_DIST_ID_KEY | SIGN_DIST |\ | |||
There was a problem hiding this comment.
I don't think so, but I do wonder if this change belongs in the release notes or migration guides? This is a functional change in some way, but I don't see why it should a problem.
There was a problem hiding this comment.
I can add this to the release notes as this changes Host behavior (though not sure if anyone relied on this).
There was a problem hiding this comment.
It seems like there is no place for such change in the new format of release notes.
There was a problem hiding this comment.
Hmm, yeah. The new format does somewhat discourage documentation functional changes that does not require a migration guide entry (if there's nothing to migrate to)
|
I'm confused. I thought zephyr/subsys/bluetooth/host/Kconfig Lines 441 to 449 in 8f2d3e7 This PR seems to modify |
Fair point, I missed that. Then I need to think about another solution for this issue. |
If the Privacy feature is disabled, drop the IdKey flag in the Initiator Key Distribution/Generation field in the Pairing Response.
It was observed that when built without the Privacy feature and bonding with a Central device that supports Privacy, the Central device will send the IRK and BD_ADDR through the Identity Information and Identity Address Information commands.
This causes the Host to attempt to add the Peer IRK and Peer Identity Address to the Controller's Resolving List. However, this operation will fail because the Privacy feature is disabled.
Additionally, the Host will attempt to check for conflicting IRKs. If such a conflict exists, it will either reject the pairing or remove the conflicting IRK based on its configuration. This behavior is unnecessary since the Privacy feature is disabled and IRKs are not used.