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

Fix CTKD failure introduced by Host RPA generation #528

Merged
merged 1 commit into from
Aug 12, 2024
Merged

Conversation

zxzxwu
Copy link
Collaborator

@zxzxwu zxzxwu commented Aug 9, 2024

  • Fix the bug which device doesn't generate a static address by default
  • Properly distribute public or random address if no identity address type is set
  • Add configurable identity address type and io capabilities

@zxzxwu zxzxwu requested a review from barbibulle August 9, 2024 15:56
bumble/smp.py Outdated
@@ -1079,7 +1079,7 @@ def send_pairing_dhkey_check_command(self) -> None:

def send_identity_address_command(self) -> None:
identity_address = {
None: self.manager.device.static_address,
None: self.connection.self_address,
Copy link
Collaborator

Choose a reason for hiding this comment

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

It doesn't seem right to send an identity address that's not the device's static random or public address.
The spec, BLUETOOTH CORE SPECIFICATION Version 5.4 | Vol 6, Part B Paragraph 1.3 does say "A device's Identity Address is a Public Device Address or Random Static Device Address that it uses in packets it transmits. If a device is using Resolvable Private Addresses, it shall also have an Identity Address.",
so if the connection address is an RPA, that can't be used as an identity address (and the peer would also get confused because they would then be resolving further RPAs from this device as resolving to that connection address.
Can you explain in which way you think this breaks CTKD?

Copy link
Collaborator Author

@zxzxwu zxzxwu Aug 9, 2024

Choose a reason for hiding this comment

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

Consider the following enviroment:

  • Bumble has Classic and CTKD over Classic enabled
  • Bumble doesn't set preferred identity address type

In this case, Bumble will send its Static Random address as identity address, even in a CTKD over Classic session, which is very confusing to remote device - but scenario commonly exists in most of Bumble's Classic-only examples.

One thing very difficult to handle is that Bumble allows device to have public and random static address simultaneously, so it's hard to tell which one should be considered as identity address unless user always sets the perferred identity address.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. So maybe the right thing here is that if the user doesn't choose static or public identity in the pairing config (default), then the choice should be:

  • the public address if it is set (not == ANY)
  • else the static random address

If the user wants the static random address even if the controller has a public address, then they should set it explicitly in the PairingConfig.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Make sense, but not sure what's the impact to existing users.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks good to them.

@zxzxwu zxzxwu merged commit 0c31713 into google:main Aug 12, 2024
57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants