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.

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

Multiple periphals with RPA won't be able to be identified. #24

Closed
qinwang717 opened this issue Apr 1, 2021 · 8 comments
Closed

Multiple periphals with RPA won't be able to be identified. #24

qinwang717 opened this issue Apr 1, 2021 · 8 comments

Comments

@qinwang717
Copy link

Hi, I have 2 esp32 devkits configured as ble peripherals running in parallel. And with the RPA feature enabled. I found ble central sometimes won't able to tell those 2 devices apart when the peripheral's address changes.

Two devices have advertising names as "nimble-bleprph-1" and "nimble-bleprph-2" separately. Both of the devkits are running the same code from idf v4.2 bleprph example, with RPA enabled. The patch I am using to enable RPA is attached.
[
NimBLE_RPA_Example_use_bleprph (1).txt
](url). This is from this issue: #8

I'm trying to connect to devices separately via a central - my mobile phone, using apps like nRF and other ble scanner apps. I noticed that two devices are not identified separately: sometimes in the scanning, they are shown as one device, with the name flickering between "nimble-bleprph-1" and "nimble-bleprph-2". Under this situation I can't really tell which device is this.

Even when the flickering doesn't appear, so when the devices are shown as two separate devices named nimble-bleprph-1 and nimble-bleprph-2, the connection will go across the devices sometimes, especially when the previously connected device has changed the address. E.g. I connect to nimble-bleprph-1, disconnect, then connect, somehow the connect request will goes to nimble-bleprph-2. So the central thinks that nimble-bleprph-2 is the new nimble-bleprph-1 after the address changed.

I attached a video to demonstrate this issue, in this video, on the left side of the screen is nimble-bleprph-1, and on the right is nimble-bleprph-2, you can see that in the video. I've been trying to interact with nimble-bleprph-1 on my phone, but around 1min9sec, the connection status in nimble-bleprph-2 is changed, seems like the central is requesting connect to nimble-bleprph-2 instead.
video here:
https://drive.google.com/file/d/1yh_ECcmfc-AiKl-GncQE6e2lOYd98dV8/view?usp=sharing

I have tried running 2 peripherals without the RPA patch, which seems working fine. They are always identified separately. Also I have tried other apps, so I don't think it's because of how nRF is handling it.

Anyone else has seen this issue? Is this because the RPA in the peripherals is not configured properly? Why is the central can't tell the devices apart? Is it something to do with how the IRK is resolved I guess? Thanks!

@prasad-alatkar
Copy link

@qinwang717 , I believe it is because both the ESP32 devices use same(default) local IRK (Identity Resolving keys). Can you please try calling ble_hs_pvcy_set_our_irk to set different/custom IRKs for 2 devices ? Please be noted that this is a private API declared in $IDF_PATH/components/bt/host/nimble/nimble/nimble/host/src/ble_hs_pvcy_priv.h, you will have to include this header file accordingly. Please let me know if it resolves your issue.

@qinwang717
Copy link
Author

Hi @prasad-alatkar Thanks a lot! It seems to be the issue! I called ble_hs_pvcy_set_our_irk in sync_cb now two devices seems to get identified!

However how the central is not resolving the new address - if I pair and bond one device, and when the device changes to a new address, I need to do the pairing and bonding again on the central. Seems like customised IRK is not fully working.
I saw this issue on the apache nimble, mentions that the default key is force load at start-up, so I called ble_hs_pvcy_set_our_irk() and then ble_hs_misc_restore_irks() in on_sync callback, but issue still exists.
Any idea?

@qinwang717
Copy link
Author

qinwang717 commented Apr 12, 2021

This is the code snap:

static int loadCustomedIrk(void)
{
    uint8_t rand_irk[16];
    int rc; 
    rc = ble_hs_hci_util_rand(rand_irk, 16);
    if(rc)
    {
        return rc;
    }
    rc = ble_hs_pvcy_set_our_irk(rand_irk);
    if(rc)
    {
        return rc;
    }
    // Loading an IRK will clear all peer IRKs, so reload them from the store.
    rc = ble_hs_misc_restore_irks();
    return rc;
}

