You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by cc...@apache.org on 2016/08/08 01:14:32 UTC
[3/4] incubator-mynewt-core git commit: BLE Host - validate write
upon rx of prep write
BLE Host - validate write upon rx of prep write
Before, upon receiving a prepare write request, the code was only
verifying that the specified attribute existed. There are a number of
checks that need to be performed in addition to that one:
1. Insufficient authorization.
2. Insufficient authentication.
3. Insufficient encryption key size.
4. Insufficient encryption (check if persisted bond exists for
unencrypted peer).
5. Invalid handle.
6. Write not permitted.
The host does not properly handle 3 and 4 yet (for any writes).
Now the host rejects a prepare write request if any of the above issues
are detected (except for 3 and 4).
Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/ee510c02
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/ee510c02
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/ee510c02
Branch: refs/heads/develop
Commit: ee510c02dff3daba9808317b33f7442375bc6f55
Parents: 36de005
Author: Christopher Collins <cc...@apache.org>
Authored: Sun Aug 7 17:48:44 2016 -0700
Committer: Christopher Collins <cc...@apache.org>
Committed: Sun Aug 7 18:11:00 2016 -0700
----------------------------------------------------------------------
net/nimble/host/src/ble_att_svr.c | 69 ++++++++++++++----------
net/nimble/host/src/test/ble_att_svr_test.c | 62 +++++++++++++++++----
2 files changed, 95 insertions(+), 36 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/ee510c02/net/nimble/host/src/ble_att_svr.c
----------------------------------------------------------------------
diff --git a/net/nimble/host/src/ble_att_svr.c b/net/nimble/host/src/ble_att_svr.c
index 1b420b5..f282093 100644
--- a/net/nimble/host/src/ble_att_svr.c
+++ b/net/nimble/host/src/ble_att_svr.c
@@ -237,9 +237,9 @@ ble_att_svr_get_sec_state(uint16_t conn_handle,
}
static int
-ble_att_svr_check_security(uint16_t conn_handle, int is_read,
- struct ble_att_svr_entry *entry,
- uint8_t *out_att_err)
+ble_att_svr_check_perms(uint16_t conn_handle, int is_read,
+ struct ble_att_svr_entry *entry,
+ uint8_t *out_att_err)
{
struct ble_gap_sec_state sec_state;
int author;
@@ -248,10 +248,20 @@ ble_att_svr_check_security(uint16_t conn_handle, int is_read,
int rc;
if (is_read) {
+ if (!(entry->ha_flags & BLE_ATT_F_READ)) {
+ *out_att_err = BLE_ATT_ERR_READ_NOT_PERMITTED;
+ return BLE_HS_ENOTSUP;
+ }
+
enc = entry->ha_flags & BLE_ATT_F_READ_ENC;
authen = entry->ha_flags & BLE_ATT_F_READ_AUTHEN;
author = entry->ha_flags & BLE_ATT_F_READ_AUTHOR;
} else {
+ if (!(entry->ha_flags & BLE_ATT_F_WRITE)) {
+ *out_att_err = BLE_ATT_ERR_WRITE_NOT_PERMITTED;
+ return BLE_HS_ENOTSUP;
+ }
+
enc = entry->ha_flags & BLE_ATT_F_WRITE_ENC;
authen = entry->ha_flags & BLE_ATT_F_WRITE_AUTHEN;
author = entry->ha_flags & BLE_ATT_F_WRITE_AUTHOR;
@@ -264,6 +274,8 @@ ble_att_svr_check_security(uint16_t conn_handle, int is_read,
rc = ble_att_svr_get_sec_state(conn_handle, &sec_state);
if (rc != 0) {
+ /* Peer no longer connected. */
+ *out_att_err = 0;
return rc;
}
@@ -300,13 +312,7 @@ ble_att_svr_read(uint16_t conn_handle,
att_err = 0; /* Silence gcc warning. */
if (conn_handle != BLE_HS_CONN_HANDLE_NONE) {
- if (!(entry->ha_flags & BLE_ATT_F_READ)) {
- att_err = BLE_ATT_ERR_READ_NOT_PERMITTED;
- rc = BLE_HS_ENOTSUP;
- goto err;
- }
-
- rc = ble_att_svr_check_security(conn_handle, 1, entry, &att_err);
+ rc = ble_att_svr_check_perms(conn_handle, 1, entry, &att_err);
if (rc != 0) {
goto err;
}
@@ -448,13 +454,7 @@ ble_att_svr_write(uint16_t conn_handle, struct ble_att_svr_entry *entry,
BLE_HS_DBG_ASSERT(!ble_hs_locked_by_cur_task());
if (conn_handle != BLE_HS_CONN_HANDLE_NONE) {
- if (!(entry->ha_flags & BLE_ATT_F_WRITE)) {
- att_err = BLE_ATT_ERR_WRITE_NOT_PERMITTED;
- rc = BLE_HS_ENOTSUP;
- goto done;
- }
-
- rc = ble_att_svr_check_security(conn_handle, 0, entry, &att_err);
+ rc = ble_att_svr_check_perms(conn_handle, 0, entry, &att_err);
if (rc != 0) {
goto done;
}
@@ -2341,19 +2341,17 @@ ble_att_svr_prep_write(uint16_t conn_handle,
while (!SLIST_EMPTY(prep_list)) {
ble_att_svr_prep_extract(prep_list, &attr_handle, &om);
+ /* Attribute existence was verified during prepare-write request
+ * processing.
+ */
attr = ble_att_svr_find_by_handle(attr_handle);
- *err_handle = attr_handle;
- if (attr == NULL) {
- rc = BLE_ATT_ERR_INVALID_HANDLE;
- } else {
- rc = ble_att_svr_write(conn_handle, attr, 0, &om, &att_err);
- if (rc != 0) {
- rc = att_err;
- }
- }
+ BLE_HS_DBG_ASSERT(attr != NULL);
+
+ rc = ble_att_svr_write(conn_handle, attr, 0, &om, &att_err);
os_mbuf_free_chain(om);
if (rc != 0) {
- return rc;
+ *err_handle = attr_handle;
+ return att_err;
}
}
@@ -2400,12 +2398,29 @@ ble_att_svr_rx_prep_write(uint16_t conn_handle, struct os_mbuf **rxom)
os_mbuf_adj(*rxom, BLE_ATT_PREP_WRITE_CMD_BASE_SZ);
attr_entry = ble_att_svr_find_by_handle(req.bapc_handle);
+
+ /* A prepare write request gets rejected for the following reasons:
+ * 1. Insufficient authorization.
+ * 2. Insufficient authentication.
+ * 3. Insufficient encryption key size (XXX: Not checked).
+ * 4. Insufficient encryption.
+ * 5. Invalid handle.
+ * 6. Write not permitted.
+ */
+
+ /* <5> */
if (attr_entry == NULL) {
rc = BLE_HS_ENOENT;
att_err = BLE_ATT_ERR_INVALID_HANDLE;
goto done;
}
+ /* <1>, <2>, <4>, <6> */
+ rc = ble_att_svr_check_perms(conn_handle, 0, attr_entry, &att_err);
+ if (rc != 0) {
+ goto done;
+ }
+
prep_entry = ble_att_svr_prep_alloc();
if (prep_entry == NULL) {
att_err = BLE_ATT_ERR_PREPARE_QUEUE_FULL;
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/ee510c02/net/nimble/host/src/test/ble_att_svr_test.c
----------------------------------------------------------------------
diff --git a/net/nimble/host/src/test/ble_att_svr_test.c b/net/nimble/host/src/test/ble_att_svr_test.c
index 09e7bf9..86565e3 100644
--- a/net/nimble/host/src/test/ble_att_svr_test.c
+++ b/net/nimble/host/src/test/ble_att_svr_test.c
@@ -319,6 +319,15 @@ ble_att_svr_test_misc_attr_fn_w_2(uint16_t conn_handle, uint16_t attr_handle,
}
}
+static int
+ble_att_svr_test_misc_attr_fn_w_fail(uint16_t conn_handle,
+ uint16_t attr_handle,
+ uint8_t op, uint16_t offset,
+ struct os_mbuf **om, void *arg)
+{
+ return BLE_ATT_ERR_INVALID_ATTR_VALUE_LEN;
+}
+
static void
ble_att_svr_test_misc_verify_w_1(void *data, int data_len)
{
@@ -2071,6 +2080,7 @@ TEST_CASE(ble_att_svr_test_read_group_type)
TEST_CASE(ble_att_svr_test_prep_write)
{
+ struct ble_hs_conn *conn;
uint16_t conn_handle;
int i;
@@ -2089,9 +2099,22 @@ TEST_CASE(ble_att_svr_test_prep_write)
ble_att_svr_test_misc_register_uuid16(0x8989, HA_FLAG_PERM_RW, 2,
ble_att_svr_test_misc_attr_fn_w_2);
- /* Register a third attribute that is not writable. */
+ /* 3: not writable. */
ble_att_svr_test_misc_register_uuid16(0xabab, BLE_ATT_F_READ, 3,
ble_att_svr_test_misc_attr_fn_r_1);
+ /* 4: Encryption required. */
+ ble_att_svr_test_misc_register_uuid16(
+ 0xabac, BLE_ATT_F_WRITE | BLE_ATT_F_WRITE_ENC, 4,
+ ble_att_svr_test_misc_attr_fn_w_1);
+
+ /* 5: Encryption+authentication required. */
+ ble_att_svr_test_misc_register_uuid16(
+ 0xabad, BLE_ATT_F_WRITE | BLE_ATT_F_WRITE_ENC | BLE_ATT_F_WRITE_AUTHEN,
+ 5, ble_att_svr_test_misc_attr_fn_w_1);
+
+ /* 6: Write callback always fails. */
+ ble_att_svr_test_misc_register_uuid16(
+ 0xabae, BLE_ATT_F_WRITE, 6, ble_att_svr_test_misc_attr_fn_w_fail);
/*** Empty write succeeds. */
ble_att_svr_test_misc_exec_write(conn_handle, BLE_ATT_EXEC_WRITE_F_CONFIRM,
@@ -2104,6 +2127,28 @@ TEST_CASE(ble_att_svr_test_prep_write)
ble_att_svr_test_misc_prep_write(conn_handle, 53525, 0, data, 10,
BLE_ATT_ERR_INVALID_HANDLE);
+ /*** Failure due to write-not-permitted. */
+ ble_att_svr_test_misc_prep_write(conn_handle, 3, 0, data, 35,
+ BLE_ATT_ERR_WRITE_NOT_PERMITTED);
+
+ /*** Failure due to insufficient authentication (encryption required). */
+ ble_att_svr_test_misc_prep_write(conn_handle, 4, 0, data, 1,
+ BLE_ATT_ERR_INSUFFICIENT_AUTHEN);
+
+ /*** Encrypt connection; ensure previous prep write now succeeds. */
+ ble_hs_lock();
+ conn = ble_hs_conn_find(2);
+ TEST_ASSERT_FATAL(conn != NULL);
+ conn->bhc_sec_state.encrypted = 1;
+ ble_hs_unlock();
+
+ ble_att_svr_test_misc_prep_write(conn_handle, 4, 0, data, 1, 0);
+ ble_att_svr_test_misc_exec_write(conn_handle, 0, 0, 0);
+
+ /*** Failure due to insufficient authentication (not authenticated). */
+ ble_att_svr_test_misc_prep_write(conn_handle, 5, 0, data, 35,
+ BLE_ATT_ERR_INSUFFICIENT_AUTHEN);
+
/*** Failure for write starting at nonzero offset. */
ble_att_svr_test_misc_prep_write(conn_handle, 1, 1, data, 10, 0);
ble_att_svr_test_misc_exec_write(conn_handle, BLE_ATT_EXEC_WRITE_F_CONFIRM,
@@ -2136,14 +2181,6 @@ TEST_CASE(ble_att_svr_test_prep_write)
BLE_ATT_ERR_INVALID_ATTR_VALUE_LEN, 1);
ble_att_svr_test_misc_verify_w_1(NULL, 0);
- /*** Failure due to attribute callback. */
- ble_att_svr_test_misc_prep_write(conn_handle, 3, 0, data, 35, 0);
- ble_att_svr_test_misc_prep_write(conn_handle, 3, 35, data + 35, 43, 0);
- ble_att_svr_test_misc_prep_write(conn_handle, 3, 78, data + 78, 1, 0);
- ble_att_svr_test_misc_exec_write(conn_handle, BLE_ATT_EXEC_WRITE_F_CONFIRM,
- BLE_ATT_ERR_WRITE_NOT_PERMITTED, 0);
- ble_att_svr_test_misc_verify_w_1(NULL, 0);
-
/*** Successful two part write. */
ble_att_svr_test_misc_prep_write(conn_handle, 1, 0, data, 20, 0);
ble_att_svr_test_misc_prep_write(conn_handle, 1, 20, data + 20, 20, 0);
@@ -2188,6 +2225,13 @@ TEST_CASE(ble_att_svr_test_prep_write)
0, 0);
ble_att_svr_test_misc_verify_w_1(data, 12);
ble_att_svr_test_misc_verify_w_2(data, 61);
+
+ /*** Fail due to attribute callback error. */
+ ble_att_svr_test_misc_prep_write(conn_handle, 6, 0, data, 35, 0);
+ ble_att_svr_test_misc_prep_write(conn_handle, 6, 35, data + 35, 43, 0);
+ ble_att_svr_test_misc_prep_write(conn_handle, 6, 78, data + 78, 1, 0);
+ ble_att_svr_test_misc_exec_write(conn_handle, BLE_ATT_EXEC_WRITE_F_CONFIRM,
+ BLE_ATT_ERR_INVALID_ATTR_VALUE_LEN, 6);
}
TEST_CASE(ble_att_svr_test_notify)