All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH BlueZ 0/2] Optionally require security for notify/indicate
@ 2021-09-29 16:16 Dagan Martinez
  2021-09-29 16:16 ` [PATCH BlueZ 1/2] gatt: allow GATT server to dicate CCC permissions Dagan Martinez
  2021-09-29 16:16 ` [PATCH BlueZ 2/2] doc/gatt-api: Add 'X-asynchronous` permissions Dagan Martinez
  0 siblings, 2 replies; 5+ messages in thread
From: Dagan Martinez @ 2021-09-29 16:16 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-asynchronous`
flags that allow a user to restrict access to a characteristic's CCCD
(as well as documentation for those flags).

Dagan Martinez (2):
  gatt: allow GATT server to dicate CCC permissions
  doc/gatt-api: Add 'X-asynchronous` permissions

 doc/gatt-api.txt       | 11 ++++++++++-
 src/gatt-database.c    | 29 +++++++++++++++++++++++++----
 src/shared/att-types.h |  4 ++++
 3 files changed, 39 insertions(+), 5 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH BlueZ 1/2] gatt: allow GATT server to dicate CCC permissions
  2021-09-29 16:16 [PATCH BlueZ 0/2] Optionally require security for notify/indicate Dagan Martinez
@ 2021-09-29 16:16 ` Dagan Martinez
  2021-09-29 16:49   ` Optionally require security for notify/indicate bluez.test.bot
  2021-09-29 16:16 ` [PATCH BlueZ 2/2] doc/gatt-api: Add 'X-asynchronous` permissions Dagan Martinez
  1 sibling, 1 reply; 5+ messages in thread
From: Dagan Martinez @ 2021-09-29 16:16 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Dagan Martinez

Allow a GATT server to impose permissions/restrictions on a CCC by
setting additional `X-asynchronous` 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    | 29 +++++++++++++++++++++++++----
 src/shared/att-types.h |  4 ++++
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/src/gatt-database.c b/src/gatt-database.c
index 1f7ce5f02..bb65656bd 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -1044,6 +1044,7 @@ 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;
@@ -1052,9 +1053,23 @@ 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
+	 */
+	uint32_t permissions = BT_ATT_PERM_READ | BT_ATT_PERM_WRITE;
+
+	if (parent_permissions & BT_ATT_PERM_ASYNCHRONOUS_ENCRYPT)
+		permissions |= BT_ATT_PERM_WRITE_ENCRYPT;
+
+	if (parent_permissions & BT_ATT_PERM_ASYNCHRONOUS_AUTHEN)
+		permissions |= BT_ATT_PERM_WRITE_AUTHEN;
+
+	if (parent_permissions & BT_ATT_PERM_ASYNCHRONOUS_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");
@@ -1211,7 +1226,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,
@@ -1674,6 +1689,12 @@ 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-asynchronous", flag)) {
+			*perm |= BT_ATT_PERM_ASYNCHRONOUS_ENCRYPT;
+		} else if (!strcmp("encrypt-authenticated-asynchronous", flag)) {
+			*perm |= BT_ATT_PERM_ASYNCHRONOUS_AUTHEN;
+		} else if (!strcmp("secure-asynchronous", flag)) {
+			*perm |= BT_ATT_PERM_ASYNCHRONOUS_SECURE;
 		} else {
 			error("Invalid characteristic flag: %s", flag);
 			return false;
@@ -2773,7 +2794,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..554441aca 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_ASYNCHRONOUS_ENCRYPT 0x0400
+#define BT_ATT_PERM_ASYNCHRONOUS_AUTHEN 0x0800
+#define BT_ATT_PERM_ASYNCHRONOUS_SECURE 0x1000
 
 /* GATT Characteristic Properties Bitfield values */
 #define BT_GATT_CHRC_PROP_BROADCAST			0x01
-- 
2.31.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH BlueZ 2/2] doc/gatt-api: Add 'X-asynchronous` permissions
  2021-09-29 16:16 [PATCH BlueZ 0/2] Optionally require security for notify/indicate Dagan Martinez
  2021-09-29 16:16 ` [PATCH BlueZ 1/2] gatt: allow GATT server to dicate CCC permissions Dagan Martinez
@ 2021-09-29 16:16 ` Dagan Martinez
  2021-09-29 18:04   ` Luiz Augusto von Dentz
  1 sibling, 1 reply; 5+ messages in thread
From: Dagan Martinez @ 2021-09-29 16:16 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Dagan Martinez

Update docs to reflect the addition of `X-asynchronous` permissions,
which allow a GATT server to restrict CCC write permissions via
permissions set on its associated characteristic.
---
 doc/gatt-api.txt | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
index 04789c6d3..2550510ba 100644
--- a/doc/gatt-api.txt
+++ b/doc/gatt-api.txt
@@ -253,7 +253,13 @@ 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-asynchronous" flags allow a characteristic to impose write
+			restrictions on its client characteristic configuration descriptor,
+			if applicable, restricting access to notifications and indications.
+
+			Allowed values:
 
 				"broadcast"
 				"read"
@@ -267,10 +273,13 @@ Properties	string UUID [read-only]
 				"writable-auxiliaries"
 				"encrypt-read"
 				"encrypt-write"
+				"encrypt-asynchronous" (Server only)
 				"encrypt-authenticated-read"
 				"encrypt-authenticated-write"
+				"encrypt-authenticated-asynchronous" (Server only)
 				"secure-read" (Server only)
 				"secure-write" (Server only)
+				"secure-asynchronous" (Server only)
 				"authorize"
 
 		uint16 Handle [read-write, optional] (Server Only)
-- 
2.31.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: Optionally require security for notify/indicate
  2021-09-29 16:16 ` [PATCH BlueZ 1/2] gatt: allow GATT server to dicate CCC permissions Dagan Martinez
