Connect and bond with a passkey

[evergreen22]

Add Feature - Connect and bond with a passkey

• Passkey pairing - passkey is displayed on watch and entered on phone
• Swipe down to clear passkey screen
• Connection encryption
• Connection bonding
• Automatic reconnects to bonded peripheral*
• Trusted device on Android
• Persist bond between reboots

* Rebooting the watch will cause reconnect failures. You must delete the bond from the phone to reconnect/pair. Persisting the bond between reboots will resolve this.

Closes #224
Closes #302
Closes #502

There are several tasks listed below that I have listed as future work in order to make this PR easier to review and test.

Future work includes the following tasks:

• Investigate encrypting other characteristics
• Explore if a delete bond option in settings is desirable/needed
• Enable RPA
• Automatically clear the passkey screen on pairing completion
• Enable the whitelist to conserve power and prevent non-bonded devices from pairing

[lman0]

hi
@evergreen22 i've tested this pull and it work great!
(i tested with gadgetbridge and by stop and starting the Bluetooth of the phone)

yet there is some point that would make it better:

• the passkey screen should not be tied to the timeout screen because if the screen timeout is 5 s , for example , then there is not enough time to read the passkey properly.
• could you add a button , in settings (for example), in order to initiate again a new bond (with the passkey screen) with the phone?
  because the only way , when someone don't manage to read the key , either because of the screen time out or closing the screen by mistake, is to restart infinitime in order to restart the bonding ...
  (resetting infinitime mean losing notification , timer ....) that would not be the case anymore if you add a button to do the initiate again a bonding.

[evergreen22]

Thanks for testing @lman0 and your feedback.
All of the the items you suggested are good ideas and listed in the "Future work" section.

[lman0]

is these will be on another pr?
or it will be added in this pr @evergreen22 ?
because the first time , i had to restart infinitme because the screen passkey was dimmed....too fast

[evergreen22]

@lman0 - you can tap on the screen to wake it up and see the passkey again - see the "Wake Up" setting on your phone to set how fast your screen turns off.

If you miss the passkey and can't get it back by tapping, just wait. Your phone will restart pairing automatically if you don't enter the passkey.

If that doesn't work, press cancel on your phone and then wait. Your phone will restart pairing automatically if you cancel the pairing attempt.

I am currently planning on the "Future work" items to be a new PR.

[lman0]

there is a big bug @evergreen22 !

if the phone (or it's infintime?) request more than once the bonding , with the passkey screen, there is a screen scrambling:
if you don't validate the passkey and you don't cancel on phone ,(or even if you cancel) the next time the passkey show up on infinitime
it will corrupt the screen
and the more the passkey restart itself , the more the screen become scrambled and unreadable
and only a restart is able it restore to normalcy

[evergreen22]

Thanks for the feedback, I'll see if I can reproduce the issue.

I'd recommend not running it yet on a sealed watch. I'm only running it on my dev-kit right now because it is not finished. Once it is merged then it is ready to install on a sealed watch.

[lman0]

it's seem that the bug show itself when the passkey is still shown when the new passkey is generated that the scrambling begin @evergreen22

[geekbozu]

How many bonds do we want to store? 1 or more? I can attempt soon to get it to store to littleFS since I'm...well versed in that how that works right now :) quick dirty and functional would be 1 bond, no more.

[evergreen22]

I am just storing 1 in my offline test code. Nimble is set to persist 3, but I don't see any reason to keep more than 1 in the fs. I wrote two functions; StoreBond and RestoreBond. Every time I access the fs, the bus freezes and then the watchdog fires. Your expertise would be greatly appreciated.

[geekbozu]

I will give this an attempt tonight then :) I'll be changing the nimble config to 1 as we routines in the stack use that value for knowing when to delete or rotate pairs

[Avamander]

Formatting wise seems to be fine.

Needs testing on physical device.

[evergreen22]

I am testing a proposed fix for the problem reported by @iman0 and will push later today if it resolves the issue. Tested and pushed - 13a6fbf

[geekbozu]

So I almost have an PR for this PR to support storing bonds to flash... Seems the device wants to store 2 SEC key pairs and 2 CCCD entries per device. I'm only allowing one CCCD entry... hopefully that will make it persist.

[evergreen22]

Awesome! I am trying to store one key set for us, one set for the peer, and 2 CCCD entries since I think that is what it will take to restore the connection. If the peer subscribes to everything we offer that number can grow. GB does not subscribe to the additional items. I can make nrfconnect subscribe and that causes nimble to store the additional info.

[geekbozu]

