* [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 related [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
* [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 related [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 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).