feat(ble): Add on-demand security support for Bluedroid (#12363)

* feat(ble): Add on-demand security support for Bluedroid

* fix(ble): Fix copilot comments

* ci(pre-commit): Apply automatic fixes

* fix(workflow): Fix formatting

---------

Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>

.github/workflows/tests_hw_wokwi.yml

@@ -13,6 +13,7 @@ permissions:
 jobs:
   get-artifacts:
     name: Get required artifacts
+    if: github.event.workflow_run.conclusion != 'cancelled'
     runs-on: ubuntu-latest
     permissions:
       actions: read

.gitlab/workflows/hw_test_template.yml

@@ -46,7 +46,6 @@ hw-test-template:
 
   script:
     - echo "Using binaries for $TEST_CHIP"
-    - ls -laR ~/.arduino/tests || true
     - |
       set -e
       rc=0

libraries/BLE/src/BLEAdvertising.cpp

@@ -54,9 +54,7 @@ BLEAdvertising::BLEAdvertising() {
  */
 void BLEAdvertising::addServiceUUID(BLEUUID serviceUUID) {
   m_serviceUUIDs.push_back(serviceUUID);
-#ifdef CONFIG_NIMBLE_ENABLED
   m_advDataSet = false;
-#endif
 }  // addServiceUUID
 
 /**
@@ -65,9 +63,6 @@ void BLEAdvertising::addServiceUUID(BLEUUID serviceUUID) {
  */
 void BLEAdvertising::addServiceUUID(const char *serviceUUID) {
   addServiceUUID(BLEUUID(serviceUUID));
-#ifdef CONFIG_NIMBLE_ENABLED
-  m_advDataSet = false;
-#endif
 }  // addServiceUUID
 
 /**
@@ -83,9 +78,7 @@ bool BLEAdvertising::removeServiceUUID(int index) {
   }
 
   m_serviceUUIDs.erase(m_serviceUUIDs.begin() + index);
-#ifdef CONFIG_NIMBLE_ENABLED
   m_advDataSet = false;
-#endif
   return true;
 }
 
@@ -121,8 +114,8 @@ void BLEAdvertising::setAppearance(uint16_t appearance) {
   m_advData.appearance = appearance;
 #ifdef CONFIG_NIMBLE_ENABLED
   m_advData.appearance_is_present = 1;
-  m_advDataSet = false;
 #endif
+  m_advDataSet = false;
 }  // setAppearance
 
 void BLEAdvertising::setAdvertisementType(uint8_t adv_type) {
@@ -158,6 +151,7 @@ void BLEAdvertising::setMaxInterval(uint16_t maxinterval) {
 void BLEAdvertising::setMinPreferred(uint16_t mininterval) {
 #ifdef CONFIG_BLUEDROID_ENABLED
   m_advData.min_interval = mininterval;
+  m_advDataSet = false;
 #endif
 
 #if defined(CONFIG_NIMBLE_ENABLED)
@@ -189,6 +183,7 @@ void BLEAdvertising::setMinPreferred(uint16_t mininterval) {
 void BLEAdvertising::setMaxPreferred(uint16_t maxinterval) {
 #ifdef CONFIG_BLUEDROID_ENABLED
   m_advData.max_interval = maxinterval;
+  m_advDataSet = false;
 #endif
 
 #if defined(CONFIG_NIMBLE_ENABLED)
@@ -217,9 +212,7 @@ void BLEAdvertising::setMaxPreferred(uint16_t maxinterval) {
 
 void BLEAdvertising::setScanResponse(bool set) {
   m_scanResp = set;
-#ifdef CONFIG_NIMBLE_ENABLED
   m_advDataSet = false;
-#endif
 }
 
 /**
@@ -659,8 +652,30 @@ void BLEAdvertising::reset() {
 
   m_customAdvData = false;           // No custom advertising data
   m_customScanResponseData = false;  // No custom scan response data
+  m_advDataSet = false;              // Force advertising data reconfiguration
+  m_advConfiguring = false;          // Not currently configuring
 }  // BLEAdvertising
 
+void BLEAdvertising::freeServiceUUIDs() {
+  free(m_advData.p_service_uuid);
+  m_advData.p_service_uuid = nullptr;
+}
+
+bool BLEAdvertising::configureScanResponseData() {
+  memcpy(&m_scanRespData, &m_advData, sizeof(esp_ble_adv_data_t));
+  m_scanRespData.set_scan_rsp = true;
+  m_scanRespData.include_name = true;
+  m_scanRespData.include_txpower = true;
+  m_scanRespData.appearance = 0;
+  m_scanRespData.flag = 0;
+  esp_err_t errRc = ::esp_ble_gap_config_adv_data(&m_scanRespData);
+  if (errRc != ESP_OK) {
+    log_e("esp_ble_gap_config_adv_data (Scan response): rc=%d %s", errRc, GeneralUtils::errorToString(errRc));
+    return false;
+  }
+  return true;
+}
+
 void BLEAdvertising::setAdvertisementChannelMap(esp_ble_adv_channel_t channel_map) {
   m_advParams.channel_map = channel_map;
 }  // setAdvertisementChannelMap
@@ -671,80 +686,88 @@ void BLEAdvertising::setAdvertisementChannelMap(esp_ble_adv_channel_t channel_ma
  * @return N/A.
  */
 bool BLEAdvertising::start() {
-  log_v(">> start: customAdvData: %d, customScanResponseData: %d", m_customAdvData, m_customScanResponseData);
+  log_v(">> start: customAdvData: %d, customScanResponseData: %d, advDataSet: %d", m_customAdvData, m_customScanResponseData, m_advDataSet);
 
-  // We have a vector of service UUIDs that we wish to advertise.  In order to use the
-  // ESP-IDF framework, these must be supplied in a contiguous array of their 128bit (16 byte)
-  // representations.  If we have 1 or more services to advertise then we allocate enough
-  // storage to host them and then copy them in one at a time into the contiguous storage.
-  int numServices = m_serviceUUIDs.size();
-  if (numServices > 0) {
-    m_advData.service_uuid_len = 16 * numServices;
-    m_advData.p_service_uuid = (uint8_t *)malloc(m_advData.service_uuid_len);
-    if (!m_advData.p_service_uuid) {
-      log_e(">> start failed: out of memory");
-      return false;
-    }
-
-    uint8_t *p = m_advData.p_service_uuid;
-    for (int i = 0; i < numServices; i++) {
-      log_d("- advertising service: %s", m_serviceUUIDs[i].toString().c_str());
-      BLEUUID serviceUUID128 = m_serviceUUIDs[i].to128();
-      memcpy(p, serviceUUID128.getNative()->uuid.uuid128, 16);
-      p += 16;
-    }
-  } else {
-    m_advData.service_uuid_len = 0;
-    log_d("- no services advertised");
+  // If async configuration is already in progress, don't re-enter.
+  // The GAP event handler will start advertising when the config completes.
+  if (m_advConfiguring) {
+    log_w("Advertising configuration already in progress");
+    return true;
   }
 
-  esp_err_t errRc;
+  // If advertising data needs to be (re)configured, kick off the async configuration.
+  // Bluedroid's esp_ble_gap_config_adv_data() is asynchronous and fires a GAP completion
+  // event when done. We must NOT block here with semaphores because start() can be called
+  // from BT callbacks (e.g., the disconnect handler), and both GATT and GAP callbacks are
+  // dispatched on the same BTC task - blocking would deadlock.
+  // Instead, we use the same non-blocking approach as NimBLE: kick off the config and let
+  // the GAP event handler (handleGAPEvent) chain the remaining steps.
+  if (!m_advDataSet) {
+    freeServiceUUIDs();
 
-  if (!m_customAdvData) {
-    // Set the configuration for advertising.
-    // This is an async operation - we must wait for ESP_GAP_BLE_ADV_DATA_SET_COMPLETE_EVT
-    m_advData.set_scan_rsp = false;
-    m_advData.include_name = !m_scanResp;
-    m_advData.include_txpower = !m_scanResp;
-    m_semaphoreSetAdv.take("config_adv_data");
-    errRc = ::esp_ble_gap_config_adv_data(&m_advData);
-    if (errRc != ESP_OK) {
-      log_e("<< esp_ble_gap_config_adv_data: rc=%d %s", errRc, GeneralUtils::errorToString(errRc));
-      m_semaphoreSetAdv.give();
-      return false;
+    // We have a vector of service UUIDs that we wish to advertise.  In order to use the
+    // ESP-IDF framework, these must be supplied in a contiguous array of their 128bit (16 byte)
+    // representations.  If we have 1 or more services to advertise then we allocate enough
+    // storage to host them and then copy them in one at a time into the contiguous storage.
+    int numServices = m_serviceUUIDs.size();
+    if (numServices > 0) {
+      m_advData.service_uuid_len = 16 * numServices;
+      m_advData.p_service_uuid = (uint8_t *)malloc(m_advData.service_uuid_len);
+      if (!m_advData.p_service_uuid) {
+        log_e(">> start failed: out of memory");
+        return false;
+      }
+
+      uint8_t *p = m_advData.p_service_uuid;
+      for (int i = 0; i < numServices; i++) {
+        log_d("- advertising service: %s", m_serviceUUIDs[i].toString().c_str());
+        BLEUUID serviceUUID128 = m_serviceUUIDs[i].to128();
+        memcpy(p, serviceUUID128.getNative()->uuid.uuid128, 16);
+        p += 16;
+      }
+    } else {
+      m_advData.service_uuid_len = 0;
+      log_d("- no services advertised");
     }
-    m_semaphoreSetAdv.wait("config_adv_data");
-    log_d("Advertising data configured");
+
+    if (!m_customAdvData) {
+      // Configure advertising data asynchronously.
+      // The GAP event handler will continue the chain when config completes:
+      //   ADV_DATA_SET_COMPLETE -> (scan response config if needed) -> start advertising
+      m_advData.set_scan_rsp = false;
+      m_advData.include_name = !m_scanResp;
+      m_advData.include_txpower = !m_scanResp;
+      esp_err_t errRc = ::esp_ble_gap_config_adv_data(&m_advData);
+      if (errRc != ESP_OK) {
+        log_e("<< esp_ble_gap_config_adv_data: rc=%d %s", errRc, GeneralUtils::errorToString(errRc));
+        freeServiceUUIDs();
+        return false;
+      }
+      m_advConfiguring = true;
+      log_d("Advertising data configuration started (async)");
+      return true;
+    }
+
+    if (!m_customScanResponseData && m_scanResp) {
+      // Custom adv data but auto scan response - configure scan response asynchronously.
+      // The GAP event handler will start advertising when config completes.
+      if (!configureScanResponseData()) {
+        freeServiceUUIDs();
+        return false;
+      }
+      m_advConfiguring = true;
+      log_d("Scan response data configuration started (async)");
+      return true;
+    }
+
+    // Both adv data and scan response are custom (or no scan response needed).
+    // No async operations, mark as configured and proceed to start.
+    m_advDataSet = true;
+    freeServiceUUIDs();
   }
 
-  if (!m_customScanResponseData && m_scanResp) {
-    // Set the configuration for scan response.
-    // This is an async operation - we must wait for ESP_GAP_BLE_SCAN_RSP_DATA_SET_COMPLETE_EVT
-    memcpy(&m_scanRespData, &m_advData, sizeof(esp_ble_adv_data_t));  // Copy the content of m_advData.
-    m_scanRespData.set_scan_rsp = true;                               // Define this struct as scan response data
-    m_scanRespData.include_name = true;                               // Caution: This may lead to a crash if the device name has more than 29 characters
-    m_scanRespData.include_txpower = true;
-    m_scanRespData.appearance = 0;  // If defined the 'Appearance' attribute is already included in the advertising data
-    m_scanRespData.flag = 0;        // 'Flags' attribute should no be included in the scan response
-
-    m_semaphoreSetAdv.take("config_scan_rsp");
-    errRc = ::esp_ble_gap_config_adv_data(&m_scanRespData);
-    if (errRc != ESP_OK) {
-      log_e("<< esp_ble_gap_config_adv_data (Scan response): rc=%d %s", errRc, GeneralUtils::errorToString(errRc));
-      m_semaphoreSetAdv.give();
-      return false;
-    }
-    m_semaphoreSetAdv.wait("config_scan_rsp");
-    log_d("Scan response data configured");
-  }
-
-  // If we had services to advertise then we previously allocated some storage for them.
-  // Here we release that storage.
-  free(m_advData.p_service_uuid);  //TODO change this variable to local scope?
-  m_advData.p_service_uuid = nullptr;
-
-  // Start advertising.
-  errRc = ::esp_ble_gap_start_advertising(&m_advParams);
+  // Advertising data is already configured, just start advertising.
+  esp_err_t errRc = ::esp_ble_gap_start_advertising(&m_advParams);
   if (errRc != ESP_OK) {
     log_e("<< esp_ble_gap_start_advertising: rc=%d %s", errRc, GeneralUtils::errorToString(errRc));
   } else {
@@ -773,18 +796,56 @@ void BLEAdvertising::handleGAPEvent(esp_gap_ble_cb_event_t event, esp_ble_gap_cb
   log_d("handleGAPEvent [event no: %d]", (int)event);
 
   switch (event) {
+    // Advertising data has been configured. Next step: configure scan response if needed,
+    // otherwise start advertising directly. This chains the async operations started by start().
     case ESP_GAP_BLE_ADV_DATA_SET_COMPLETE_EVT:
     {
       log_d("Advertising data set complete, status=%d", param->adv_data_cmpl.status);
-      m_semaphoreSetAdv.give();
+      if (param->adv_data_cmpl.status != ESP_BT_STATUS_SUCCESS) {
+        log_e("Advertising data set failed, status=%d", param->adv_data_cmpl.status);
+        freeServiceUUIDs();
+        m_advConfiguring = false;
+        break;
+      }
+
+      if (!m_customScanResponseData && m_scanResp) {
+        if (!configureScanResponseData()) {
+          freeServiceUUIDs();
+          m_advConfiguring = false;
+        }
+      } else {
+        m_advDataSet = true;
+        m_advConfiguring = false;
+        freeServiceUUIDs();
+        esp_err_t errRc = ::esp_ble_gap_start_advertising(&m_advParams);
+        if (errRc != ESP_OK) {
+          log_e("esp_ble_gap_start_advertising: rc=%d %s", errRc, GeneralUtils::errorToString(errRc));
+        }
+      }
       break;
     }
+
+    // Scan response data has been configured. Final step: start advertising.
     case ESP_GAP_BLE_SCAN_RSP_DATA_SET_COMPLETE_EVT:
     {
       log_d("Scan response data set complete, status=%d", param->scan_rsp_data_cmpl.status);
-      m_semaphoreSetAdv.give();
+      if (param->scan_rsp_data_cmpl.status != ESP_BT_STATUS_SUCCESS) {
+        log_e("Scan response data set failed, status=%d", param->scan_rsp_data_cmpl.status);
+        freeServiceUUIDs();
+        m_advConfiguring = false;
+        break;
+      }
+
+      m_advDataSet = true;
+      m_advConfiguring = false;
+      freeServiceUUIDs();
+      esp_err_t errRc = ::esp_ble_gap_start_advertising(&m_advParams);
+      if (errRc != ESP_OK) {
+        log_e("esp_ble_gap_start_advertising: rc=%d %s", errRc, GeneralUtils::errorToString(errRc));
+      }
       break;
     }
+
     case ESP_GAP_BLE_ADV_START_COMPLETE_EVT:
     {
       log_d("Advertising start complete, status=%d", param->adv_start_cmpl.status);
@@ -793,7 +854,6 @@ void BLEAdvertising::handleGAPEvent(esp_gap_ble_cb_event_t event, esp_ble_gap_cb
     case ESP_GAP_BLE_ADV_STOP_COMPLETE_EVT:
     {
       log_i("STOP advertising");
-      //start();
       break;
     }
     default: break;

libraries/BLE/src/BLEAdvertising.h

@@ -23,7 +23,6 @@
 
 #include "BLEUUID.h"
 #include <vector>
-#include "RTOS.h"
 #include "BLEUtils.h"
 
 /***************************************************************************
@@ -192,7 +191,8 @@ private:
   std::vector<BLEUUID> m_serviceUUIDs;
   bool m_customAdvData = false;
   bool m_customScanResponseData = false;
-  FreeRTOS::Semaphore m_semaphoreSetAdv = FreeRTOS::Semaphore("startAdvert");
+  bool m_advDataSet = false;
+  bool m_advConfiguring = false;
   bool m_scanResp = true;
 
   /***************************************************************************
@@ -203,6 +203,8 @@ private:
   esp_ble_adv_data_t m_advData;
   esp_ble_adv_data_t m_scanRespData;
   esp_ble_adv_params_t m_advParams;
+  bool configureScanResponseData();
+  void freeServiceUUIDs();
 #endif
 
   /***************************************************************************
@@ -213,7 +215,6 @@ private:
   ble_hs_adv_fields m_advData;
   ble_hs_adv_fields m_scanData;
   ble_gap_adv_params m_advParams;
-  bool m_advDataSet;
   void (*m_advCompCB)(BLEAdvertising *pAdv);
   uint8_t m_slaveItvl[4];
   uint32_t m_duration;

libraries/BLE/src/BLEClient.cpp

@@ -178,8 +178,19 @@ esp_gatt_if_t BLEClient::getGattcIf() {
  */
 bool BLEClient::secureConnection() {
 #if defined(CONFIG_BLUEDROID_ENABLED)
-  log_i("secureConnection() does not need to be called for Bluedroid");
-  return true;
+  if (BLESecurity::m_authenticationComplete) {
+    return true;
+  }
+  if (!BLESecurity::m_securityEnabled) {
+    log_i("Security not enabled");
+    return true;
+  }
+  if (!BLESecurity::startSecurity(m_peerAddress.getNative())) {
+    log_e("Failed to start security");
+    return false;
+  }
+  BLESecurity::waitForAuthenticationComplete();
+  return BLESecurity::m_authenticationComplete;
 #endif
 
 #if defined(CONFIG_NIMBLE_ENABLED)
@@ -1141,7 +1152,7 @@ int BLEClient::handleGAPEvent(struct ble_gap_event *event, void *arg) {
           break;
         }
 
-        if (BLESecurity::m_securityEnabled) {
+        if (BLESecurity::m_securityEnabled && BLESecurity::m_forceSecurity) {
           BLESecurity::startSecurity(client->m_conn_id);
         }
 

libraries/BLE/src/BLEDevice.cpp

@@ -1243,9 +1243,13 @@ void BLEDevice::gapEventHandler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_par
     {
       log_i("ESP_GAP_BLE_AUTH_CMPL_EVT");
 #ifdef CONFIG_BLE_SMP_ENABLE  // Check that BLE SMP (security) is configured in make menuconfig
-      // Signal that authentication has completed
-      // This unblocks any GATT operations waiting for pairing when bonding is enabled
-      BLESecurity::signalAuthenticationComplete();
+      // Call user callback BEFORE signaling completion.
+      // This ensures callback output (e.g., "Authentication complete") appears before
+      // any waiting GATT operations are unblocked and produce their own output.
+      // This matches NimBLE's ordering where the callback fires before the task is released.
+      if (BLEDevice::m_securityCallbacks != nullptr) {
+        BLEDevice::m_securityCallbacks->onAuthenticationComplete(param->ble_security.auth_cmpl);
+      }
 
       // Restore CCCD values for bonded device reconnection
       // Per GATT spec, CCCD values should persist for bonded devices
@@ -1255,9 +1259,8 @@ void BLEDevice::gapEventHandler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_par
         m_pServer->restoreCCCDValues(peerAddress);
       }
 
-      if (BLEDevice::m_securityCallbacks != nullptr) {
-        BLEDevice::m_securityCallbacks->onAuthenticationComplete(param->ble_security.auth_cmpl);
-      }
+      // Signal completion last - this unblocks any GATT operations waiting for pairing
+      BLESecurity::signalAuthenticationComplete();
 #endif  // CONFIG_BLE_SMP_ENABLE
     } break;
     case ESP_GAP_BLE_UPDATE_CONN_PARAMS_EVT:

libraries/BLE/src/BLERemoteCharacteristic.cpp

@@ -448,7 +448,7 @@ void BLERemoteCharacteristic::gattClientEventHandler(esp_gattc_cb_event_t event,
         m_value = "";
       }
 
-      m_semaphoreReadCharEvt.give();
+      m_semaphoreReadCharEvt.give(evtParam->read.status);
       break;
     }  // ESP_GATTC_READ_CHAR_EVT
 
@@ -499,7 +499,7 @@ void BLERemoteCharacteristic::gattClientEventHandler(esp_gattc_cb_event_t event,
 
       // There is nothing further we need to do here.  This is merely an indication
       // that the write has completed and we can unlock the caller.
-      m_semaphoreWriteCharEvt.give();
+      m_semaphoreWriteCharEvt.give(evtParam->write.status);
       break;
     }  // ESP_GATTC_WRITE_CHAR_EVT
 
@@ -578,30 +578,47 @@ String BLERemoteCharacteristic::readValue() {
     return String();
   }
 
-  // Wait for authentication to complete if bonding is enabled
-  // This prevents the read request from being made while pairing is in progress
+  // Wait for authentication to complete if security was started on connection
   BLESecurity::waitForAuthenticationComplete();
 
-  m_semaphoreReadCharEvt.take("readValue");
+  int retryCount = 1;
+  uint32_t status = ESP_GATT_OK;
 
-  // Ask the BLE subsystem to retrieve the value for the remote hosted characteristic.
-  // This is an asynchronous request which means that we must block waiting for the response
-  // to become available.
-  esp_err_t errRc = ::esp_ble_gattc_read_char(
-    m_pRemoteService->getClient()->getGattcIf(),
-    m_pRemoteService->getClient()->getConnId(),  // The connection ID to the BLE server
-    getHandle(),                                 // The handle of this characteristic
-    (esp_gatt_auth_req_t)m_auth
-  );  // Security
+  do {
+    m_semaphoreReadCharEvt.take("readValue");
 
-  if (errRc != ESP_OK) {
-    log_e("esp_ble_gattc_read_char: rc=%d %s", errRc, GeneralUtils::errorToString(errRc));
-    return "";
-  }
+    // Ask the BLE subsystem to retrieve the value for the remote hosted characteristic.
+    // This is an asynchronous request which means that we must block waiting for the response
+    // to become available.
+    esp_err_t errRc = ::esp_ble_gattc_read_char(
+      m_pRemoteService->getClient()->getGattcIf(),
+      m_pRemoteService->getClient()->getConnId(),  // The connection ID to the BLE server
+      getHandle(),                                 // The handle of this characteristic
+      (esp_gatt_auth_req_t)m_auth
+    );  // Security
 
-  // Block waiting for the event that indicates that the read has completed.  When it has, the String found
-  // in m_value will contain our data.
-  m_semaphoreReadCharEvt.wait("readValue");
+    if (errRc != ESP_OK) {
+      log_e("esp_ble_gattc_read_char: rc=%d %s", errRc, GeneralUtils::errorToString(errRc));
+      m_semaphoreReadCharEvt.give();
+      return "";
+    }
+
+    // Block waiting for the event that indicates that the read has completed.
+    // The GATT status is passed through the semaphore value.
+    status = m_semaphoreReadCharEvt.wait("readValue");
+
+    switch (status) {
+      case ESP_GATT_OK: break;
+      case ESP_GATT_INSUF_AUTHENTICATION:
+      case ESP_GATT_INSUF_AUTHORIZATION:
+      case ESP_GATT_INSUF_ENCRYPTION:
+        if (BLESecurity::m_securityEnabled && retryCount && getRemoteService()->getClient()->secureConnection()) {
+          break;
+        }
+      /* Else falls through. */
+      default: log_e("readValue: status=%d", status); break;
+    }
+  } while (status != ESP_GATT_OK && retryCount--);
 
   log_v("<< readValue(): length: %d", m_value.length());
   return m_value;
@@ -624,26 +641,46 @@ bool BLERemoteCharacteristic::writeValue(uint8_t *data, size_t length, bool resp
     return false;
   }
 
-  // Wait for authentication to complete if bonding is enabled
-  // This prevents the write request from being made while pairing is in progress
+  // Wait for authentication to complete if security was started on connection
   BLESecurity::waitForAuthenticationComplete();
 
-  m_semaphoreWriteCharEvt.take("writeValue");
-  // Invoke the ESP-IDF API to perform the write.
-  esp_err_t errRc = ::esp_ble_gattc_write_char(
-    m_pRemoteService->getClient()->getGattcIf(), m_pRemoteService->getClient()->getConnId(), getHandle(), length, data,
-    response ? ESP_GATT_WRITE_TYPE_RSP : ESP_GATT_WRITE_TYPE_NO_RSP, (esp_gatt_auth_req_t)m_auth
-  );
+  int retryCount = 1;
+  uint32_t status = ESP_GATT_OK;
 
-  if (errRc != ESP_OK) {
-    log_e("esp_ble_gattc_write_char: rc=%d %s", errRc, GeneralUtils::errorToString(errRc));
-    return false;
-  }
+  do {
+    m_semaphoreWriteCharEvt.take("writeValue");
 
-  m_semaphoreWriteCharEvt.wait("writeValue");
+    // Invoke the ESP-IDF API to perform the write.
+    esp_err_t errRc = ::esp_ble_gattc_write_char(
+      m_pRemoteService->getClient()->getGattcIf(), m_pRemoteService->getClient()->getConnId(), getHandle(), length, data,
+      response ? ESP_GATT_WRITE_TYPE_RSP : ESP_GATT_WRITE_TYPE_NO_RSP, (esp_gatt_auth_req_t)m_auth
+    );
+
+    if (errRc != ESP_OK) {
+      log_e("esp_ble_gattc_write_char: rc=%d %s", errRc, GeneralUtils::errorToString(errRc));
+      m_semaphoreWriteCharEvt.give();
+      return false;
+    }
+
+    // Block waiting for the event that indicates that the write has completed.
+    // The GATT status is passed through the semaphore value.
+    status = m_semaphoreWriteCharEvt.wait("writeValue");
+
+    switch (status) {
+      case ESP_GATT_OK: break;
+      case ESP_GATT_INSUF_AUTHENTICATION:
+      case ESP_GATT_INSUF_AUTHORIZATION:
+      case ESP_GATT_INSUF_ENCRYPTION:
+        if (BLESecurity::m_securityEnabled && retryCount && getRemoteService()->getClient()->secureConnection()) {
+          break;
+        }
+      /* Else falls through. */
+      default: log_e("writeValue: status=%d", status); break;
+    }
+  } while (status != ESP_GATT_OK && retryCount--);
 
   log_v("<< writeValue");
-  return true;
+  return (status == ESP_GATT_OK);
 }  // writeValue
 
 #endif

libraries/BLE/src/BLERemoteDescriptor.cpp

@@ -163,14 +163,15 @@ void BLERemoteDescriptor::gattClientEventHandler(esp_gattc_cb_event_t event, esp
         m_value = "";
       }
       // Unlock the semaphore to ensure that the requester of the data can continue.
-      m_semaphoreReadDescrEvt.give();
+      // Pass the GATT status through the semaphore value for retry logic.
+      m_semaphoreReadDescrEvt.give(evtParam->read.status);
       break;
 
     case ESP_GATTC_WRITE_DESCR_EVT:
       if (evtParam->write.handle != getHandle()) {
         break;
       }
-      m_semaphoreWriteDescrEvt.give();
+      m_semaphoreWriteDescrEvt.give(evtParam->write.status);
       break;
     default: break;
   }
@@ -179,30 +180,53 @@ void BLERemoteDescriptor::gattClientEventHandler(esp_gattc_cb_event_t event, esp
 String BLERemoteDescriptor::readValue() {
   log_v(">> readValue: %s", toString().c_str());
 
+  BLEClient *pClient = getRemoteCharacteristic()->getRemoteService()->getClient();
+
   // Check to see that we are connected.
-  if (!getRemoteCharacteristic()->getRemoteService()->getClient()->isConnected()) {
+  if (!pClient->isConnected()) {
     log_e("Disconnected");
     return String();
   }
 
-  m_semaphoreReadDescrEvt.take("readValue");
+  // Wait for authentication to complete if security was started on connection
+  BLESecurity::waitForAuthenticationComplete();
 
-  // Ask the BLE subsystem to retrieve the value for the remote hosted characteristic.
-  esp_err_t errRc = ::esp_ble_gattc_read_char_descr(
-    m_pRemoteCharacteristic->getRemoteService()->getClient()->getGattcIf(),
-    m_pRemoteCharacteristic->getRemoteService()->getClient()->getConnId(),  // The connection ID to the BLE server
-    getHandle(),                                                            // The handle of this characteristic
-    (esp_gatt_auth_req_t)m_auth
-  );  // Security
+  int retryCount = 1;
+  uint32_t status = ESP_GATT_OK;
 
-  if (errRc != ESP_OK) {
-    log_e("esp_ble_gattc_read_char: rc=%d %s", errRc, GeneralUtils::errorToString(errRc));
-    return "";
-  }
+  do {
+    m_semaphoreReadDescrEvt.take("readValue");
 
-  // Block waiting for the event that indicates that the read has completed.  When it has, the String found
-  // in m_value will contain our data.
-  m_semaphoreReadDescrEvt.wait("readValue");
+    // Ask the BLE subsystem to retrieve the value for the remote hosted descriptor.
+    esp_err_t errRc = ::esp_ble_gattc_read_char_descr(
+      pClient->getGattcIf(),
+      pClient->getConnId(),  // The connection ID to the BLE server
+      getHandle(),           // The handle of this descriptor
+      (esp_gatt_auth_req_t)m_auth
+    );  // Security
+
+    if (errRc != ESP_OK) {
+      log_e("esp_ble_gattc_read_char_descr: rc=%d %s", errRc, GeneralUtils::errorToString(errRc));
+      m_semaphoreReadDescrEvt.give();
+      return "";
+    }
+
+    // Block waiting for the event that indicates that the read has completed.
+    // The GATT status is passed through the semaphore value.
+    status = m_semaphoreReadDescrEvt.wait("readValue");
+
+    switch (status) {
+      case ESP_GATT_OK: break;
+      case ESP_GATT_INSUF_AUTHENTICATION:
+      case ESP_GATT_INSUF_AUTHORIZATION:
+      case ESP_GATT_INSUF_ENCRYPTION:
+        if (BLESecurity::m_securityEnabled && retryCount && pClient->secureConnection()) {
+          break;
+        }
+      /* Else falls through. */
+      default: log_e("readValue: status=%d", status); break;
+    }
+  } while (status != ESP_GATT_OK && retryCount--);
 
   log_v("<< readValue(): length: %d", m_value.length());
   return m_value;
@@ -217,27 +241,56 @@ String BLERemoteDescriptor::readValue() {
  */
 bool BLERemoteDescriptor::writeValue(uint8_t *data, size_t length, bool response) {
   log_v(">> writeValue: %s", toString().c_str());
+
+  BLEClient *pClient = getRemoteCharacteristic()->getRemoteService()->getClient();
+
   // Check to see that we are connected.
-  if (!getRemoteCharacteristic()->getRemoteService()->getClient()->isConnected()) {
+  if (!pClient->isConnected()) {
     log_e("Disconnected");
     return false;
   }
 
-  m_semaphoreWriteDescrEvt.take("writeValue");
+  // Wait for authentication to complete if security was started on connection
+  BLESecurity::waitForAuthenticationComplete();
 
-  esp_err_t errRc = ::esp_ble_gattc_write_char_descr(
-    m_pRemoteCharacteristic->getRemoteService()->getClient()->getGattcIf(), m_pRemoteCharacteristic->getRemoteService()->getClient()->getConnId(), getHandle(),
-    length,  // Data length
-    data,    // Data
-    response ? ESP_GATT_WRITE_TYPE_RSP : ESP_GATT_WRITE_TYPE_NO_RSP, (esp_gatt_auth_req_t)m_auth
-  );
-  if (errRc != ESP_OK) {
-    log_e("esp_ble_gattc_write_char_descr: %d", errRc);
-  }
+  int retryCount = 1;
+  uint32_t status = ESP_GATT_OK;
+
+  do {
+    m_semaphoreWriteDescrEvt.take("writeValue");
+
+    esp_err_t errRc = ::esp_ble_gattc_write_char_descr(
+      pClient->getGattcIf(), pClient->getConnId(), getHandle(),
+      length,  // Data length
+      data,    // Data
+      response ? ESP_GATT_WRITE_TYPE_RSP : ESP_GATT_WRITE_TYPE_NO_RSP, (esp_gatt_auth_req_t)m_auth
+    );
+
+    if (errRc != ESP_OK) {
+      log_e("esp_ble_gattc_write_char_descr: %d", errRc);
+      m_semaphoreWriteDescrEvt.give();
+      return false;
+    }
+
+    // Block waiting for the event that indicates that the write has completed.
+    // The GATT status is passed through the semaphore value.
+    status = m_semaphoreWriteDescrEvt.wait("writeValue");
+
+    switch (status) {
+      case ESP_GATT_OK: break;
+      case ESP_GATT_INSUF_AUTHENTICATION:
+      case ESP_GATT_INSUF_AUTHORIZATION:
+      case ESP_GATT_INSUF_ENCRYPTION:
+        if (BLESecurity::m_securityEnabled && retryCount && pClient->secureConnection()) {
+          break;
+        }
+      /* Else falls through. */
+      default: log_e("writeValue: status=%d", status); break;
+    }
+  } while (status != ESP_GATT_OK && retryCount--);
 
-  m_semaphoreWriteDescrEvt.wait("writeValue");
   log_v("<< writeValue");
-  return (errRc == ESP_OK);
+  return (status == ESP_GATT_OK);
 }  // writeValue
 
 #endif

libraries/BLE/src/BLESecurity.cpp

@@ -37,8 +37,16 @@
  *                         Common properties                               *
  ***************************************************************************/
 
-// If true, the security will be enforced on connection even if no security is needed
-// TODO: Make this configurable without breaking Bluedroid/NimBLE compatibility
+// Controls when authentication is initiated:
+// - true (default): authentication starts immediately upon connection, running concurrently
+//   with service discovery. Because Bluedroid and NimBLE handle this concurrency differently,
+//   the relative order of events (e.g. "Authentication complete" vs "Service discovered")
+//   is not guaranteed to be the same across stacks.
+// - false: authentication is deferred until a protected characteristic is accessed. This
+//   serializes the flow (connect -> discover -> read -> auth -> retry), producing a
+//   consistent and predictable event order on both stacks.
+// Can be changed at runtime via setForceAuthentication().
+// To Do: Change default to false in v4.0.0
 bool BLESecurity::m_forceSecurity = true;
 
 bool BLESecurity::m_securityEnabled = false;
@@ -189,6 +197,20 @@ void BLESecurity::regenPassKeyOnConnect(bool enable) {
   m_regenOnConnect = enable;
 }
 
+// This function sets whether authentication should be forced immediately on connection.
+// When true, security is initiated as soon as the connection is established.
+// When false, security is triggered on-demand when accessing a protected characteristic.
+// Default: true for backward compatibility
+// To Do: Change default to false in v4.0.0
+void BLESecurity::setForceAuthentication(bool force) {
+  m_forceSecurity = force;
+}
+
+// This function returns whether authentication is forced on connection.
+bool BLESecurity::getForceAuthentication() {
+  return m_forceSecurity;
+}
+
 // This function resets the security state on disconnect.
 void BLESecurity::resetSecurity() {
   log_d("resetSecurity: Resetting security state");

libraries/BLE/src/BLESecurity.h

@@ -105,6 +105,8 @@ public:
   static uint32_t generateRandomPassKey();
   static void regenPassKeyOnConnect(bool enable = false);
   static void resetSecurity();
+  static void setForceAuthentication(bool force);
+  static bool getForceAuthentication();
   static void waitForAuthenticationComplete(uint32_t timeoutMs = 10000);
   static void signalAuthenticationComplete();
 

tests/validation/ble/client/client.ino

@@ -16,10 +16,6 @@ static BLEClient *pClient = nullptr;
 static BLERemoteCharacteristic *pRemoteInsecureCharacteristic = nullptr;
 static BLERemoteCharacteristic *pRemoteSecureCharacteristic = nullptr;
 static BLEAdvertisedDevice *myDevice = nullptr;
-static bool serviceDiscovered = false;
-static bool pinPending = false;
-static bool pinLogged = false;
-static uint32_t pendingPin = 0;
 
 class MyClientCallback : public BLEClientCallbacks {
   void onConnect(BLEClient *pclient) {
@@ -35,13 +31,8 @@ class MyClientCallback : public BLEClientCallbacks {
 class MySecurityCallbacks : public BLESecurityCallbacks {
   // Numeric Comparison callback - both devices display the same PIN
   bool onConfirmPIN(uint32_t pin) override {
-    pendingPin = pin;
-    pinPending = true;
-    if (serviceDiscovered && !pinLogged) {
-      Serial.printf("[CLIENT] Numeric comparison PIN: %lu\n", (unsigned long)pendingPin);
-      Serial.println("[CLIENT] Confirming PIN match");
-      pinLogged = true;
-    }
+    Serial.printf("[CLIENT] Numeric comparison PIN: %lu\n", (unsigned long)pin);
+    Serial.println("[CLIENT] Confirming PIN match");
     // Automatically confirm for testing
     return true;
   }
@@ -109,12 +100,6 @@ bool connectToServer() {
     return false;
   }
   Serial.println("[CLIENT] Found service");
-  serviceDiscovered = true;
-  if (pinPending && !pinLogged) {
-    Serial.printf("[CLIENT] Numeric comparison PIN: %lu\n", (unsigned long)pendingPin);
-    Serial.println("[CLIENT] Confirming PIN match");
-    pinLogged = true;
-  }
 
   pRemoteInsecureCharacteristic = pRemoteService->getCharacteristic(insecureCharUUID);
   if (pRemoteInsecureCharacteristic == nullptr) {
@@ -142,7 +127,7 @@ bool connectToServer() {
   // Set auth requirement for secure characteristic (Bluedroid)
   pRemoteSecureCharacteristic->setAuth(ESP_GATT_AUTH_REQ_MITM);
 
-  // Read secure characteristic (triggers authentication in NimBLE)
+  // Read secure characteristic (triggers on-demand authentication for both stacks)
   if (pRemoteSecureCharacteristic->canRead()) {
     Serial.println("[CLIENT] Reading secure characteristic...");
     String value = pRemoteSecureCharacteristic->readValue();
@@ -219,6 +204,9 @@ void setup() {
   pSecurity->setCapability(ESP_IO_CAP_IO);
   // Enable bonding, MITM (required for Numeric Comparison), and secure connection
   pSecurity->setAuthenticationMode(true, true, true);
+  // Disable forced auth on connect so authentication is triggered on-demand
+  // when reading the secure characteristic (consistent ordering for testing)
+  pSecurity->setForceAuthentication(false);
   BLEDevice::setSecurityCallbacks(new MySecurityCallbacks());
 
   Serial.print("[CLIENT] Scanning for server: ");

tests/validation/ble/server/server.ino

@@ -137,6 +137,9 @@ void setup() {
   pSecurity->setCapability(ESP_IO_CAP_IO);
   // Enable bonding, MITM (required for Numeric Comparison), and secure connection
   pSecurity->setAuthenticationMode(true, true, true);
+  // Disable forced auth on connect so authentication is triggered on-demand
+  // when reading the secure characteristic (consistent ordering for testing)
+  pSecurity->setForceAuthentication(false);
   BLEDevice::setSecurityCallbacks(new MySecurityCallbacks());
 
   // Create server

tests/validation/ble/test_ble.py

@@ -61,13 +61,29 @@ def test_ble(dut, ci_job_id):
     client.expect_exact("[CLIENT] Physical connection established", timeout=20)
     server.expect_exact("[SERVER] Client connected", timeout=10)
 
-    # Verify service discovery starts
+    # With m_forceSecurity=false, authentication is NOT triggered on connect.
+    # Instead, it is triggered on-demand when reading the secure characteristic.
+    # This ensures consistent ordering across both Bluedroid and NimBLE stacks:
+    # connect -> service discovery -> insecure read -> secure read (triggers auth) -> auth complete
+
+    # Verify service discovery
     LOGGER.info("Verifying service discovery...")
     client.expect_exact("[CLIENT] Found service", timeout=10)
 
+    # Continue with characteristics discovery
+    client.expect_exact("[CLIENT] Found insecure characteristic", timeout=10)
+    client.expect_exact("[CLIENT] Found secure characteristic", timeout=10)
+
+    # Verify insecure characteristic read (no auth needed)
+    LOGGER.info("Verifying insecure characteristic access...")
+    client.expect_exact("[CLIENT] Insecure characteristic value: Insecure Hello World!", timeout=10)
+
+    # Reading the secure characteristic triggers authentication on-demand
+    LOGGER.info("Verifying secure characteristic access...")
+    client.expect_exact("[CLIENT] Reading secure characteristic...", timeout=10)
+
     # Wait for numeric comparison PIN display on both devices
-    # Note: Bluedroid (ESP32) initiates security on-connect, NimBLE initiates on-demand
-    # So numeric comparison may happen during service discovery (Bluedroid) or later (NimBLE)
+    # The PIN appears during the on-demand authentication triggered by the secure read
     LOGGER.info("Waiting for numeric comparison...")
     m_server = server.expect(r"\[SERVER\] Numeric comparison PIN: (\d+)", timeout=30)
     server_pin = m_server.group(1).decode()
@@ -83,13 +99,6 @@ def test_ble(dut, ci_job_id):
     assert server_pin == client_pin, f"PIN mismatch! Server: {server_pin}, Client: {client_pin}"
     LOGGER.info(f"PIN match confirmed: {server_pin}")
 
-    # Continue with characteristics discovery
-    client.expect_exact("[CLIENT] Found insecure characteristic", timeout=10)
-    client.expect_exact("[CLIENT] Found secure characteristic", timeout=10)
-
-    # Verify insecure characteristic read
-    LOGGER.info("Verifying insecure characteristic access...")
-
     # Wait for authentication to complete on both devices
     LOGGER.info("Waiting for authentication to complete...")
     client.expect_exact("[CLIENT] Authentication complete", timeout=30)
@@ -98,22 +107,13 @@ def test_ble(dut, ci_job_id):
     LOGGER.info("Verifying IRK retrieval on client...")
     client.expect(r"\[CLIENT\] Successfully retrieved peer IRK: ([0-9a-fA-F:]+)", timeout=10)
 
-    # Server-side authentication may complete after characteristic read
-    server.expect_exact("[SERVER] Characteristic read: beb5483e-36e1-4688-b7f5-ea07361b26a8", timeout=10)
     server.expect_exact("[SERVER] Authentication complete", timeout=30)
 
     # Verify IRK retrieval on server side
     LOGGER.info("Verifying IRK retrieval on server...")
     server.expect(r"\[SERVER\] Successfully retrieved peer IRK: ([0-9a-fA-F:]+)", timeout=10)
 
-    client.expect_exact("[CLIENT] Insecure characteristic value: Insecure Hello World!", timeout=10)
-
-    # Verify secure characteristic read
-    LOGGER.info("Verifying secure characteristic access...")
-    client.expect_exact("[CLIENT] Reading secure characteristic...", timeout=10)
-
-    # Verify secure characteristic was read successfully
-    server.expect_exact("[SERVER] Characteristic read: ff1d2614-e2d6-4c87-9154-6625d39ca7f8", timeout=10)
+    # Verify secure characteristic was read successfully (after auth)
     client.expect_exact("[CLIENT] Secure characteristic value: Secure Hello World!", timeout=10)
 
     # Verify connection is established
@@ -126,7 +126,6 @@ def test_ble(dut, ci_job_id):
     server.expect_exact("[SERVER] Received write: Test Insecure Write", timeout=10)
 
     client.expect_exact("[CLIENT] Read from insecure characteristic: Test Insecure Write", timeout=10)
-    server.expect_exact("[SERVER] Characteristic read: beb5483e-36e1-4688-b7f5-ea07361b26a8", timeout=10)
     LOGGER.info("Insecure characteristic write/read verified")
 
     # Verify write and read operations on secure characteristic
@@ -135,7 +134,6 @@ def test_ble(dut, ci_job_id):
     server.expect_exact("[SERVER] Received write: Test Secure Write", timeout=10)
 
     client.expect_exact("[CLIENT] Read from secure characteristic: Test Secure Write", timeout=10)
-    server.expect_exact("[SERVER] Characteristic read: ff1d2614-e2d6-4c87-9154-6625d39ca7f8", timeout=10)
     LOGGER.info("Secure characteristic write/read verified")
 
     # Verify test completion