Merge pull request #11812 from lucasssvaz/fix/ble_bugfixes

fix(BLE): Fix double callback and secure examples

Author: me-no-dev

libraries/BLE/README.md

@@ -12,16 +12,17 @@
   This means that in NimBLE you can read the insecure characteristic without entering
   the passkey. This is not possible in Bluedroid.
 
-  Also, the SoC stores the authentication info in the NVS memory. After a successful
-  connection it is possible that a passkey change will be ineffective.
-  To avoid this, clear the memory of the SoC's between security tests.
+  IMPORTANT: MITM (Man-In-The-Middle protection) must be enabled for password prompts
+  to work. Without MITM, the BLE stack assumes no user interaction is needed and will use
+  "Just Works" pairing method (with encryption if secure connection is enabled).
 
   Based on examples from Neil Kolban and h2zero.
   Created by lucasssvaz.
 */
 
 #include "BLEDevice.h"
 #include "BLESecurity.h"
+#include "nvs_flash.h"
 
 // The remote service we wish to connect to.
 static BLEUUID serviceUUID("4fafc201-1fb5-459e-8fcc-c5c9c331914b");
@@ -116,7 +117,7 @@ bool connectToServer() {
 
   // For Bluedroid, we need to set the authentication request type for the secure characteristic
   // This is not needed for NimBLE and will be ignored.
-  pRemoteSecureCharacteristic->setAuth(ESP_GATT_AUTH_REQ_NO_MITM);
+  pRemoteSecureCharacteristic->setAuth(ESP_GATT_AUTH_REQ_MITM);
 
   // Try to read the secure characteristic (this will trigger security negotiation in NimBLE)
   if (pRemoteSecureCharacteristic->canRead()) {
@@ -167,6 +168,12 @@ void setup() {
   Serial.begin(115200);
   Serial.println("Starting Secure BLE Client application...");
 
+  // Clear NVS to remove any cached pairing information
+  // This ensures fresh authentication for testing
+  Serial.println("Clearing NVS pairing data...");
+  nvs_flash_erase();
+  nvs_flash_init();
+
   BLEDevice::init("Secure BLE Client");
 
   // Set up security with the same passkey as the server
@@ -177,16 +184,16 @@ void setup() {
   // - IO capability is set to NONE
   // - Initiator and responder key distribution flags are set to both encryption and identity keys.
   // - Passkey is set to BLE_SM_DEFAULT_PASSKEY (123456). It will warn if you don't change it.
-  // - Max key size is set to 16 bytes
+  // - Key size is set to 16 bytes
 
   // Set the same static passkey as the server
   // The first argument defines if the passkey is static or random.
   // The second argument is the passkey (ignored when using a random passkey).
   pSecurity->setPassKey(true, CLIENT_PIN);
 
   // Set authentication mode to match server requirements
-  // Enable secure connection only for this example
-  pSecurity->setAuthenticationMode(false, false, true);
+  // Enable secure connection and MITM (for password prompts) for this example
+  pSecurity->setAuthenticationMode(false, true, true);
 
   // Retrieve a Scanner and set the callback we want to use to be informed when we
   // have detected a new device. Specify that we want active scanning and start the

libraries/BLE/examples/Client_secure_static_passkey/Client_secure_static_passkey.ino

@@ -0,0 +1,160 @@
+/*
+  Simple BLE Server Authorization Example
+
+  This example demonstrates how to create a BLE server with authorization
+  requirements. It shows the essential setup for:
+  - Authorization with static passkey
+  - Secure connection
+  - MITM (Man-In-The-Middle) protection
+
+  The server creates a single characteristic that requires authorization
+  to access. Clients must provide the correct passkey (123456) to read
+  or write to the characteristic.
+
+  Note that ESP32 uses Bluedroid by default and the other SoCs use NimBLE.
+  Bluedroid initiates security on-connect, while NimBLE initiates security on-demand.
+
+  Due to a bug in ESP-IDF's Bluedroid, this example will currently not work with ESP32.
+
+  IMPORTANT: MITM (Man-In-The-Middle protection) must be enabled for password prompts
+  to work. Without MITM, the BLE stack assumes no user interaction is needed and will use
+  "Just Works" pairing method (with encryption if secure connection is enabled).
+
+  Created by lucasssvaz.
+*/
+
+#include <BLEDevice.h>
+#include <BLEUtils.h>
+#include <BLEServer.h>
+#include <BLESecurity.h>
+#include <nvs_flash.h>
+
+// See the following for generating UUIDs:
+// https://www.uuidgenerator.net/
+
+#define SERVICE_UUID        "4fafc201-1fb5-459e-8fcc-c5c9c331914b"
+#define CHARACTERISTIC_UUID "beb5483e-36e1-4688-b7f5-ea07361b26a8"
+
+// Example passkey - change this for production use
+#define AUTH_PASSKEY 123456
+
+static int s_readCount = 0;
+static BLECharacteristic *s_pCharacteristic;
+
+class MySecurityCallbacks : public BLESecurityCallbacks {
+  bool onAuthorizationRequest(uint16_t connHandle, uint16_t attrHandle, bool isRead) {
+    Serial.println("Authorization request received");
+    if (isRead) {
+      s_readCount++;
+      // Keep value length <= (MTU - 1) to avoid a follow-up read request
+      uint16_t maxLen = BLEDevice::getServer()->getPeerMTU(connHandle) - 1;
+      String msg = "Authorized #" + String(s_readCount);
+      if (msg.length() > maxLen) {
+        msg = msg.substring(0, maxLen);
+      }
+      s_pCharacteristic->setValue(msg);
+      // Grant authorization to the first 3 reads
+      if (s_readCount <= 3) {
+        Serial.println("Authorization granted");
+        return true;
+      } else {
+        Serial.println("Authorization denied, read count exceeded");
+        Serial.println("Please reset the read counter to continue");
+        return false;
+      }
+    }
+    // Fallback to deny
+    Serial.println("Authorization denied");
+    return false;
+  }
+};
+
+void setup() {
+  Serial.begin(115200);
+  Serial.println("Starting BLE Authorization Example!");
+
+  // Initialize the BOOT pin for resetting the read count
+  pinMode(BOOT_PIN, INPUT_PULLUP);
+
+  // Clear NVS to remove any cached pairing information
+  // This ensures fresh authentication for testing
+  Serial.println("Clearing NVS pairing data...");
+  nvs_flash_erase();
+  nvs_flash_init();
+
+  Serial.print("Using BLE stack: ");
+  Serial.println(BLEDevice::getBLEStackString());
+
+  BLEDevice::init("BLE Auth Server");
+
+  // Set MTU to 517 to avoid a follow-up read request
+  BLEDevice::setMTU(517);
+
+  // Configure BLE Security
+  BLESecurity *pSecurity = new BLESecurity();
+
+  // Set static passkey for authentication
+  pSecurity->setPassKey(true, AUTH_PASSKEY);
+
+  // Set IO capability to DisplayOnly for MITM authentication
+  pSecurity->setCapability(ESP_IO_CAP_OUT);
+
+  // Enable authorization requirements:
+  // - bonding: true (for persistent storage of the keys)
+  // - MITM: true (enables Man-In-The-Middle protection for password prompts)
+  // - secure connection: true (enables secure connection for encryption)
+  pSecurity->setAuthenticationMode(true, true, true);
+
+  // Set the security callbacks
+  BLEDevice::setSecurityCallbacks(new MySecurityCallbacks());
+
+  // Create BLE Server
+  BLEServer *pServer = BLEDevice::createServer();
+  pServer->advertiseOnDisconnect(true);
+
+  // Create BLE Service
+  BLEService *pService = pServer->createService(SERVICE_UUID);
+
+  // Create characteristic with read and write properties
+  uint32_t properties = BLECharacteristic::PROPERTY_READ;
+
+  // For NimBLE: Add authentication properties
+  // These properties ensure the characteristic requires authorization
+  // (ignored by Bluedroid but harmless)
+  properties |= BLECharacteristic::PROPERTY_READ_AUTHEN | BLECharacteristic::PROPERTY_READ_AUTHOR;
+
+  s_pCharacteristic = pService->createCharacteristic(CHARACTERISTIC_UUID, properties);
+
+  // For Bluedroid: Set access permissions that require encryption and MITM
+  // This ensures authorization is required (ignored by NimBLE)
+  s_pCharacteristic->setAccessPermissions(ESP_GATT_PERM_READ_ENC_MITM | ESP_GATT_PERM_READ_AUTHORIZATION);
+
+  // Set initial value
+  s_pCharacteristic->setValue("Hello! You needed authorization to read this!");
+
+  // Start the service
+  pService->start();
+
+  // Configure and start advertising
+  BLEAdvertising *pAdvertising = BLEDevice::getAdvertising();
+  pAdvertising->addServiceUUID(SERVICE_UUID);
+  pAdvertising->setScanResponse(true);
+  pAdvertising->setMinPreferred(0x06);  // helps with iPhone connections
+  pAdvertising->setMinPreferred(0x12);
+
+  BLEDevice::startAdvertising();
+
+  Serial.println("BLE Server is running!");
+  Serial.println("Authorization is required to access the characteristic.");
+  Serial.printf("Use passkey: %d when prompted\n", AUTH_PASSKEY);
+}
+
+void loop() {
+  // Reset the read count if the BOOT pin is pressed
+  if (digitalRead(BOOT_PIN) == LOW) {
+    s_readCount = 0;
+    Serial.println("Read count reset");
+  }
+
+  delay(100);
+}

libraries/BLE/examples/Server_secure_authorization/Server_secure_authorization.ino

@@ -0,0 +1,9 @@
+{
+  "targets": {
+    "esp32": false
+  },
+  "fqbn_append": "PartitionScheme=huge_app",
+  "requires": [
+    "CONFIG_SOC_BLE_SUPPORTED=y"
+  ]
+}

libraries/BLE/examples/Server_secure_authorization/ci.json

@@ -15,9 +15,9 @@
   This means that in NimBLE you can read the insecure characteristic without entering
   the passkey. This is not possible in Bluedroid.
 
-  Also, the SoC stores the authentication info in the NVS memory. After a successful
-  connection it is possible that a passkey change will be ineffective.
-  To avoid this, clear the memory of the SoC's between security tests.
+  IMPORTANT: MITM (Man-In-The-Middle protection) must be enabled for password prompts
+  to work. Without MITM, the BLE stack assumes no user interaction is needed and will use
+  "Just Works" pairing method (with encryption if secure connection is enabled).
 
   Based on examples from Neil Kolban and h2zero.
   Created by lucasssvaz.
@@ -27,13 +27,14 @@
 #include <BLEUtils.h>
 #include <BLEServer.h>
 #include <BLESecurity.h>
+#include <nvs_flash.h>
 #include <string>
 
 // See the following for generating UUIDs:
 // https://www.uuidgenerator.net/
 
 #define SERVICE_UUID                 "4fafc201-1fb5-459e-8fcc-c5c9c331914b"
-#define Insecure_CHARACTERISTIC_UUID "beb5483e-36e1-4688-b7f5-ea07361b26a8"
+#define INSECURE_CHARACTERISTIC_UUID "beb5483e-36e1-4688-b7f5-ea07361b26a8"
 #define SECURE_CHARACTERISTIC_UUID   "ff1d2614-e2d6-4c87-9154-6625d39ca7f8"
 
 // This is an example passkey. You should use a different or random passkey.
@@ -43,6 +44,12 @@ void setup() {
   Serial.begin(115200);
   Serial.println("Starting BLE work!");
 
+  // Clear NVS to remove any cached pairing information
+  // This ensures fresh authentication for testing
+  Serial.println("Clearing NVS pairing data...");
+  nvs_flash_erase();
+  nvs_flash_init();
+
   Serial.print("Using BLE stack: ");
   Serial.println(BLEDevice::getBLEStackString());
 
@@ -55,16 +62,22 @@ void setup() {
   // - IO capability is set to NONE
   // - Initiator and responder key distribution flags are set to both encryption and identity keys.
   // - Passkey is set to BLE_SM_DEFAULT_PASSKEY (123456). It will warn if you don't change it.
-  // - Max key size is set to 16 bytes
+  // - Key size is set to 16 bytes
 
   // Set static passkey
   // The first argument defines if the passkey is static or random.
   // The second argument is the passkey (ignored when using a random passkey).
   pSecurity->setPassKey(true, SERVER_PIN);
 
+  // Set IO capability to DisplayOnly
+  // We need the proper IO capability for MITM authentication even
+  // if the passkey is static and won't be shown to the user
+  // See https://www.bluetooth.com/blog/bluetooth-pairing-part-2-key-generation-methods/
+  pSecurity->setCapability(ESP_IO_CAP_OUT);
+
   // Set authentication mode
-  // Require secure connection only for this example
-  pSecurity->setAuthenticationMode(false, false, true);
+  // Require secure connection and MITM (for password prompts) for this example
+  pSecurity->setAuthenticationMode(false, true, true);
 
   BLEServer *pServer = BLEDevice::createServer();
   pServer->advertiseOnDisconnect(true);
@@ -78,16 +91,16 @@ void setup() {
   // These special permission properties are not supported by Bluedroid and will be ignored.
   // This can be removed if only using Bluedroid (ESP32).
   // Check the BLECharacteristic.h file for more information.
-  secure_properties |= BLECharacteristic::PROPERTY_READ_ENC | BLECharacteristic::PROPERTY_WRITE_ENC;
+  secure_properties |= BLECharacteristic::PROPERTY_READ_AUTHEN | BLECharacteristic::PROPERTY_WRITE_AUTHEN;
 
   BLECharacteristic *pSecureCharacteristic = pService->createCharacteristic(SECURE_CHARACTERISTIC_UUID, secure_properties);
-  BLECharacteristic *pInsecureCharacteristic = pService->createCharacteristic(Insecure_CHARACTERISTIC_UUID, insecure_properties);
+  BLECharacteristic *pInsecureCharacteristic = pService->createCharacteristic(INSECURE_CHARACTERISTIC_UUID, insecure_properties);
 
   // Bluedroid uses permissions to secure characteristics.
   // This is the same as using the properties above.
   // NimBLE does not use permissions and will ignore these calls.
   // This can be removed if only using NimBLE (any SoC except ESP32).
-  pSecureCharacteristic->setAccessPermissions(ESP_GATT_PERM_READ_ENCRYPTED | ESP_GATT_PERM_WRITE_ENCRYPTED);
+  pSecureCharacteristic->setAccessPermissions(ESP_GATT_PERM_READ_ENC_MITM | ESP_GATT_PERM_WRITE_ENC_MITM);
   pInsecureCharacteristic->setAccessPermissions(ESP_GATT_PERM_READ | ESP_GATT_PERM_WRITE);
 
   // Set value for secure characteristic

libraries/BLE/examples/Server_secure_static_passkey/Server_secure_static_passkey.ino

@@ -923,7 +923,6 @@ int BLECharacteristic::handleGATTServerEvent(uint16_t conn_handle, uint16_t attr
         if (ctxt->om->om_pkthdr_len > 8) {
           rc = ble_gap_conn_find(conn_handle, &desc);
           assert(rc == 0);
-          pCharacteristic->m_pCallbacks->onRead(pCharacteristic);
           pCharacteristic->m_pCallbacks->onRead(pCharacteristic, &desc);
         }
 
@@ -957,7 +956,6 @@ int BLECharacteristic::handleGATTServerEvent(uint16_t conn_handle, uint16_t attr
         rc = ble_gap_conn_find(conn_handle, &desc);
         assert(rc == 0);
         pCharacteristic->setValue(buf, len);
-        pCharacteristic->m_pCallbacks->onWrite(pCharacteristic);
         pCharacteristic->m_pCallbacks->onWrite(pCharacteristic, &desc);
 
         return 0;

libraries/BLE/src/BLECharacteristic.cpp

@@ -536,6 +536,8 @@ void BLEClient::gattClientEventHandler(esp_gattc_cb_event_t event, esp_gatt_if_t
       m_semaphoreRssiCmplEvt.give();
       m_semaphoreSearchCmplEvt.give(1);
       BLEDevice::removePeerDevice(m_appId, true);
+      // Reset security state on disconnect
+      BLESecurity::resetSecurity();
       if (m_wasConnected && m_pClientCallbacks != nullptr) {
         m_pClientCallbacks->onDisconnect(this);
       }
@@ -981,6 +983,8 @@ int BLEClient::handleGAPEvent(struct ble_gap_event *event, void *arg) {
 
       BLEDevice::removePeerDevice(client->m_appId, true);
       client->m_isConnected = false;
+      // Reset security state on disconnect
+      BLESecurity::resetSecurity();
       if (client->m_pClientCallbacks != nullptr) {
         client->m_pClientCallbacks->onDisconnect(client);
       }

libraries/BLE/src/BLEClient.cpp

@@ -47,7 +47,7 @@ bool BLESecurity::m_securityStarted = false;
 bool BLESecurity::m_passkeySet = false;
 bool BLESecurity::m_staticPasskey = true;
 bool BLESecurity::m_regenOnConnect = false;
-uint8_t BLESecurity::m_iocap = 0;
+uint8_t BLESecurity::m_iocap = ESP_IO_CAP_NONE;
 uint8_t BLESecurity::m_authReq = 0;
 uint8_t BLESecurity::m_initKey = 0;
 uint8_t BLESecurity::m_respKey = 0;
@@ -188,6 +188,12 @@ void BLESecurity::regenPassKeyOnConnect(bool enable) {
   m_regenOnConnect = enable;
 }
 
+// This function resets the security state on disconnect.
+void BLESecurity::resetSecurity() {
+  log_d("resetSecurity: Resetting security state");
+  m_securityStarted = false;
+}
+
 // This function sets the authentication mode with bonding, MITM, and secure connection options.
 void BLESecurity::setAuthenticationMode(bool bonding, bool mitm, bool sc) {
   log_d("setAuthenticationMode: bonding=%d, mitm=%d, sc=%d", bonding, mitm, sc);
@@ -270,6 +276,7 @@ void BLESecurity::setEncryptionLevel(esp_ble_sec_act_t level) {
 
 bool BLESecurity::startSecurity(esp_bd_addr_t bd_addr, int *rcPtr) {
 #ifdef CONFIG_BLE_SMP_ENABLE
+  log_d("startSecurity: bd_addr=%s", BLEAddress(bd_addr).toString().c_str());
   if (m_securityStarted) {
     log_w("Security already started for bd_addr=%s", BLEAddress(bd_addr).toString().c_str());
     if (rcPtr) {
@@ -333,6 +340,7 @@ void BLESecurityCallbacks::onAuthenticationComplete(esp_ble_auth_cmpl_t param) {
 #if defined(CONFIG_NIMBLE_ENABLED)
 // This function initiates security for a given connection handle.
 bool BLESecurity::startSecurity(uint16_t connHandle, int *rcPtr) {
+  log_d("startSecurity: connHandle=%d", connHandle);
   if (m_securityStarted) {
     log_w("Security already started for connHandle=%d", connHandle);
     if (rcPtr) {