-
Notifications
You must be signed in to change notification settings - Fork 7.7k
fix(ble): Fix BLESecurity and add examples #11681
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
Conversation
👋 Hello lucasssvaz, we appreciate your contribution to this project! 📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more. 🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project. Click to see more instructions ...
Review and merge process you can expect ...
|
Test Results
76 files 76 suites 15m 4s ⏱️
38 tests 38 ✅ 0 💤 0 ❌
241 runs 241 ✅ 0 💤 0 ❌
Results for commit 586e497.
♻️ This comment has been updated with latest results.
Memory usage test (comparing PR against master branch)The table below shows the summary of memory usage change (decrease - increase) in bytes and percentage for each target.
Click to expand the detailed deltas report [usage change in BYTES]
|
3f549d3
to
f83d7f3
Compare
f83d7f3
to
511c1d4
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request fixes missing static member initialization in the BLESecurity class and adds comprehensive security examples. The changes improve BLE security functionality by properly initializing static variables, enhancing error handling, and providing clearer security configuration APIs.
- Initializes missing static members in BLESecurity class to prevent undefined behavior
- Improves security callback handling and removes deprecated server/client callback methods
- Adds comprehensive secure BLE server and client examples with static passkey authentication
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.
Show a summary per file
File | Description |
---|---|
BLESecurity.h/cpp | Adds static member initialization, new security APIs, and improved callback handling |
BLEServer.h/cpp | Removes deprecated security callbacks and adds advertise-on-disconnect functionality |
BLEClient.h/cpp | Removes deprecated security callbacks and improves security integration |
BLEDevice.h/cpp | Moves security level handling to BLESecurity class and adds stack detection |
BLEUtils.h/cpp | Fixes const correctness and logging macro usage |
BLECharacteristic.h/cpp | Adds authorization support and improves permission handling |
BLEDescriptor.h/cpp | Updates permission type from uint8_t to uint16_t |
BLERemoteCharacteristic.cpp | Adds security enabled checks before attempting secure connections |
BLERemoteDescriptor.cpp | Adds security enabled checks before attempting secure connections |
Examples | Adds comprehensive secure BLE server and client examples with documentation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
libraries/BLE/examples/Server_secure_static_passkey/Server_secure_static_passkey.ino
Outdated
Show resolved
Hide resolved
libraries/BLE/examples/Client_secure_static_passkey/Client_secure_static_passkey.ino
Outdated
Show resolved
Hide resolved
I note that your above "BLESecurity.cpp" has the static class function "setAuthenticationMode()" containing:
uint8_t BLESecurity::m_authReq = 0;
...
void BLESecurity::setAuthenticationMode(uint8_t auth_req) {
log_d("setAuthenticationMode: auth_req=%d", auth_req);
m_authReq = auth_req;
#if defined(CONFIG_BLUEDROID_ENABLED)
esp_ble_gap_set_security_param(ESP_BLE_SM_AUTHEN_REQ_MODE, &m_authReq, sizeof(uint8_t));
#elif defined(CONFIG_NIMBLE_ENABLED)
BLESecurity::setAuthenticationMode(
(auth_req & BLE_SM_PAIR_AUTHREQ_BOND) != 0, (auth_req & BLE_SM_PAIR_AUTHREQ_MITM) != 0, (auth_req & BLE_SM_PAIR_AUTHREQ_SC) != 0
);
#endif
}
But I wonder why it is necessary to have "uint8_t BLESecurity::m_authReq = 0;" at all, as would it not make more sence just to use the passed "auth_req" directly, or instead to just to define it inside the function itself?
(And similarly for m_iocap, m_initKey and m_respKey?)
PS. Thankyou for the new "static passkey" examples!
@lucasssvaz please take care of the typos https://github.com/espressif/arduino-esp32/actions/runs/17464095894/job/49595433898?pr=11681#step:9:29
@Rob58329 These values are accessed directly by other classes by declaring them as friend classes.
@me-no-dev Done
@lucasssvaz
Note that for me your "Secure_server_with_static_passkey.ino" example does not work with my NodeMCU ESP32.
Speifically if you delete all saved bonds (eg. using the code below), my Android phone can connect to the ESP32 fine, and even though it gets the pop-up "Bluetooth paring request" box, you click on "pair" and it pairs without asking for or checking any paring code).
(In fact I cant currently get any of my old SDK v3.1.1 paring-code software to work!)
(I am using Windows10 with Arduino IDE v1.8.19 and https://github.com/espressif/arduino-esp32 as at 5Sep25)
The "delete saves bonds" code I am using is:
void loop() {
while (Serial.available()) {
char c=Serial.read();
if (c>=32) Serial.write(c);
else Serial.println();
if (c=='d') show_bonded_devices();
if (c=='e') remove_all_bonded_devices();
}
delay(100);
}
char bda_str[18];
char *bda2str(const uint8_t *bda) {
sprintf(bda_str, "%02x:%02x:%02x:%02x:%02x:%02x", bda[0], bda[1], bda[2], bda[3], bda[4], bda[5]);
return bda_str;
}
void show_bonded_devices() {
int dev_num = esp_ble_get_bond_device_num();
esp_ble_bond_dev_t *dev_list = (esp_ble_bond_dev_t *)malloc(sizeof(esp_ble_bond_dev_t) * dev_num);
esp_ble_get_bond_device_list(&dev_num, dev_list);
Serial.printf("\nBonded devices number: %d\n", dev_num);
for (int i = 0; i < dev_num; i++) {Serial.printf("Found bonded device #%d = %s\n", i+1, bda2str(dev_list[i].bd_addr));}
free(dev_list);
}
void remove_all_bonded_devices() { // static void __attribute__((unused)) remove_all_bonded_devices(void) {
Serial.println("\nRemoving all bonded devices...");
int dev_num = esp_ble_get_bond_device_num();
esp_ble_bond_dev_t *dev_list = (esp_ble_bond_dev_t *)malloc(sizeof(esp_ble_bond_dev_t) * dev_num);
esp_ble_get_bond_device_list(&dev_num, dev_list);
for (int i = 0; i < dev_num; i++) esp_ble_remove_bond_device(dev_list[i].bd_addr);
free(dev_list);
}
@Rob58329 I tested everything with the nrf connect and it was working as expected. Could you please try erasing the flash before the sketch upload to clear the nvs ?
@Rob58329 I tested everything with the nrf connect and it was working as expected. Could you please try erasing the flash before the sketch upload to clear the nvs ?
@lucasssvaz I've just done a "Flash memory erased successfully in 6.0 seconds." and then re-flashed your Secure_server_with_static_passkey.ino" example . But with my ESP32 (orig) I still can connect to the ESP32 using BLE with "nRF Connect" on my Android phone without enterning any PIN code and read the "Secure Hello World" message (as well as the "Insecure Hello World" message).
PS. also tried on a brand new NodeMCU ESP32 which has never been flashed before and it too connected without needing to enter any PIN (after pressing the "PAIR" option on the phone it just connects without giving you the option to enter a PIN).
@lucasssvaz: Your above example "Secure_server_with_static_passkey.ino" sketches only work on "CONFIG_NIMBLE_ENABLED" devices, and not on "CONFIG_BLUEDROID_ENABLED" devices such as the ESP32 (original).
For Bluedroid devices I attach below the following "Secure server with static passkey using Bluedroid" sketch, which works on the ESP32 (original) with the latest IDE SDK=v3.3.0.
/*
Secure server with static passkey using Bluedroid (used on the ESP32)
This example demonstrates how to create a secure BLE server using a static passkey on the ESP32 (Bluedroid).
Note that ESP32 uses Bluedroid by default and the other SoCs use NimBLE.
Bluedroid initiates security on-connect, while NimBLE initiates security on-demand.
This means that in NimBLE you can read the unsecure 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.
Based on examples from Neil Kolban and h2zero and lucasssvaz
Created by Rob58329
*/
#if !defined(CONFIG_BLUEDROID_ENABLED)
#error "This will only compile on Bluedroid based devices such as the ESP32 (ie. not the ESP32S3 etc)"
#endif
#include <BLEDevice.h>
#include <BLEServer.h>
BLEServer *pServer=nullptr;
uint32_t BLE_PASSKEY=123456; // for BLE must be exactly 6 digits long! // once bonded passkey is n/a (can be changed without effecting bonded devices)
volatile boolean BLE_connected=false;
volatile boolean BLE_security=false;
class MySecurityCallbacks : public BLESecurityCallbacks {
//bool onConfirmPIN(uint32_t pin) {Serial.printf("onConfirmPIN=%u\n",pin); return (pin==BLE_PASSKEY);}
//uint32_t onPassKeyRequest() {Serial.println("onPassKeyRequest"); return BLE_PASSKEY;}
void onPassKeyNotify(uint32_t pass_key) {Serial.printf("onPassKeyNotify=%u\n", pass_key);}
bool onSecurityRequest() {Serial.println("onSecurityRequest"); return true;}
void onAuthenticationComplete(esp_ble_auth_cmpl_t cmpl) {Serial.printf("Auth %s\n",cmpl.success ? "OK" : "FAIL"); BLE_security=(cmpl.success);}
};
class MyServerCallbacks : public BLEServerCallbacks {
void onConnect(BLEServer *pServer) {BLE_connected=true; Serial.printf("Device connected (ID=%u)\n",pServer->getConnId());}
void onDisconnect(BLEServer *pServer) {BLE_connected=false; BLE_security=false; Serial.println("Device Disconnected");}
};
// After BLE_connected allow 1s for BLE_security to change/succeed, and if it doesn't disconnect the BLE-client.
// If you want to allow PAIRING all the time, set change the above to "DISCONNECT_TIMER 15000".
// But note that I beleive that there is a bug in the "Bluedroid" software that after reboot it will only give you the
// prompt to enter a PIN the 1st time you try to connect, and if you fail to enter the correct PIN the 1st time, it
// will disconnect you, but subsequently will allow you to connect but not go through any securuty-authentication.
// This means if you fail to enter the correct PIN the 1st time, you have to reboot the ESP32 or do
// a BLEDevice::de/reinit before you can try again.
// It also means that a few seconds after any connection you need to check to see if BLE_security has succeeded
// and if not disconnect the Client - see the below example.
#define DISCONNECT_TIMER 1000 // 1s // increase this to 2s if necessary
#define DISCONNECT_TIMER_PAIRING 15000 // 15s
uint32_t disconnect_timer=DISCONNECT_TIMER;
boolean BLE_secured_connection() {
static uint32_t millis_disconnect=0;
if (BLE_security or !BLE_connected) millis_disconnect=0;
else if (millis_disconnect) {if (millis_disconnect<=millis() and (pServer)) pServer->disconnect(pServer->getConnId());}
else {millis_disconnect=millis()+disconnect_timer; Serial.println("Start connect timer (1 or 15s)"); disconnect_timer=DISCONNECT_TIMER;}
return BLE_security;
}
void setup() {
Serial.begin(115200); delay(2999);
Serial.printf("ESP_ARDUINO=v%s\n",ESP_ARDUINO_VERSION_STR); // ESP_ARDUINO=v3.3.0
Serial.println("Starting BLE work!");
Serial.print("Using BLE stack: "); Serial.println(BLEDevice::getBLEStackString()); // Using BLE stack: Bluedroid // defined(CONFIG_BLUEDROID_ENABLED)
Serial.println("BLEDevice::init...");
BLEDevice::init("Secure BLE Client");
BLEDevice::setSecurityCallbacks(new MySecurityCallbacks());
pServer = BLEDevice::createServer(); // BLEServer *
pServer->advertiseOnDisconnect(true);
pServer->setCallbacks(new MyServerCallbacks());
BLEAdvertising *pAdvertising = pServer->getAdvertising();
pAdvertising->start();
BLESecurity *pSecurity = new BLESecurity();
pSecurity->setCapability(ESP_IO_CAP_OUT); // Not sure if this is necessary?
pSecurity->setPassKey(true,BLE_PASSKEY);
pSecurity->setAuthenticationMode(true,false,false);
pSecurity->setAuthenticationMode(ESP_LE_AUTH_REQ_SC_MITM_BOND); // override the "setAuthenticationMode(true,0,0);" // Not sure if this is necessary?
BLESecurity::setEncryptionLevel(ESP_BLE_SEC_ENCRYPT); // Not sure if this is necessary?
uint8_t auth_option = ESP_BLE_ONLY_ACCEPT_SPECIFIED_AUTH_ENABLE; // essential to stop Clients with No-PIN connecting
esp_ble_gap_set_security_param(ESP_BLE_SM_ONLY_ACCEPT_SPECIFIED_SEC_AUTH, &auth_option, sizeof(uint8_t));
pSecurity->setInitEncryptionKey(ESP_BLE_ENC_KEY_MASK | ESP_BLE_ID_KEY_MASK); // Not sure if this is necessary?
pSecurity->setRespEncryptionKey(ESP_BLE_ENC_KEY_MASK | ESP_BLE_ID_KEY_MASK); // Not sure if this is necessary?
Serial.println("BLE Started. You can now connect from your phone");
Serial.println("Serial Commands: b=Initiate Bonding, d=Display Bonds, e=Erase All-Bonds");
}
void loop() {
boolean secure=BLE_secured_connection();
static boolean secure_old=false; if (secure_old!=secure) {secure_old=secure; Serial.printf("**Secure Connection=%u\n",secure);}
delay(100);
while (Serial.available()) {
char c=Serial.read();
if (c>=32) Serial.write(c);
else Serial.println();
if (c=='b') {
disconnect_timer=DISCONNECT_TIMER_PAIRING;
Serial.println("\nSet disconnect_timer=15s (NB. you need to do this BEFORE to try to PAIR,\notherwise do a reboot or BLEDevice::de/reinit and try again)");
}
if (c=='d') show_bonded_devices();
if (c=='e') remove_all_bonded_devices();
}
}
char bda_str[18];
char *bda2str(const uint8_t *bda) {
sprintf(bda_str, "%02x:%02x:%02x:%02x:%02x:%02x", bda[0], bda[1], bda[2], bda[3], bda[4], bda[5]);
return bda_str;
}
void show_bonded_devices() {
int dev_num = esp_ble_get_bond_device_num();
esp_ble_bond_dev_t *dev_list = (esp_ble_bond_dev_t *)malloc(sizeof(esp_ble_bond_dev_t) * dev_num);
esp_ble_get_bond_device_list(&dev_num, dev_list);
Serial.printf("\nBonded devices number: %d\n", dev_num);
for (int i = 0; i < dev_num; i++) {Serial.printf("Found bonded device #%d = %s\n", i+1, bda2str(dev_list[i].bd_addr));}
free(dev_list);
}
void remove_all_bonded_devices() {
Serial.println("\nRemoving all bonded devices");
int dev_num = esp_ble_get_bond_device_num();
esp_ble_bond_dev_t *dev_list = (esp_ble_bond_dev_t *)malloc(sizeof(esp_ble_bond_dev_t) * dev_num);
esp_ble_get_bond_device_list(&dev_num, dev_list);
for (int i = 0; i < dev_num; i++) esp_ble_remove_bond_device(dev_list[i].bd_addr);
free(dev_list);
}
Description of Change
This pull request fixes the missing static members initialization in BLESecurity.
Tests scenarios
Tested locally
Related links
Closes #11671