Skip to content

nimble/host: characteristic value reading for notification/indication compromises security set-up #1962

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

nimble/host: characteristic value reading for notification/indication compromises security set-up #1962

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions nimble/host/src/ble_att_priv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ void ble_att_svr_prep_clear(struct ble_att_prep_entry_list *prep_list);
int ble_att_svr_read_handle(uint16_t conn_handle, uint16_t attr_handle,
uint16_t offset, struct os_mbuf *om,
uint8_t *out_att_err);
int ble_att_svr_read_local_with_perms(uint16_t conn_handle, uint16_t attr_handle, struct os_mbuf **out_om);
void ble_att_svr_reset(void);
int ble_att_svr_init(void);

Expand Down
12 changes: 9 additions & 3 deletions nimble/host/src/ble_att_svr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ ble_att_svr_read_handle(uint16_t conn_handle, uint16_t attr_handle,
}

int
ble_att_svr_read_local(uint16_t attr_handle, struct os_mbuf **out_om)
ble_att_svr_read_local_with_perms(uint16_t conn_handle, uint16_t attr_handle, struct os_mbuf **out_om)
{
struct os_mbuf *om;
int rc;
Expand All @@ -507,20 +507,26 @@ ble_att_svr_read_local(uint16_t attr_handle, struct os_mbuf **out_om)
goto err;
}

rc = ble_att_svr_read_handle(BLE_HS_CONN_HANDLE_NONE, attr_handle, 0, om,
NULL);
rc = ble_att_svr_read_handle(conn_handle, attr_handle, 0, om, NULL);
if (rc != 0) {
goto err;
}

*out_om = om;

return 0;

err:
os_mbuf_free_chain(om);
return rc;
}

int
ble_att_svr_read_local(uint16_t attr_handle, struct os_mbuf **out_om)
{
return ble_att_svr_read_local_with_perms(BLE_HS_CONN_HANDLE_NONE, attr_handle, out_om);
}

static int
ble_att_svr_write(uint16_t conn_handle, struct ble_att_svr_entry *entry,
uint16_t offset, struct os_mbuf **om, uint8_t *out_att_err)
Expand Down
112 changes: 60 additions & 52 deletions nimble/host/src/ble_gattc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4368,11 +4368,27 @@ ble_gatts_notify_custom(uint16_t conn_handle, uint16_t chr_val_handle,
rc = BLE_HS_ENOMEM;
goto done;
}
rc = ble_att_svr_read_handle(BLE_HS_CONN_HANDLE_NONE,
chr_val_handle, 0, txom, NULL);

