Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-2758-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240325061841.22387-1-hui.wang@canonical.com>
In-Reply-To: <20240325061841.22387-1-hui.wang@canonical.com>
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date: Mon, 25 Mar 2024 12:59:53 -0400
Message-ID: <CABBYNZLNQtYnh=DbPV7vzmt_Aje87dBuEJrhXiJRabOr5a3O0w@mail.gmail.com>
Subject: Re: [PATCH] Bluetooth: hci_event: conn is already encryped before
 conn establishes
To: Hui Wang <hui.wang@canonical.com>
Cc: linux-bluetooth@vger.kernel.org, marcel@holtmann.org, 
	johan.hedberg@gmail.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Hui,

On Mon, Mar 25, 2024 at 2:19=E2=80=AFAM Hui Wang <hui.wang@canonical.com> w=
rote:
>
> 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.

> 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.


> 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 =3D=3D 1 && !test_bit(HCI_CONN_ENCRYPT,=
 &conn->flags) &&
> +                   ev->link_type =3D=3D ACL_LINK) {
> +                       struct link_key *key;
> +                       struct hci_cp_read_enc_key_size cp;
> +
> +                       key =3D 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 =3D HCI_LINK_K=
EY_SIZE;
> +                                       goto notify;
> +                               }
> +
> +                               cp.handle =3D cpu_to_le16(conn->handle);
> +                               if (hci_send_cmd(hdev, HCI_OP_READ_ENC_KE=
Y_SIZE,
> +                                                sizeof(cp), &cp)) {
> +                                       bt_dev_err(hdev, "sending read ke=
y size failed");
> +                                       conn->enc_key_size =3D HCI_LINK_K=
EY_SIZE;
> +                               }
> +notify:
> +                               hci_encrypt_cfm(conn, ev->status);
> +                       }
> +               }
> +
>                 /* Get remote features */
>                 if (conn->type =3D=3D ACL_LINK) {
>                         struct hci_cp_read_remote_features cp;
> --
> 2.34.1
>


--
Luiz Augusto von Dentz