espruino / Espruino

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

ble: allow configuring device privacy (v2) #2506

Closed ssievert42 closed 1 month ago

ssievert42 commented 1 month ago

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

Currently only enabled on Bangle.js 2.

NRF.setSecurity() now accepts an additional option privacy that is either a bool or looks like this:

privacy: {
    mode : string // The privacy mode that should be used.
    addr_type : string // The type of address to use.
    addr_cycle_s : number // How often the address should change, in seconds.
}

Example usage:

NRF.setSecurity({
  privacy: {
    mode: "device_privacy",
    addr_type: "random_private_resolvable",
    addr_cycle_s: 900
  },
  passkey: "123456", mitm: 1, display: 1,
});

Short version (that basically does the same thing):

NRF.setSecurity({privacy: 1, passkey: "123456", mitm: 1, display: 1});

And NRF.getSecurityStatus() also returns the current privacy config.

There's some more info in the docs for NRF.setSecurity().

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.

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.

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.

Note that this alone probably is not enough to avoid being trackable, because a unique-enough device name, like "Bangle.js ffff", might still be enough for a device to be identified. A workaround may be to change the bootloader to call NRF.setAdvertising({}, {showName: false});, but this has to be turned off for pairing with Gadgetbridge to work.

Because builds like to fail when adding something that uses more flash, I put this feature behind a flag that I called ESPR_BLE_PRIVATE_ADDRESS_SUPPORT, that is currently only present on Bangle.js 2.

This is my second attempt at making a PR for this; the first one is here. The main difference is, that the first PR introduced new methods for setting and getting the privacy config, while this PR integrates the privacy options into NRF.setSecurity() and NRF.getSecurityStatus().

I didn't touch NRF.getAddress() - that method still returns the real address while a private one is in use.

Feedback is very much appreciated :)

ssievert42 commented 1 month ago

About NRF.getAddress(): I failed to find a way to ask for the address that is currently in use, if the whole privacy and random address stuff is turned on. There is pm_id_addr_get(), but that always returns the actual address, and not the one that was randomly generated.

Maybe I missed something, which is kind of likely, considering the absolutely gigantic size of the SDK, and me being relatively unfamiliar with it, but I'm just gonna leave NRF.getAddress() alone for the time being ¯\_(ツ)_/¯

ssievert42 commented 1 month ago

I did some testing and everything seems to be ok:

Weirdly enough the watch keeps using a random address when doing a clean boot, but it also still requires a passkey to pair, so I guess this doesn't introduce that behavior.

bobrippling commented 1 month ago

A workaround may be to change the bootloader to call NRF.setAdvertising({}, {showName: false});

I plan on writing an app to set this up, along with configuring a private random address using the privacy field introduced here - thanks for your work on the firmware side

ssievert42 commented 1 month ago

@bobrippling You're welcome :) About writing an app: Wouldn't it be better to integrate setting up privacy stuff into the bootloader and the settings app? In the bootloader, we're already calling NRF.setSecurity() anyways, at least if a passkey is set.

I've been mulling about adding a shorthand for:

privacy: {
  mode: "device_privacy",
  addr_type: "random_private_resolvable",
  addr_cycle_s: 900
}

Maybe privacy could accept either a bool (for false -> off and true -> the above defaults), or an object? That would allow us to save a bit of RAM in the most common case, at the expense of readability. Kind of nervous about accidental type conversions though...

bobrippling commented 1 month ago

Wouldn't it be better to integrate setting up privacy stuff into the bootloader and the settings app? In the bootloader, we're already calling NRF.setSecurity() anyways, at least if a passkey is set.

Yeah that's a much better idea, more likely to be found by users rather than stashed in an app too.

Maybe privacy could accept either a bool (for false -> off and true -> the above defaults), or an object? That would allow us to save a bit of RAM in the most common case, at the expense of readability. Kind of nervous about accidental type conversions though...

I like the sound of true for sensible defaults, I suppose we could assert it's a boolean rather than allowing conversions via jsvGetBool, for example

ssievert42 commented 1 month ago

I've changed it to allow either undefined, a bool, something numeric that is then converted to a bool with jsvGetBool() (so that we can do NRF.setSecurity({privacy:1})), or an object for more fine-grained control :+1:

If that bool is true, we're now using (hopefully sensible) defaults of:

gfwilliams commented 1 month ago

This looks great, thanks! I've just tweaked the docs a little bit, but merging now. It's a shame about not adding to non-Bangle.js2 devices, but it's a constant stuggle to fit stuff in the available flash so it seems best to leave this out as you have unless it's something others are requesting a lot.

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

This is interesting as a softdevice restart isn't always required when we do a non-fastload transition - it's probably kicked off by the setServices call. For instance https://github.com/espruino/Espruino/blob/master/libs/bluetooth/jswrap_bluetooth.c#L1510

It might be worth investigating as it's possible we can be a bit more careful about under what circumstances we do a restart (right now we don't restart if ANCS is enabled and we request it again, but we do always restart if HID is used). It'd not only fix your problem but would really speed up app transitions too.

bobrippling commented 2 weeks ago

This is great, thank you!