I'll just expand on the CCCD store on my local copy to hold up to the config entries then. Its not a huge deal either way....I was taking the lazy way out and just using files called
BLE_STORE_PEER_SEC , BLE_STORE_OUR_SEC, and BLE_STORE_PEER_CCCD....aka one key for each. Ill have to expand CCCD to use a struct of the proper type and persist that...not trivial but not hard persay either! Just going to take more time!

[geekbozu]

Do you have it working with the BLE_RAM_STORE in the host folder? If so I can easily modify that to persist...currently all of what i'm doing is pretty much just implementing that in our NimbleController class...which feels really bad.

[lman0]

I am testing a proposed fix for the problem reported by @iman0 and will push later today if it resolves the issue. Tested and pushed - 13a6fbf

Hi
first and foremost i"m @lman0 and not @iman0 , L not i at the beginning , i remind you.

i have tested the pr with the fix and it work great with no bug of screen scrambling anymore !

[trman]

@evergreen22 , @geekbozu any news on the pr?

i'am also quite interested by this pr and have this pr on my pinetime for quite some time
since it resolve these issue
#707
#302

and 2/3 this one #628 , because we can't still choose which smartphone unlike peeble for example.

do the task 7 :Persist bond between reboots ,still need to be necessary for this pr @evergreen22 ?

for what i have read @geekbozu will do a pr that will resolve this task so it might be not necessary to let this part be blocker of this pr completion .

for instance , i use only this pr for 1 week already and i didn't need to restart infinitime since gadgetbridge pickup infinitime instantly.
that a great improvement on Bluetooth reliability on par , or even above ,with @danielgjackson PR !

maybe @JF002 will even make a release only for this pr on this completion since the improvement on Bluetooth are this great!

@evergreen22 keep the good work on this pr !
@geekbozu good luck on the persist part as it will be the needed base for be the creation of a screen where user will be able to select which smartphone will be removed or remembered !

@Avamander , do you thinks that could that the pr could be merged as is?

[evergreen22]

@Riksu9000 thanks for the feedback.

[geekbozu]

@evergreen22 , @geekbozu any news on the pr?

i'am also quite interested by this pr and have this pr on my pinetime for quite some time since it resolve these issue #707 #302

and 2/3 this one #628 , because we can't still choose which smartphone unlike peeble for example.

do the task 7 :Persist bond between reboots ,still need to be necessary for this pr @evergreen22 ?

for what i have read @geekbozu will do a pr that will resolve this task so it might be not necessary to let this part be blocker of this pr completion .

for instance , i use only this pr for 1 week already and i didn't need to restart infinitime since gadgetbridge pickup infinitime instantly. that a great improvement on Bluetooth reliability on par , or even above ,with @danielgjackson PR !

maybe @JF002 will even make a release only for this pr on this completion since the improvement on Bluetooth are this great!

@evergreen22 keep the good work on this pr ! @geekbozu good luck on the persist part as it will be the needed base for be the creation of a screen where user will be able to select which smartphone will be removed or remembered !

@Avamander , do you thinks that could that the pr could be merged as is?

No update on my end. I passed the hand back to @evergreen22 for now. After forwarding the issues regarding File System Access they were running into. I am waiting to hear more as well! I do however think this is a requirement in this PR to have persist working. :)

[trman]

No update on my end. I passed the hand back to @evergreen22 for now. After forwarding the issues regarding File System Access they were running into. I am waiting to hear more as well! I do however think this is a requirement in this PR to have persist working. :)

@geekbozu , you have written in a previous comment that you had a pr nearly done for this pr , on the persist task , yet you say now that you have passed the hand back to @evergreen22 .

does it mean that you have given to @evergreen22 , the nearly finished code in order for him to include it in his pr?
i don't understand otherwise , your statement n this part.

And for the he persist ,for me , is not necessary to be included in this pr at least.
It's for a later pr , because we will need to have a screen to remove persisted device , and it seem that screen will be for a latter pr as per @evergreen22
(' Explore if a delete bond option in settings is desirable/needed ' and 'Enable the whitelist to conserve power and prevent non-bonded devices from pairing' need a screento choose an select device device and remove a device from whitlelist)

[Avamander]

I'll add that to my list of future work to investigate.

I think this is a good idea for one more reason, requiring bonding might be a reason for a major version increment as it's a very strong API breakage.

[JF002]

The pairing encryption process is triggered when either the central or peripheral attempts to read a characteristic and the insufficient authorization message is returned. Therefore, we need at least one characteristic on the watch that requires authentication and encryption.

Once the pairing encryption process is successful, will all following exchanges be encrypted? Or will it be enabled only on characteristics that are explicitly secured (BLE_GATT_CHR_F_READ_ENC | BLE_GATT_CHR_F_READ_AUTHEN)

