Message ID | 20240829132507.4545-2-iulia.tanasescu@nxp.com |
---|---|
State | New |
Headers | show |
Series | client/assistant: Enter Broadcast Code as string | expand |
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)