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/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/include/bluetooth/conn.h b/include/bluetooth/conn.h index c1055e2a50f..983840e4420 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_NONE, /** 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/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/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 9941fba25a0..7797292844a 100644 --- a/net/bluetooth/att.c +++ b/net/bluetooth/att.c @@ -92,30 +92,20 @@ 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 ATT indications packets. This is required since indication can be - * sent in parallel to requests. + * 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 ind_data; -static NET_BUF_POOL(ind_pool, - CONFIG_BLUETOOTH_ATT_REQ_COUNT * CONFIG_BLUETOOTH_MAX_CONN, +static struct nano_fifo rsp_data; +static NET_BUF_POOL(rsp_pool, 1, 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); + &rsp_data, NULL, BT_BUF_USER_DATA_MIN); static void att_req_destroy(struct bt_att_req *req) { @@ -199,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; } @@ -1530,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; @@ -1796,12 +1778,23 @@ 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. + 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 as this is + * usually sent from RX fiber it shall never block. */ - buf = bt_l2cap_create_pdu(&ind_data, 0); + buf = bt_l2cap_create_pdu(&rsp_data, 0); break; default: buf = bt_l2cap_create_pdu(&req_data, 0); @@ -1983,9 +1976,8 @@ void bt_att_init(void) .accept = bt_att_accept, }; - net_buf_pool_init(ind_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 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); } diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c index 4a203def480..d15fe70be16 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"); @@ -601,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); @@ -624,6 +637,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 +1363,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_NONE) { + 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..39a8cc6ce14 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_NONE) { 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_NONE && + 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_NONE && + 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_NONE && + psm != L2CAP_BR_PSM_SDP) { + chan->required_sec_level = BT_SECURITY_LOW; + } + switch (chan->state) { case BT_L2CAP_CONNECTED: /* Already connected */ 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 diff --git a/samples/bluetooth/hci-uart/src/main.c b/samples/bluetooth/hci-uart/src/main.c index f2960d6c9cd..b103f059809 100644 --- a/samples/bluetooth/hci-uart/src/main.c +++ b/samples/bluetooth/hci-uart/src/main.c @@ -33,14 +33,13 @@ #include #include #include -#include #include #include 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 + \ @@ -64,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: @@ -76,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) @@ -174,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 */ 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, "" }, 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