From 9ea6e72bae9c400ce553281d76b89ee9d4f56258 Mon Sep 17 00:00:00 2001 From: Kim Sekkelund Date: Tue, 26 Nov 2019 08:22:21 +0100 Subject: [PATCH] Bluetooth: host: cfg_write callback to return error code Current implementation of application's cfg_write callback only has the possibility of returning boolean status, which in case of failure only allows for one error code; BT_ATT_ERR_WRITE_NOT_PERMITTED. This change makes the application able to add own security check on characteristic subscription in the cfg_write callback and report a more relevant error code (e.g. BT_ATT_ERR_AUTHORIZATION). Signed-off-by: Kim Sekkelund --- include/bluetooth/gatt.h | 46 ++++++++++++++++++++++++++++------- subsys/bluetooth/host/gatt.c | 20 ++++++++++----- subsys/bluetooth/mesh/proxy.c | 14 +++++------ 3 files changed, 58 insertions(+), 22 deletions(-) diff --git a/include/bluetooth/gatt.h b/include/bluetooth/gatt.h index 7108aebd823..0e19593958a 100644 --- a/include/bluetooth/gatt.h +++ b/include/bluetooth/gatt.h @@ -571,15 +571,43 @@ struct bt_gatt_ccc_cfg { /* Internal representation of CCC value */ struct _bt_gatt_ccc { - struct bt_gatt_ccc_cfg cfg[BT_GATT_CCC_MAX]; - u16_t value; - void (*cfg_changed)(const struct bt_gatt_attr *attr, - u16_t value); - bool (*cfg_write)(struct bt_conn *conn, - const struct bt_gatt_attr *attr, - u16_t value); - bool (*cfg_match)(struct bt_conn *conn, - const struct bt_gatt_attr *attr); + /** Configuration for each connection */ + struct bt_gatt_ccc_cfg cfg[BT_GATT_CCC_MAX]; + + /** Highest value of all connected peer's subscriptions */ + u16_t value; + + /** CCC attribute changed callback + * + * @param attr The attribute that's changed value + * @param value New value + */ + void (*cfg_changed)(const struct bt_gatt_attr *attr, u16_t value); + + /** CCC attribute write validation callback + * + * @param conn The connection that is requesting to write + * @param attr The attribute that's being written + * @param value CCC value to write + * + * @return Number of bytes to write, or in case of an error + * BT_GATT_ERR() with a specific error code. + */ + ssize_t (*cfg_write)(struct bt_conn *conn, + const struct bt_gatt_attr *attr, u16_t value); + + /** CCC attribute match handler + * Indicate if it is OK to send a notification or indication + * to the subscriber. + * + * @param conn The connection that is being checked + * @param attr The attribute that's being checked + * + * @return true if application has approved notification/indication, + * false if application does not approve. + */ + bool (*cfg_match)(struct bt_conn *conn, + const struct bt_gatt_attr *attr); }; /** @brief Read Client Characteristic Configuration Attribute helper. diff --git a/subsys/bluetooth/host/gatt.c b/subsys/bluetooth/host/gatt.c index e1260484f8b..c17b02c8b9c 100644 --- a/subsys/bluetooth/host/gatt.c +++ b/subsys/bluetooth/host/gatt.c @@ -359,9 +359,8 @@ done: } } -static bool sc_ccc_cfg_write(struct bt_conn *conn, - const struct bt_gatt_attr *attr, - u16_t value) +static ssize_t sc_ccc_cfg_write(struct bt_conn *conn, + const struct bt_gatt_attr *attr, u16_t value) { BT_DBG("value 0x%04x", value); @@ -378,7 +377,7 @@ static bool sc_ccc_cfg_write(struct bt_conn *conn, } } - return true; + return sizeof(value); } static struct _bt_gatt_ccc sc_ccc = BT_GATT_CCC_INITIALIZER(NULL, @@ -1483,8 +1482,17 @@ ssize_t bt_gatt_attr_write_ccc(struct bt_conn *conn, } /* Confirm write if cfg is managed by application */ - if (ccc->cfg_write && !ccc->cfg_write(conn, attr, value)) { - return BT_GATT_ERR(BT_ATT_ERR_WRITE_NOT_PERMITTED); + if (ccc->cfg_write) { + ssize_t write = ccc->cfg_write(conn, attr, value); + + if (write < 0) { + return write; + } + + /* Accept size=1 for backwards compatibility */ + if (write != sizeof(value) && write != 1) { + return BT_GATT_ERR(BT_ATT_ERR_UNLIKELY); + } } cfg->value = value; diff --git a/subsys/bluetooth/mesh/proxy.c b/subsys/bluetooth/mesh/proxy.c index 2c9a84d07a1..af4d5420398 100644 --- a/subsys/bluetooth/mesh/proxy.c +++ b/subsys/bluetooth/mesh/proxy.c @@ -612,7 +612,7 @@ static void prov_ccc_changed(const struct bt_gatt_attr *attr, u16_t value) BT_DBG("value 0x%04x", value); } -static bool prov_ccc_write(struct bt_conn *conn, +static ssize_t prov_ccc_write(struct bt_conn *conn, const struct bt_gatt_attr *attr, u16_t value) { struct bt_mesh_proxy_client *client; @@ -621,7 +621,7 @@ static bool prov_ccc_write(struct bt_conn *conn, if (value != BT_GATT_CCC_NOTIFY) { BT_WARN("Client wrote 0x%04x instead enabling notify", value); - return false; + return BT_GATT_ERR(BT_ATT_ERR_VALUE_NOT_ALLOWED); } /* If a connection exists there must be a client */ @@ -633,7 +633,7 @@ static bool prov_ccc_write(struct bt_conn *conn, bt_mesh_pb_gatt_open(conn); } - return true; + return sizeof(value); } /* Mesh Provisioning Service Declaration */ @@ -731,8 +731,8 @@ static void proxy_ccc_changed(const struct bt_gatt_attr *attr, u16_t value) BT_DBG("value 0x%04x", value); } -static bool proxy_ccc_write(struct bt_conn *conn, - const struct bt_gatt_attr *attr, u16_t value) +static ssize_t proxy_ccc_write(struct bt_conn *conn, + const struct bt_gatt_attr *attr, u16_t value) { struct bt_mesh_proxy_client *client; @@ -740,7 +740,7 @@ static bool proxy_ccc_write(struct bt_conn *conn, if (value != BT_GATT_CCC_NOTIFY) { BT_WARN("Client wrote 0x%04x instead enabling notify", value); - return false; + return BT_GATT_ERR(BT_ATT_ERR_VALUE_NOT_ALLOWED); } /* If a connection exists there must be a client */ @@ -752,7 +752,7 @@ static bool proxy_ccc_write(struct bt_conn *conn, k_work_submit(&client->send_beacons); } - return true; + return sizeof(value); } /* Mesh Proxy Service Declaration */