loadCustomedIrk is called first thing in bleprph_on_sync.

Any idea what am I missing? Thanks a lot!

@VincentBeltman
Copy link

VincentBeltman commented Sep 21, 2021

In case other people run into this problem. ble_hs_pvcy_set_our_irk is within a private header and I think it is not meant to be used for us developers.

However, I do not see another solution. De problem qinwang717 is running into looks to be caused by the way ble_hs_pvcy_set_our_irk is implemented. When this method is called and detects that the IRK has changed (note that NimBLE loads the default IRK first) it will remove the peer records from NVS. This causes the bug that the addresses don't seem to be resolved after some time.

I've patched this in IDF 4.1.1 with the patch file below (MIT license).

This patch changes the default IRK constantthat is used by nimble to a method that you can implement in your own code.

diff --git a/nimble/host/src/ble_hs_pvcy.c b/nimble/host/src/ble_hs_pvcy.c
index 69f3da54..fdf5b193 100644
--- a/nimble/host/src/ble_hs_pvcy.c
+++ b/nimble/host/src/ble_hs_pvcy.c
@@ -27,11 +27,8 @@
 static uint8_t ble_hs_pvcy_started;
 static uint8_t ble_hs_pvcy_irk[16];
 
-/** Use this as a default IRK if none gets set. */
-const uint8_t ble_hs_pvcy_default_irk[16] = {
-    0xef, 0x8d, 0xe2, 0x16, 0x4f, 0xec, 0x43, 0x0d,
-    0xbf, 0x5b, 0xdd, 0x34, 0xc0, 0x53, 0x1e, 0xb8,
-};
+extern const uint8_t* ble_hs_pvcy_default_irk(void);
 
 static int
 ble_hs_pvcy_set_addr_timeout(uint16_t timeout)
@@ -236,7 +233,7 @@ ble_hs_pvcy_set_our_irk(const uint8_t *irk)
     if (irk != NULL) {
         memcpy(new_irk, irk, 16);
     } else {
-        memcpy(new_irk, ble_hs_pvcy_default_irk, 16);
+        memcpy(new_irk, ble_hs_pvcy_default_irk(), 16);
     }
 
     /* Clear the resolving list if this is a new IRK. */
diff --git a/nimble/host/src/ble_hs_pvcy_priv.h b/nimble/host/src/ble_hs_pvcy_priv.h
index 86157da0..1d96045c 100644
--- a/nimble/host/src/ble_hs_pvcy_priv.h
+++ b/nimble/host/src/ble_hs_pvcy_priv.h
@@ -26,7 +26,7 @@
 extern "C" {
 #endif
 
-extern const uint8_t ble_hs_pvcy_default_irk[16];
+// extern const uint8_t ble_hs_pvcy_default_irk[16];
 
 int ble_hs_pvcy_set_our_irk(const uint8_t *irk);
 int ble_hs_pvcy_our_irk(const uint8_t **out_irk);

@SlightlyStoopid
Copy link

Any update on this?

@rahult-github
Copy link
Collaborator

Current IDF version have below functionality now:

  1. Upon fresh boot, a new local IRK is generated per device --> Previously the same local IRK was used, which caused issues.
  2. The IRK is saved in NVS so as to have the same IRK across reboot by default --> This helps to ensure address resolution even across reboots.
  3. A ble_hs_pvcy_set_our_irk still exists to let user reset the local IRK with their own choice --> This gives flexibility for solutions as per their use case. This though needs to explicitly include the ble_hs_pvcy_priv.h which is as per current upstream nimble implementation kept private .
  4. Current code reads NVS to restore IRKs upon reboot . Also if user calls the ble_hs_pvcy_set_our_irk explicitly, then it will clear the entries in controller and attempt to restore IRK from NVS.

So, latest codebase should address the issues.

@greenaddress
Copy link

@rahult-github could you comment on which tags address the issue? if none, which branches? thanks

@rahult-github
Copy link
Collaborator

rahult-github commented Mar 19, 2024

Hi @greenaddress ,

The change is on nimble-1.5.0-idf .. so v5.2 and v5.1 only

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants