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.

Sign up for GitHub

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

Jump to bottom

Optional secure pairing with a passkey #886

Merged
merged 1 commit into from
Dec 22, 2021
Merged

Optional secure pairing with a passkey #886

merged 1 commit into from
Dec 22, 2021

Conversation

evergreen22
Copy link
Contributor

Support passkey pairing when requested by the central.
Also, fixes a comment and a reorder warning introduced by a faulty conflict resolution.

@evergreen22
Optional secure pairing with a passkey
eca0588 
Support passkey pairing when requested by the central
Fix a comment and reorder warning
@evergreen22 evergreen22 mentioned this pull request Dec 21, 2021
Closed
geekbozu
geekbozu approved these changes Dec 21, 2021
View reviewed changes
Copy link
Member

@geekbozu geekbozu 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,
Thanks for staying on top of this!

@lman0
Copy link

lman0 commented Dec 21, 2021 •
edited
Loading

work properly .
moreover unlike before , the passkey is shown only if gadgetbridge request a bonding
(basically , the passkey was automatically shown once bluetooth was activated , now it show itself only when we add infinitme as bond from gadgetbridge by the add device button).

thanks @evergreen22

Avamander
Avamander approved these changes Dec 22, 2021
View reviewed changes
src/components/ble/NimbleController.cpp
@@ -274,7 +272,7 @@ int NimbleController::OnGAPEvent(ble_gap_event* event) {
* display capability only so we only handle the "display" action here.
*
* Standards insist that the rand() PRNG be deterministic.
* Use the nimble TRNG here since rand() is predictable.
* Use the tinycrypt prng here since rand() is predictable.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is intentional?

@evergreen22
Copy link
Contributor Author

Closes #880

@JF002
Copy link
Collaborator

JF002 commented Dec 22, 2021

Thank you very much, @evergreen22! Bluetooth pairing now works exactly as expected:

  • Gadgetbridge : when connecting to the PinetTime, Gadgetbridge asks if the user wants to pair with the device. If he chose to pair, the PIN code is displayed on the PinetTime and can be entered in Gadgetbridge. If the user does not want to pair, vanilla connection is established.
  • On Linux (Manjaro, KDE plasma), Siglo and Amazfish work exactly like before : they connect and communicate with the watch. Sometimes, KDE asks me if I want to "authorize and trust" my PineTime. I don't know what it does, but it doesn't ask for the PIN code and siglo/amazfish cannot connect anymore.
  • On Linux, I can manually pair using bluetoothctl (type the command pair xx:yy:zz:11:22:33 and enter the PIN code displayed on the PineTime). Once it's done, Siglo/Amazfish can connect to the PineTime again.
  • ITD supports secure pairing out of the box, and it works!

Great job everyone!

If I had only 1 suggestion : add a visual indicator showing if the connectivity is secured or not (ble icon with another color? another icon? something else?).

@JF002 JF002 added this to the 1.8.0 milestone Dec 22, 2021
@JF002 JF002 merged commit 3b0b480 into InfiniTimeOrg:develop Dec 22, 2021
@JF002
Copy link
Collaborator

JF002 commented Dec 22, 2021

Update : according to my observations, OTA is much more reliable on not bonded connections! Using my new phone, it's nearly impossible to start an OTA (it fails at 0% after a few seconds) when bonded. WIthout the bond, OTA works as expected (and a lot faster with my new phone : 12kbps).

For some reason, it still works (bonded or not) on my older phone (but it's so slow... 2-3 kbps).

@evergreen22
Copy link
Contributor Author

evergreen22 commented Dec 23, 2021 via email

@evergreen22 evergreen22 deleted the passkey-option branch December 24, 2021 00:21
@GIJack GIJack mentioned this pull request Jun 21, 2022
Open
@ghost ghost mentioned this pull request Jan 22, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Avamander Avamander Avamander approved these changes

@geekbozu geekbozu geekbozu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.8.0
Development

Successfully merging this pull request may close these issues.
5 participants
@evergreen22 @lman0 @JF002 @geekbozu @Avamander