* [PATCH BlueZ v3 0/2] Optionally require security for notify/indicate @ 2021-09-30 15:08 Dagan Martinez 2021-09-30 15:08 ` [PATCH BlueZ v3 1/2] gatt: Allow GATT server to dicate CCC permissions Dagan Martinez 2021-09-30 15:08 ` [PATCH BlueZ v3 2/2] doc/gatt-api: Add 'X-notify`/`X-indicate` Dagan Martinez 0 siblings, 2 replies; 4+ messages in thread From: Dagan Martinez @ 2021-09-30 15:08 UTC (permalink / raw) To: linux-bluetooth; +Cc: Dagan Martinez In some cases, it is desirable to require encryption and/or authentication for server-initiated updates, as they may contain sensitive data. Currently, there is no way to do this with BlueZ. Here is a query about this feature from 2019: https://stackoverflow.com/questions/55884233 This patch implements this feature by introducing new `x-notify` and `x-indicate` flags that allow a user to restrict access to a characteristic's CCCD (as well as documentation for those flags). Note that `x-notify` and `x-indicate` each enforce security for ALL server-initiated updates. That is, you cannot require one level of security for notifications and another security level for indications on the same CCCD. I could not think of a reason why somebody would want that feature, and did not think the accuracy of terms would be worth the introduced complexity, so I didn't implement it. --- Changes in v3: - Split the `x-asynchronous` flags into `x-notify` and `x-indicate` - Fix a mixed code and declaration error Changes in v2: - Fix line-width issues brought up by CI Dagan Martinez (2): gatt: Allow GATT server to dicate CCC permissions doc/gatt-api: Add 'X-notify`/`X-indicate` doc/gatt-api.txt | 15 ++++++++++++++- src/gatt-database.c | 42 ++++++++++++++++++++++++++++++++++++++---- src/shared/att-types.h | 4 ++++ 3 files changed, 56 insertions(+), 5 deletions(-) -- 2.31.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH BlueZ v3 1/2] gatt: Allow GATT server to dicate CCC permissions 2021-09-30 15:08 [PATCH BlueZ v3 0/2] Optionally require security for notify/indicate Dagan Martinez @ 2021-09-30 15:08 ` Dagan Martinez 2021-09-30 20:25 ` Luiz Augusto von Dentz 2021-09-30 15:08 ` [PATCH BlueZ v3 2/2] doc/gatt-api: Add 'X-notify`/`X-indicate` Dagan Martinez 1 sibling, 1 reply; 4+ messages in thread From: Dagan Martinez @ 2021-09-30 15:08 UTC (permalink / raw) To: linux-bluetooth; +Cc: Dagan Martinez Allow a GATT server to impose permissions/restrictions on a CCC by setting additional `X-notify` and `X-indicate` permissions on its associated characteristic. This allows a developer to require encryption/authentication in order for a GATT client to subscribe to server-initiated updates. Test procedure: Attempt to read/write with a "low" security level on an unprotected CCC using gatttool, and succeed Attempt to READ with a "low" security level on an protected CCC using gatttool, and succeed Attempt to WRITE with a "low" security level on an protected CCC using gatttool, and fail Attempt to read/write while paired on a protected CCC using `bluetoothctl`, and succeed --- src/gatt-database.c | 42 ++++++++++++++++++++++++++++++++++++++---- src/shared/att-types.h | 4 ++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/gatt-database.c b/src/gatt-database.c index 68f411ba4..fd4a39166 100644 --- a/src/gatt-database.c +++ b/src/gatt-database.c @@ -1060,17 +1060,33 @@ service_add_ccc(struct gatt_db_attribute *service, struct btd_gatt_database *database, btd_gatt_database_ccc_write_t write_callback, void *user_data, + uint32_t parent_permissions, btd_gatt_database_destroy_t destroy) { struct gatt_db_attribute *ccc; struct ccc_cb_data *ccc_cb; bt_uuid_t uuid; + uint32_t permissions; ccc_cb = new0(struct ccc_cb_data, 1); + /* + * Provide a way for the permissions on a characteristic to dictate + * the permissions on the CCC + */ + permissions = BT_ATT_PERM_READ | BT_ATT_PERM_WRITE; + + if (parent_permissions & BT_ATT_PERM_SERVER_INITIATED_UPDATE_ENCRYPT) + permissions |= BT_ATT_PERM_WRITE_ENCRYPT; + + if (parent_permissions & BT_ATT_PERM_SERVER_INITIATED_UPDATE_AUTHEN) + permissions |= BT_ATT_PERM_WRITE_AUTHEN; + + if (parent_permissions & BT_ATT_PERM_SERVER_INITIATED_UPDATE_SECURE) + permissions |= BT_ATT_PERM_WRITE_SECURE; + bt_uuid16_create(&uuid, GATT_CLIENT_CHARAC_CFG_UUID); - ccc = gatt_db_service_add_descriptor(service, &uuid, - BT_ATT_PERM_READ | BT_ATT_PERM_WRITE, + ccc = gatt_db_service_add_descriptor(service, &uuid, permissions, gatt_ccc_read_cb, gatt_ccc_write_cb, database); if (!ccc) { error("Failed to create CCC entry in database"); @@ -1227,7 +1243,7 @@ static void populate_gatt_service(struct btd_gatt_database *database) NULL, NULL, database); database->svc_chngd_ccc = service_add_ccc(service, database, NULL, NULL, - NULL); + 0, NULL); bt_uuid16_create(&uuid, GATT_CHARAC_CLI_FEAT); database->cli_feat = gatt_db_service_add_characteristic(service, @@ -1690,6 +1706,24 @@ static bool parse_chrc_flags(DBusMessageIter *array, uint8_t *props, *perm |= BT_ATT_PERM_WRITE | BT_ATT_PERM_WRITE_SECURE; } else if (!strcmp("authorize", flag)) { *req_prep_authorization = true; + } else if (!strcmp("encrypt-notify", flag)) { + *perm |= BT_ATT_PERM_SERVER_INITIATED_UPDATE_ENCRYPT; + *props |= BT_GATT_CHRC_PROP_NOTIFY; + } else if (!strcmp("encrypt-authenticated-notify", flag)) { + *perm |= BT_ATT_PERM_SERVER_INITIATED_UPDATE_AUTHEN; + *props |= BT_GATT_CHRC_PROP_NOTIFY; + } else if (!strcmp("secure-notify", flag)) { + *perm |= BT_ATT_PERM_SERVER_INITIATED_UPDATE_SECURE; + *props |= BT_GATT_CHRC_PROP_NOTIFY; + } else if (!strcmp("encrypt-indicate", flag)) { + *perm |= BT_ATT_PERM_SERVER_INITIATED_UPDATE_ENCRYPT; + *props |= BT_GATT_CHRC_PROP_INDICATE; + } else if (!strcmp("encrypt-authenticated-indicate", flag)) { + *perm |= BT_ATT_PERM_SERVER_INITIATED_UPDATE_AUTHEN; + *props |= BT_GATT_CHRC_PROP_INDICATE; + } else if (!strcmp("secure-indicate", flag)) { + *perm |= BT_ATT_PERM_SERVER_INITIATED_UPDATE_SECURE; + *props |= BT_GATT_CHRC_PROP_INDICATE; } else { error("Invalid characteristic flag: %s", flag); return false; @@ -2796,7 +2830,7 @@ static bool database_add_ccc(struct external_service *service, return true; chrc->ccc = service_add_ccc(service->attrib, service->app->database, - ccc_write_cb, chrc, NULL); + ccc_write_cb, chrc, chrc->perm, NULL); if (!chrc->ccc) { error("Failed to create CCC entry for characteristic"); return false; diff --git a/src/shared/att-types.h b/src/shared/att-types.h index a08b24155..eb5def503 100644 --- a/src/shared/att-types.h +++ b/src/shared/att-types.h @@ -137,6 +137,10 @@ struct bt_att_pdu_error_rsp { BT_ATT_PERM_WRITE_AUTHEN | \ BT_ATT_PERM_WRITE_ENCRYPT | \ BT_ATT_PERM_WRITE_SECURE) +/* Permissions to be applied to the CCC*/ +#define BT_ATT_PERM_SERVER_INITIATED_UPDATE_ENCRYPT 0x0400 +#define BT_ATT_PERM_SERVER_INITIATED_UPDATE_AUTHEN 0x0800 +#define BT_ATT_PERM_SERVER_INITIATED_UPDATE_SECURE 0x1000 /* GATT Characteristic Properties Bitfield values */ #define BT_GATT_CHRC_PROP_BROADCAST 0x01 -- 2.31.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH BlueZ v3 1/2] gatt: Allow GATT server to dicate CCC permissions 2021-09-30 15:08 ` [PATCH BlueZ v3 1/2] gatt: Allow GATT server to dicate CCC permissions Dagan Martinez @ 2021-09-30 20:25 ` Luiz Augusto von Dentz 0 siblings, 0 replies; 4+ messages in thread From: Luiz Augusto von Dentz @ 2021-09-30 20:25 UTC (permalink / raw) To: Dagan Martinez; +Cc: linux-bluetooth Hi Dagan, On Thu, Sep 30, 2021 at 8:24 AM Dagan Martinez <dmartinez@starry.com> wrote: > > Allow a GATT server to impose permissions/restrictions on a CCC by > setting additional `X-notify` and `X-indicate` permissions on its > associated characteristic. > > This allows a developer to require encryption/authentication in order > for a GATT client to subscribe to server-initiated updates. > > Test procedure: > Attempt to read/write with a "low" security level on an unprotected CCC > using gatttool, and succeed > Attempt to READ with a "low" security level on an protected CCC > using gatttool, and succeed > Attempt to WRITE with a "low" security level on an protected CCC > using gatttool, and fail > Attempt to read/write while paired on a protected CCC using > `bluetoothctl`, and succeed > --- > src/gatt-database.c | 42 ++++++++++++++++++++++++++++++++++++++---- > src/shared/att-types.h | 4 ++++ > 2 files changed, 42 insertions(+), 4 deletions(-) > > diff --git a/src/gatt-database.c b/src/gatt-database.c > index 68f411ba4..fd4a39166 100644 > --- a/src/gatt-database.c > +++ b/src/gatt-database.c > @@ -1060,17 +1060,33 @@ service_add_ccc(struct gatt_db_attribute *service, > struct btd_gatt_database *database, > btd_gatt_database_ccc_write_t write_callback, > void *user_data, > + uint32_t parent_permissions, > btd_gatt_database_destroy_t destroy) > { > struct gatt_db_attribute *ccc; > struct ccc_cb_data *ccc_cb; > bt_uuid_t uuid; > + uint32_t permissions; > > ccc_cb = new0(struct ccc_cb_data, 1); > > + /* > + * Provide a way for the permissions on a characteristic to dictate > + * the permissions on the CCC > + */ > + permissions = BT_ATT_PERM_READ | BT_ATT_PERM_WRITE; > + > + if (parent_permissions & BT_ATT_PERM_SERVER_INITIATED_UPDATE_ENCRYPT) > + permissions |= BT_ATT_PERM_WRITE_ENCRYPT; > + > + if (parent_permissions & BT_ATT_PERM_SERVER_INITIATED_UPDATE_AUTHEN) > + permissions |= BT_ATT_PERM_WRITE_AUTHEN; > + > + if (parent_permissions & BT_ATT_PERM_SERVER_INITIATED_UPDATE_SECURE) > + permissions |= BT_ATT_PERM_WRITE_SECURE; > + > bt_uuid16_create(&uuid, GATT_CLIENT_CHARAC_CFG_UUID); > - ccc = gatt_db_service_add_descriptor(service, &uuid, > - BT_ATT_PERM_READ | BT_ATT_PERM_WRITE, > + ccc = gatt_db_service_add_descriptor(service, &uuid, permissions, > gatt_ccc_read_cb, gatt_ccc_write_cb, database); > if (!ccc) { > error("Failed to create CCC entry in database"); > @@ -1227,7 +1243,7 @@ static void populate_gatt_service(struct btd_gatt_database *database) > NULL, NULL, database); > > database->svc_chngd_ccc = service_add_ccc(service, database, NULL, NULL, > - NULL); > + 0, NULL); > > bt_uuid16_create(&uuid, GATT_CHARAC_CLI_FEAT); > database->cli_feat = gatt_db_service_add_characteristic(service, > @@ -1690,6 +1706,24 @@ static bool parse_chrc_flags(DBusMessageIter *array, uint8_t *props, > *perm |= BT_ATT_PERM_WRITE | BT_ATT_PERM_WRITE_SECURE; > } else if (!strcmp("authorize", flag)) { > *req_prep_authorization = true; > + } else if (!strcmp("encrypt-notify", flag)) { > + *perm |= BT_ATT_PERM_SERVER_INITIATED_UPDATE_ENCRYPT; > + *props |= BT_GATT_CHRC_PROP_NOTIFY; > + } else if (!strcmp("encrypt-authenticated-notify", flag)) { > + *perm |= BT_ATT_PERM_SERVER_INITIATED_UPDATE_AUTHEN; > + *props |= BT_GATT_CHRC_PROP_NOTIFY; > + } else if (!strcmp("secure-notify", flag)) { > + *perm |= BT_ATT_PERM_SERVER_INITIATED_UPDATE_SECURE; > + *props |= BT_GATT_CHRC_PROP_NOTIFY; > + } else if (!strcmp("encrypt-indicate", flag)) { > + *perm |= BT_ATT_PERM_SERVER_INITIATED_UPDATE_ENCRYPT; > + *props |= BT_GATT_CHRC_PROP_INDICATE; > + } else if (!strcmp("encrypt-authenticated-indicate", flag)) { > + *perm |= BT_ATT_PERM_SERVER_INITIATED_UPDATE_AUTHEN; > + *props |= BT_GATT_CHRC_PROP_INDICATE; > + } else if (!strcmp("secure-indicate", flag)) { > + *perm |= BT_ATT_PERM_SERVER_INITIATED_UPDATE_SECURE; > + *props |= BT_GATT_CHRC_PROP_INDICATE; > } else { > error("Invalid characteristic flag: %s", flag); > return false; > @@ -2796,7 +2830,7 @@ static bool database_add_ccc(struct external_service *service, > return true; > > chrc->ccc = service_add_ccc(service->attrib, service->app->database, > - ccc_write_cb, chrc, NULL); > + ccc_write_cb, chrc, chrc->perm, NULL); > if (!chrc->ccc) { > error("Failed to create CCC entry for characteristic"); > return false; > diff --git a/src/shared/att-types.h b/src/shared/att-types.h > index a08b24155..eb5def503 100644 > --- a/src/shared/att-types.h > +++ b/src/shared/att-types.h > @@ -137,6 +137,10 @@ struct bt_att_pdu_error_rsp { > BT_ATT_PERM_WRITE_AUTHEN | \ > BT_ATT_PERM_WRITE_ENCRYPT | \ > BT_ATT_PERM_WRITE_SECURE) > +/* Permissions to be applied to the CCC*/ > +#define BT_ATT_PERM_SERVER_INITIATED_UPDATE_ENCRYPT 0x0400 > +#define BT_ATT_PERM_SERVER_INITIATED_UPDATE_AUTHEN 0x0800 > +#define BT_ATT_PERM_SERVER_INITIATED_UPDATE_SECURE 0x1000 I don't think we really need these above, how about we do something like the following: https://gist.github.com/Vudentz/af6899625df3d83b62cfbc61bbd4b94b That way we don't need to define more permissions since we can just reuse the existing one which imo are easier to understand. > /* GATT Characteristic Properties Bitfield values */ > #define BT_GATT_CHRC_PROP_BROADCAST 0x01 > -- > 2.31.1 Btw, it would be great to have an example using the new flags in the patch description, bluetoothctl> register-characteristic... -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH BlueZ v3 2/2] doc/gatt-api: Add 'X-notify`/`X-indicate` 2021-09-30 15:08 [PATCH BlueZ v3 0/2] Optionally require security for notify/indicate Dagan Martinez 2021-09-30 15:08 ` [PATCH BlueZ v3 1/2] gatt: Allow GATT server to dicate CCC permissions Dagan Martinez @ 2021-09-30 15:08 ` Dagan Martinez 1 sibling, 0 replies; 4+ messages in thread From: Dagan Martinez @ 2021-09-30 15:08 UTC (permalink / raw) To: linux-bluetooth; +Cc: Dagan Martinez Update docs to reflect the addition of the `X-notify` and `X-indicate` permissions, which allow a GATT server to restrict CCC write permissions via permissions set on its associated characteristic. --- doc/gatt-api.txt | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt index 120b23d1a..af0aa761d 100644 --- a/doc/gatt-api.txt +++ b/doc/gatt-api.txt @@ -253,7 +253,14 @@ Properties string UUID [read-only] Defines how the characteristic value can be used. See Core spec "Table 3.5: Characteristic Properties bit field", and "Table 3.8: Characteristic Extended - Properties bit field". Allowed values: + Properties bit field". + + The "x-notify" and "x-indicate" flags restrict access + to notifications and indications by imposing write + restrictions on a characteristic's client + characteristic configuration descriptor. + + Allowed values: "broadcast" "read" @@ -267,10 +274,16 @@ Properties string UUID [read-only] "writable-auxiliaries" "encrypt-read" "encrypt-write" + "encrypt-notify" (Server only) + "encrypt-indicate" (Server only) "encrypt-authenticated-read" "encrypt-authenticated-write" + "encrypt-authenticated-notify" (Server only) + "encrypt-authenticated-indicate" (Server only) "secure-read" (Server only) "secure-write" (Server only) + "secure-notify" (Server only) + "secure-indicate" (Server only) "authorize" uint16 Handle [read-write, optional] (Server Only) -- 2.31.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-09-30 20:25 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-30 15:08 [PATCH BlueZ v3 0/2] Optionally require security for notify/indicate Dagan Martinez 2021-09-30 15:08 ` [PATCH BlueZ v3 1/2] gatt: Allow GATT server to dicate CCC permissions Dagan Martinez 2021-09-30 20:25 ` Luiz Augusto von Dentz 2021-09-30 15:08 ` [PATCH BlueZ v3 2/2] doc/gatt-api: Add 'X-notify`/`X-indicate` Dagan Martinez
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).