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

[PATCH BlueZ v3 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 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

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

[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

[PATCH BlueZ v3 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`
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

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

[Luiz Augusto von Dentz]

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