| Message ID | 20240829132507.4545-2-iulia.tanasescu@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 2ec7799354968f66dd00832bb33895134029c3bb |
| Headers | show |
| Series | client/assistant: Enter Broadcast Code as string | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/BuildEll | success | Build ELL PASS |
| tedd_an/BluezMake | success | Bluez Make PASS |
| tedd_an/MakeCheck | success | Bluez Make Check PASS |
| tedd_an/MakeDistcheck | success | Make Distcheck PASS |
| tedd_an/CheckValgrind | success | Check Valgrind PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/bluezmakeextell | success | Make External ELL PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
| tedd_an/ScanBuild | success | Scan Build PASS |
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=884775 ---Test result--- Test Summary: CheckPatch PASS 1.61 seconds GitLint PASS 0.38 seconds BuildEll PASS 25.57 seconds BluezMake PASS 1900.51 seconds MakeCheck PASS 13.68 seconds MakeDistcheck PASS 187.15 seconds CheckValgrind PASS 261.31 seconds CheckSmatch PASS 366.95 seconds bluezmakeextell PASS 126.65 seconds IncrementalBuild PASS 3394.10 seconds ScanBuild PASS 1079.53 seconds --- Regards, Linux Bluetooth
Hi Iulia, On Thu, Aug 29, 2024 at 9:25 AM Iulia Tanasescu <iulia.tanasescu@nxp.com> wrote: > > Currently, the user sets the Broadcast Code as an array of bytes > when prompted from the assistant submenu. However, the Bluetooth > Core Specification requires that, on the UI level, the Broadcast > Code shall be represented as a string (Vol 3, Part C, 3.2.6). Interesting, but we could go one step further can hash the string using bt_crypto_AES-CMAC since that would always generate a 16 bits hash, we actually have done something similar for SIRK on main.conf: https://github.com/bluez/bluez/blob/master/src/main.conf#L268 > This commit makes the Broadcast Code be parsed as a string from > the assistant prompt. The bluetoothctl log below shows a Broadcast > Assistant pushing an encrypted stream to a peer: > > client/bluetoothctl > [bluetooth]# [CHG] Controller 00:60:37:31:7E:3F Pairable: yes > [bluetooth]# AdvertisementMonitor path registered > [bluetooth]# scan on > [bluetooth]# [NEW] Device 00:60:37:31:7E:3F 00-60-37-31-7E-3F > [bluetooth]# connect 00:60:37:31:7E:3F > Attempting to connect to 00:60:37:31:7E:3F > [CHG] Device 00:60:37:31:7E:3F Connected: yes > [00-60-37-31-7E-3F]# Connection successful > [00-60-37-31-7E-3F]# [NEW] Device 19:9A:7A:71:E5:8B 19-9A-7A-71-E5-8B > [00-60-37-31-7E-3F]# [NEW] Assistant > /org/bluez/hci0/src_19_9A_7A_71_E5_8B/dev_00_60_37_31_7E_3F/bis1 > [00-60-37-31-7E-3F]# assistant.push > /org/bluez/hci0/src_19_9A_7A_71_E5_8B/dev_00_60_37_31_7E_3F/bis1 > [Assistant] Enter Metadata (auto/value): a > [Assistant] Enter Broadcast Code (auto/value): Borne House > [00-60-37-31-7E-3F]# [CHG] Assistant > /org/bluez/hci0/src_19_9A_7A_71_E5_8B/dev_00_60_37_31_7E_3F/bis1 > State: pending > [00-60-37-31-7E-3F]# Assistant > /org/bluez/hci0/src_19_9A_7A_71_E5_8B/dev_00_60_37_31_7E_3F/bis1 > pushed > [00-60-37-31-7E-3F]# [CHG] Assistant > /org/bluez/hci0/src_19_9A_7A_71_E5_8B/dev_00_60_37_31_7E_3F/bis1 > State: requesting > [00-60-37-31-7E-3F]# [CHG] Assistant > /org/bluez/hci0/src_19_9A_7A_71_E5_8B/dev_00_60_37_31_7E_3F/bis1 > State: active > > The btmon log below shows the way the Broadcast Code string is converted > into a byte array and sent to the peer via GATT: > > bluetoothd[6013]: < ACL Data TX: Handle 0 flags 0x00 dlen 28 > ATT: Write Command (0x52) len 23 > Handle: 0x0040 Type: Broadcast Audio Scan Control Point (0x2bc7) > Data[21]: 02018be5717a9a1900db5e3a02ffff010100000000 > Opcode: Add Source (0x02) > Source_Address_Type: 1 > Source_Address: 19:9A:7A:71:E5:8B > Source_Adv_SID: 0 > Broadcast_ID: 0x3a5edb > PA_Sync_State: Synchronize to PA - PAST not available > PA_Interval: 0xffff > Num_Subgroups: 1 > Subgroup #0: > BIS_Sync State: 0x00000001 > > ACL Data RX: Handle 0 flags 0x01 dlen 2 > ATT: Handle Multiple Value Notification (0x23) len 24 > Length: 0x0014 > Handle: 0x003a Type: Broadcast Receive State (0x2bc8) > Data[20]: 01018be5717a9a1900db5e3a0201010000000000 > Source_ID: 1 > Source_Address_Type: 1 > Source_Address: 19:9A:7A:71:E5:8B > Source_Adv_SID: 0 > Broadcast_ID: 0x3a5edb > PA_Sync_State: Synchronized to PA > BIG_Encryption: Broadcast_Code required > Num_Subgroups: 1 > Subgroup #0: > BIS_Sync State: 0x00000000 > bluetoothd[6013]: < ACL Data TX: Handle 0 flags 0x00 dlen 25 > ATT: Write Command (0x52) len 20 > Handle: 0x0040 Type: Broadcast Audio Scan Control Point (0x2bc7) > Data[18]: 040142c3b8726e6520486f75736500000000 > Opcode: Set Broadcast_Code (0x04) > Source_ID: 1 > Broadcast_Code[16]: 426f726e6520486f7573650000000000 > > ACL Data RX: Handle 0 flags 0x01 dlen 2 > ATT: Handle Multiple Value Notification (0x23) len 24 > Length: 0x0014 > Handle: 0x003a Type: Broadcast Receive State (0x2bc8) > Data[20]: 01018be5717a9a1900db5e3a0202010100000000 > Source_ID: 1 > Source_Address_Type: 1 > Source_Address: 19:9A:7A:71:E5:8B > Source_Adv_SID: 0 > Broadcast_ID: 0x3a5edb > PA_Sync_State: Synchronized to PA > BIG_Encryption: Decrypting > Num_Subgroups: 1 > Subgroup #0: > BIS_Sync State: 0x00000001 > --- > client/assistant.c | 24 ++++++++++++++---------- > 1 file changed, 14 insertions(+), 10 deletions(-) > > diff --git a/client/assistant.c b/client/assistant.c > index 77fb78329..16e94664a 100644 > --- a/client/assistant.c > +++ b/client/assistant.c > @@ -255,28 +255,32 @@ static void push_reply(DBusMessage *message, void *user_data) > static void assistant_set_bcode_cfg(const char *input, void *user_data) > { > struct assistant_config *cfg = user_data; > - char *endptr; > - uint8_t *bcode = cfg->qos.bcast.bcode; > > if (!strcasecmp(input, "a") || !strcasecmp(input, "auto")) { > - memset(bcode, 0, BCODE_LEN); > + memset(cfg->qos.bcast.bcode, 0, BCODE_LEN); > } else { > - bcode[0] = strtol(input, &endptr, 16); > + if (strlen(input) > BCODE_LEN) { > + bt_shell_printf("Input string too long %s\n", input); > + goto fail; > + } > > - for (uint8_t i = 1; i < BCODE_LEN; i++) > - bcode[i] = strtol(endptr, &endptr, 16); > + memcpy(cfg->qos.bcast.bcode, input, strlen(input)); > } > > if (!g_dbus_proxy_method_call(cfg->proxy, "Push", > push_setup, push_reply, > cfg, NULL)) { > bt_shell_printf("Failed to push assistant\n"); > + goto fail; > + } > > - free(cfg->meta); > - g_free(cfg); > + return; > > - return bt_shell_noninteractive_quit(EXIT_FAILURE); > - } > +fail: > + free(cfg->meta); > + g_free(cfg); > + > + return bt_shell_noninteractive_quit(EXIT_FAILURE); > } > > static void assistant_set_metadata_cfg(const char *input, void *user_data) > -- > 2.39.2 >
Hi Luiz, > -----Original Message----- > From: Luiz Augusto von Dentz <luiz.dentz@gmail.com> > Sent: Thursday, August 29, 2024 6:50 PM > To: Iulia Tanasescu <iulia.tanasescu@nxp.com> > Cc: linux-bluetooth@vger.kernel.org; Claudia Cristina Draghicescu > <claudia.rosu@nxp.com>; Mihai-Octavian Urzica <mihai- > octavian.urzica@nxp.com>; Vlad Pruteanu <vlad.pruteanu@nxp.com>; Andrei > Istodorescu <andrei.istodorescu@nxp.com> > Subject: Re: [PATCH BlueZ 1/2] client/assistant: Enter Broadcast Code as > string > > Hi Iulia, > > On Thu, Aug 29, 2024 at 9:25 AM Iulia Tanasescu <iulia.tanasescu@nxp.com> > wrote: > > > > Currently, the user sets the Broadcast Code as an array of bytes when > > prompted from the assistant submenu. However, the Bluetooth Core > > Specification requires that, on the UI level, the Broadcast Code shall > > be represented as a string (Vol 3, Part C, 3.2.6). > > Interesting, but we could go one step further can hash the string using > bt_crypto_AES-CMAC since that would always generate a 16 bits hash, we > actually have done something similar for SIRK on main.conf: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu > b.com%2Fbluez%2Fbluez%2Fblob%2Fmaster%2Fsrc%2Fmain.conf%23L268 > &data=05%7C02%7Ciulia.tanasescu%40nxp.com%7C55a1c6477d7440ca5aa > 208dcc842487c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6 > 38605434214981441%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw > MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7 > C&sdata=3fX9C6T9JKo2KBn1KzJI0NfOJmFelje3sQdCv6HElYo%3D&reserved= > 0 I think this would violate the spec, which states that the string should be converted into a number simply by placing the bytes into a 128-bit value (Vol 3, Part C, 3.2.6). Also, by hashing the string, the bytes will not match the key used by the Broadcast Source to encrypt the stream, so the controller on the Scan Delegator will use an incorrect key for decrypting. > > > This commit makes the Broadcast Code be parsed as a string from the > > assistant prompt. The bluetoothctl log below shows a Broadcast > > Assistant pushing an encrypted stream to a peer: > > > > client/bluetoothctl > > [bluetooth]# [CHG] Controller 00:60:37:31:7E:3F Pairable: yes > > [bluetooth]# AdvertisementMonitor path registered [bluetooth]# scan on > > [bluetooth]# [NEW] Device 00:60:37:31:7E:3F 00-60-37-31-7E-3F > > [bluetooth]# connect 00:60:37:31:7E:3F Attempting to connect to > > 00:60:37:31:7E:3F [CHG] Device 00:60:37:31:7E:3F Connected: yes > > [00-60-37-31-7E-3F]# Connection successful [00-60-37-31-7E-3F]# [NEW] > > Device 19:9A:7A:71:E5:8B 19-9A-7A-71-E5-8B [00-60-37-31-7E-3F]# > [NEW] > > Assistant > > > /org/bluez/hci0/src_19_9A_7A_71_E5_8B/dev_00_60_37_31_7E_3F/bis1 > > [00-60-37-31-7E-3F]# assistant.push > > > /org/bluez/hci0/src_19_9A_7A_71_E5_8B/dev_00_60_37_31_7E_3F/bis1 > > [Assistant] Enter Metadata (auto/value): a [Assistant] Enter Broadcast > > Code (auto/value): Borne House [00-60-37-31-7E-3F]# [CHG] Assistant > > > /org/bluez/hci0/src_19_9A_7A_71_E5_8B/dev_00_60_37_31_7E_3F/bis1 > > State: pending > > [00-60-37-31-7E-3F]# Assistant > > > /org/bluez/hci0/src_19_9A_7A_71_E5_8B/dev_00_60_37_31_7E_3F/bis1 > > pushed > > [00-60-37-31-7E-3F]# [CHG] Assistant > > > /org/bluez/hci0/src_19_9A_7A_71_E5_8B/dev_00_60_37_31_7E_3F/bis1 > > State: requesting > > [00-60-37-31-7E-3F]# [CHG] Assistant > > > /org/bluez/hci0/src_19_9A_7A_71_E5_8B/dev_00_60_37_31_7E_3F/bis1 > > State: active > > > > The btmon log below shows the way the Broadcast Code string is > > converted into a byte array and sent to the peer via GATT: > > > > bluetoothd[6013]: < ACL Data TX: Handle 0 flags 0x00 dlen 28 > > ATT: Write Command (0x52) len 23 > > Handle: 0x0040 Type: Broadcast Audio Scan Control Point (0x2bc7) > > Data[21]: 02018be5717a9a1900db5e3a02ffff010100000000 > > Opcode: Add Source (0x02) > > Source_Address_Type: 1 > > Source_Address: 19:9A:7A:71:E5:8B > > Source_Adv_SID: 0 > > Broadcast_ID: 0x3a5edb > > PA_Sync_State: Synchronize to PA - PAST not available > > PA_Interval: 0xffff > > Num_Subgroups: 1 > > Subgroup #0: > > BIS_Sync State: 0x00000001 > > > ACL Data RX: Handle 0 flags 0x01 dlen 2 > > ATT: Handle Multiple Value Notification (0x23) len 24 > > Length: 0x0014 > > Handle: 0x003a Type: Broadcast Receive State (0x2bc8) > > Data[20]: 01018be5717a9a1900db5e3a0201010000000000 > > Source_ID: 1 > > Source_Address_Type: 1 > > Source_Address: 19:9A:7A:71:E5:8B > > Source_Adv_SID: 0 > > Broadcast_ID: 0x3a5edb > > PA_Sync_State: Synchronized to PA > > BIG_Encryption: Broadcast_Code required > > Num_Subgroups: 1 > > Subgroup #0: > > BIS_Sync State: 0x00000000 > > bluetoothd[6013]: < ACL Data TX: Handle 0 flags 0x00 dlen 25 > > ATT: Write Command (0x52) len 20 > > Handle: 0x0040 Type: Broadcast Audio Scan Control Point (0x2bc7) > > Data[18]: 040142c3b8726e6520486f75736500000000 > > Opcode: Set Broadcast_Code (0x04) > > Source_ID: 1 > > Broadcast_Code[16]: 426f726e6520486f7573650000000000 > > > ACL Data RX: Handle 0 flags 0x01 dlen 2 > > ATT: Handle Multiple Value Notification (0x23) len 24 > > Length: 0x0014 > > Handle: 0x003a Type: Broadcast Receive State (0x2bc8) > > Data[20]: 01018be5717a9a1900db5e3a0202010100000000 > > Source_ID: 1 > > Source_Address_Type: 1 > > Source_Address: 19:9A:7A:71:E5:8B > > Source_Adv_SID: 0 > > Broadcast_ID: 0x3a5edb > > PA_Sync_State: Synchronized to PA > > BIG_Encryption: Decrypting > > Num_Subgroups: 1 > > Subgroup #0: > > BIS_Sync State: 0x00000001 > > --- > > client/assistant.c | 24 ++++++++++++++---------- > > 1 file changed, 14 insertions(+), 10 deletions(-) > > > > diff --git a/client/assistant.c b/client/assistant.c index > > 77fb78329..16e94664a 100644 > > --- a/client/assistant.c > > +++ b/client/assistant.c > > @@ -255,28 +255,32 @@ static void push_reply(DBusMessage *message, > > void *user_data) static void assistant_set_bcode_cfg(const char > > *input, void *user_data) { > > struct assistant_config *cfg = user_data; > > - char *endptr; > > - uint8_t *bcode = cfg->qos.bcast.bcode; > > > > if (!strcasecmp(input, "a") || !strcasecmp(input, "auto")) { > > - memset(bcode, 0, BCODE_LEN); > > + memset(cfg->qos.bcast.bcode, 0, BCODE_LEN); > > } else { > > - bcode[0] = strtol(input, &endptr, 16); > > + if (strlen(input) > BCODE_LEN) { > > + bt_shell_printf("Input string too long %s\n", input); > > + goto fail; > > + } > > > > - for (uint8_t i = 1; i < BCODE_LEN; i++) > > - bcode[i] = strtol(endptr, &endptr, 16); > > + memcpy(cfg->qos.bcast.bcode, input, strlen(input)); > > } > > > > if (!g_dbus_proxy_method_call(cfg->proxy, "Push", > > push_setup, push_reply, > > cfg, NULL)) { > > bt_shell_printf("Failed to push assistant\n"); > > + goto fail; > > + } > > > > - free(cfg->meta); > > - g_free(cfg); > > + return; > > > > - return bt_shell_noninteractive_quit(EXIT_FAILURE); > > - } > > +fail: > > + free(cfg->meta); > > + g_free(cfg); > > + > > + return bt_shell_noninteractive_quit(EXIT_FAILURE); > > } > > > > static void assistant_set_metadata_cfg(const char *input, void > > *user_data) > > -- > > 2.39.2 > > > > > -- > Luiz Augusto von Dentz Regards, Iulia
diff --git a/client/assistant.c b/client/assistant.c index 77fb78329..16e94664a 100644 --- a/client/assistant.c +++ b/client/assistant.c @@ -255,28 +255,32 @@ static void push_reply(DBusMessage *message, void *user_data) static void assistant_set_bcode_cfg(const char *input, void *user_data) { struct assistant_config *cfg = user_data; - char *endptr; - uint8_t *bcode = cfg->qos.bcast.bcode; if (!strcasecmp(input, "a") || !strcasecmp(input, "auto")) { - memset(bcode, 0, BCODE_LEN); + memset(cfg->qos.bcast.bcode, 0, BCODE_LEN); } else { - bcode[0] = strtol(input, &endptr, 16); + if (strlen(input) > BCODE_LEN) { + bt_shell_printf("Input string too long %s\n", input); + goto fail; + } - for (uint8_t i = 1; i < BCODE_LEN; i++) - bcode[i] = strtol(endptr, &endptr, 16); + memcpy(cfg->qos.bcast.bcode, input, strlen(input)); } if (!g_dbus_proxy_method_call(cfg->proxy, "Push", push_setup, push_reply, cfg, NULL)) { bt_shell_printf("Failed to push assistant\n"); + goto fail; + } - free(cfg->meta); - g_free(cfg); + return; - return bt_shell_noninteractive_quit(EXIT_FAILURE); - } +fail: + free(cfg->meta); + g_free(cfg); + + return bt_shell_noninteractive_quit(EXIT_FAILURE); } static void assistant_set_metadata_cfg(const char *input, void *user_data)
Currently, the user sets the Broadcast Code as an array of bytes when prompted from the assistant submenu. However, the Bluetooth Core Specification requires that, on the UI level, the Broadcast Code shall be represented as a string (Vol 3, Part C, 3.2.6). This commit makes the Broadcast Code be parsed as a string from the assistant prompt. The bluetoothctl log below shows a Broadcast Assistant pushing an encrypted stream to a peer: client/bluetoothctl [bluetooth]# [CHG] Controller 00:60:37:31:7E:3F Pairable: yes [bluetooth]# AdvertisementMonitor path registered [bluetooth]# scan on [bluetooth]# [NEW] Device 00:60:37:31:7E:3F 00-60-37-31-7E-3F [bluetooth]# connect 00:60:37:31:7E:3F Attempting to connect to 00:60:37:31:7E:3F [CHG] Device 00:60:37:31:7E:3F Connected: yes [00-60-37-31-7E-3F]# Connection successful [00-60-37-31-7E-3F]# [NEW] Device 19:9A:7A:71:E5:8B 19-9A-7A-71-E5-8B [00-60-37-31-7E-3F]# [NEW] Assistant /org/bluez/hci0/src_19_9A_7A_71_E5_8B/dev_00_60_37_31_7E_3F/bis1 [00-60-37-31-7E-3F]# assistant.push /org/bluez/hci0/src_19_9A_7A_71_E5_8B/dev_00_60_37_31_7E_3F/bis1 [Assistant] Enter Metadata (auto/value): a [Assistant] Enter Broadcast Code (auto/value): Borne House [00-60-37-31-7E-3F]# [CHG] Assistant /org/bluez/hci0/src_19_9A_7A_71_E5_8B/dev_00_60_37_31_7E_3F/bis1 State: pending [00-60-37-31-7E-3F]# Assistant /org/bluez/hci0/src_19_9A_7A_71_E5_8B/dev_00_60_37_31_7E_3F/bis1 pushed [00-60-37-31-7E-3F]# [CHG] Assistant /org/bluez/hci0/src_19_9A_7A_71_E5_8B/dev_00_60_37_31_7E_3F/bis1 State: requesting [00-60-37-31-7E-3F]# [CHG] Assistant /org/bluez/hci0/src_19_9A_7A_71_E5_8B/dev_00_60_37_31_7E_3F/bis1 State: active The btmon log below shows the way the Broadcast Code string is converted into a byte array and sent to the peer via GATT: bluetoothd[6013]: < ACL Data TX: Handle 0 flags 0x00 dlen 28 ATT: Write Command (0x52) len 23 Handle: 0x0040 Type: Broadcast Audio Scan Control Point (0x2bc7) Data[21]: 02018be5717a9a1900db5e3a02ffff010100000000 Opcode: Add Source (0x02) Source_Address_Type: 1 Source_Address: 19:9A:7A:71:E5:8B Source_Adv_SID: 0 Broadcast_ID: 0x3a5edb PA_Sync_State: Synchronize to PA - PAST not available PA_Interval: 0xffff Num_Subgroups: 1 Subgroup #0: BIS_Sync State: 0x00000001 > ACL Data RX: Handle 0 flags 0x01 dlen 2 ATT: Handle Multiple Value Notification (0x23) len 24 Length: 0x0014 Handle: 0x003a Type: Broadcast Receive State (0x2bc8) Data[20]: 01018be5717a9a1900db5e3a0201010000000000 Source_ID: 1 Source_Address_Type: 1 Source_Address: 19:9A:7A:71:E5:8B Source_Adv_SID: 0 Broadcast_ID: 0x3a5edb PA_Sync_State: Synchronized to PA BIG_Encryption: Broadcast_Code required Num_Subgroups: 1 Subgroup #0: BIS_Sync State: 0x00000000 bluetoothd[6013]: < ACL Data TX: Handle 0 flags 0x00 dlen 25 ATT: Write Command (0x52) len 20 Handle: 0x0040 Type: Broadcast Audio Scan Control Point (0x2bc7) Data[18]: 040142c3b8726e6520486f75736500000000 Opcode: Set Broadcast_Code (0x04) Source_ID: 1 Broadcast_Code[16]: 426f726e6520486f7573650000000000 > ACL Data RX: Handle 0 flags 0x01 dlen 2 ATT: Handle Multiple Value Notification (0x23) len 24 Length: 0x0014 Handle: 0x003a Type: Broadcast Receive State (0x2bc8) Data[20]: 01018be5717a9a1900db5e3a0202010100000000 Source_ID: 1 Source_Address_Type: 1 Source_Address: 19:9A:7A:71:E5:8B Source_Adv_SID: 0 Broadcast_ID: 0x3a5edb PA_Sync_State: Synchronized to PA BIG_Encryption: Decrypting Num_Subgroups: 1 Subgroup #0: BIS_Sync State: 0x00000001 --- client/assistant.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)