From 03b9ce487cce6b7bb13bde9cefaba752fdd87238 Mon Sep 17 00:00:00 2001 From: Luiz Augusto von Dentz Date: Wed, 4 Sep 2019 14:15:07 +0300 Subject: [PATCH] Bluetooth: GATT: Add support to setting permission on CCCD MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds support to set different permissions to CCCD so security can be checked when enabling notification which conforms to: BLUETOOTH CORE SPECIFICATION Version 5.1 | Vol 3, Part G page 2360: '3.3.3.3 Client Characteristic Configuration Authentication and authorization may be required by the server to write the configuration descriptor.' In addition to that also ensure that notification are not re-enabled until the proper security level is reached to conform to the following statement: '10.3.1.1 Handling of GATT indications and notifications A client “requests” a server to send indications and notifications by appropriately configuring the server via a Client Characteristic Configuration Descriptor. Since the configuration is persistent across a disconnection and reconnection, security requirements must be checked against the configuration upon a reconnection before sending indications or notifications. When a server reconnects to a client to send an indication or notification for which security is required, the server shall initiate or request encryption with the client prior to sending an indication or notification. If the client does not have an LTK indicating that the client has lost the bond, enabling encryption will fail.' Fixes #17983 Signed-off-by: Luiz Augusto von Dentz --- include/bluetooth/gatt.h | 15 +++-- samples/bluetooth/peripheral/src/cts.c | 2 +- samples/bluetooth/peripheral/src/main.c | 3 +- samples/bluetooth/peripheral_csc/src/main.c | 6 +- samples/bluetooth/peripheral_esp/src/main.c | 6 +- samples/bluetooth/peripheral_hids/src/hog.c | 3 +- samples/bluetooth/peripheral_ht/src/hts.c | 3 +- samples/boards/bbc_microbit/pong/src/ble.c | 3 +- subsys/bluetooth/host/att.c | 2 + subsys/bluetooth/host/gatt.c | 75 +++++++++++++++++++-- subsys/bluetooth/host/gatt_internal.h | 1 + subsys/bluetooth/mesh/proxy.c | 6 +- subsys/bluetooth/services/bas.c | 3 +- subsys/bluetooth/services/hrs.c | 3 +- subsys/bluetooth/shell/gatt.c | 3 +- subsys/mgmt/smp_bt.c | 2 +- tests/bluetooth/gatt/src/main.c | 3 +- tests/bluetooth/tester/src/gatt.c | 4 +- 18 files changed, 115 insertions(+), 28 deletions(-) diff --git a/include/bluetooth/gatt.h b/include/bluetooth/gatt.h index 65e4dcebe6f..3d759a94215 100644 --- a/include/bluetooth/gatt.h +++ b/include/bluetooth/gatt.h @@ -641,10 +641,10 @@ ssize_t bt_gatt_attr_write_ccc(struct bt_conn *conn, * Helper macro to declare a Managed CCC attribute. * * @param _ccc CCC attribute user data, shall point to a _bt_gatt_ccc. + * @param _perm CCC access permissions. */ -#define BT_GATT_CCC_MANAGED(_ccc) \ - BT_GATT_ATTRIBUTE(BT_UUID_GATT_CCC, \ - BT_GATT_PERM_READ | BT_GATT_PERM_WRITE, \ +#define BT_GATT_CCC_MANAGED(_ccc, _perm) \ + BT_GATT_ATTRIBUTE(BT_UUID_GATT_CCC, _perm, \ bt_gatt_attr_read_ccc, bt_gatt_attr_write_ccc, \ _ccc) @@ -653,11 +653,12 @@ ssize_t bt_gatt_attr_write_ccc(struct bt_conn *conn, * * Helper macro to declare a CCC attribute. * - * @param _cfg_changed Configuration changed callback. + * @param _changed Configuration changed callback. + * @param _perm CCC access permissions. */ -#define BT_GATT_CCC(_cfg_changed) \ - BT_GATT_CCC_MANAGED((&(struct _bt_gatt_ccc) \ - BT_GATT_CCC_INITIALIZER(_cfg_changed, NULL, NULL))) +#define BT_GATT_CCC(_changed, _perm) \ + BT_GATT_CCC_MANAGED((&(struct _bt_gatt_ccc) \ + BT_GATT_CCC_INITIALIZER(_changed, NULL, NULL)), _perm) /** @brief Read Characteristic Extended Properties Attribute helper * diff --git a/samples/bluetooth/peripheral/src/cts.c b/samples/bluetooth/peripheral/src/cts.c index d7217aa4545..3bb76df9bcb 100644 --- a/samples/bluetooth/peripheral/src/cts.c +++ b/samples/bluetooth/peripheral/src/cts.c @@ -62,7 +62,7 @@ BT_GATT_SERVICE_DEFINE(cts_cvs, BT_GATT_CHRC_NOTIFY | BT_GATT_CHRC_WRITE, BT_GATT_PERM_READ | BT_GATT_PERM_WRITE, read_ct, write_ct, ct), - BT_GATT_CCC(ct_ccc_cfg_changed), + BT_GATT_CCC(ct_ccc_cfg_changed, BT_GATT_PERM_READ | BT_GATT_PERM_WRITE), ); static void generate_current_time(u8_t *buf) diff --git a/samples/bluetooth/peripheral/src/main.c b/samples/bluetooth/peripheral/src/main.c index a1f053eed5a..1b2d1eed46a 100644 --- a/samples/bluetooth/peripheral/src/main.c +++ b/samples/bluetooth/peripheral/src/main.c @@ -194,7 +194,8 @@ BT_GATT_SERVICE_DEFINE(vnd_svc, BT_GATT_PERM_READ_ENCRYPT | BT_GATT_PERM_WRITE_ENCRYPT, read_vnd, write_vnd, vnd_value), - BT_GATT_CCC(vnd_ccc_cfg_changed), + BT_GATT_CCC(vnd_ccc_cfg_changed, + BT_GATT_PERM_READ | BT_GATT_PERM_WRITE_ENCRYPT), BT_GATT_CHARACTERISTIC(&vnd_auth_uuid.uuid, BT_GATT_CHRC_READ | BT_GATT_CHRC_WRITE, BT_GATT_PERM_READ_AUTHEN | diff --git a/samples/bluetooth/peripheral_csc/src/main.c b/samples/bluetooth/peripheral_csc/src/main.c index c8103e1bf97..4122ff22691 100644 --- a/samples/bluetooth/peripheral_csc/src/main.c +++ b/samples/bluetooth/peripheral_csc/src/main.c @@ -202,7 +202,8 @@ BT_GATT_SERVICE_DEFINE(csc_svc, BT_GATT_PRIMARY_SERVICE(BT_UUID_CSC), BT_GATT_CHARACTERISTIC(BT_UUID_CSC_MEASUREMENT, BT_GATT_CHRC_NOTIFY, 0x00, NULL, NULL, NULL), - BT_GATT_CCC(csc_meas_ccc_cfg_changed), + BT_GATT_CCC(csc_meas_ccc_cfg_changed, + BT_GATT_PERM_READ | BT_GATT_PERM_WRITE), BT_GATT_CHARACTERISTIC(BT_UUID_SENSOR_LOCATION, BT_GATT_CHRC_READ, BT_GATT_PERM_READ, read_location, NULL, &sensor_location), @@ -212,7 +213,8 @@ BT_GATT_SERVICE_DEFINE(csc_svc, BT_GATT_CHRC_WRITE | BT_GATT_CHRC_INDICATE, BT_GATT_PERM_WRITE, NULL, write_ctrl_point, &sensor_location), - BT_GATT_CCC(ctrl_point_ccc_cfg_changed), + BT_GATT_CCC(ctrl_point_ccc_cfg_changed, + BT_GATT_PERM_READ | BT_GATT_PERM_WRITE), ); struct sc_ctrl_point_ind { diff --git a/samples/bluetooth/peripheral_esp/src/main.c b/samples/bluetooth/peripheral_esp/src/main.c index 16bf01177db..3a5bfd71b3d 100644 --- a/samples/bluetooth/peripheral_esp/src/main.c +++ b/samples/bluetooth/peripheral_esp/src/main.c @@ -298,7 +298,8 @@ BT_GATT_SERVICE_DEFINE(ess_svc, BT_GATT_DESCRIPTOR(BT_UUID_ES_TRIGGER_SETTING, BT_GATT_PERM_READ, read_temp_trigger_setting, NULL, &sensor_1), - BT_GATT_CCC(temp_ccc_cfg_changed), + BT_GATT_CCC(temp_ccc_cfg_changed, + BT_GATT_PERM_READ | BT_GATT_PERM_WRITE), /* Temperature Sensor 2 */ BT_GATT_CHARACTERISTIC(BT_UUID_TEMPERATURE, @@ -313,7 +314,8 @@ BT_GATT_SERVICE_DEFINE(ess_svc, BT_GATT_DESCRIPTOR(BT_UUID_ES_TRIGGER_SETTING, BT_GATT_PERM_READ, read_temp_trigger_setting, NULL, &sensor_2), - BT_GATT_CCC(temp_ccc_cfg_changed), + BT_GATT_CCC(temp_ccc_cfg_changed, + BT_GATT_PERM_READ | BT_GATT_PERM_WRITE), /* Humidity Sensor */ BT_GATT_CHARACTERISTIC(BT_UUID_HUMIDITY, BT_GATT_CHRC_READ, diff --git a/samples/bluetooth/peripheral_hids/src/hog.c b/samples/bluetooth/peripheral_hids/src/hog.c index c4d0a8341c3..cc8e21ab0b9 100644 --- a/samples/bluetooth/peripheral_hids/src/hog.c +++ b/samples/bluetooth/peripheral_hids/src/hog.c @@ -150,7 +150,8 @@ BT_GATT_SERVICE_DEFINE(hog_svc, BT_GATT_CHRC_READ | BT_GATT_CHRC_NOTIFY, BT_GATT_PERM_READ_AUTHEN, read_input_report, NULL, NULL), - BT_GATT_CCC(input_ccc_changed), + BT_GATT_CCC(input_ccc_changed, + BT_GATT_PERM_READ_AUTHEN | BT_GATT_PERM_WRITE_AUTHEN), BT_GATT_DESCRIPTOR(BT_UUID_HIDS_REPORT_REF, BT_GATT_PERM_READ, read_report, NULL, &input), BT_GATT_CHARACTERISTIC(BT_UUID_HIDS_CTRL_POINT, diff --git a/samples/bluetooth/peripheral_ht/src/hts.c b/samples/bluetooth/peripheral_ht/src/hts.c index 3ccdacb1a69..3c5b971b1db 100644 --- a/samples/bluetooth/peripheral_ht/src/hts.c +++ b/samples/bluetooth/peripheral_ht/src/hts.c @@ -46,7 +46,8 @@ BT_GATT_SERVICE_DEFINE(hts_svc, BT_GATT_PRIMARY_SERVICE(BT_UUID_HTS), BT_GATT_CHARACTERISTIC(BT_UUID_HTS_MEASUREMENT, BT_GATT_CHRC_INDICATE, BT_GATT_PERM_NONE, NULL, NULL, NULL), - BT_GATT_CCC(htmc_ccc_cfg_changed), + BT_GATT_CCC(htmc_ccc_cfg_changed, + BT_GATT_PERM_READ | BT_GATT_PERM_WRITE), /* more optional Characteristics */ ); diff --git a/samples/boards/bbc_microbit/pong/src/ble.c b/samples/boards/bbc_microbit/pong/src/ble.c index 0f02c4feebb..b82dd8e0970 100644 --- a/samples/boards/bbc_microbit/pong/src/ble.c +++ b/samples/boards/bbc_microbit/pong/src/ble.c @@ -512,7 +512,8 @@ BT_GATT_SERVICE_DEFINE(pong_svc, BT_GATT_PRIMARY_SERVICE(&pong_svc_uuid.uuid), BT_GATT_CHARACTERISTIC(&pong_chr_uuid.uuid, BT_GATT_CHRC_NOTIFY, BT_GATT_PERM_NONE, NULL, NULL, NULL), - BT_GATT_CCC(pong_ccc_cfg_changed), + BT_GATT_CCC(pong_ccc_cfg_changed, + BT_GATT_PERM_READ | BT_GATT_PERM_WRITE), ); void ble_init(void) diff --git a/subsys/bluetooth/host/att.c b/subsys/bluetooth/host/att.c index a9b2513d00c..fe140597fe1 100644 --- a/subsys/bluetooth/host/att.c +++ b/subsys/bluetooth/host/att.c @@ -2212,6 +2212,8 @@ static void bt_att_encrypt_change(struct bt_l2cap_chan *chan, return; } + bt_gatt_encrypt_change(conn); + if (conn->sec_level == BT_SECURITY_L1) { return; } diff --git a/subsys/bluetooth/host/gatt.c b/subsys/bluetooth/host/gatt.c index f1b6d797d71..73d21626503 100644 --- a/subsys/bluetooth/host/gatt.c +++ b/subsys/bluetooth/host/gatt.c @@ -505,7 +505,7 @@ BT_GATT_SERVICE_DEFINE(_1_gatt_svc, */ BT_GATT_CHARACTERISTIC(BT_UUID_GATT_SC, BT_GATT_CHRC_INDICATE, BT_GATT_PERM_NONE, NULL, NULL, NULL), - BT_GATT_CCC(sc_ccc_cfg_changed), + BT_GATT_CCC(sc_ccc_cfg_changed, BT_GATT_PERM_READ | BT_GATT_PERM_WRITE), #if defined(CONFIG_BT_GATT_CACHING) BT_GATT_CHARACTERISTIC(BT_UUID_GATT_CLIENT_FEATURES, BT_GATT_CHRC_READ | BT_GATT_CHRC_WRITE, @@ -1714,9 +1714,15 @@ static void sc_restore(struct bt_gatt_ccc_cfg *cfg) } #endif /* CONFIG_BT_GATT_DYNAMIC_DB */ -static u8_t connected_cb(const struct bt_gatt_attr *attr, void *user_data) +struct conn_data { + struct bt_conn *conn; + bt_security_t sec; +}; + +static u8_t update_ccc(const struct bt_gatt_attr *attr, void *user_data) { - struct bt_conn *conn = user_data; + struct conn_data *data = user_data; + struct bt_conn *conn = data->conn; struct _bt_gatt_ccc *ccc; size_t i; @@ -1733,6 +1739,26 @@ static u8_t connected_cb(const struct bt_gatt_attr *attr, void *user_data) continue; } +#if defined(CONFIG_BT_SMP) + /* Check if attribute requires encryption/authentication */ + if (attr->perm & + (BT_GATT_PERM_WRITE_ENCRYPT | BT_GATT_PERM_WRITE_AUTHEN)) { + bt_security_t sec = BT_SECURITY_L2; + + if (attr->perm & BT_GATT_PERM_WRITE_AUTHEN) { + sec = BT_SECURITY_L3; + } + + /* Check if current security is enough */ + if (conn->sec_level < sec) { + if (data->sec < sec) { + data->sec = sec; + } + continue; + } + } +#endif + if (ccc->cfg[i].value) { gatt_ccc_changed(attr, ccc); #if defined(CONFIG_BT_GATT_DYNAMIC_DB) @@ -3229,13 +3255,54 @@ static void add_subscriptions(struct bt_conn *conn) void bt_gatt_connected(struct bt_conn *conn) { + struct conn_data data; + BT_DBG("conn %p", conn); - bt_gatt_foreach_attr(0x0001, 0xffff, connected_cb, conn); + + data.conn = conn; + data.sec = BT_SECURITY_L1; + + bt_gatt_foreach_attr(0x0001, 0xffff, update_ccc, &data); + +#if defined(CONFIG_BT_SMP) + /* BLUETOOTH CORE SPECIFICATION Version 5.1 | Vol 3, Part C page 2192: + * + * 10.3.1.1 Handling of GATT indications and notifications + * + * A client “requests” a server to send indications and notifications + * by appropriately configuring the server via a Client Characteristic + * Configuration Descriptor. Since the configuration is persistent + * across a disconnection and reconnection, security requirements must + * be checked against the configuration upon a reconnection before + * sending indications or notifications. When a server reconnects to a + * client to send an indication or notification for which security is + * required, the server shall initiate or request encryption with the + * client prior to sending an indication or notification. If the client + * does not have an LTK indicating that the client has lost the bond, + * enabling encryption will fail. + */ + if (conn->sec_level < data.sec) { + bt_conn_set_security(conn, data.sec); + } +#endif /* CONFIG_BT_SMP */ + #if defined(CONFIG_BT_GATT_CLIENT) add_subscriptions(conn); #endif /* CONFIG_BT_GATT_CLIENT */ } +void bt_gatt_encrypt_change(struct bt_conn *conn) +{ + struct conn_data data; + + BT_DBG("conn %p", conn); + + data.conn = conn; + data.sec = BT_SECURITY_L1; + + bt_gatt_foreach_attr(0x0001, 0xffff, update_ccc, &data); +} + bool bt_gatt_change_aware(struct bt_conn *conn, bool req) { #if defined(CONFIG_BT_GATT_CACHING) diff --git a/subsys/bluetooth/host/gatt_internal.h b/subsys/bluetooth/host/gatt_internal.h index d716c1b13f6..95c544a76e2 100644 --- a/subsys/bluetooth/host/gatt_internal.h +++ b/subsys/bluetooth/host/gatt_internal.h @@ -13,6 +13,7 @@ void bt_gatt_init(void); void bt_gatt_connected(struct bt_conn *conn); +void bt_gatt_encrypt_change(struct bt_conn *conn); void bt_gatt_disconnected(struct bt_conn *conn); bool bt_gatt_change_aware(struct bt_conn *conn, bool req); diff --git a/subsys/bluetooth/mesh/proxy.c b/subsys/bluetooth/mesh/proxy.c index d4cddcc87be..9bc2ee8c78e 100644 --- a/subsys/bluetooth/mesh/proxy.c +++ b/subsys/bluetooth/mesh/proxy.c @@ -651,7 +651,8 @@ static struct bt_gatt_attr prov_attrs[] = { BT_GATT_CHARACTERISTIC(BT_UUID_MESH_PROV_DATA_OUT, BT_GATT_CHRC_NOTIFY, BT_GATT_PERM_NONE, NULL, NULL, NULL), - BT_GATT_CCC_MANAGED(&prov_ccc), + BT_GATT_CCC_MANAGED(&prov_ccc, + BT_GATT_PERM_READ | BT_GATT_PERM_WRITE), }; static struct bt_gatt_service prov_svc = BT_GATT_SERVICE(prov_attrs); @@ -770,7 +771,8 @@ static struct bt_gatt_attr proxy_attrs[] = { BT_GATT_CHRC_NOTIFY, BT_GATT_PERM_NONE, NULL, NULL, NULL), - BT_GATT_CCC_MANAGED(&proxy_ccc), + BT_GATT_CCC_MANAGED(&proxy_ccc, + BT_GATT_PERM_READ | BT_GATT_PERM_WRITE), }; static struct bt_gatt_service proxy_svc = BT_GATT_SERVICE(proxy_attrs); diff --git a/subsys/bluetooth/services/bas.c b/subsys/bluetooth/services/bas.c index f38c1d933c5..d112557f61b 100644 --- a/subsys/bluetooth/services/bas.c +++ b/subsys/bluetooth/services/bas.c @@ -53,7 +53,8 @@ BT_GATT_SERVICE_DEFINE(bas, BT_GATT_CHRC_READ | BT_GATT_CHRC_NOTIFY, BT_GATT_PERM_READ, read_blvl, NULL, &battery_level), - BT_GATT_CCC(blvl_ccc_cfg_changed), + BT_GATT_CCC(blvl_ccc_cfg_changed, + BT_GATT_PERM_READ | BT_GATT_PERM_WRITE), ); static int bas_init(struct device *dev) diff --git a/subsys/bluetooth/services/hrs.c b/subsys/bluetooth/services/hrs.c index 4aa65ec8ca5..6b3cd09d87f 100644 --- a/subsys/bluetooth/services/hrs.c +++ b/subsys/bluetooth/services/hrs.c @@ -48,7 +48,8 @@ BT_GATT_SERVICE_DEFINE(hrs_svc, BT_GATT_PRIMARY_SERVICE(BT_UUID_HRS), BT_GATT_CHARACTERISTIC(BT_UUID_HRS_MEASUREMENT, BT_GATT_CHRC_NOTIFY, BT_GATT_PERM_NONE, NULL, NULL, NULL), - BT_GATT_CCC(hrmc_ccc_cfg_changed), + BT_GATT_CCC(hrmc_ccc_cfg_changed, + BT_GATT_PERM_READ | BT_GATT_PERM_WRITE), BT_GATT_CHARACTERISTIC(BT_UUID_HRS_BODY_SENSOR, BT_GATT_CHRC_READ, BT_GATT_PERM_READ, read_blsc, NULL, NULL), BT_GATT_CHARACTERISTIC(BT_UUID_HRS_CONTROL_POINT, BT_GATT_CHRC_WRITE, diff --git a/subsys/bluetooth/shell/gatt.c b/subsys/bluetooth/shell/gatt.c index e410d461da2..328b3f1c15b 100644 --- a/subsys/bluetooth/shell/gatt.c +++ b/subsys/bluetooth/shell/gatt.c @@ -731,7 +731,8 @@ static struct bt_gatt_attr vnd1_attrs[] = { BT_GATT_CHRC_WRITE_WITHOUT_RESP | BT_GATT_CHRC_NOTIFY, BT_GATT_PERM_WRITE, NULL, write_vnd1, NULL), - BT_GATT_CCC(vnd1_ccc_cfg_changed), + BT_GATT_CCC(vnd1_ccc_cfg_changed, + BT_GATT_PERM_READ | BT_GATT_PERM_WRITE), }; static struct bt_gatt_service vnd1_svc = BT_GATT_SERVICE(vnd1_attrs); diff --git a/subsys/mgmt/smp_bt.c b/subsys/mgmt/smp_bt.c index 60d96e61e30..b2f885f2787 100644 --- a/subsys/mgmt/smp_bt.c +++ b/subsys/mgmt/smp_bt.c @@ -78,7 +78,7 @@ static struct bt_gatt_attr smp_bt_attrs[] = { BT_GATT_CHRC_NOTIFY, BT_GATT_PERM_WRITE, NULL, smp_bt_chr_write, NULL), - BT_GATT_CCC(smp_bt_ccc_changed), + BT_GATT_CCC(smp_bt_ccc_changed, BT_GATT_PERM_READ | BT_GATT_PERM_WRITE), }; static struct bt_gatt_service smp_bt_svc = BT_GATT_SERVICE(smp_bt_attrs); diff --git a/tests/bluetooth/gatt/src/main.c b/tests/bluetooth/gatt/src/main.c index 6c18d2df5aa..8759611471c 100644 --- a/tests/bluetooth/gatt/src/main.c +++ b/tests/bluetooth/gatt/src/main.c @@ -83,7 +83,8 @@ static struct bt_gatt_attr test1_attrs[] = { BT_GATT_CHARACTERISTIC(&test1_nfy_uuid.uuid, BT_GATT_CHRC_NOTIFY, BT_GATT_PERM_NONE, NULL, NULL, &nfy_enabled), - BT_GATT_CCC(test1_ccc_cfg_changed), + BT_GATT_CCC(test1_ccc_cfg_changed, + BT_GATT_PERM_READ | BT_GATT_PERM_WRITE), }; static struct bt_gatt_service test1_svc = BT_GATT_SERVICE(test1_attrs); diff --git a/tests/bluetooth/tester/src/gatt.c b/tests/bluetooth/tester/src/gatt.c index 1da5908acd1..af215106eb7 100644 --- a/tests/bluetooth/tester/src/gatt.c +++ b/tests/bluetooth/tester/src/gatt.c @@ -470,7 +470,9 @@ static void ccc_cfg_changed(const struct bt_gatt_attr *attr, u16_t value) ccc_value = value; } -static struct bt_gatt_attr ccc = BT_GATT_CCC(ccc_cfg_changed); +static struct bt_gatt_attr ccc = BT_GATT_CCC(ccc_cfg_changed, + BT_GATT_PERM_READ | + BT_GATT_PERM_WRITE); static struct bt_gatt_attr *add_ccc(const struct bt_gatt_attr *attr) {