rc = ble_att_svr_read_handle(conn_handle, chr_val_handle, 0, txom, NULL);
if (rc != 0) {
/* Fatal error; application disallowed attribute read. */
rc = BLE_HS_EAPP;
switch (rc) {
case BLE_HS_ATT_ERR(BLE_ATT_ERR_INSUFFICIENT_ENC):
rc = BLE_HS_EENCRYPT;
break;

case BLE_HS_ATT_ERR(BLE_ATT_ERR_INSUFFICIENT_AUTHEN):
rc = BLE_HS_EAUTHEN;
break;

case BLE_HS_ATT_ERR(BLE_ATT_ERR_INSUFFICIENT_AUTHOR):
rc = BLE_HS_EAUTHOR;
break;

default:
/* Fatal error; application disallowed attribute read. */
rc = BLE_HS_EAPP;
break;
}
goto done;
}
}
Expand Down Expand Up @@ -4437,43 +4453,41 @@ ble_gatts_notify_multiple_custom(uint16_t conn_handle,
struct os_mbuf *txom;
struct ble_hs_conn *conn;

txom = ble_hs_mbuf_att_pkt();
if (txom == NULL) {
return BLE_HS_ENOMEM;
}
BLE_HS_LOG_DEBUG("conn_handle %d\n", conn_handle);

conn = ble_hs_conn_find(conn_handle);
if (conn == NULL) {
return BLE_HS_ENOTCONN;
}

/* Read missing values */
for (i = 0; i < chr_count; i++) {
if (tuples->handle == 0) {
rc = BLE_HS_EINVAL;
goto done;
}
if (tuples[i].value == NULL) {
rc = ble_att_svr_read_local(tuples[i].handle, &tuples[i].value);
BLE_HS_LOG_DEBUG("ble_gatts_notify_multiple: peer_cl_sup_feat %d\n",
conn->bhc_gatt_svr.peer_cl_sup_feat[0]);

/* If peer does not support fall back to multiple single value Notifications */
if ((conn->bhc_gatt_svr.peer_cl_sup_feat[0] & 0x04) == 0) {
for (i = 0; i < chr_count; i++) {
rc = ble_gatts_notify(conn_handle, tuples[i].handle);
if (rc != 0) {
goto done;
}
}

return 0;
}

/* If peer does not support fall back to multiple single value
* Notifications */
if ((conn->bhc_gatt_svr.peer_cl_sup_feat[0] & 0x04) == 0) {
for (i = 0; i < chr_count; i++) {
rc = ble_att_clt_tx_notify(conn_handle, tuples[chr_count].handle,
tuples[chr_count].value);
txom = ble_hs_mbuf_att_pkt();
if (txom == NULL) {
return BLE_HS_ENOMEM;
}

for (i = 0; i < chr_count; i++) {
if (tuples[i].value == NULL) {
rc = ble_att_svr_read_local_with_perms(conn_handle, tuples[i].handle, &tuples[i].value);
if (rc != 0) {
goto done;
}
}
}

for (i = 0; i < chr_count; i++) {
if (txom->om_len + tuples[i].value->om_len > mtu && cur_chr_cnt < 2) {
rc = ble_att_clt_tx_notify(conn_handle, tuples[i].handle,
tuples[i].value);
Expand Down Expand Up @@ -4520,37 +4534,17 @@ ble_gatts_notify_multiple(uint16_t conn_handle,
#if !MYNEWT_VAL(BLE_GATT_NOTIFY_MULTIPLE)
return BLE_HS_ENOTSUP;
#endif
int rc, i;
struct ble_gatt_notif tuples[num_handles];
struct ble_hs_conn *conn;

BLE_HS_LOG_DEBUG("conn_handle %d\n", conn_handle);
conn = ble_hs_conn_find(conn_handle);
if (conn == NULL) {
return BLE_HS_ENOTCONN;
}

/** Skip sending to client that doesn't support this feature */
BLE_HS_LOG_DEBUG("ble_gatts_notify_multiple: peer_cl_sup_feat %d\n",
conn->bhc_gatt_svr.peer_cl_sup_feat[0]);
if ((conn->bhc_gatt_svr.peer_cl_sup_feat[0] & 0x04) == 0) {
for (i = 0; i < num_handles; i++) {
rc = ble_gatts_notify(conn_handle, chr_val_handles[i]);
if (rc != 0) {
return rc;
}
}
return 0;
}
int i;
struct ble_gatt_notif tuples[num_handles];

for (i = 0; i < num_handles; i++) {
tuples[i].handle = chr_val_handles[i];
tuples[i].value = NULL;
BLE_HS_LOG(DEBUG, "handle 0x%02x\n", tuples[i].handle);
}

rc = ble_gatts_notify_multiple_custom(conn_handle, num_handles, tuples);
return rc;
return ble_gatts_notify_multiple_custom(conn_handle, num_handles, tuples);
}

/**
Expand Down Expand Up @@ -4677,12 +4671,26 @@ ble_gatts_indicate_custom(uint16_t conn_handle, uint16_t chr_val_handle,
goto done;
}

rc = ble_att_svr_read_handle(BLE_HS_CONN_HANDLE_NONE, chr_val_handle,
0, txom, NULL);
rc = ble_att_svr_read_handle(conn_handle, chr_val_handle, 0, txom, NULL);
if (rc != 0) {
/* Fatal error; application disallowed attribute read. */
BLE_HS_DBG_ASSERT(0);
rc = BLE_HS_EAPP;
switch (rc) {
case BLE_HS_ATT_ERR(BLE_ATT_ERR_INSUFFICIENT_ENC):
rc = BLE_HS_EENCRYPT;
break;

case BLE_HS_ATT_ERR(BLE_ATT_ERR_INSUFFICIENT_AUTHEN):
rc = BLE_HS_EAUTHEN;
break;

case BLE_HS_ATT_ERR(BLE_ATT_ERR_INSUFFICIENT_AUTHOR):
rc = BLE_HS_EAUTHOR;
break;

default:
/* Fatal error; application disallowed attribute read. */
rc = BLE_HS_EAPP;
break;
}
goto done;
}
}
Expand Down
Loading