| Message ID | 20240325061841.22387-1-hui.wang@canonical.com |
|---|---|
| State | Accepted |
| Commit | 60b6277569799d143fcf12a563016d5a37e8091a |
| Headers | show |
| Series | Bluetooth: hci_event: conn is already encryped before conn establishes | expand |
Dear Hui, Thank you for your patch. Some minor nits from my side. Regarding the summary (encryp*t*ed), please describe the action of the change and not the issue. Am 25.03.24 um 07:18 schrieb Hui Wang: > We have a BT headset (Lenovo Thinkplus XT99), the pairing and > connecting has no problem, once this headset is paired, bluez will > remember this device and will auto re-connect it whenever the device > is power on. The auto re-connecting works well with Windows and power*ed* > Android, but with Linux, it always fails. Through debugging, we found > at the rfcomm connection stage, the bluetooth stack reports > "Connection refused - security block (0x0003)". > > For this device, the re-connecting negotiation process is different > from other BT headsets, it sends the Link_KEY_REQUEST command before > the CONNECT_REQUEST completes, and it doesn't send ENCRYPT_CHANGE > command during the negotiation. When the device sends the "connect > complete" to hci, the ev->encr_mode is 1. Is that in accordance with the specification or a violation? > So here in the conn_complete_evt(), if ev->encr_mode is 1, link type > is ACL and HCI_CONN_ENCRYPT is not set, we set HCI_CONN_ENCRYPT to > this conn, and update conn->enc_key_size accordingly. > > After this change, this BT headset could re-connect with Linux > successfully. Despite this being a generic issue, could you please document with what controller you tested this? Do you have any bug/issue tracker URL, you can reference? > Signed-off-by: Hui Wang <hui.wang@canonical.com> > --- > This is the btmon log for auto re-connecting negotiation: > Before applying this patch: > https://pastebin.com/NUa9RJv8 > > After applying the patch: > https://pastebin.com/GqviChTC > > net/bluetooth/hci_event.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > index 4ae224824012..0c45beb08cb2 100644 > --- a/net/bluetooth/hci_event.c > +++ b/net/bluetooth/hci_event.c > @@ -3208,6 +3208,32 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data, > if (test_bit(HCI_ENCRYPT, &hdev->flags)) > set_bit(HCI_CONN_ENCRYPT, &conn->flags); > > + /* "Link key request" completed ahead of "connect request" completes */ > + if (ev->encr_mode == 1 && !test_bit(HCI_CONN_ENCRYPT, &conn->flags) && > + ev->link_type == ACL_LINK) { > + struct link_key *key; > + struct hci_cp_read_enc_key_size cp; > + > + key = hci_find_link_key(hdev, &ev->bdaddr); > + if (key) { > + set_bit(HCI_CONN_ENCRYPT, &conn->flags); > + > + if (!(hdev->commands[20] & 0x10)) { > + conn->enc_key_size = HCI_LINK_KEY_SIZE; > + goto notify; > + } > + > + cp.handle = cpu_to_le16(conn->handle); > + if (hci_send_cmd(hdev, HCI_OP_READ_ENC_KEY_SIZE, > + sizeof(cp), &cp)) { > + bt_dev_err(hdev, "sending read key size failed"); > + conn->enc_key_size = HCI_LINK_KEY_SIZE; > + } > +notify: > + hci_encrypt_cfm(conn, ev->status); Is goto and labels necessary? > + } > + } > + > /* Get remote features */ > if (conn->type == ACL_LINK) { > struct hci_cp_read_remote_features cp; Kind regards, Paul
On 3/26/24 00:59, Luiz Augusto von Dentz wrote: > Hi Hui, > > On Mon, Mar 25, 2024 at 2:19 AM Hui Wang <hui.wang@canonical.com> wrote: >> We have a BT headset (Lenovo Thinkplus XT99), the pairing and >> connecting has no problem, once this headset is paired, bluez will >> remember this device and will auto re-connect it whenever the device >> is power on. The auto re-connecting works well with Windows and >> Android, but with Linux, it always fails. Through debugging, we found >> at the rfcomm connection stage, the bluetooth stack reports >> "Connection refused - security block (0x0003)". >> >> For this device, the re-connecting negotiation process is different >> from other BT headsets, it sends the Link_KEY_REQUEST command before >> the CONNECT_REQUEST completes, and it doesn't send ENCRYPT_CHANGE >> command during the negotiation. When the device sends the "connect >> complete" to hci, the ev->encr_mode is 1. >> >> So here in the conn_complete_evt(), if ev->encr_mode is 1, link type >> is ACL and HCI_CONN_ENCRYPT is not set, we set HCI_CONN_ENCRYPT to >> this conn, and update conn->enc_key_size accordingly. >> >> After this change, this BT headset could re-connect with Linux >> successfully. > I suspect it is doing Security Mode 3, so it establishes the > encryption _before_ the connection handle which probably disables > EncryptionChange since there is no handle at that point. That said I > don't remember what we shall do with respect to AES since the > Encryption_Enabled field can only be set to 0x00 or 0x01 so I assume > the later can only be: > > Link Level Encryption is ON with E0 for BR/EDR. Thanks for sharing this, it is sth I will study. :-) >> Signed-off-by: Hui Wang <hui.wang@canonical.com> >> --- >> This is the btmon log for auto re-connecting negotiation: >> Before applying this patch: >> https://pastebin.com/NUa9RJv8 >> >> After applying the patch: >> https://pastebin.com/GqviChTC > Lets add these to the patch description to be more informative about > the change, you can strip the unrelated traffic at the start and end > and just focus on what changes to the traffic the patches introduces. OK, will address it in the v2. Thanks, Hui. > >> net/bluetooth/hci_event.c | 26 ++++++++++++++++++++++++++ >> 1 file changed, 26 insertions(+) >> >> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c >> index 4ae224824012..0c45beb08cb2 100644 >> --- a/net/bluetooth/hci_event.c >> +++ b/net/bluetooth/hci_event.c >> @@ -3208,6 +3208,32 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data, >> if (test_bit(HCI_ENCRYPT, &hdev->flags)) >> set_bit(HCI_CONN_ENCRYPT, &conn->flags); >> >> + /* "Link key request" completed ahead of "connect request" completes */ >> + if (ev->encr_mode == 1 && !test_bit(HCI_CONN_ENCRYPT, &conn->flags) && >> + ev->link_type == ACL_LINK) { >> + struct link_key *key; >> + struct hci_cp_read_enc_key_size cp; >> + >> + key = hci_find_link_key(hdev, &ev->bdaddr); >> + if (key) { >> + set_bit(HCI_CONN_ENCRYPT, &conn->flags); >> + >> + if (!(hdev->commands[20] & 0x10)) { >> + conn->enc_key_size = HCI_LINK_KEY_SIZE; >> + goto notify; >> + } >> + >> + cp.handle = cpu_to_le16(conn->handle); >> + if (hci_send_cmd(hdev, HCI_OP_READ_ENC_KEY_SIZE, >> + sizeof(cp), &cp)) { >> + bt_dev_err(hdev, "sending read key size failed"); >> + conn->enc_key_size = HCI_LINK_KEY_SIZE; >> + } >> +notify: >> + hci_encrypt_cfm(conn, ev->status); >> + } >> + } >> + >> /* Get remote features */ >> if (conn->type == ACL_LINK) { >> struct hci_cp_read_remote_features cp; >> -- >> 2.34.1 >> > > -- > Luiz Augusto von Dentz
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 4ae224824012..0c45beb08cb2 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -3208,6 +3208,32 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data, if (test_bit(HCI_ENCRYPT, &hdev->flags)) set_bit(HCI_CONN_ENCRYPT, &conn->flags); + /* "Link key request" completed ahead of "connect request" completes */ + if (ev->encr_mode == 1 && !test_bit(HCI_CONN_ENCRYPT, &conn->flags) && + ev->link_type == ACL_LINK) { + struct link_key *key; + struct hci_cp_read_enc_key_size cp; + + key = hci_find_link_key(hdev, &ev->bdaddr); + if (key) { + set_bit(HCI_CONN_ENCRYPT, &conn->flags); + + if (!(hdev->commands[20] & 0x10)) { + conn->enc_key_size = HCI_LINK_KEY_SIZE; + goto notify; + } + + cp.handle = cpu_to_le16(conn->handle); + if (hci_send_cmd(hdev, HCI_OP_READ_ENC_KEY_SIZE, + sizeof(cp), &cp)) { + bt_dev_err(hdev, "sending read key size failed"); + conn->enc_key_size = HCI_LINK_KEY_SIZE; + } +notify: + hci_encrypt_cfm(conn, ev->status); + } + } + /* Get remote features */ if (conn->type == ACL_LINK) { struct hci_cp_read_remote_features cp;
We have a BT headset (Lenovo Thinkplus XT99), the pairing and connecting has no problem, once this headset is paired, bluez will remember this device and will auto re-connect it whenever the device is power on. The auto re-connecting works well with Windows and Android, but with Linux, it always fails. Through debugging, we found at the rfcomm connection stage, the bluetooth stack reports "Connection refused - security block (0x0003)". For this device, the re-connecting negotiation process is different from other BT headsets, it sends the Link_KEY_REQUEST command before the CONNECT_REQUEST completes, and it doesn't send ENCRYPT_CHANGE command during the negotiation. When the device sends the "connect complete" to hci, the ev->encr_mode is 1. So here in the conn_complete_evt(), if ev->encr_mode is 1, link type is ACL and HCI_CONN_ENCRYPT is not set, we set HCI_CONN_ENCRYPT to this conn, and update conn->enc_key_size accordingly. After this change, this BT headset could re-connect with Linux successfully. Signed-off-by: Hui Wang <hui.wang@canonical.com> --- This is the btmon log for auto re-connecting negotiation: Before applying this patch: https://pastebin.com/NUa9RJv8 After applying the patch: https://pastebin.com/GqviChTC net/bluetooth/hci_event.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)