diff --git a/include/zephyr/bluetooth/gatt.h b/include/zephyr/bluetooth/gatt.h index 3597c7c5fdb..2c7d36a6758 100644 --- a/include/zephyr/bluetooth/gatt.h +++ b/include/zephyr/bluetooth/gatt.h @@ -918,6 +918,7 @@ ssize_t bt_gatt_attr_read_service(struct bt_conn *conn, * Read include service attribute value from local database storing the result * into buffer after encoding it. * @note Only use this with attributes which user_data is a ``bt_gatt_include``. + * The function returns EINVAL if @p attr or @p attr->user_data is NULL. * * @param conn Connection object. * @param attr Attribute to read. diff --git a/include/zephyr/bluetooth/l2cap.h b/include/zephyr/bluetooth/l2cap.h index f8781d791f0..380b52ad508 100644 --- a/include/zephyr/bluetooth/l2cap.h +++ b/include/zephyr/bluetooth/l2cap.h @@ -111,6 +111,20 @@ extern "C" { */ #define BT_L2CAP_ECRED_MIN_MPS 64 +/** @brief L2CAP maximum MTU + * + * The maximum MTU for an L2CAP Based Connection. This is the same with or without ECRED. This + * requirement is taken from text in Core 3.A.4.22 and 3.A.4.26 v6.0. + */ +#define BT_L2CAP_MAX_MTU UINT16_MAX + +/** @brief L2CAP maximum MPS + * + * The maximum MPS for an L2CAP Based Connection. This is the same with or without ECRED. This + * requirement is taken from text in Core 3.A.4.22 and 3.A.4.26 v6.0. + */ +#define BT_L2CAP_MAX_MPS 65533 + /** @brief The maximum number of channels in ECRED L2CAP signaling PDUs * * Currently, this is the maximum number of channels referred to in the diff --git a/subsys/bluetooth/host/gatt.c b/subsys/bluetooth/host/gatt.c index 3092801554a..be642fec9fe 100644 --- a/subsys/bluetooth/host/gatt.c +++ b/subsys/bluetooth/host/gatt.c @@ -1917,6 +1917,10 @@ ssize_t bt_gatt_attr_read_included(struct bt_conn *conn, const struct bt_gatt_attr *attr, void *buf, uint16_t len, uint16_t offset) { + if ((attr == NULL) || (attr->user_data == NULL)) { + return -EINVAL; + } + struct bt_gatt_attr *incl = attr->user_data; uint16_t handle = bt_gatt_attr_get_handle(incl); struct bt_uuid *uuid = incl->user_data; diff --git a/subsys/bluetooth/host/l2cap.c b/subsys/bluetooth/host/l2cap.c index 9fa5638d961..caf5424717b 100644 --- a/subsys/bluetooth/host/l2cap.c +++ b/subsys/bluetooth/host/l2cap.c @@ -46,6 +46,7 @@ LOG_MODULE_REGISTER(bt_l2cap, CONFIG_BT_L2CAP_LOG_LEVEL); #define CHAN_RX(_w) CONTAINER_OF(_w, struct bt_l2cap_le_chan, rx_work) #define L2CAP_LE_MIN_MTU 23 +#define L2CAP_LE_MIN_MPS 23 #define L2CAP_LE_MAX_CREDITS (BT_BUF_ACL_RX_COUNT - 1) @@ -1478,11 +1479,6 @@ static void le_conn_req(struct bt_l2cap *l2cap, uint8_t ident, LOG_DBG("psm 0x%02x scid 0x%04x mtu %u mps %u credits %u", psm, scid, mtu, mps, credits); - if (mtu < L2CAP_LE_MIN_MTU || mps < L2CAP_LE_MIN_MTU) { - LOG_ERR("Invalid LE-Conn Req params: mtu %u mps %u", mtu, mps); - return; - } - buf = l2cap_create_le_sig_pdu(BT_L2CAP_LE_CONN_RSP, ident, sizeof(*rsp)); if (!buf) { @@ -1492,6 +1488,16 @@ static void le_conn_req(struct bt_l2cap *l2cap, uint8_t ident, rsp = net_buf_add(buf, sizeof(*rsp)); (void)memset(rsp, 0, sizeof(*rsp)); + /* Validate parameters. Requirements are from Core Spec v6.0, Vol 3.A.4.22. Valid credit + * range is from 0 to UINT16_MAX, thus no credit validation is needed. + */ + if (!IN_RANGE(mtu, L2CAP_LE_MIN_MTU, BT_L2CAP_MAX_MTU) || + !IN_RANGE(mps, L2CAP_LE_MIN_MPS, BT_L2CAP_MAX_MPS)) { + LOG_ERR("Invalid le conn req params: mtu %u mps %u", mtu, mps); + result = BT_L2CAP_LE_ERR_UNACCEPT_PARAMS; + goto rsp; + } + /* Check if there is a server registered */ server = bt_l2cap_server_lookup_psm(psm); if (!server) { @@ -1577,8 +1583,12 @@ static void le_ecred_conn_req(struct bt_l2cap *l2cap, uint8_t ident, LOG_DBG("psm 0x%02x mtu %u mps %u credits %u", psm, mtu, mps, credits); - if (mtu < BT_L2CAP_ECRED_MIN_MTU || mps < BT_L2CAP_ECRED_MIN_MTU) { - LOG_ERR("Invalid ecred conn req params. mtu %u mps %u", mtu, mps); + /* Validate parameters. Requirements are from Core Spec v6.0, Vol 3.A.4.25. */ + if (!IN_RANGE(mtu, BT_L2CAP_ECRED_MIN_MTU, BT_L2CAP_MAX_MTU) || + !IN_RANGE(mps, BT_L2CAP_ECRED_MIN_MPS, BT_L2CAP_MAX_MPS) || + !IN_RANGE(credits, BT_L2CAP_ECRED_CREDITS_MIN, BT_L2CAP_ECRED_CREDITS_MAX)) { + LOG_ERR("Invalid le ecred conn req params: mtu %u mps %u credits %u", mtu, mps, + credits); result = BT_L2CAP_LE_ERR_INVALID_PARAMS; goto response; } @@ -1685,7 +1695,7 @@ static void le_ecred_reconf_req(struct bt_l2cap *l2cap, uint8_t ident, mtu = sys_le16_to_cpu(req->mtu); mps = sys_le16_to_cpu(req->mps); - if (mps < BT_L2CAP_ECRED_MIN_MTU) { + if (mps < BT_L2CAP_ECRED_MIN_MPS) { result = BT_L2CAP_RECONF_OTHER_UNACCEPT; goto response; } @@ -1981,13 +1991,24 @@ static void le_ecred_conn_rsp(struct bt_l2cap *l2cap, uint8_t ident, LOG_DBG("dcid 0x%04x", dcid); - /* If a Destination CID is 0x0000, the channel was not + /* Validate parameters before assignment. Requirements are from Core Spec + * v6.0, Vol 3.A.4.26. If a Destination CID is 0x0000, the channel was not * established. */ - if (!dcid) { + if (dcid == 0U) { bt_l2cap_chan_remove(conn, &chan->chan); bt_l2cap_chan_del(&chan->chan); continue; + } else if (!L2CAP_LE_CID_IS_DYN(dcid) || + !IN_RANGE(mtu, BT_L2CAP_ECRED_MIN_MTU, BT_L2CAP_MAX_MTU) || + !IN_RANGE(mps, BT_L2CAP_ECRED_MIN_MPS, BT_L2CAP_MAX_MPS) || + !IN_RANGE(credits, BT_L2CAP_ECRED_CREDITS_MIN, + BT_L2CAP_ECRED_CREDITS_MAX)) { + LOG_WRN("Invalid ecred conn rsp params: dcid 0x%04x mtu %u mps %u " + "credits %u. Disconnecting.", + dcid, mtu, mps, credits); + bt_conn_disconnect(conn, BT_HCI_ERR_UNACCEPT_CONN_PARAM); + return; } c = bt_l2cap_le_lookup_tx_cid(conn, dcid); @@ -2085,6 +2106,20 @@ static void le_conn_rsp(struct bt_l2cap *l2cap, uint8_t ident, switch (result) { case BT_L2CAP_LE_SUCCESS: + /* Validate parameters on successful connection. Requirements are from Core Spec + * v6.0, Vol 3.A.4.23. Valid credit range is from 0 to UINT16_MAX, thus no credit + * validation is needed. + */ + if ((!L2CAP_LE_CID_IS_DYN(dcid) || + !IN_RANGE(mtu, L2CAP_LE_MIN_MTU, BT_L2CAP_MAX_MTU) || + !IN_RANGE(mps, L2CAP_LE_MIN_MPS, BT_L2CAP_MAX_MPS))) { + LOG_WRN("Invalid conn rsp params: dcid 0x%04x mtu %u mps %u. " + "Disconnecting.", + dcid, mtu, mps); + bt_conn_disconnect(conn, BT_HCI_ERR_UNACCEPT_CONN_PARAM); + return; + } + chan->tx.cid = dcid; chan->tx.mtu = mtu; chan->tx.mps = mps; @@ -2157,6 +2192,17 @@ static void le_credits(struct bt_l2cap *l2cap, uint8_t ident, cid = sys_le16_to_cpu(ev->cid); credits = sys_le16_to_cpu(ev->credits); + if (!L2CAP_LE_CID_IS_DYN(cid)) { + LOG_WRN("Can't add credits to non-dynamic channel %p (cid 0x%04x)", &l2cap->chan, + cid); + return; + } + + if (credits == 0U) { + LOG_WRN("Ignoring zero credit packet"); + return; + } + LOG_DBG("cid 0x%04x credits %u", cid, credits); chan = bt_l2cap_le_lookup_tx_cid(conn, cid); diff --git a/subsys/bluetooth/host/l2cap_internal.h b/subsys/bluetooth/host/l2cap_internal.h index 07bd428334a..61fa7416dd5 100644 --- a/subsys/bluetooth/host/l2cap_internal.h +++ b/subsys/bluetooth/host/l2cap_internal.h @@ -125,6 +125,9 @@ struct bt_l2cap_le_credits { uint16_t credits; } __packed; +#define BT_L2CAP_ECRED_CREDITS_MIN 1 +#define BT_L2CAP_ECRED_CREDITS_MAX UINT16_MAX + #define BT_L2CAP_ECRED_CONN_REQ 0x17 struct bt_l2cap_ecred_conn_req { uint16_t psm; diff --git a/subsys/bluetooth/host/smp.c b/subsys/bluetooth/host/smp.c index 916bb661bb9..ff920fb0f3a 100644 --- a/subsys/bluetooth/host/smp.c +++ b/subsys/bluetooth/host/smp.c @@ -2555,22 +2555,35 @@ static uint8_t legacy_pairing_req(struct bt_smp *smp) static uint8_t legacy_pairing_random(struct bt_smp *smp) { struct bt_conn *conn = smp->chan.chan.conn; - uint8_t tmp[16]; + uint8_t tmp[16], cfm_i[16]; int err; LOG_DBG(""); - /* calculate confirmation */ + /* calculate LP_CONFIRM_R */ err = smp_c1(smp->tk, smp->rrnd, smp->preq, smp->prsp, &conn->le.init_addr, &conn->le.resp_addr, tmp); if (err) { return BT_SMP_ERR_UNSPECIFIED; } + /* calculate LP_CONFIRM_I */ + err = smp_c1(smp->tk, smp->prnd, smp->preq, smp->prsp, + &conn->le.init_addr, &conn->le.resp_addr, cfm_i); + if (err) { + return BT_SMP_ERR_UNSPECIFIED; + } + LOG_DBG("pcnf %s", bt_hex(smp->pcnf, 16)); - LOG_DBG("cfm %s", bt_hex(tmp, 16)); + LOG_DBG("cfm (remote) %s", bt_hex(tmp, 16)); + LOG_DBG("cfm (local) %s", bt_hex(cfm_i, 16)); - if (memcmp(smp->pcnf, tmp, sizeof(smp->pcnf))) { + /* Core Specification, Vol 3, Part H, section 2.3.5.5 (Errata ES-24491): If the computed + * LP_CONFIRM_R value is not equal to the received LP_CONFIRM_R value, or the received + * LP_CONFIRM_R value is equal to the LP_CONFIRM_I value, fail pairing. + */ + if (memcmp(smp->pcnf, tmp, sizeof(smp->pcnf)) || + !memcmp(smp->pcnf, cfm_i, sizeof(smp->pcnf))) { return BT_SMP_ERR_CONFIRM_FAILED; } @@ -4486,7 +4499,7 @@ static uint8_t smp_public_key(struct bt_smp *smp, struct net_buf *buf) } } else if (!bt_pub_key_is_valid(smp->pkey)) { LOG_WRN("Received invalid public key"); - return BT_SMP_ERR_INVALID_PARAMS; + return BT_SMP_ERR_DHKEY_CHECK_FAILED; } if (IS_ENABLED(CONFIG_BT_CENTRAL) && diff --git a/tests/bsim/bluetooth/host/l2cap/many_conns/src/main.c b/tests/bsim/bluetooth/host/l2cap/many_conns/src/main.c index ff9f3030d1e..a4f45bc1890 100644 --- a/tests/bsim/bluetooth/host/l2cap/many_conns/src/main.c +++ b/tests/bsim/bluetooth/host/l2cap/many_conns/src/main.c @@ -29,7 +29,7 @@ DEFINE_FLAG_STATIC(flag_l2cap_connected); #define NUM_PERIPHERALS CONFIG_BT_MAX_CONN #define L2CAP_CHANS NUM_PERIPHERALS #define SDU_NUM 1 -#define SDU_LEN 10 +#define SDU_LEN 23 /* Only one SDU per link will be transmitted */ NET_BUF_POOL_DEFINE(sdu_tx_pool,