-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
Implement pairing with btstack. #14291
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
base: master
Are you sure you want to change the base?
Conversation
Adds nimble equivalent behavior to btstack BLE implementations
Code size report:
|
Btstack offers two abstraction layers for secret storage, one called "device db" and another called "tlv". Pairing information is stored in the "device db", additional secrets (like own keys) are stored directly in the tlv. Luckily there is a "device db" implementation using tlv, so we only need to provide one interface. Additionally, I've removed some btstack files from compilation that were not referenced.
030d129
to
6634fea
Compare
@brianreinhold you might like to compare notes here! |
@felixdoerre We have been trying to get pairing to work on the PICO W which uses the btstack. We have a working implementation (except for interactive passkeys) but it is sharply geared toward our needs. We do not have python expertise and are new to Micropython and the btstack (I have done a lot with other BTLE C stacks designed for specific platforms such as Nordic and Renesas). Do you have an 'official' version supported through aioble? We found that some of the trickiest parts were devices that sent security requests to invoke the client to begin either pairing or encryption or both and sometimes as a replacement to an insufficient authentication error. The difficulty was that the btstack handles the security request and aioble and the app know nothing about it BUT the app (via aioble) has to handle the insufficient encryption/authentication errors. To solve this problem I had to add a method to aioble that would register for an encrytion changed event but do nothing else. In the modbluetooth_btstack.c file I had to add more events to those that would be fed to aioble's encryption changed event. I would run this listener as a task in the background after connection. In this manner I could inform aioble and my application of a pairing or re-encryption event due to security request. It works but it is not nice. Ideally a security request event should be passed up to aioble and the application so the application could invoke pairing/encryption as it does with an insufficient authentication/encryption error. I suppose I should add to this discussion on pairing the problem of bonding. Many devices (at least health devices that we are interested in) assume that on a bonded reconnect that they can send their data as soon as encryption is established and disconnect (saves battery). aioble does not save service discovery results so these devices often disconnect before aioble completes re-doing service discovery. aioble even invokes descriptor discovery when one tries to 'subscribe' which is slow exasperating the problem. I tried to see if it was possible to save the results of service discovery but it is not as they are objects and not just data and on the next connection (or after a power cycle) this information is no longer valid and one is forced to redo these transactions. |
I aim that this will be the 'official' version, but that depends on maintainers picking it up. I have not tested it with aioble, but in the scenario that I tested this with, I don't see anything that would prevent it from working with aioble.
I did have an issue, where btstack would not answer a pairing request (bluekitchen/btstack#584), but that was a mistake in btstack and it got resolved. The scenario that I have working is micropython/btstack as a ble peripheral (ble hid keyboard) and an android device as ble central. The android device initiates pairing, and micropython/btstack correctly responds and stores bonding information. Reconnection across micropython restarts work flawlessly. This works now both with public and with rpa addresses.
So you have micropython acting as ble central/gatt client? I have not tested that.
I don't get which problem you are trying to avoid here. I have not tested btstack as ble central, so I am not sure how pairing in response to a insufficient authentication/encryption error would work, but for BLE peripheral/gatt server, I just flag the attribute as authentication/encryption required and btstack handles the corresponding responses.
I thought that you only need to "subscribe" once per device/bond and that this information is supposed to be saved persistently. |
@felixdoerre our application is a BTLE central running on the PICO W. The central/client is responsible for initiating pairing (first time) encryption (reconnects). An insufficient authentication/encryption happens when the peripheral has secured an action on a characteristic/descriptor and the client tries to perform that action pre-pairing or pre-encryption. Peripherals do not have to deal with that error. A client is supposed to either pair or encrypt and then redo the action that caused the error. The btstack does not automatically handle this error (the Android client does) so my application has to catch the GattError exception raised by aioble and use aioble to initiate pairing (same method works for encryption since btstack knows if it is paired). However, a peripheral can also invoke a security request. Many health devices do this. This tells the client to get off its butt and initiate pairing/encryption. The btstack handles this automatically (as does the Android client) and aioble (and my application) has no idea that it ever happened. Ideally the btstack would handle BOTH insufficient authentication/encryption errors and the security requests automatically (as on Android), or pass both situations up to the application and let the application do it. I would prefer the latter as ongoing pairing the application may not know about could lead to timeouts on operations the application is trying to do. This is a big pain in Android as I have to check every write and read operation to see if pairing is ongoing and wait until it is finished. As far as bonding is concerned you are correct - in theory you should only need to subscribe once and if you are bonded that should not have to be repeated. We have not tested that by itself, however, service discovery results are not persisted and that is part of bonding. We need to read the Current Time Service and other characteristics like battery level every connection which requires that we redo service discovery. Also, many BTLE devices require that one re-subscribe event though they say they support bonding. That is just a bad implementation by the peripheral but such devices are on the market and we need to work with them. Resubscribing is fast if the results of service discovery are persisted so it has not been a problem (on Android). IN any case, that is where we are at. We have it working including some passkey support and unpairing/bonding support at least for a PICO W client. We have no need at this time for peripheral support. |
I would assume that this is what the
To me, this feels like a limitation of |
I fully agree that the btstack has no limitations. Once can do all Bluetooth transactions in that library. The problem with the _IRQ_ENCRYPTION_UPDATE event is that you have to invoke some method that anticipates it, for example, the pairing method in aioble's security module. In order to detect the pairing caused by a security request I added a method to aioble which did nothing but register for that _IRQ_ENCRYPTION_UPDATE event. I invoke a background task on connection which calls that aioble method and when a security request happens, that task gets notified (and I also notify the DeviceConnection object of aioble). It's not the most elegant approach but it does work. |
Thanks @felixdoerre for this BTstack pairing implementation, it's definitely something we want and need to support. From what I can tell the code here looks correct. But when I run the existing BLE multitests for pairing/bonding, they do not work. Looking at Investigating this, I see that BTstack is emitting the In your testing do you ever see the |
@@ -16,45 +16,45 @@ target_include_directories(micropy_extmod_btstack INTERFACE | |||
|
|||
target_sources(micropy_extmod_btstack INTERFACE | |||
${BTSTACK_LIB_DIR}/platform/embedded/hci_dump_embedded_stdout.c | |||
${BTSTACK_LIB_DIR}/src/ad_parser.c | |||
#${BTSTACK_LIB_DIR}/src/ad_parser.c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to just remove these unused lines, rather than comment them out.
#if MICROPY_PY_BLUETOOTH_ENABLE_PAIRING_BONDING | ||
|
||
static int btstack_tlv_mp_get_tag(void *context, uint32_t tag, uint8_t *buffer, uint32_t buffer_size) { | ||
UNUSED(context); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please just use (void)context;
to match the rest of the code
} | ||
|
||
static void btstack_tlv_mp_delete_tag(void *context, uint32_t tag) { | ||
mp_bluetooth_gap_on_set_secret(0, (uint8_t *)&tag, sizeof(tag), NULL, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could do with a (void)context;
here
@@ -35,7 +35,7 @@ | |||
#define MAX_NR_LE_DEVICE_DB_ENTRIES 4 | |||
|
|||
// Link Key DB and LE Device DB using TLV on top of Flash Sector interface | |||
// #define NVM_NUM_DEVICE_DB_ENTRIES 16 | |||
#define NVM_NUM_DEVICE_DB_ENTRIES 16 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this here, the event _IRQ_GET_SECRET
is called 16 times, with the string <n>DBT
for n in 0..15 inclusive. I'm not sure exactly what this is needed for, do you know?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the le_device_db implementation of btstack: https://github.com/bluekitchen/btstack/blob/2b49e57bd1fae85ac32ac1f41cdb7c794de335f6/src/ble/le_device_db_tlv.c#L208. Basically it implements the device db as a (fixed) list of device, and this code scans through all slots to find the least recently used empty slot.
I only ever tested pairing as initiated by the non-btstack device and in this case I see
I've just tested it with btstack as pairing initiator and it seems to have worked as well:
So this triggers an ENCRYPTION_UPDATE event as well. I'm testing with I'll look closer at |
The multitests can be run either way around. So if you have a non-Pico-W that has BLE (eg an esp32 device) it's possible to test both cases. As for the configuration:
|
@dpgeorge I had to add extra code in the modbluetooth_btstack.c file in order to get the _IRQ_ENCRYPTION_UPDATE on the python side due to peripherals sending security requests. When a peripheral sends a security request, the btstack client side responds by invoking pairing (if a first time connect) and invoking encryption (on a bonded reconnect). I had to map the completion of those processes to the _IRQ_ENCRYPTION_UPDATE. I don't recall the details as I did it many months ago. As I stated, it works, but I don't like the asymmetric behavior (the app handles the insufficient authentication error but the stack handles security requests). |
The first step is to get BTstack on par with the NimBLE implementation, and that means getting the above-mentioned multtests to pass on BTstack. The semantics of |
Btstack offers two abstraction layers for secret storage, one
called "device db" and another called "tlv". Pairing information
is stored in the "device db", additional secrets (like own keys)
are stored directly in the tlv. Luckily there is a "device db"
implementation using tlv, so we only need to provide one interface.
Additionally, I've removed some btstack files from compilation that
were not referenced.
This PR builds upon #10838 where, from what I can tell, only the
signed-off
-header is missing. If that's a problem I could re-implement those changes.With these changes I've tested bonding (with mp/btstack as peripheral), with random, public and RPA address.