Fix crash when clearing NRF52 BLE bonds (#6609)

* Fix crash before clearing BLE bonds

* Prevent clients re-pairing BLE during factory reset
Clients seem able to re-pair BLE after clearing bonds during factory reset, even after advertising disabled. This seems to primarily occur on Android devices, which seem to more actively maintain the BLE connection.
As a workaround, `NRF52Bluetooth::shutdown` swaps the BLE pairing callback to one which actively rejects new connections.

---------

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>

arch/esp32/esp32.ini

@@ -50,7 +50,7 @@ lib_deps =
   # renovate: datasource=git-refs depName=meshtastic-esp32_https_server packageName=https://github.com/meshtastic/esp32_https_server gitBranch=master
   https://github.com/meshtastic/esp32_https_server/archive/896f1771ceb5979987a0b41028bf1b4e7aad419b.zip
   # renovate: datasource=custom.pio depName=NimBLE-Arduino packageName=h2zero/library/NimBLE-Arduino
-  h2zero/NimBLE-Arduino@^1.4.3
+  h2zero/NimBLE-Arduino@^2.2.3
   # renovate: datasource=git-refs depName=libpax packageName=https://github.com/dbinfrago/libpax gitBranch=master
   https://github.com/dbinfrago/libpax/archive/3cdc0371c375676a97967547f4065607d4c53fd1.zip
   # renovate: datasource=custom.pio depName=XPowersLib packageName=lewisxhe/library/XPowersLib

src/mesh/NodeDB.cpp

@@ -450,7 +450,6 @@ bool NodeDB::factoryReset(bool eraseBleBonds)
         nvs_flash_erase();
 #endif
 #ifdef ARCH_NRF52
-        Bluefruit.begin();
         LOG_INFO("Clear bluetooth bonds!");
         bond_print_list(BLE_GAP_ROLE_PERIPH);
         bond_print_list(BLE_GAP_ROLE_CENTRAL);

src/nimble/NimbleBluetooth.cpp

@@ -49,7 +49,7 @@ static uint8_t lastToRadio[MAX_TO_FROM_RADIO_SIZE];
 
 class NimbleBluetoothToRadioCallback : public NimBLECharacteristicCallbacks
 {
-    virtual void onWrite(NimBLECharacteristic *pCharacteristic)
+    virtual void onWrite(NimBLECharacteristic *pCharacteristic, NimBLEConnInfo &connInfo)
     {
         LOG_DEBUG("To Radio onwrite");
         auto val = pCharacteristic->getValue();
@@ -66,7 +66,7 @@ class NimbleBluetoothToRadioCallback : public NimBLECharacteristicCallbacks
 
 class NimbleBluetoothFromRadioCallback : public NimBLECharacteristicCallbacks
 {
-    virtual void onRead(NimBLECharacteristic *pCharacteristic)
+    virtual void onRead(NimBLECharacteristic *pCharacteristic, NimBLEConnInfo &connInfo)
     {
         uint8_t fromRadioBytes[meshtastic_FromRadio_size];
         size_t numBytes = bluetoothPhoneAPI->getFromRadio(fromRadioBytes);
@@ -79,7 +79,7 @@ class NimbleBluetoothFromRadioCallback : public NimBLECharacteristicCallbacks
 
 class NimbleBluetoothServerCallback : public NimBLEServerCallbacks
 {
-    virtual uint32_t onPassKeyRequest()
+    virtual uint32_t onPassKeyDisplay()
     {
         uint32_t passkey = config.bluetooth.fixed_pin;
 
@@ -125,7 +125,7 @@ class NimbleBluetoothServerCallback : public NimBLEServerCallbacks
         return passkey;
     }
 
-    virtual void onAuthenticationComplete(ble_gap_conn_desc *desc)
+    virtual void onAuthenticationComplete(NimBLEConnInfo &connInfo)
     {
         LOG_INFO("BLE authentication complete");
 
@@ -138,9 +138,9 @@ class NimbleBluetoothServerCallback : public NimBLEServerCallbacks
         }
     }
 
-    virtual void onDisconnect(NimBLEServer *pServer, ble_gap_conn_desc *desc)
+    virtual void onDisconnect(NimBLEServer *pServer, NimBLEConnInfo &connInfo, int reason)
     {
-        LOG_INFO("BLE disconnect");
+        LOG_INFO("BLE disconnect. Reason %i", reason);
 
         bluetoothStatus->updateStatus(
             new meshtastic::BluetoothStatus(meshtastic::BluetoothStatus::ConnectionState::DISCONNECTED));
@@ -191,7 +191,7 @@ int NimbleBluetooth::getRssi()
     if (bleServer && isConnected()) {
         auto service = bleServer->getServiceByUUID(MESH_SERVICE_UUID);
         uint16_t handle = service->getHandle();
-        return NimBLEDevice::getClientByID(handle)->getRssi();
+        return NimBLEDevice::getClientByHandle(handle)->getRssi();
     }
     return 0; // FIXME figure out where to source this
 }
@@ -216,6 +216,7 @@ void NimbleBluetooth::setup()
 
     NimbleBluetoothServerCallback *serverCallbacks = new NimbleBluetoothServerCallback();
     bleServer->setCallbacks(serverCallbacks, true);
+    bleServer->advertiseOnDisconnect(true);
     setupService();
     startAdvertising();
 }
@@ -259,7 +260,7 @@ void NimbleBluetooth::setupService()
     BatteryCharacteristic = batteryService->createCharacteristic( // 0x2A19 is the Battery Level characteristic)
         (uint16_t)0x2a19, NIMBLE_PROPERTY::READ | NIMBLE_PROPERTY::NOTIFY, 1);
 
-    NimBLE2904 *batteryLevelDescriptor = (NimBLE2904 *)BatteryCharacteristic->createDescriptor((uint16_t)0x2904);
+    NimBLE2904 *batteryLevelDescriptor = (NimBLE2904 *)BatteryCharacteristic->create2904();
     batteryLevelDescriptor->setFormat(NimBLE2904::FORMAT_UINT8);
     batteryLevelDescriptor->setNamespace(1);
     batteryLevelDescriptor->setUnit(0x27ad);

src/platform/nrf52/NRF52Bluetooth.cpp

@@ -210,17 +210,8 @@ void NRF52Bluetooth::shutdown()
 {
     // Shutdown bluetooth for minimum power draw
     LOG_INFO("Disable NRF52 bluetooth");
-    uint8_t connection_num = Bluefruit.connected();
+    Bluefruit.Security.setPairPasskeyCallback(NRF52Bluetooth::onUnwantedPairing); // Actively refuse (during factory reset)
-    if (connection_num) {
+    disconnect();
-        for (uint8_t i = 0; i < connection_num; i++) {
-            LOG_INFO("NRF52 bluetooth disconnecting handle %d", i);
-            Bluefruit.disconnect(i);
-        }
-        // Wait for disconnection
-        while (Bluefruit.connected())
-            yield();
-        LOG_INFO("All bluetooth connections ended");
-    }
     Bluefruit.Advertising.stop();
 }
 void NRF52Bluetooth::startDisabled()
@@ -372,6 +363,33 @@ bool NRF52Bluetooth::onPairingPasskey(uint16_t conn_handle, uint8_t const passke
     LOG_INFO("BLE passkey pair: match_request=%i", match_request);
     return true;
 }
+
+// Actively refuse new BLE pairings
+// After clearing bonds (at factory reset), clients seem initially able to attempt to re-pair, even with advertising disabled.
+// On NRF52Bluetooth::shutdown, we change the pairing callback to this method, to aggressively refuse any connection attempts.
+bool NRF52Bluetooth::onUnwantedPairing(uint16_t conn_handle, uint8_t const passkey[6], bool match_request)
+{
+    NRF52Bluetooth::disconnect();
+    return false;
+}
+
+// Disconnect any BLE connections
+void NRF52Bluetooth::disconnect()
+{
+    uint8_t connection_num = Bluefruit.connected();
+    if (connection_num) {
+        // Close all connections. We're only expecting one.
+        for (uint8_t i = 0; i < connection_num; i++)
+            Bluefruit.disconnect(i);
+
+        // Wait for disconnection
+        while (Bluefruit.connected())
+            yield();
+
+        LOG_INFO("Ended BLE connection");
+    }
+}
+
 void NRF52Bluetooth::onPairingCompleted(uint16_t conn_handle, uint8_t auth_status)
 {
     if (auth_status == BLE_GAP_SEC_STATUS_SUCCESS) {

src/platform/nrf52/NRF52Bluetooth.h

@@ -19,4 +19,7 @@ class NRF52Bluetooth : BluetoothApi
     static void onConnectionSecured(uint16_t conn_handle);
     static bool onPairingPasskey(uint16_t conn_handle, uint8_t const passkey[6], bool match_request);
     static void onPairingCompleted(uint16_t conn_handle, uint8_t auth_status);
+
+    static bool onUnwantedPairing(uint16_t conn_handle, uint8_t const passkey[6], bool match_request);
+    static void disconnect();
 };