Skip to content

Commit 8ec38d7

Browse files
committed
fix(ble): Fix authentication deadlock
1 parent 7a9a10c commit 8ec38d7

File tree

4 files changed

+74
-0
lines changed

4 files changed

+74
-0
lines changed

libraries/BLE/src/BLEDevice.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -960,6 +960,10 @@ void BLEDevice::gapEventHandler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_par
960960
{
961961
log_i("ESP_GAP_BLE_AUTH_CMPL_EVT");
962962
#ifdef CONFIG_BLE_SMP_ENABLE // Check that BLE SMP (security) is configured in make menuconfig
963+
// Signal that authentication has completed
964+
// This unblocks any GATT operations waiting for pairing when bonding is enabled
965+
BLESecurity::signalAuthenticationComplete();
966+
963967
if (BLEDevice::m_securityCallbacks != nullptr) {
964968
BLEDevice::m_securityCallbacks->onAuthenticationComplete(param->ble_security.auth_cmpl);
965969
}

libraries/BLE/src/BLERemoteCharacteristic.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,10 @@ String BLERemoteCharacteristic::readValue() {
565565
return String();
566566
}
567567

568+
// Wait for authentication to complete if bonding is enabled
569+
// This prevents the read request from being made while pairing is in progress
570+
BLESecurity::waitForAuthenticationComplete();
571+
568572
m_semaphoreReadCharEvt.take("readValue");
569573

570574
// Ask the BLE subsystem to retrieve the value for the remote hosted characteristic.
@@ -607,6 +611,10 @@ bool BLERemoteCharacteristic::writeValue(uint8_t *data, size_t length, bool resp
607611
return false;
608612
}
609613

614+
// Wait for authentication to complete if bonding is enabled
615+
// This prevents the write request from being made while pairing is in progress
616+
BLESecurity::waitForAuthenticationComplete();
617+
610618
m_semaphoreWriteCharEvt.take("writeValue");
611619
// Invoke the ESP-IDF API to perform the write.
612620
esp_err_t errRc = ::esp_ble_gattc_write_char(

libraries/BLE/src/BLESecurity.cpp

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ bool BLESecurity::m_securityStarted = false;
4646
bool BLESecurity::m_passkeySet = false;
4747
bool BLESecurity::m_staticPasskey = true;
4848
bool BLESecurity::m_regenOnConnect = false;
49+
bool BLESecurity::m_authenticationComplete = false;
4950
uint8_t BLESecurity::m_iocap = ESP_IO_CAP_NONE;
5051
uint8_t BLESecurity::m_authReq = 0;
5152
uint8_t BLESecurity::m_initKey = 0;
@@ -59,6 +60,7 @@ uint32_t BLESecurity::m_passkey = BLE_SM_DEFAULT_PASSKEY;
5960
#if defined(CONFIG_BLUEDROID_ENABLED)
6061
uint8_t BLESecurity::m_keySize = 16;
6162
esp_ble_sec_act_t BLESecurity::m_securityLevel;
63+
FreeRTOS::Semaphore *BLESecurity::m_authCompleteSemaphore = nullptr;
6264
#endif
6365

6466
/***************************************************************************
@@ -191,6 +193,7 @@ void BLESecurity::regenPassKeyOnConnect(bool enable) {
191193
void BLESecurity::resetSecurity() {
192194
log_d("resetSecurity: Resetting security state");
193195
m_securityStarted = false;
196+
m_authenticationComplete = false;
194197
}
195198

196199
// This function sets the authentication mode with bonding, MITM, and secure connection options.
@@ -263,6 +266,48 @@ bool BLESecurityCallbacks::onAuthorizationRequest(uint16_t connHandle, uint16_t
263266
return true;
264267
}
265268

269+
// This function waits for authentication to complete when bonding is enabled
270+
// It prevents GATT operations from proceeding before pairing completes
271+
void BLESecurity::waitForAuthenticationComplete(uint32_t timeoutMs) {
272+
#if defined(CONFIG_BLUEDROID_ENABLED)
273+
// Only wait if bonding is enabled
274+
if ((m_authReq & ESP_LE_AUTH_BOND) == 0) {
275+
return;
276+
}
277+
278+
// If already authenticated, no need to wait
279+
if (m_authenticationComplete) {
280+
return;
281+
}
282+
283+
// Semaphore should have been created in startSecurity()
284+
if (m_authCompleteSemaphore == nullptr) {
285+
log_e("Authentication semaphore not initialized");
286+
return;
287+
}
288+
289+
// Wait for authentication with timeout
290+
bool success = m_authCompleteSemaphore->timedWait("waitForAuthenticationComplete", timeoutMs / portTICK_PERIOD_MS);
291+
292+
if (!success) {
293+
log_w("Timeout waiting for authentication to complete");
294+
}
295+
#endif
296+
}
297+
298+
// This function signals that authentication has completed
299+
// Called from ESP_GAP_BLE_AUTH_CMPL_EVT handler
300+
void BLESecurity::signalAuthenticationComplete() {
301+
#if defined(CONFIG_BLUEDROID_ENABLED)
302+
m_authenticationComplete = true;
303+
304+
// Signal waiting threads if semaphore exists
305+
if (m_authCompleteSemaphore != nullptr) {
306+
m_authCompleteSemaphore->give();
307+
}
308+
#endif
309+
}
310+
266311
/***************************************************************************
267312
* Bluedroid functions *
268313
***************************************************************************/
@@ -285,6 +330,18 @@ bool BLESecurity::startSecurity(esp_bd_addr_t bd_addr, int *rcPtr) {
285330
}
286331

287332
if (m_securityEnabled) {
333+
// Initialize semaphore before starting security to avoid race condition
334+
if (m_authCompleteSemaphore == nullptr) {
335+
m_authCompleteSemaphore = new FreeRTOS::Semaphore("AuthComplete");
336+
}
337+
338+
// Reset authentication complete flag when starting new security negotiation
339+
m_authenticationComplete = false;
340+
341+
// Consume any pending semaphore signals from previous operations
342+
// This ensures the next wait will block until the new auth completes
343+
m_authCompleteSemaphore->take("startSecurity-reset");
344+
288345
int rc = esp_ble_set_encryption(bd_addr, m_securityLevel);
289346
if (rc != ESP_OK) {
290347
log_e("esp_ble_set_encryption: rc=%d %s", rc, GeneralUtils::errorToString(rc));

libraries/BLE/src/BLESecurity.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "BLEDevice.h"
2626
#include "BLEClient.h"
2727
#include "BLEServer.h"
28+
#include "RTOS.h"
2829

2930
/***************************************************************************
3031
* Bluedroid includes *
@@ -104,6 +105,8 @@ class BLESecurity {
104105
static uint32_t generateRandomPassKey();
105106
static void regenPassKeyOnConnect(bool enable = false);
106107
static void resetSecurity();
108+
static void waitForAuthenticationComplete(uint32_t timeoutMs = 10000);
109+
static void signalAuthenticationComplete();
107110

108111
/***************************************************************************
109112
* Bluedroid public declarations *
@@ -140,6 +143,7 @@ class BLESecurity {
140143
static bool m_passkeySet;
141144
static bool m_staticPasskey;
142145
static bool m_regenOnConnect;
146+
static bool m_authenticationComplete;
143147
static uint8_t m_iocap;
144148
static uint8_t m_authReq;
145149
static uint8_t m_initKey;
@@ -153,6 +157,7 @@ class BLESecurity {
153157
#if defined(CONFIG_BLUEDROID_ENABLED)
154158
static uint8_t m_keySize;
155159
static esp_ble_sec_act_t m_securityLevel;
160+
static class FreeRTOS::Semaphore *m_authCompleteSemaphore;
156161
#endif
157162

158163
}; // BLESecurity

0 commit comments

Comments
 (0)