I guess there is no way for the central or peripheral to request the pairing encryption process at connection time, even before a characteristic is accessed?

I agree, in principle, it would be best to require auth+enc on all characteristics.

Do BLE/GATT allow this? Can we enable security feature on all services/characteristics and still be "BLE compliant"? Or do some of them still need to be available without those features?

I think this is a good idea for one more reason, requiring bonding might be a reason for a major version increment as it's a very strong API breakage.

Yep, I was going to ask if companion apps needed to do anything to support these new security features and checked with Amazfish : it does not work properly anymore. I guess it fails when trying to read the battery level. I'm not sure if there's a generic way to implement bonding on Linux apps or if the companion app need to implement it from scratch, but I guess that means that enabling security is a major API break.

So now I'm wondering : should we

• enable auth+sec on a characteristic every companion app will read to ensure that the secure bonding process will be triggered ?
• enable them on all services/characteristics so we are 100% secure
• enable 2 modes:
  • legacy/unsecure : infinitime will work exactly as it is working now which would require no work from companion app
  • secure : auth+enc is enabled, and additional characteristics are available
• anything else?

[JF002]

FYI I just did a size check : this PR uses 2156B in Flash. I have to assume I was afraid it would be much more than that! 👍

[Avamander]

I would try avoid making this toggle-able once implemented. This saves both companion complexity and doesn't increase this PR's scope.

[trman]

hi,

@evergreen22 first thanks for adding the persist , and add way to remove bonded state !

but sadly there is a bug when i tested the pr .
here the step to reproduce it :
1a - first bond the smartphone with infinitime ( with the 6 digit of this pr)
1b - once complete bonded (gadget bridge show 'connected' ) ,
1c - stop smartphone buetooth , in order to force infinitme store the persist bond
1d - then enable smartphone bluetooth (unitl it show connected) again

2 reset infinitime (hold around 5 sec the button)
- during reset stop the smartphone bluetooth

3- when infinitme is fully restarted , wait 3 sec and start the smartphone bluetooth

3 -b ) smartphone (and gadgetbridge) show that infinitme is connected
---> 4) then shortly after crash of infinitme (the bug) and reset

5. infinitme lose the persisted bond and want again to be bonded with 6 digit passkey

[evergreen22]

Once the pairing encryption process is successful, will all following exchanges be encrypted? Or will it be enabled only on characteristics that are explicitly secured (BLE_GATT_CHR_F_READ_ENC | BLE_GATT_CHR_F_READ_AUTHEN)

I don't have any way to find out since I don't own a BLE sniffer. It would be useful if someone who does own a sniffer could do a capture and analyze it to determine if all traffic is encrypted after bonding. FWIW nimble tells me that we are encrypted, authenticated, and bonded after the passkey pairing is complete, but nimble has lied to us before, so I don't really know.

I guess there is no way for the central or peripheral to request the pairing encryption process at connection time, even before a characteristic is accessed?

Based on my understanding of comments in nimble and the ble spec, only the central should initiate a secure connection. Of course that would require a change in every companion app as well as changes in InfiniTime.

The approach I've taken is to return an auth+enc required message to the central to force it to initiate security. This is how the spec says to do it (as well as several online guides). This, obviously, does not require a change in the companion as long it already handles the auth+enc required response.

Do BLE/GATT allow this? Can we enable security feature on all services/characteristics and still be "BLE compliant"? Or do some of them still need to be available without those features?

Yes, again based on my understanding of the spec, this would be allowed. As @Avamander and @kieranc pointed out previously it has the potential to cause a lot of breakage.

Yep, I was going to ask if companion apps needed to do anything to support these new security features and checked with Amazfish : it does not work properly anymore. I guess it fails when trying to read the battery level. I'm not sure if there's a generic way to implement bonding on Linux apps or if the companion app need to implement it from scratch, but I guess that means that enabling security is a major API break.

Try re-anabling "Legacy pairing" and see if that fixes Amazfish.

I disabled it to force all messages to be encrypted. Since I don't own a ble sniffer I can't prove, or disprove, that it does force an encrypted link for all messages. But, based on my understanding of the ble spec it does. I invite someone with a ble sniffer to confirm or refute my claim.

So now I'm wondering : should we

* enable auth+sec on a characteristic every companion app will read to ensure that the secure bonding process will be triggered ?

That is exactly my reasoning for requiring auth+enc on battery level. I use GB and it always reads the battery level on connect.

As stated before, I also tried the CTS and device information characteristics but they did not work for me. I concede that was in early days so I'll probably revisit them (future work).

* enable them on all services/characteristics so we are 100% secure

