vandy opened a new pull request, #1962:
URL: https://github.com/apache/mynewt-nimble/pull/1962
If a GATT server provides the characteristic which requires encryption
(`BLE_GATT_CHR_F_READ_ENC`) than a client should connect and pair with the
server to read the characteristic. But if notification
(`BLE_GATT_CHR_F_NOTIFY`) or indication (`BLE_GATT_CHR_F_INDICATE`) is enabled
for the characteristic than the client could connect and subscribe to
notifications/indications getting characteristic value even if the connection
is not encrypted. So the security of the characteristic is compromised.
As I see it there are 3 alternatives for the solution:
1. Created CCCD should also has READ_ENC, WRITE_ENC (same as characteristic
itself), so that the client shall pair before proceeding if encryption required.
2. Check permissions during reading (which is the current implementation).
Nimble stack should respect permissions during reading for notifications (now
it doesn't check permissions).
3. Application developer should ensure that connection parameters are
correct for each notification. He should keep track on what characteristics
what permissions require before calling `ble_gatts_notify`. But it's kind of
weird.
```c
// service definitions
static struct ble_gatt_svc_def environmentService[] = {
{
.type = BLE_GATT_SVC_TYPE_PRIMARY,
.uuid = BLE_UUID16_DECLARE(BLE_UUID_SERVICE_ENV_SENSING),
.characteristics = (struct ble_gatt_chr_def[]) {
{
.uuid =
BLE_UUID16_DECLARE(BLE_UUID_CHARACTERISTIC_TEMPERATURE),
.access_cb = characteristicValue,
.val_handle = &esTemperatureCharacteristicAttributeHandle,
.flags = BLE_GATT_CHR_F_READ | BLE_GATT_CHR_F_READ_ENC |
BLE_GATT_CHR_F_NOTIFY,
},
{ 0 }
},
},
{ BLE_GATT_SVC_TYPE_END },
};
// nimble configuration
ble_hs_cfg.sm_sc = 1;
ble_hs_cfg.sm_bonding = 1;
ble_hs_cfg.sm_our_key_dist |= BLE_SM_PAIR_KEY_DIST_ENC;
ble_hs_cfg.sm_their_key_dist |= BLE_SM_PAIR_KEY_DIST_ENC;
ble_hs_cfg.sm_our_key_dist |= BLE_SM_PAIR_KEY_DIST_ID;
ble_hs_cfg.sm_their_key_dist |= BLE_SM_PAIR_KEY_DIST_ID;
ble_hs_cfg.reset_cb = nimble_callback_on_reset;
ble_hs_cfg.sync_cb = nimble_callback_on_sync;
ble_hs_cfg.store_status_cb = ble_store_util_status_rr;
ble_store_config_init();
ble_svc_gap_init();
ble_svc_gatt_init();
error = ble_gatts_count_cfg(environmentService);
if (error) {
return error;
}
error = ble_gatts_add_svcs(environmentService);
if (error) {
return error;
}
// set random temperature value and call notify at interval
ble_gatts_notify(conn_handle, esTemperatureCharacteristicAttributeHandle);
```
We could reproduce the behavior using nRF Connect app on Android. Start the
device, open the app, connect to the advertising device.
1. If try to read temperature characteristic the Android will show pairing
dialog. Value couldn't be read unless paired;
2. Subscribe to notification without reading the value. Android app
subscribes and value is coming at each interval.
**Review:**
1. Current code doesn't check handle in `ble_gatts_notify_multiple`, so I
removed it from `ble_gatts_notify_multiple_custom` as well. Eventually the
handle is checked during reading.
2. I've made code more consistent. E.g. `ble_gatts_notify` calls
`ble_gatts_notify_custom`, therefor I moved logic from
`ble_gatts_notify_multiple` to `ble_gatts_notify_multiple_custom`, so it
handles both cases. (As it did before, made it more DRY).
3. It seems there is a bug in the current implementation in
`ble_gatts_notify_multiple_custom` when multiple notifications are not
supported (iterating over passed handles-values structure taking out of border
element on each iteration and also not returning after the for loop).
If this changes are ok, a consideration should be taken of the necessity of
`ble_att_svr_read_local` function as it was used to bypass permission checks.
And is not used in other functions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
