Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 8af12a5

Browse files
fix(ble): Fix security for Bluedroid
1 parent 20c0dde commit 8af12a5

15 files changed

+648
-144
lines changed

‎libraries/BLE/src/BLECharacteristic.cpp

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,34 @@ void BLECharacteristic::handleGATTServerEvent(esp_gatts_cb_event_t event, esp_ga
582582
// we save the new value. Next we look at the need_rsp flag which indicates whether or not we need
583583
// to send a response. If we do, then we formulate a response and send it.
584584
if (param->write.handle == m_handle) {
585+
586+
// Check for authorization requirement
587+
if (m_permissions & ESP_GATT_PERM_WRITE_AUTHORIZATION) {
588+
bool authorized = false;
589+
590+
if (BLEDevice::m_securityCallbacks != nullptr) {
591+
log_i("Authorization required for write operation. Checking authorization...");
592+
authorized = BLEDevice::m_securityCallbacks->onAuthorizationRequest(
593+
param->write.conn_id, m_handle, false
594+
);
595+
} else {
596+
log_w("onAuthorizationRequest not implemented. Rejecting write authorization request");
597+
}
598+
599+
if (!authorized) {
600+
log_i("Write authorization rejected");
601+
if (param->write.need_rsp) {
602+
esp_err_t errRc = ::esp_ble_gatts_send_response(gatts_if, param->write.conn_id, param->write.trans_id, ESP_GATT_INSUF_AUTHORIZATION, nullptr);
603+
if (errRc != ESP_OK) {
604+
log_e("esp_ble_gatts_send_response (authorization failed): rc=%d %s", errRc, GeneralUtils::errorToString(errRc));
605+
}
606+
}
607+
return; // Exit early, don't process the write
608+
} else {
609+
log_i("Write authorization granted");
610+
}
611+
}
612+
585613
if (param->write.is_prep) {
586614
m_value.addPart(param->write.value, param->write.len);
587615
m_writeEvt = true;
@@ -637,6 +665,33 @@ void BLECharacteristic::handleGATTServerEvent(esp_gatts_cb_event_t event, esp_ga
637665
{
638666
if (param->read.handle == m_handle) {
639667

668+
// Check for authorization requirement
669+
if (m_permissions & ESP_GATT_PERM_READ_AUTHORIZATION) {
670+
bool authorized = false;
671+
672+
if (BLEDevice::m_securityCallbacks != nullptr) {
673+
log_i("Authorization required for read operation. Checking authorization...");
674+
authorized = BLEDevice::m_securityCallbacks->onAuthorizationRequest(
675+
param->read.conn_id, m_handle, true
676+
);
677+
} else {
678+
log_w("onAuthorizationRequest not implemented. Rejecting read authorization request");
679+
}
680+
681+
if (!authorized) {
682+
log_i("Read authorization rejected");
683+
if (param->read.need_rsp) {
684+
esp_err_t errRc = ::esp_ble_gatts_send_response(gatts_if, param->read.conn_id, param->read.trans_id, ESP_GATT_INSUF_AUTHORIZATION, nullptr);
685+
if (errRc != ESP_OK) {
686+
log_e("esp_ble_gatts_send_response (authorization failed): rc=%d %s", errRc, GeneralUtils::errorToString(errRc));
687+
}
688+
}
689+
return; // Exit early, don't process the read
690+
} else {
691+
log_i("Read authorization granted");
692+
}
693+
}
694+
640695
// Here's an interesting thing. The read request has the option of saying whether we need a response
641696
// or not. What would it "mean" to receive a read request and NOT send a response back? That feels like
642697
// a very strange read.

‎libraries/BLE/src/BLECharacteristic.h

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,17 +140,25 @@ class BLECharacteristic {
140140

141141
#if defined(CONFIG_BLUEDROID_ENABLED)
142142
static const uint32_t PROPERTY_READ = 1 << 0;
143+
static const uint32_t PROPERTY_READ_ENC = 0; // Not supported by Bluedroid. Use setAccessPermissions() instead.
144+
static const uint32_t PROPERTY_READ_AUTHEN = 0; // Not supported by Bluedroid. Use setAccessPermissions() instead.
145+
static const uint32_t PROPERTY_READ_AUTHOR = 0; // Not supported by Bluedroid. Use setAccessPermissions() instead.
143146
static const uint32_t PROPERTY_WRITE = 1 << 1;
147+
static const uint32_t PROPERTY_WRITE_NR = 1 << 5;
148+
static const uint32_t PROPERTY_WRITE_ENC = 0; // Not supported by Bluedroid. Use setAccessPermissions() instead.
149+
static const uint32_t PROPERTY_WRITE_AUTHEN = 0; // Not supported by Bluedroid. Use setAccessPermissions() instead.
150+
static const uint32_t PROPERTY_WRITE_AUTHOR = 0; // Not supported by Bluedroid. Use setAccessPermissions() instead.
144151
static const uint32_t PROPERTY_NOTIFY = 1 << 2;
145152
static const uint32_t PROPERTY_BROADCAST = 1 << 3;
146153
static const uint32_t PROPERTY_INDICATE = 1 << 4;
147-
static const uint32_t PROPERTY_WRITE_NR = 1 << 5;
148154
#endif
149155

150156
/***************************************************************************
151157
* NimBLE public properties *
152158
***************************************************************************/
153159

160+
161+
154162
#if defined(CONFIG_NIMBLE_ENABLED)
155163
static const uint32_t PROPERTY_READ = BLE_GATT_CHR_F_READ;
156164
static const uint32_t PROPERTY_READ_ENC = BLE_GATT_CHR_F_READ_ENC;
@@ -161,8 +169,8 @@ class BLECharacteristic {
161169
static const uint32_t PROPERTY_WRITE_ENC = BLE_GATT_CHR_F_WRITE_ENC;
162170
static const uint32_t PROPERTY_WRITE_AUTHEN = BLE_GATT_CHR_F_WRITE_AUTHEN;
163171
static const uint32_t PROPERTY_WRITE_AUTHOR = BLE_GATT_CHR_F_WRITE_AUTHOR;
164-
static const uint32_t PROPERTY_BROADCAST = BLE_GATT_CHR_F_BROADCAST;
165172
static const uint32_t PROPERTY_NOTIFY = BLE_GATT_CHR_F_NOTIFY;
173+
static const uint32_t PROPERTY_BROADCAST = BLE_GATT_CHR_F_BROADCAST;
166174
static const uint32_t PROPERTY_INDICATE = BLE_GATT_CHR_F_INDICATE;
167175
#endif
168176

‎libraries/BLE/src/BLEClient.cpp

Lines changed: 59 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
* Common includes *
2020
***************************************************************************/
2121

22+
#include "Arduino.h"
2223
#include <esp_bt.h>
2324
#include "BLEClient.h"
2425
#include "BLEUtils.h"
@@ -29,6 +30,7 @@
2930
#include <unordered_set>
3031
#include "BLEDevice.h"
3132
#include "esp32-hal-log.h"
33+
#include "BLESecurity.h"
3234

3335
/***************************************************************************
3436
* Bluedroid includes *
@@ -598,11 +600,11 @@ void BLEClient::gattClientEventHandler(esp_gattc_cb_event_t event, esp_gatt_if_t
598600
if (errRc != ESP_OK) {
599601
log_e("esp_ble_gattc_send_mtu_req: rc=%d %s", errRc, GeneralUtils::errorToString(errRc));
600602
}
601-
#ifdef CONFIG_BLE_SMP_ENABLE // Check that BLE SMP (security) is configured in make menuconfig
602-
if (BLEDevice::m_securityLevel) {
603-
esp_ble_set_encryption(evtParam->connect.remote_bda, BLEDevice::m_securityLevel);
603+
// Set encryption on connect for BlueDroid when security is enabled
604+
// This ensures security is established before any secure operations
605+
if (BLESecurity::m_securityEnabled && BLESecurity::m_forceSecurity) {
606+
BLESecurity::startSecurity(evtParam->connect.remote_bda);
604607
}
605-
#endif // CONFIG_BLE_SMP_ENABLE
606608
break;
607609
} // ESP_GATTC_CONNECT_EVT
608610

@@ -1006,6 +1008,10 @@ int BLEClient::handleGAPEvent(struct ble_gap_event *event, void *arg) {
10061008
break;
10071009
}
10081010

1011+
if(BLESecurity::m_securityEnabled) {
1012+
BLESecurity::startSecurity(client->m_conn_id);
1013+
}
1014+
10091015
// In the case of a multiconnecting device we ignore this device when
10101016
// scanning since we are already connected to it
10111017
// BLEDevice::addIgnored(client->m_peerAddress);
@@ -1136,8 +1142,6 @@ int BLEClient::handleGAPEvent(struct ble_gap_event *event, void *arg) {
11361142
ble_store_util_delete_peer(&desc.peer_id_addr);
11371143
} else if (BLEDevice::m_securityCallbacks != nullptr) {
11381144
BLEDevice::m_securityCallbacks->onAuthenticationComplete(&desc);
1139-
} else {
1140-
client->m_pClientCallbacks->onAuthenticationComplete(&desc);
11411145
}
11421146
}
11431147

@@ -1164,52 +1168,88 @@ int BLEClient::handleGAPEvent(struct ble_gap_event *event, void *arg) {
11641168
}
11651169

11661170
if (event->passkey.params.action == BLE_SM_IOACT_DISP) {
1171+
// Display the passkey on this device
1172+
log_d("BLE_SM_IOACT_DISP");
1173+
11671174
pkey.action = event->passkey.params.action;
1168-
pkey.passkey = BLESecurity::m_passkey; // This is the passkey to be entered on peer
1175+
pkey.passkey = BLESecurity::getPassKey(); // This is the passkey to be entered on peer
1176+
1177+
if(!BLESecurity::m_passkeySet) {
1178+
log_w("No passkey set");
1179+
}
1180+
1181+
if (BLESecurity::m_staticPasskey && pkey.passkey == BLE_SM_DEFAULT_PASSKEY) {
1182+
log_w("*ATTENTION* Using default passkey: %06d", BLE_SM_DEFAULT_PASSKEY);
1183+
log_w("*ATTENTION* Please use a random passkey or set a different static passkey");
1184+
} else {
1185+
log_i("Passkey: %d", pkey.passkey);
1186+
}
1187+
1188+
if (BLEDevice::m_securityCallbacks != nullptr) {
1189+
BLEDevice::m_securityCallbacks->onPassKeyNotify(pkey.passkey);
1190+
}
1191+
11691192
rc = ble_sm_inject_io(event->passkey.conn_handle, &pkey);
11701193
log_d("ble_sm_inject_io result: %d", rc);
11711194

11721195
} else if (event->passkey.params.action == BLE_SM_IOACT_NUMCMP) {
1196+
// Check if the passkey on the peer device is correct
1197+
log_d("BLE_SM_IOACT_NUMCMP");
1198+
11731199
log_d("Passkey on device's display: %d", event->passkey.params.numcmp);
11741200
pkey.action = event->passkey.params.action;
1175-
// Compatibility only - Do not use, should be removed the in future
1201+
11761202
if (BLEDevice::m_securityCallbacks != nullptr) {
11771203
pkey.numcmp_accept = BLEDevice::m_securityCallbacks->onConfirmPIN(event->passkey.params.numcmp);
1178-
////////////////////////////////////////////////////
11791204
} else {
1180-
pkey.numcmp_accept = client->m_pClientCallbacks->onConfirmPIN(event->passkey.params.numcmp);
1205+
log_e("onConfirmPIN not implemented. Rejecting connection");
1206+
pkey.numcmp_accept = 0;
11811207
}
11821208

11831209
rc = ble_sm_inject_io(event->passkey.conn_handle, &pkey);
11841210
log_d("ble_sm_inject_io result: %d", rc);
11851211

1186-
//TODO: Handle out of band pairing
11871212
} else if (event->passkey.params.action == BLE_SM_IOACT_OOB) {
1213+
// Out of band pairing
1214+
// TODO: Handle out of band pairing
1215+
log_w("BLE_SM_IOACT_OOB: Not implemented");
1216+
11881217
static uint8_t tem_oob[16] = {0};
11891218
pkey.action = event->passkey.params.action;
11901219
for (int i = 0; i < 16; i++) {
11911220
pkey.oob[i] = tem_oob[i];
11921221
}
11931222
rc = ble_sm_inject_io(event->passkey.conn_handle, &pkey);
11941223
log_d("ble_sm_inject_io result: %d", rc);
1195-
////////
11961224
} else if (event->passkey.params.action == BLE_SM_IOACT_INPUT) {
1197-
log_d("Enter the passkey");
1225+
// Input passkey from peer device
1226+
log_d("BLE_SM_IOACT_INPUT");
1227+
11981228
pkey.action = event->passkey.params.action;
1229+
pkey.passkey = BLESecurity::getPassKey();
1230+
1231+
if (!BLESecurity::m_passkeySet) {
1232+
if (BLEDevice::m_securityCallbacks != nullptr) {
1233+
log_i("No passkey set, getting passkey from onPassKeyRequest");
1234+
pkey.passkey = BLEDevice::m_securityCallbacks->onPassKeyRequest();
1235+
} else {
1236+
log_w("*ATTENTION* onPassKeyRequest not implemented and no static passkey set.");
1237+
}
1238+
}
11991239

1200-
// Compatibility only - Do not use, should be removed the in future
1201-
if (BLEDevice::m_securityCallbacks != nullptr) {
1202-
pkey.passkey = BLEDevice::m_securityCallbacks->onPassKeyRequest();
1203-
/////////////////////////////////////////////
1240+
if (BLESecurity::m_staticPasskey && pkey.passkey == BLE_SM_DEFAULT_PASSKEY) {
1241+
log_w("*ATTENTION* Using default passkey: %06d", BLE_SM_DEFAULT_PASSKEY);
1242+
log_w("*ATTENTION* Please use a random passkey or set a different static passkey");
12041243
} else {
1205-
pkey.passkey = client->m_pClientCallbacks->onPassKeyRequest();
1244+
log_i("Passkey: %d", pkey.passkey);
12061245
}
12071246

12081247
rc = ble_sm_inject_io(event->passkey.conn_handle, &pkey);
12091248
log_d("ble_sm_inject_io result: %d", rc);
12101249

12111250
} else if (event->passkey.params.action == BLE_SM_IOACT_NONE) {
1212-
log_d("No passkey action required");
1251+
log_d("BLE_SM_IOACT_NONE");
1252+
log_i("No passkey action required");
12131253
}
12141254

12151255
return 0;
@@ -1255,20 +1295,6 @@ bool BLEClientCallbacks::onConnParamsUpdateRequest(BLEClient *pClient, const ble
12551295
return true;
12561296
}
12571297

1258-
uint32_t BLEClientCallbacks::onPassKeyRequest() {
1259-
log_d("onPassKeyRequest: default: 123456");
1260-
return 123456;
1261-
}
1262-
1263-
void BLEClientCallbacks::onAuthenticationComplete(ble_gap_conn_desc *desc) {
1264-
log_d("onAuthenticationComplete: default");
1265-
}
1266-
1267-
bool BLEClientCallbacks::onConfirmPIN(uint32_t pin) {
1268-
log_d("onConfirmPIN: default: true");
1269-
return true;
1270-
}
1271-
12721298
#endif // CONFIG_NIMBLE_ENABLED
12731299

12741300
#endif /* CONFIG_BLUEDROID_ENABLED || CONFIG_NIMBLE_ENABLED */

‎libraries/BLE/src/BLEClient.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,9 +209,6 @@ class BLEClientCallbacks {
209209

210210
#if defined(CONFIG_NIMBLE_ENABLED)
211211
virtual bool onConnParamsUpdateRequest(BLEClient *pClient, const ble_gap_upd_params *params);
212-
virtual uint32_t onPassKeyRequest();
213-
virtual void onAuthenticationComplete(ble_gap_conn_desc *desc);
214-
virtual bool onConfirmPIN(uint32_t pin);
215212
#endif
216213
};
217214

‎libraries/BLE/src/BLEDescriptor.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@
4242
#include <host/ble_att.h>
4343
#include "BLEConnInfo.h"
4444

45+
// Bluedroid compatibility
46+
// NimBLE does not support signed reads and writes
4547
#define ESP_GATT_PERM_READ BLE_ATT_F_READ
4648
#define ESP_GATT_PERM_WRITE BLE_ATT_F_WRITE
4749
#define ESP_GATT_PERM_READ_ENCRYPTED BLE_ATT_F_READ_ENC

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /