[PATCH BlueZ v4 0/2] Optionally require security for notify/indicate

[PATCH BlueZ v4 0/2] Optionally require security for notify/indicate

[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 v4:
- Directly set flags on CCCD instead of having new intermediate
    permissions

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 | 52 ++++++++++++++++++++++++++++++++++++---------
 2 files changed, 56 insertions(+), 11 deletions(-)

-- 
2.31.1

[PATCH BlueZ v4 1/2] gatt: Allow GATT server to dicate CCC permissions

[Dagan Martinez]

Allow a GATT server to impose write permissions/restrictions on a CCC by
setting additional `X-notify` and `X-indicate` flags on its associated
characteristic.

This allows a developer to require encryption/authentication in order
for a GATT client to subscribe to server-initiated updates.

```
[bluetooth]# register-characteristic\
	4b75f0f8-1f23-46b1-900c-5bbabcd5ca93 encrypt-read,encrypt-notify

[NEW] Characteristic (Handle 0x0000)
        /org/bluez/app/service0/chrc17
        4b75f0f8-1f23-46b1-900c-5bbabcd5ca93
        Vendor specific
[/org/bluez/app/service0/chrc17] Enter value: 42
```

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 | 52 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/src/gatt-database.c b/src/gatt-database.c
index 68f411ba4..475e7873c 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -115,6 +115,7 @@ struct external_chrc {
 	uint8_t props;
 	uint8_t ext_props;
 	uint32_t perm;
+	uint32_t ccc_perm;
 	uint16_t mtu;
 	struct io *write_io;
 	struct io *notify_io;
@@ -1059,7 +1060,7 @@ static struct gatt_db_attribute *
 service_add_ccc(struct gatt_db_attribute *service,
 				struct btd_gatt_database *database,
 				btd_gatt_database_ccc_write_t write_callback,
-				void *user_data,
+				void *user_data, uint32_t perm,
 				btd_gatt_database_destroy_t destroy)
 {
 	struct gatt_db_attribute *ccc;
@@ -1068,9 +1069,14 @@ service_add_ccc(struct gatt_db_attribute *service,
 
 	ccc_cb = new0(struct ccc_cb_data, 1);
 
+	/*
+	 * Provide a way for the permissions on a characteristic to dictate
+	 * the permissions on the CCC
+	 */
+	perm |= BT_ATT_PERM_READ | BT_ATT_PERM_WRITE;
+
 	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, perm,
 				gatt_ccc_read_cb, gatt_ccc_write_cb, database);
 	if (!ccc) {
 		error("Failed to create CCC entry in database");
@@ -1227,7 +1233,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,
@@ -1633,11 +1639,18 @@ static bool incr_attr_count(struct external_service *service, uint16_t incr)
 
 static bool parse_chrc_flags(DBusMessageIter *array, uint8_t *props,
 					uint8_t *ext_props, uint32_t *perm,
+					uint32_t *ccc_perm,
 					bool *req_prep_authorization)
 {
 	const char *flag;
 
-	*props = *ext_props = 0;
+	if (!props || !ext_props || !perm || !ccc_perm)
+		return false;
+
+	*props = 0;
+	*ext_props = 0;
+	*perm = 0;
+	*ccc_perm = 0;
 
 	do {
 		if (dbus_message_iter_get_arg_type(array) != DBUS_TYPE_STRING)
@@ -1690,6 +1703,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)) {
+			*ccc_perm |= BT_ATT_PERM_WRITE_ENCRYPT;
+			*props |= BT_GATT_CHRC_PROP_NOTIFY;
+		} else if (!strcmp("encrypt-authenticated-notify", flag)) {
+			*ccc_perm |= BT_ATT_PERM_WRITE_AUTHEN;
+			*props |= BT_GATT_CHRC_PROP_NOTIFY;
+		} else if (!strcmp("secure-notify", flag)) {
+			*ccc_perm |= BT_ATT_PERM_WRITE_SECURE;
+			*props |= BT_GATT_CHRC_PROP_NOTIFY;
+		} else if (!strcmp("encrypt-indicate", flag)) {
+			*ccc_perm |= BT_ATT_PERM_WRITE_ENCRYPT;
+			*props |= BT_GATT_CHRC_PROP_INDICATE;
+		} else if (!strcmp("encrypt-authenticated-indicate", flag)) {
+			*ccc_perm |= BT_ATT_PERM_WRITE_AUTHEN;
+			*props |= BT_GATT_CHRC_PROP_INDICATE;
+		} else if (!strcmp("secure-indicate", flag)) {
+			*ccc_perm |= BT_ATT_PERM_WRITE_SECURE;
+			*props |= BT_GATT_CHRC_PROP_INDICATE;
 		} else {
 			error("Invalid characteristic flag: %s", flag);
 			return false;
@@ -1743,7 +1774,8 @@ static bool parse_desc_flags(DBusMessageIter *array, uint32_t *perm,
 }
 
 static bool parse_flags(GDBusProxy *proxy, uint8_t *props, uint8_t *ext_props,
-				uint32_t *perm, bool *req_prep_authorization)
+					    uint32_t *perm, uint32_t *ccc_perm,
+					    bool *req_prep_authorization)
 {
 	DBusMessageIter iter, array;
 	const char *iface;
@@ -1760,7 +1792,7 @@ static bool parse_flags(GDBusProxy *proxy, uint8_t *props, uint8_t *ext_props,
 	if (!strcmp(iface, GATT_DESC_IFACE))
 		return parse_desc_flags(&array, perm, req_prep_authorization);
 
-	return parse_chrc_flags(&array, props, ext_props, perm,
+	return parse_chrc_flags(&array, props, ext_props, perm, ccc_perm,
 							req_prep_authorization);
 }
 
@@ -1810,7 +1842,7 @@ static struct external_chrc *chrc_create(struct gatt_app *app,
 	 * created.
 	 */
 	if (!parse_flags(proxy, &chrc->props, &chrc->ext_props, &chrc->perm,
-					&chrc->req_prep_authorization)) {
+			&chrc->ccc_perm, &chrc->req_prep_authorization)) {
 		error("Failed to parse characteristic properties");
 		goto fail;
 	}
@@ -1892,7 +1924,7 @@ static struct external_desc *desc_create(struct gatt_app *app,
 	 * Parse descriptors flags here since they are used to
 	 * determine the permission the descriptor should have
 	 */
-	if (!parse_flags(proxy, NULL, NULL, &desc->perm,
+	if (!parse_flags(proxy, NULL, NULL, &desc->perm, NULL,
 					&desc->req_prep_authorization)) {
 		error("Failed to parse characteristic properties");
 		goto fail;
@@ -2796,7 +2828,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->ccc_perm, NULL);
 	if (!chrc->ccc) {
 		error("Failed to create CCC entry for characteristic");
 		return false;
-- 
2.31.1

[PATCH BlueZ v4 2/2] doc/gatt-api: Add 'X-notify`/`X-indicate`

[Dagan Martinez]

Update docs to reflect the addition of the `X-notify` and `X-indicate`
characteristic flags, which allow a GATT server to restrict CCC write
permissions.
---
 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

Re: [PATCH BlueZ v4 0/2] Optionally require security for notify/indicate

[Luiz Augusto von Dentz]

Hi Dagan,

On Fri, Oct 1, 2021 at 7:19 AM Dagan Martinez <dmartinez@starry.com> wrote:
>
> 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 v4:
> - Directly set flags on CCCD instead of having new intermediate
>     permissions
>
> 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 | 52 ++++++++++++++++++++++++++++++++++++---------
>  2 files changed, 56 insertions(+), 11 deletions(-)
>
> --
> 2.31.1

Applied, thanks.

-- 
Luiz Augusto von Dentz