espruino / Espruino

The Espruino JavaScript interpreter - Official Repo
http://www.espruino.com/
Other
2.74k stars 741 forks source link

ble: allow configuring device privacy #2401

Closed ssievert42 closed 1 month ago

ssievert42 commented 11 months ago

This allows using a random private resolvable address to prevent being tracked.

There are now two new methods: NRF.setPrivacy() and NRF.getPrivacy(), to set and get the current privacy settings.

One thing that (currently) cannot be configured is our IRK. Should be possible to add this as well, but I'd rather not do that, because that would complicate things a bit.

I always get a bit nervous when working directly with pointers, hopefully I'm not leaking any memory and / or freeing memory too early.

The initial setup with Gadgetbridge can be kind of weird and create a bunch of devices that don't actually exist, but everything works without (noticable) issues after that.

Something that bugs me is that if we run this on startup in addition to NRF.setSecurity() we may trigger a softdevice restart twice.

ToDo:

gfwilliams commented 11 months ago

I didn't even realise this was a thing - seems like a good idea. But there are definitely memory leaks from handling JsVars.

if we run this on startup in addition to NRF.setSecurity() we may trigger a softdevice restart twice.

It's probably not a huge deal, but what if we just add these as options to setSecurity and getSecurityStatus? It feels like it'd be cleaner than having another set of API functions, and it avoids the restart issue

ssievert42 commented 11 months ago

The leaks should be fixed now; I'm still not used to working with refcounting manually like this :sweat_smile:

About adding these as options to NRF.setSecurity() and NRF.getSecurityStatus():
I tried doing that, but just can't get myself to like having that many, only kind of related, options in one place.
If privacy was a part of NRF.setSecurity(), the description that it "sets the security options used when connecting/pairing" wouldn't quite fit anymore, because the privacy stuff is relevant no matter the connection state. The same would apply to NRF.getSecurityStatus() that currently returns "information about the security state of the current peripheral connection", which is not where I would expect to find the current privacy config. Nordic also provides separate functions for security and privacy.
I guess I could live with two softdevice restarts, especially since you said that it's not a huge deal. Although I've got no idea what the cost of having more API functions is.
Obsiously it's your call where this goes in the end :)

bobrippling commented 1 month ago

I've been tinkering with BLE addresses recently and would be interested in this - is there much left to bring it to being mergeable?

ssievert42 commented 1 month ago

@bobrippling I haven't touched this in some time, but a slightly modified version has been working fine for me for the last couple of months (firmware I'm currently running).

Well, apart from the button on my Bangle randomly deciding to remain pressed and triggering reboots, that cause the watch to forget all stored bonding info and sometimes also the time :cry:

The issue with Gadgetbridge adding new devices (seemingly at random, but likely while the phone has GPS turned on, the main screen listing devices is visible and the watch decides to change its address) still exists. I wanted to take a look at the Gadgetbridge code, but somehow never got around to doing that. That was / is the main blocker from my perspective.

This feature also uses some additional flash, which is apparently enough for the Puck.js and Pixl.js builds to fail. (At least on my "flash_me" branch.)

I've since mostly come to terms with having the random address options as part of NRF.setSecurity(), but haven't done that change (yet).

Something I don't really like, is that the random address does not only change every 15 minutes, but also every time the softdevice restarts, which happens every time there is a non-fastload transition between apps. But I guess this isn't really avoidable.

Speaking of softdevice restarts: Having random addresses becomes pretty useless (at least if your intention is to avoid being trackable) if doing a non-fastload transition between apps may shortly send a beacon with your actual address, before the bootloader is able to configure the watch to use a random address. Which is why my "flash_me" branch includes some changes made by Gordon here, along with a commit by me (that I should probably clean up) with the title "ble: cleaner init", to prevent us from restarting the softdevice before all config options are set, and accidentally sending our real address. Not sure if the stuff I added actually does what I want it to do though :sweat_smile:

Having random addresses also quickly becomes useless if we are sending beacons that contain a unique-enough device name, like "Bangle.js ffff". For that reason, I've changed my bootloader to call NRF.setAdvertising({}, {showName: false});, but this has to be turned off for pairing with Gadgetbridge to work.

There's also at least one philosophical question, which is which address NRF.getAddress() should return.

In short: I've been using this mostly without serious issues and could definitely tidy this up a bit to make it mergeable :)

ssievert42 commented 1 month ago

Superseded by #2506

ssievert42 commented 1 month ago

This can be closed now.

bobrippling commented 1 month ago

I see, fair points about the name being shown and the brief window before the softdevice can be configured - thanks for the details. That lines up / helps answer some thoughts/questions with what I've been tinkering with.

I guess I'd not be able to use the random private addresses with OpenHaystack too, since it pretty much fixes the address as a public one, but I've not looked into OpenHaystack to see if we can advertise a public key as part of a private random address - an investigation in progress!

There's also at least one philosophical question, which is which address NRF.getAddress() should return.

Certainly! I found this and ended up with #2503