@ 2021-09-29 16:49   ` bluez.test.bot
  0 siblings, 0 replies; 5+ messages in thread
From: bluez.test.bot @ 2021-09-29 16:49 UTC (permalink / raw)
  To: linux-bluetooth, dmartinez

[-- Attachment #1: Type: text/plain, Size: 3663 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=555113

---Test result---

Test Summary:
CheckPatch                    FAIL      0.88 seconds
GitLint                       PASS      0.59 seconds
Prep - Setup ELL              PASS      44.86 seconds
Build - Prep                  PASS      0.18 seconds
Build - Configure             PASS      7.90 seconds
Build - Make                  FAIL      161.42 seconds
Make Check                    FAIL      1.37 seconds
Make Distcheck                PASS      223.29 seconds
Build w/ext ELL - Configure   PASS      7.79 seconds
Build w/ext ELL - Make        FAIL      143.08 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
[BlueZ,1/2] gatt: allow GATT server to dicate CCC permissions
WARNING:LONG_LINE: line length of 81 exceeds 80 columns
#144: FILE: src/gatt-database.c:1229:
+									0, NULL);

WARNING:LONG_LINE: line length of 81 exceeds 80 columns
#154: FILE: src/gatt-database.c:1694:
+		} else if (!strcmp("encrypt-authenticated-asynchronous", flag)) {

WARNING:LONG_LINE: line length of 86 exceeds 80 columns
#166: FILE: src/gatt-database.c:2797:
+						ccc_write_cb, chrc, chrc->perm, NULL);

/github/workspace/src/12526275.patch total: 0 errors, 3 warnings, 70 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/12526275.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: Build - Make - FAIL
Desc: Build the BlueZ source tree
Output:
src/gatt-database.c: In function ‘service_add_ccc’:
src/gatt-database.c:1060:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 1060 |  uint32_t permissions = BT_ATT_PERM_READ | BT_ATT_PERM_WRITE;
      |  ^~~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:9212: src/bluetoothd-gatt-database.o] Error 1
make: *** [Makefile:4151: all] Error 2


##############################
Test: Make Check - FAIL
Desc: Run 'make check'
Output:
src/gatt-database.c: In function ‘service_add_ccc’:
src/gatt-database.c:1060:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 1060 |  uint32_t permissions = BT_ATT_PERM_READ | BT_ATT_PERM_WRITE;
      |  ^~~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:9212: src/bluetoothd-gatt-database.o] Error 1
make: *** [Makefile:10443: check] Error 2


##############################
Test: Build w/ext ELL - Make - FAIL
Desc: Build BlueZ source with '--enable-external-ell' configuration
Output:
src/gatt-database.c: In function ‘service_add_ccc’:
src/gatt-database.c:1060:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 1060 |  uint32_t permissions = BT_ATT_PERM_READ | BT_ATT_PERM_WRITE;
      |  ^~~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:9212: src/bluetoothd-gatt-database.o] Error 1
make: *** [Makefile:4151: all] Error 2




---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH BlueZ 2/2] doc/gatt-api: Add 'X-asynchronous` permissions
  2021-09-29 16:16 ` [PATCH BlueZ 2/2] doc/gatt-api: Add 'X-asynchronous` permissions Dagan Martinez
@ 2021-09-29 18:04   ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2021-09-29 18:04 UTC (permalink / raw)
  To: Dagan Martinez; +Cc: linux-bluetooth

Hi Dagan,

On Wed, Sep 29, 2021 at 10:42 AM Dagan Martinez <dmartinez@starry.com> wrote:
>
> Update docs to reflect the addition of `X-asynchronous` permissions,
> which allow a GATT server to restrict CCC write permissions via
> permissions set on its associated characteristic.
> ---
>  doc/gatt-api.txt | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
> index 04789c6d3..2550510ba 100644
> --- a/doc/gatt-api.txt
> +++ b/doc/gatt-api.txt
> @@ -253,7 +253,13 @@ 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-asynchronous" flags allow a characteristic to impose write
> +                       restrictions on its client characteristic configuration descriptor,
> +                       if applicable, restricting access to notifications and indications.

Don't really like the asynchronous name, it doesn't really reflect
what we want to accomplish.

> +                       Allowed values:
>
>                                 "broadcast"
>                                 "read"
> @@ -267,10 +273,13 @@ Properties        string UUID [read-only]
>                                 "writable-auxiliaries"
>                                 "encrypt-read"
>                                 "encrypt-write"
> +                               "encrypt-asynchronous" (Server only)
>                                 "encrypt-authenticated-read"
>                                 "encrypt-authenticated-write"
> +                               "encrypt-authenticated-asynchronous" (Server only)

I think for CCC we may just inherit the read/write permissions, or
have it as encrypt-notify/encrypt-indicate, etc.

>                                 "secure-read" (Server only)
>                                 "secure-write" (Server only)
> +                               "secure-asynchronous" (Server only)
>                                 "authorize"
>
>                 uint16 Handle [read-write, optional] (Server Only)
> --
> 2.31.1
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-09-29 18:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-29 16:16 [PATCH BlueZ 0/2] Optionally require security for notify/indicate Dagan Martinez
2021-09-29 16:16 ` [PATCH BlueZ 1/2] gatt: allow GATT server to dicate CCC permissions Dagan Martinez
2021-09-29 16:49   ` Optionally require security for notify/indicate bluez.test.bot
2021-09-29 16:16 ` [PATCH BlueZ 2/2] doc/gatt-api: Add 'X-asynchronous` permissions Dagan Martinez
2021-09-29 18:04   ` Luiz Augusto von Dentz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.