From bbe538fde2c794eb1295eaa8dcd44231150c353b Mon Sep 17 00:00:00 2001 From: Luiz Augusto von Dentz Date: Fri, 28 Oct 2016 18:44:18 +0300 Subject: [PATCH 01/12] Bluetooth: ATT: Fix not responding when there is a request in parallel If there is a request ongoing it may block responses to be generated since they were using the same buffer pool, so this introduces a dedicated pool for responses making the code able to act as both server and client at same time. Change-Id: I5fe3e19f9c5c0c2e0dfadedf77b7684f0960572c Signed-off-by: Luiz Augusto von Dentz --- net/bluetooth/att.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/net/bluetooth/att.c b/net/bluetooth/att.c index 9941fba25a0..461df5a4767 100644 --- a/net/bluetooth/att.c +++ b/net/bluetooth/att.c @@ -97,6 +97,15 @@ static NET_BUF_POOL(req_pool, BT_L2CAP_BUF_SIZE(CONFIG_BLUETOOTH_ATT_MTU), &req_data, NULL, BT_BUF_USER_DATA_MIN); +/* + * Pool for outgoing ATT responses packets. + */ +static struct nano_fifo rsp_data; +static NET_BUF_POOL(rsp_pool, + CONFIG_BLUETOOTH_ATT_REQ_COUNT * CONFIG_BLUETOOTH_MAX_CONN, + BT_L2CAP_BUF_SIZE(CONFIG_BLUETOOTH_ATT_MTU), + &rsp_data, NULL, BT_BUF_USER_DATA_MIN); + /* * Pool for ATT indications packets. This is required since indication can be * sent in parallel to requests. @@ -1803,6 +1812,23 @@ struct net_buf *bt_att_create_pdu(struct bt_conn *conn, uint8_t op, size_t len) */ buf = bt_l2cap_create_pdu(&ind_data, 0); break; + case BT_ATT_OP_ERROR_RSP: + case BT_ATT_OP_MTU_RSP: + case BT_ATT_OP_FIND_INFO_RSP: + case BT_ATT_OP_FIND_TYPE_RSP: + case BT_ATT_OP_READ_TYPE_RSP: + case BT_ATT_OP_READ_RSP: + case BT_ATT_OP_READ_BLOB_RSP: + case BT_ATT_OP_READ_MULT_RSP: + case BT_ATT_OP_READ_GROUP_RSP: + case BT_ATT_OP_WRITE_RSP: + case BT_ATT_OP_PREPARE_WRITE_RSP: + case BT_ATT_OP_EXEC_WRITE_RSP: + /* Use a different buffer pool for responses since they can be + * sent in parallel. + */ + buf = bt_l2cap_create_pdu(&rsp_data, 0); + break; default: buf = bt_l2cap_create_pdu(&req_data, 0); } @@ -1984,6 +2010,7 @@ void bt_att_init(void) }; net_buf_pool_init(ind_pool); + net_buf_pool_init(rsp_pool); net_buf_pool_init(req_pool); net_buf_pool_init(clone_pool); #if CONFIG_BLUETOOTH_ATT_PREPARE_COUNT > 0 From a3a74de6ba8331b17841285bf47a26469e05cf7a Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Sat, 29 Oct 2016 16:07:39 +0300 Subject: [PATCH 02/12] Bluetooth: samples/hci-uart: Remove unneeded hci_driver.h include There's no need for this app to use the HCI driver API directly. Change-Id: Ie5acfbfb09fc3f86c3f29f22fbf7da7db1ff2127 Signed-off-by: Johan Hedberg --- samples/bluetooth/hci-uart/src/main.c | 1 - 1 file changed, 1 deletion(-) diff --git a/samples/bluetooth/hci-uart/src/main.c b/samples/bluetooth/hci-uart/src/main.c index f2960d6c9cd..6050c551400 100644 --- a/samples/bluetooth/hci-uart/src/main.c +++ b/samples/bluetooth/hci-uart/src/main.c @@ -33,7 +33,6 @@ #include #include #include -#include #include #include From 079030718f12fd1c1c6009fd46910d90e7ce8ac3 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Sat, 29 Oct 2016 14:20:03 +0300 Subject: [PATCH 03/12] Bluetooth: Move hci_driver.h to include/drivers/bluetooth This header file is not intended for applications, rather than for drivers, so move it to the appropriate place. Change-Id: I7ff8158418e7e839b538c6cccbb32f3af43c12c6 Signed-off-by: Johan Hedberg --- include/{ => drivers}/bluetooth/hci_driver.h | 0 tests/bluetooth/test_bluetooth/src/bluetooth.c | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename include/{ => drivers}/bluetooth/hci_driver.h (100%) diff --git a/include/bluetooth/hci_driver.h b/include/drivers/bluetooth/hci_driver.h similarity index 100% rename from include/bluetooth/hci_driver.h rename to include/drivers/bluetooth/hci_driver.h diff --git a/tests/bluetooth/test_bluetooth/src/bluetooth.c b/tests/bluetooth/test_bluetooth/src/bluetooth.c index f214a2a153f..1199131affd 100644 --- a/tests/bluetooth/test_bluetooth/src/bluetooth.c +++ b/tests/bluetooth/test_bluetooth/src/bluetooth.c @@ -22,7 +22,7 @@ #include #include -#include +#include #define EXPECTED_ERROR -ENOSYS From 20952b75e3b00c42c5f9fb7bbaf22fabf28bebc7 Mon Sep 17 00:00:00 2001 From: Carles Cufi Date: Mon, 31 Oct 2016 12:59:12 +0100 Subject: [PATCH 04/12] Bluetooth: uart: Fix UART TX stall condition In 3 of our UART driver implementations the ISR is exclusively used and enabled for the RX path. The existing logic was susceptible to a stall situation where a polled out transmission would be interrupted by a reception and then the ISR code would loop forever due to the TX event being signalled (although the interrupt itself was disabled) causing the ISR to keep looping for an RX interrupt. Change-Id: Ic379e58b1c974aca3cee37d2d81f12c3726fb160 Signed-off-by: Carles Cufi --- drivers/bluetooth/hci/h4.c | 3 ++- drivers/bluetooth/hci/h5.c | 3 ++- samples/bluetooth/hci-uart/src/main.c | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/bluetooth/hci/h4.c b/drivers/bluetooth/hci/h4.c index 0ecf6082088..fe30a6a370d 100644 --- a/drivers/bluetooth/hci/h4.c +++ b/drivers/bluetooth/hci/h4.c @@ -144,7 +144,8 @@ static void bt_uart_isr(struct device *unused) } else { BT_DBG("spurious interrupt"); } - continue; + /* Only the UART RX path is interrupt-enabled */ + break; } /* Beginning of a new packet */ diff --git a/drivers/bluetooth/hci/h5.c b/drivers/bluetooth/hci/h5.c index c7052509251..00f3bbe3426 100644 --- a/drivers/bluetooth/hci/h5.c +++ b/drivers/bluetooth/hci/h5.c @@ -438,7 +438,8 @@ static void bt_uart_isr(struct device *unused) } else { BT_DBG("spurious interrupt"); } - continue; + /* Only the UART RX path is interrupt-enabled */ + break; } ret = uart_fifo_read(h5_dev, &byte, sizeof(byte)); diff --git a/samples/bluetooth/hci-uart/src/main.c b/samples/bluetooth/hci-uart/src/main.c index 6050c551400..ded9e68b364 100644 --- a/samples/bluetooth/hci-uart/src/main.c +++ b/samples/bluetooth/hci-uart/src/main.c @@ -173,7 +173,8 @@ static void bt_uart_isr(struct device *unused) } else { SYS_LOG_DBG("spurious interrupt"); } - continue; + /* Only the UART RX path is interrupt-enabled */ + break; } /* Beginning of a new packet */ From 07c8be1791de4f42c6f071318dbcb26b0c4c7f55 Mon Sep 17 00:00:00 2001 From: Luiz Augusto von Dentz Date: Mon, 31 Oct 2016 12:53:43 +0200 Subject: [PATCH 05/12] Bluetooth: ATT: Rework buffer pools to minimize extra data This uses net_buf_simple_{save/restore} so the same buffer can be reused if the buffer needs to be resent, also since the responses don't need to be saved a pool with 1 element is enough while it keeps the code safe from deadlocking when both request and responses use the same pool. Change-Id: Ibaa8e7ef39f4b466d5cd4d55874bd609f0a1d67c Signed-off-by: Luiz Augusto von Dentz --- include/bluetooth/att.h | 11 ++++--- net/bluetooth/Kconfig | 6 ++-- net/bluetooth/att.c | 69 ++++++++++------------------------------- 3 files changed, 26 insertions(+), 60 deletions(-) diff --git a/include/bluetooth/att.h b/include/bluetooth/att.h index 712c40b92fd..53633b7340d 100644 --- a/include/bluetooth/att.h +++ b/include/bluetooth/att.h @@ -57,12 +57,13 @@ typedef void (*bt_att_destroy_t)(void *user_data); /* ATT request context */ struct bt_att_req { - sys_snode_t node; - bt_att_func_t func; - bt_att_destroy_t destroy; - struct net_buf *buf; + sys_snode_t node; + bt_att_func_t func; + bt_att_destroy_t destroy; + struct net_buf_simple_state state; + struct net_buf *buf; #if defined(CONFIG_BLUETOOTH_SMP) - bool retrying; + bool retrying; #endif /* CONFIG_BLUETOOTH_SMP */ }; diff --git a/net/bluetooth/Kconfig b/net/bluetooth/Kconfig index 35b2e5e0742..32b57bdb9cd 100644 --- a/net/bluetooth/Kconfig +++ b/net/bluetooth/Kconfig @@ -190,11 +190,11 @@ config BLUETOOTH_ATT_PREPARE_COUNT config BLUETOOTH_ATT_REQ_COUNT int "Number of ATT request buffers" - default 1 + default BLUETOOTH_MAX_CONN range 1 64 help - Number of outgoing buffers available for ATT requests per connection, - this controls how many requests can be queued without blocking. + Number of outgoing buffers available for ATT requests, this controls + how many requests can be queued without blocking. config BLUETOOTH_SMP bool "Security Manager Protocol support" diff --git a/net/bluetooth/att.c b/net/bluetooth/att.c index 461df5a4767..7797292844a 100644 --- a/net/bluetooth/att.c +++ b/net/bluetooth/att.c @@ -92,40 +92,21 @@ static struct bt_att bt_req_pool[CONFIG_BLUETOOTH_MAX_CONN]; * Pool for outgoing ATT requests packets. */ static struct nano_fifo req_data; -static NET_BUF_POOL(req_pool, - CONFIG_BLUETOOTH_ATT_REQ_COUNT * CONFIG_BLUETOOTH_MAX_CONN, +static NET_BUF_POOL(req_pool, CONFIG_BLUETOOTH_ATT_REQ_COUNT, BT_L2CAP_BUF_SIZE(CONFIG_BLUETOOTH_ATT_MTU), &req_data, NULL, BT_BUF_USER_DATA_MIN); /* - * Pool for outgoing ATT responses packets. + * Pool for ougoing ATT responses packets. This is necessary in order not to + * block the RX fiber since otherwise req_pool would have be used but buffers + * may only be freed after a response is received which would never happen if + * the RX fiber is waiting a buffer causing a deadlock. */ static struct nano_fifo rsp_data; -static NET_BUF_POOL(rsp_pool, - CONFIG_BLUETOOTH_ATT_REQ_COUNT * CONFIG_BLUETOOTH_MAX_CONN, +static NET_BUF_POOL(rsp_pool, 1, BT_L2CAP_BUF_SIZE(CONFIG_BLUETOOTH_ATT_MTU), &rsp_data, NULL, BT_BUF_USER_DATA_MIN); -/* - * Pool for ATT indications packets. This is required since indication can be - * sent in parallel to requests. - */ -static struct nano_fifo ind_data; -static NET_BUF_POOL(ind_pool, - CONFIG_BLUETOOTH_ATT_REQ_COUNT * CONFIG_BLUETOOTH_MAX_CONN, - BT_L2CAP_BUF_SIZE(CONFIG_BLUETOOTH_ATT_MTU), - &ind_data, NULL, BT_BUF_USER_DATA_MIN); - -/* - * Pool for outstanding ATT request, this is required for resending in case - * there is a recoverable error since the original buffer is changed while - * sending. - */ -static struct nano_fifo clone_data; -static NET_BUF_POOL(clone_pool, 1 * CONFIG_BLUETOOTH_MAX_CONN, - BT_L2CAP_BUF_SIZE(CONFIG_BLUETOOTH_ATT_MTU), - &clone_data, NULL, BT_BUF_USER_DATA_MIN); - static void att_req_destroy(struct bt_att_req *req) { if (req->buf) { @@ -208,32 +189,21 @@ static uint8_t att_mtu_req(struct bt_att *att, struct net_buf *buf) return 0; } -static struct net_buf *att_req_clone(struct net_buf *buf) -{ - struct net_buf *clone; - - clone = net_buf_get(&clone_data, net_buf_headroom(buf)); - if (!clone) { - return NULL; - } - - memcpy(net_buf_add(clone, buf->len), buf->data, buf->len); - - return clone; -} - static int att_send_req(struct bt_att *att, struct bt_att_req *req) { BT_DBG("req %p", req); att->req = req; + /* Save request state so it can be resent */ + net_buf_simple_save(&req->buf->b, &req->state); + /* Start timeout work */ nano_delayed_work_submit(&att->timeout_work, ATT_TIMEOUT); - /* Send a clone to keep the original buffer intact */ + /* Keep a reference for resending in case of an error */ bt_l2cap_send(att->chan.chan.conn, BT_L2CAP_CID_ATT, - att_req_clone(req->buf)); + net_buf_ref(req->buf)); return 0; } @@ -1539,6 +1509,9 @@ static uint8_t att_error_rsp(struct bt_att *att, struct net_buf *buf) goto done; } + /* Restore state to be resent */ + net_buf_simple_restore(&att->req->buf->b, &att->req->state); + hdr = (void *)att->req->buf->data; err = rsp->request == hdr->code ? rsp->error : BT_ATT_ERR_UNLIKELY; @@ -1805,13 +1778,7 @@ struct net_buf *bt_att_create_pdu(struct bt_conn *conn, uint8_t op, size_t len) } switch (op) { - case BT_ATT_OP_INDICATE: case BT_ATT_OP_CONFIRM: - /* Use a different buffer pool for indication/confirmations - * since they can be sent in parallel. - */ - buf = bt_l2cap_create_pdu(&ind_data, 0); - break; case BT_ATT_OP_ERROR_RSP: case BT_ATT_OP_MTU_RSP: case BT_ATT_OP_FIND_INFO_RSP: @@ -1824,8 +1791,8 @@ struct net_buf *bt_att_create_pdu(struct bt_conn *conn, uint8_t op, size_t len) case BT_ATT_OP_WRITE_RSP: case BT_ATT_OP_PREPARE_WRITE_RSP: case BT_ATT_OP_EXEC_WRITE_RSP: - /* Use a different buffer pool for responses since they can be - * sent in parallel. + /* Use a different buffer pool for responses as this is + * usually sent from RX fiber it shall never block. */ buf = bt_l2cap_create_pdu(&rsp_data, 0); break; @@ -2009,10 +1976,8 @@ void bt_att_init(void) .accept = bt_att_accept, }; - net_buf_pool_init(ind_pool); - net_buf_pool_init(rsp_pool); net_buf_pool_init(req_pool); - net_buf_pool_init(clone_pool); + net_buf_pool_init(rsp_pool); #if CONFIG_BLUETOOTH_ATT_PREPARE_COUNT > 0 net_buf_pool_init(prep_pool); #endif From 237455f77fb6e0d7a62d7488affa735469b34913 Mon Sep 17 00:00:00 2001 From: Luiz Augusto von Dentz Date: Mon, 31 Oct 2016 13:20:47 +0200 Subject: [PATCH 06/12] Bluetooth: GATT: Fix calling read handler twice If length is already 0 there is no point in calling the handler again as that already mark the end of the operation. Change-Id: I212a6d8bdd9aa3d9886099bb7ed3f9d3831bcf1a Signed-off-by: Luiz Augusto von Dentz --- net/bluetooth/gatt.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/net/bluetooth/gatt.c b/net/bluetooth/gatt.c index bd58a09064f..85226cc5426 100644 --- a/net/bluetooth/gatt.c +++ b/net/bluetooth/gatt.c @@ -1355,6 +1355,11 @@ static void att_read_rsp(struct bt_conn *conn, uint8_t err, const void *pdu, return; } + /* Stop if no data left */ + if (!length) { + return; + } + /* * Core Spec 4.2, Vol. 3, Part G, 4.8.1 * If the Characteristic Value is greater than (ATT_MTU - 1) octets @@ -1410,6 +1415,10 @@ static void att_read_multiple_rsp(struct bt_conn *conn, uint8_t err, params->func(conn, 0, params, pdu, length); + if (!length) { + return; + } + /* mark read as complete since read multiple is single response */ params->func(conn, 0, params, NULL, 0); } From 9dc033cb84762ab0d161150f9d91f32774c060bb Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Tue, 1 Nov 2016 11:05:35 +0200 Subject: [PATCH 07/12] Bluetooth: Add handling for security level 0 So far bt_security_t has completely missed out on security level 0, i.e. its actual values have been one lower than in the core specification. To properly introduce for the new level (which is only applicable for BR/EDR) add proper tracking for each channel and server, and make the channels inherit their required level from the respective server. Change-Id: I9a2384d883017125c2c117880aa6e0ade30520e4 Signed-off-by: Johan Hedberg --- include/bluetooth/conn.h | 2 ++ include/bluetooth/l2cap.h | 3 +++ net/bluetooth/l2cap.c | 15 +++++++++++++++ net/bluetooth/l2cap_br.c | 24 ++++++++++++++++++++---- 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/include/bluetooth/conn.h b/include/bluetooth/conn.h index c1055e2a50f..64939582a14 100644 --- a/include/bluetooth/conn.h +++ b/include/bluetooth/conn.h @@ -241,6 +241,8 @@ struct bt_conn *bt_conn_create_slave_le(const bt_addr_le_t *peer, /** Security level. */ typedef enum __packed { + /** Only for BR/EDR special cases, like SDP */ + BT_SECURITY_ZERO, /** No encryption and no authentication. */ BT_SECURITY_LOW, /** Encryption and no authentication (no MITM). */ diff --git a/include/bluetooth/l2cap.h b/include/bluetooth/l2cap.h index 2979a1a2624..d508484cc33 100644 --- a/include/bluetooth/l2cap.h +++ b/include/bluetooth/l2cap.h @@ -206,6 +206,9 @@ struct bt_l2cap_server { /** Server PSM */ uint16_t psm; + /** Required minimim security level */ + bt_security_t sec_level; + /** Server accept callback * * This callback is called whenever a new incoming connection requires diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c index 4a203def480..15cad55f494 100644 --- a/net/bluetooth/l2cap.c +++ b/net/bluetooth/l2cap.c @@ -474,6 +474,13 @@ int bt_l2cap_server_register(struct bt_l2cap_server *server) return -EINVAL; } + if (server->sec_level > BT_SECURITY_FIPS) { + return -EINVAL; + } else if (server->sec_level < BT_SECURITY_LOW) { + /* Level 0 is only applicable for BR/EDR */ + server->sec_level = BT_SECURITY_LOW; + } + /* Check if given PSM is already in use */ if (l2cap_server_lookup_psm(server->psm)) { BT_DBG("PSM already registered"); @@ -624,6 +631,8 @@ static void le_conn_req(struct bt_l2cap *l2cap, uint8_t ident, goto rsp; } + chan->required_sec_level = server->sec_level; + if (l2cap_chan_add(conn, chan, l2cap_chan_destroy)) { struct bt_l2cap_le_chan *ch = BT_L2CAP_LE_CHAN(chan); @@ -1348,6 +1357,12 @@ int bt_l2cap_chan_connect(struct bt_conn *conn, struct bt_l2cap_chan *chan, } #endif /* CONFIG_BLUETOOTH_BREDR */ + if (chan->required_sec_level > BT_SECURITY_FIPS) { + return -EINVAL; + } else if (chan->required_sec_level == BT_SECURITY_ZERO) { + chan->required_sec_level = BT_SECURITY_LOW; + } + return l2cap_le_connect(conn, BT_L2CAP_LE_CHAN(chan), psm); } diff --git a/net/bluetooth/l2cap_br.c b/net/bluetooth/l2cap_br.c index bd86d30b894..32b9fdf37a5 100644 --- a/net/bluetooth/l2cap_br.c +++ b/net/bluetooth/l2cap_br.c @@ -708,9 +708,9 @@ l2cap_br_conn_security(struct bt_l2cap_chan *chan, const uint16_t psm) int check; /* For SDP PSM there's no need to change existing security on link */ - if (psm == L2CAP_BR_PSM_SDP) { + if (chan->required_sec_level == BT_SECURITY_ZERO) { return L2CAP_CONN_SECURITY_PASSED; - }; + } /* * No link key needed for legacy devices (pre 2.1) and when low security @@ -839,8 +839,8 @@ static void l2cap_br_conn_req(struct bt_l2cap_br *l2cap, uint8_t ident, * Report security violation for non SDP channel without encryption when * remote supports SSP. */ - if (psm != L2CAP_BR_PSM_SDP && BT_FEAT_HOST_SSP(conn->br.features) && - !conn->encrypt) { + if (server->sec_level != BT_SECURITY_ZERO && + BT_FEAT_HOST_SSP(conn->br.features) && !conn->encrypt) { result = BT_L2CAP_BR_ERR_SEC_BLOCK; goto done; } @@ -865,6 +865,8 @@ static void l2cap_br_conn_req(struct bt_l2cap_br *l2cap, uint8_t ident, goto done; } + chan->required_sec_level = server->sec_level; + l2cap_br_chan_add(conn, chan, l2cap_br_chan_destroy); BR_CHAN(chan)->tx.cid = scid; dcid = BR_CHAN(chan)->rx.cid; @@ -973,6 +975,13 @@ int bt_l2cap_br_server_register(struct bt_l2cap_server *server) return -EINVAL; } + if (server->sec_level > BT_SECURITY_FIPS) { + return -EINVAL; + } else if (server->sec_level == BT_SECURITY_ZERO && + server->psm != L2CAP_BR_PSM_SDP) { + server->sec_level = BT_SECURITY_LOW; + } + /* Check if given PSM is already in use */ if (l2cap_br_server_lookup_psm(server->psm)) { BT_DBG("PSM already registered"); @@ -1361,6 +1370,13 @@ int bt_l2cap_br_chan_connect(struct bt_conn *conn, struct bt_l2cap_chan *chan, return -EINVAL; } + if (chan->required_sec_level > BT_SECURITY_FIPS) { + return -EINVAL; + } else if (chan->required_sec_level == BT_SECURITY_ZERO && + psm != L2CAP_BR_PSM_SDP) { + chan->required_sec_level = BT_SECURITY_LOW; + } + switch (chan->state) { case BT_L2CAP_CONNECTED: /* Already connected */ From 1b804f9d74370fd4a12434d99bdecd9e59ec25ef Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Tue, 1 Nov 2016 12:40:36 +0200 Subject: [PATCH 08/12] Bluetooth: samples/hci-uart: Fix up configuration files Right now the hci-uart samples doesn't really work anywhere else than nRF5x, so make that the default configuration. Also rename the old prj.conf to generic.conf to give a generic starting point for creating configurations for other targets. Change-Id: I1c940339b457d0e77c8ddad4b2474fc7e5b12a51 Signed-off-by: Johan Hedberg --- samples/bluetooth/hci-uart/Makefile | 2 +- samples/bluetooth/hci-uart/{prj.conf => generic.conf} | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) rename samples/bluetooth/hci-uart/{prj.conf => generic.conf} (62%) diff --git a/samples/bluetooth/hci-uart/Makefile b/samples/bluetooth/hci-uart/Makefile index 6a59a114fb0..f00c8a32706 100644 --- a/samples/bluetooth/hci-uart/Makefile +++ b/samples/bluetooth/hci-uart/Makefile @@ -1,5 +1,5 @@ KERNEL_TYPE = unified -CONF_FILE ?= prj.conf +CONF_FILE ?= nrf5.conf BOARD ?= nrf52_pca10040 include $(ZEPHYR_BASE)/Makefile.inc diff --git a/samples/bluetooth/hci-uart/prj.conf b/samples/bluetooth/hci-uart/generic.conf similarity index 62% rename from samples/bluetooth/hci-uart/prj.conf rename to samples/bluetooth/hci-uart/generic.conf index 7981c2c536f..b66a93e3805 100644 --- a/samples/bluetooth/hci-uart/prj.conf +++ b/samples/bluetooth/hci-uart/generic.conf @@ -1,11 +1,7 @@ CONFIG_CONSOLE=n CONFIG_STDOUT_CONSOLE=n CONFIG_UART_CONSOLE=n -CONFIG_GPIO=y CONFIG_SERIAL=y CONFIG_UART_INTERRUPT_DRIVEN=y CONFIG_BLUETOOTH=y -CONFIG_BLUETOOTH_LE=y CONFIG_BLUETOOTH_STACK_HCI_RAW=y -CONFIG_BLUETOOTH_DEBUG_LOG=n -CONFIG_BLUETOOTH_DEBUG_MONITOR=n From 142b75fddb51b9444f0232987100f435e62d340a Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Tue, 1 Nov 2016 12:42:45 +0200 Subject: [PATCH 09/12] Bluetooth: samples/hci-uart: Fix minor style issues Fix minor style issues with excessive whitespace for alignment and lack of 'static' for the tx_fiber_stack. Change-Id: I0c761e94e6a6789a3772b08b0774cd1a5e73c2ef Signed-off-by: Johan Hedberg --- samples/bluetooth/hci-uart/src/main.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/samples/bluetooth/hci-uart/src/main.c b/samples/bluetooth/hci-uart/src/main.c index ded9e68b364..b103f059809 100644 --- a/samples/bluetooth/hci-uart/src/main.c +++ b/samples/bluetooth/hci-uart/src/main.c @@ -38,8 +38,8 @@ static struct device *hci_uart_dev; -#define STACK_SIZE 1024 -uint8_t tx_fiber_stack[STACK_SIZE]; +#define STACK_SIZE 1024 +static uint8_t tx_fiber_stack[STACK_SIZE]; /* HCI command buffers */ #define CMD_BUF_SIZE (CONFIG_BLUETOOTH_HCI_SEND_RESERVE + \ @@ -63,10 +63,10 @@ static NET_BUF_POOL(acl_tx_pool, CONFIG_BLUETOOTH_CONTROLLER_TX_BUFFERS, static struct nano_fifo tx_queue; -#define H4_CMD 0x01 -#define H4_ACL 0x02 -#define H4_SCO 0x03 -#define H4_EVT 0x04 +#define H4_CMD 0x01 +#define H4_ACL 0x02 +#define H4_SCO 0x03 +#define H4_EVT 0x04 /* Length of a discard/flush buffer. * This is sized to align with a BLE HCI packet: @@ -75,7 +75,7 @@ static struct nano_fifo tx_queue; * variable, smaller ones will force the caller to call into discard more * often. */ -#define H4_DISCARD_LEN 33 +#define H4_DISCARD_LEN 33 static int h4_read(struct device *uart, uint8_t *buf, size_t len, size_t min) From 3eb04280ede6a7f4c078a3bd67d246f125b052b0 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Tue, 1 Nov 2016 15:16:19 +0200 Subject: [PATCH 10/12] Bluetooth: Rename BT_SECURITY_ZERO to BT_SECURITY_NONE Even though this is level 0, NONE as a symbolic description is more in line with the other level names. Change-Id: Ic9d7c38b928d8744d47e759fae804bbbc08c89bf Signed-off-by: Johan Hedberg --- include/bluetooth/conn.h | 2 +- net/bluetooth/l2cap.c | 2 +- net/bluetooth/l2cap_br.c | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/bluetooth/conn.h b/include/bluetooth/conn.h index 64939582a14..983840e4420 100644 --- a/include/bluetooth/conn.h +++ b/include/bluetooth/conn.h @@ -242,7 +242,7 @@ struct bt_conn *bt_conn_create_slave_le(const bt_addr_le_t *peer, /** Security level. */ typedef enum __packed { /** Only for BR/EDR special cases, like SDP */ - BT_SECURITY_ZERO, + BT_SECURITY_NONE, /** No encryption and no authentication. */ BT_SECURITY_LOW, /** Encryption and no authentication (no MITM). */ diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c index 15cad55f494..b31f57f8438 100644 --- a/net/bluetooth/l2cap.c +++ b/net/bluetooth/l2cap.c @@ -1359,7 +1359,7 @@ int bt_l2cap_chan_connect(struct bt_conn *conn, struct bt_l2cap_chan *chan, if (chan->required_sec_level > BT_SECURITY_FIPS) { return -EINVAL; - } else if (chan->required_sec_level == BT_SECURITY_ZERO) { + } else if (chan->required_sec_level == BT_SECURITY_NONE) { chan->required_sec_level = BT_SECURITY_LOW; } diff --git a/net/bluetooth/l2cap_br.c b/net/bluetooth/l2cap_br.c index 32b9fdf37a5..39a8cc6ce14 100644 --- a/net/bluetooth/l2cap_br.c +++ b/net/bluetooth/l2cap_br.c @@ -708,7 +708,7 @@ l2cap_br_conn_security(struct bt_l2cap_chan *chan, const uint16_t psm) int check; /* For SDP PSM there's no need to change existing security on link */ - if (chan->required_sec_level == BT_SECURITY_ZERO) { + if (chan->required_sec_level == BT_SECURITY_NONE) { return L2CAP_CONN_SECURITY_PASSED; } @@ -839,7 +839,7 @@ static void l2cap_br_conn_req(struct bt_l2cap_br *l2cap, uint8_t ident, * Report security violation for non SDP channel without encryption when * remote supports SSP. */ - if (server->sec_level != BT_SECURITY_ZERO && + if (server->sec_level != BT_SECURITY_NONE && BT_FEAT_HOST_SSP(conn->br.features) && !conn->encrypt) { result = BT_L2CAP_BR_ERR_SEC_BLOCK; goto done; @@ -977,7 +977,7 @@ int bt_l2cap_br_server_register(struct bt_l2cap_server *server) if (server->sec_level > BT_SECURITY_FIPS) { return -EINVAL; - } else if (server->sec_level == BT_SECURITY_ZERO && + } else if (server->sec_level == BT_SECURITY_NONE && server->psm != L2CAP_BR_PSM_SDP) { server->sec_level = BT_SECURITY_LOW; } @@ -1372,7 +1372,7 @@ int bt_l2cap_br_chan_connect(struct bt_conn *conn, struct bt_l2cap_chan *chan, if (chan->required_sec_level > BT_SECURITY_FIPS) { return -EINVAL; - } else if (chan->required_sec_level == BT_SECURITY_ZERO && + } else if (chan->required_sec_level == BT_SECURITY_NONE && psm != L2CAP_BR_PSM_SDP) { chan->required_sec_level = BT_SECURITY_LOW; } From b1ad0c9c213ee31cd277e9d0100e4b471e5fcf5e Mon Sep 17 00:00:00 2001 From: Luiz Augusto von Dentz Date: Tue, 1 Nov 2016 14:00:21 +0200 Subject: [PATCH 11/12] Bluetooth: L2CAP: Add security check for LE CoC This implements the security check for incoming LE CoC and in case the connection cannot attend the minimun security level set by the server return a proper error: > ACL Data RX: Handle 72 flags 0x02 dlen 18 LE L2CAP: LE Connection Request (0x14) ident 1 len 10 PSM: 128 (0x0080) Source CID: 64 MTU: 672 MPS: 230 Credits: 10 < ACL Data TX: Handle 72 flags 0x00 dlen 18 LE L2CAP: LE Connection Response (0x15) ident 1 len 10 Destination CID: 0 MTU: 0 MPS: 0 Credits: 0 Result: Connection refused - insufficient authentication (0x0005) Change-Id: Ia8b2873c5d1b154eb9a7c1a49c94578c4ec22f6d Signed-off-by: Luiz Augusto von Dentz --- net/bluetooth/l2cap.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c index b31f57f8438..d15fe70be16 100644 --- a/net/bluetooth/l2cap.c +++ b/net/bluetooth/l2cap.c @@ -608,7 +608,13 @@ static void le_conn_req(struct bt_l2cap *l2cap, uint8_t ident, goto rsp; } - /* TODO: Add security check */ +#if defined(CONFIG_BLUETOOTH_SMP) + /* Check if connection has minimum required security level */ + if (conn->sec_level < server->sec_level) { + rsp->result = sys_cpu_to_le16(BT_L2CAP_ERR_AUTHENTICATION); + goto rsp; + } +#endif /* CONFIG_BLUETOOTH_SMP */ if (!L2CAP_LE_CID_IS_DYN(scid)) { rsp->result = sys_cpu_to_le16(BT_L2CAP_ERR_INVALID_SCID); From 2538811fa90584598d616f3ec18dab8f4b5499b9 Mon Sep 17 00:00:00 2001 From: Luiz Augusto von Dentz Date: Tue, 1 Nov 2016 14:25:12 +0200 Subject: [PATCH 12/12] Bluetooth: shell: Add support for setting security level to l2cap_register This adds support for setting the minimun required security level to l2cap_register command: l2cap-register [sec_level] Note that entering the security level is optional and the default is BT_SECURITY_LOW(1). Change-Id: Ib6dea92919357f41fe2f4de3050b9a10a7f553d6 Signed-off-by: Luiz Augusto von Dentz --- tests/bluetooth/shell/src/main.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/bluetooth/shell/src/main.c b/tests/bluetooth/shell/src/main.c index 664c0a54386..b8bf7189f33 100644 --- a/tests/bluetooth/shell/src/main.c +++ b/tests/bluetooth/shell/src/main.c @@ -1809,11 +1809,16 @@ static int cmd_l2cap_register(int argc, char *argv[]) server.psm = strtoul(argv[1], NULL, 16); + if (argc > 2) { + server.sec_level = strtoul(argv[2], NULL, 10); + } + if (bt_l2cap_server_register(&server) < 0) { printk("Unable to register psm\n"); server.psm = 0; } else { - printk("L2CAP psm %u registered\n", server.psm); + printk("L2CAP psm %u sec_level %u registered\n", server.psm, + server.sec_level); } return 0; @@ -2189,7 +2194,7 @@ static const struct shell_cmd commands[] = { { "hrs-simulate", cmd_hrs_simulate, "register and simulate Heart Rate Service " }, #if defined(CONFIG_BLUETOOTH_L2CAP_DYNAMIC_CHANNEL) - { "l2cap-register", cmd_l2cap_register, "" }, + { "l2cap-register", cmd_l2cap_register, " [sec_level]" }, { "l2cap-connect", cmd_l2cap_connect, "" }, { "l2cap-disconnect", cmd_l2cap_disconnect, HELP_NONE }, { "l2cap-send", cmd_l2cap_send, "" },