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

BLE Host - Don't persist security material if either key distribution field is 0 #370

Merged
merged 1 commit into from
Jul 7, 2017

Conversation

ccollins476ad
Copy link
Contributor

This issue is documented in: https://issues.apache.org/jira/browse/MYNEWT-749

This commit implements the following behavior:

If either peer indicates a key distribution flags value of 0, then don't persist security material when pairing completes.

It isn't clear from the spec what should happen when both peers indicate bonding support, but one indicates a 0 key distribution field. I think proceding with pairing and just not persisting makes the most sense. This is also what Lightblue (or CoreBluetooth, I guess) seems to do.

@ccollins476ad ccollins476ad changed the title MYNEWT-749 BLE Host - Don't persist if keydist==0 BLE Host - Don't persist security material if either key distribution field is 0 Jul 1, 2017
Copy link
Contributor

@sjanc sjanc left a comment

Choose a reason for hiding this comment

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

Hi Chris,

This will work with legacy pairing only. On LE SC EncKey bit is ignored so you may have bonding while KeyDist is all zeros.

In legacy mode, if either peer indicates a key distribution flags value
of 0, then don't persist security material when pairing completes.

It isn't clear from the spec what should happen when both peers indicate
bonding support, but one indicates a 0 key distribution field.  I think
proceding with pairing and just not persisting makes the most sense.
This is also what Lightblue (or CoreBluetooth, I guess) seems to do.
@ccollins476ad
Copy link
Contributor Author

Thanks, Szymon. That is a good catch. I have updated the original commit with a fix.

Copy link
Contributor

@sjanc sjanc left a comment

Choose a reason for hiding this comment

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

looks good to me

@ccollins476ad ccollins476ad merged commit 8f499dd into apache:master Jul 7, 2017
@ccollins476ad ccollins476ad deleted the key-dist-0 branch July 7, 2017 18:12
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.

None yet

2 participants