I added this to future tasks to determine feasibility.

IMO ble cannot ever be 100% secure because the architecture is fundamentally insecure.

* enable 2 modes:

My gut says no - the complexity would lead to a frustrating user experience for those who failed to grasp the pairing process and limitations (for example - people who tried this PR but didn't read the commit messages to understand what exactly it does and does not do). Also, the additional complexity would likely mean the code would be hard(er) to understand, debug, and maintain.

  * legacy/unsecure : infinitime will work exactly as it is working now which would require no work from companion app

Abandoning this PR would mean the watch would not work as a trusted device on Android (unlock your phone). Otherwise, I don't see any real disadvantage to the project in skipping this PR. BLE connections are reliable and easy now without bonding (thanks to everyone who spent hours and hours on the ble issues).

  * secure : auth+enc is enabled, and additional characteristics are available

* anything else?

In general, I've been trying to improve the reliability and usability of the BLE side of the watch. My pebble seamlessly bonded, reconnected, and unlocked my phone and those are all features I personally wanted in InfiniTime.

I think this PR accomplishes my goals with only one minor nit. The bond is not available to be stored until there is a disconnection or a completed successful encryption change. Maybe upgrading nimble to a newer version (or sub-module) would resolve this. However, that is a problem for another day/PR.

[trman]

@evergreen22 did you have checked the bug i've , found ?
because i have updated the step, (i forgot a step) , so you should check it again
sorry for the inconvenience caused by the forgotten step

[JF002]

Thanks for all these info, @evergreen22 !

The plan is not to drop this PR, quite the contrary! Making the connection easier will certainly improve the user experience, and secured (encrypted) connection is a must in our times!

I can setup a BLE sniffer using a laptop and a NRF52-DK board, I'll try to do it soon to confirm encrypted communication !

Regarding the bonding, indeed, it looks like the central can request it : NRFConnect has a "bond" button. So I guess companion app will be able to trigger the bonding process themselves, without needing to read a specific characteristic.

Now, do we want to keep the CTS (or any other characteristic like the one for the notification, which requires more privacy) to force the bonding process? Or should we create a new characteristic specific to infinitime for that purpose?

Sooner today, I did a few tests, but couldn't read the battery level from Gadgetbridge and NRFConnect. Gadgetbridge would just connect but did not display the battery level. NRFConnect would connect too, bond when I tried to read the battery level, but never displayed the battery level. According to the log, it was bonded but it never requested the PIN code (and infinitime never displayed it). I'll do more test to give more info about this!

[trman]

@JF002 , @geekbozu or @Avamander can you test the reset crash bug?
with the step that have been updated , it crash always at step 4.
but i would like another people to test it as well in order
to know if it's from a hardware bug on my pinetime or it's truly a bug on this pr.

because , if it's a bug on this pr , it would make the persist bond unusable .... across reset 😢

[darkdragon-001]

What is the current approach to multiple devices (e.g. Phone+Laptop)? Is only one connection supported at the moment and re-pair required to change the device? Or are multiple connected devices (maybe also at the same time) supported? If multiple devices are supported, I think there is need for a forget device UI. If pairing a new device effectively deletes the old key, I think such an UI is not needed.

[evergreen22]

InfiniTime only supports a single connection at a time. You'd have to disconnect and repair to a second device. This PR handles the possibility that you might do this.

[picomatic]

I'm not a bluetooth expert, but merged this pr into my build, and it works very well!

[trman]

Hi @evergreen22 ,

I've tested the latest build and I don't have anymore the crash so far ,so all good for me!!.

I 'be seen that you have explained on comments how to reset the persist

but it should be good as well to add an explanation on github wiki because user would not know how to do it.

[JF002]

This PR have already received some good feedback! Thanks to everyone who participated in this work, and especially to @evergreen22 !

[JF002]

I did a quick sniffing test:

InfiniTime 1.7.1 : everything is visible : characteristic handles, BLE commands and data ("Hello world" notification in this case :

Using changes from this PR:

I'm not a security expert but... it looks good to me 👍

[heinrich-ulbricht]

Just want to leave POSITIVE feedback: before this new passkey bonding I could hardly use the watch. But ever since the latest update and the passkey bonding I never ever encountered a single Bluetooth connectivity issue again, no matter how often I go in and out of flight mode or out of range and back.

Thank you thank you thank you!

(Android, Gadgetbridge)

[JF002]

@heinrich-ulbricht Thank you very much for this nice feedback! I'm glad this feature is working as well for you as it has for me and many other contributors and users!

[evergreen22]

@heinrich-ulbricht thanks for the kind words. We are all volunteers and is always nice to know our work is appreciated.