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

STALE1805d

Revision v3 of 4 in this series.

4 messages, 2 authors, 2021-09-30 · open the first message on its own page

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

From: Dagan Martinez <hidden>
Date: 2021-09-30 15:08:55

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 2/2] doc/gatt-api: Add 'X-notify`/`X-indicate`

From: Dagan Martinez <hidden>
Date: 2021-09-30 15:08:53

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

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

From: Dagan Martinez <hidden>
Date: 2021-09-30 15:08:54

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

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

From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date: 2021-09-30 20:25:36

Hi Dagan,

On Thu, Sep 30, 2021 at 8:24 AM Dagan Martinez [off-list ref] wrote:
quoted hunk
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help