Bluetooth: hci_event: Fix not using key encryption size when its known
authorLuiz Augusto von Dentz <luiz.von.dentz@intel.com>
Wed, 30 Apr 2025 19:07:03 +0000 (15:07 -0400)
committerLuiz Augusto von Dentz <luiz.von.dentz@intel.com>
Thu, 8 May 2025 14:24:15 +0000 (10:24 -0400)
This fixes the regression introduced by 50c1241e6a8a ("Bluetooth: l2cap:
Check encryption key size on incoming connection") introduced a check for
l2cap_check_enc_key_size which checks for hcon->enc_key_size which may
not be initialized if HCI_OP_READ_ENC_KEY_SIZE is still pending.

If the key encryption size is known, due previously reading it using
HCI_OP_READ_ENC_KEY_SIZE, then store it as part of link_key/smp_ltk
structures so the next time the encryption is changed their values are
used as conn->enc_key_size thus avoiding the racing against
HCI_OP_READ_ENC_KEY_SIZE.

Now that the enc_size is stored as part of key the information the code
then attempts to check that there is no downgrade of security if
HCI_OP_READ_ENC_KEY_SIZE returns a value smaller than what has been
previously stored.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=220061
Link: https://bugzilla.kernel.org/show_bug.cgi?id=220063
Fixes: 522e9ed157e3 ("Bluetooth: l2cap: Check encryption key size on incoming connection")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
include/net/bluetooth/hci_core.h
net/bluetooth/hci_conn.c
net/bluetooth/hci_event.c

index 522d837a23fa461703ddc5567e3257c6306318e6..54bfeeaa09959b48b4219010a27023cbcf045769 100644 (file)
@@ -1798,6 +1798,7 @@ struct hci_conn_params *hci_pend_le_action_lookup(struct list_head *list,
 void hci_uuids_clear(struct hci_dev *hdev);
 
 void hci_link_keys_clear(struct hci_dev *hdev);
+u8 *hci_conn_key_enc_size(struct hci_conn *conn);
 struct link_key *hci_find_link_key(struct hci_dev *hdev, bdaddr_t *bdaddr);
 struct link_key *hci_add_link_key(struct hci_dev *hdev, struct hci_conn *conn,
                                  bdaddr_t *bdaddr, u8 *val, u8 type,
index 6533e281ada3334b4f8d8fe36798140239aa33bd..946d2ae551f86cad9c68c41ab1a0dfd148d960cc 100644 (file)
@@ -3023,3 +3023,27 @@ void hci_conn_tx_dequeue(struct hci_conn *conn)
 
        kfree_skb(skb);
 }
+
+u8 *hci_conn_key_enc_size(struct hci_conn *conn)
+{
+       if (conn->type == ACL_LINK) {
+               struct link_key *key;
+
+               key = hci_find_link_key(conn->hdev, &conn->dst);
+               if (!key)
+                       return NULL;
+
+               return &key->pin_len;
+       } else if (conn->type == LE_LINK) {
+               struct smp_ltk *ltk;
+
+               ltk = hci_find_ltk(conn->hdev, &conn->dst, conn->dst_type,
+                                  conn->role);
+               if (!ltk)
+                       return NULL;
+
+               return &ltk->enc_size;
+       }
+
+       return NULL;
+}
index 6d6061111ac5679f019ba4c8212ea1979b8f3a99..c38ada69c3d7f25d91e148d4ab490e18caa73649 100644 (file)
@@ -739,10 +739,17 @@ static u8 hci_cc_read_enc_key_size(struct hci_dev *hdev, void *data,
                           handle);
                conn->enc_key_size = 0;
        } else {
+               u8 *key_enc_size = hci_conn_key_enc_size(conn);
+
                conn->enc_key_size = rp->key_size;
                status = 0;
 
-               if (conn->enc_key_size < hdev->min_enc_key_size) {
+               /* Attempt to check if the key size is too small or if it has
+                * been downgraded from the last time it was stored as part of
+                * the link_key.
+                */
+               if (conn->enc_key_size < hdev->min_enc_key_size ||
+                   (key_enc_size && conn->enc_key_size < *key_enc_size)) {
                        /* As slave role, the conn->state has been set to
                         * BT_CONNECTED and l2cap conn req might not be received
                         * yet, at this moment the l2cap layer almost does
@@ -755,6 +762,10 @@ static u8 hci_cc_read_enc_key_size(struct hci_dev *hdev, void *data,
                        clear_bit(HCI_CONN_ENCRYPT, &conn->flags);
                        clear_bit(HCI_CONN_AES_CCM, &conn->flags);
                }
+
+               /* Update the key encryption size with the connection one */
+               if (key_enc_size && *key_enc_size != conn->enc_key_size)
+                       *key_enc_size = conn->enc_key_size;
        }
 
        hci_encrypt_cfm(conn, status);
@@ -3065,6 +3076,34 @@ static void hci_inquiry_result_evt(struct hci_dev *hdev, void *edata,
        hci_dev_unlock(hdev);
 }
 
+static int hci_read_enc_key_size(struct hci_dev *hdev, struct hci_conn *conn)
+{
+       struct hci_cp_read_enc_key_size cp;
+       u8 *key_enc_size = hci_conn_key_enc_size(conn);
+
+       if (!read_key_size_capable(hdev)) {
+               conn->enc_key_size = HCI_LINK_KEY_SIZE;
+               return -EOPNOTSUPP;
+       }
+
+       bt_dev_dbg(hdev, "hcon %p", conn);
+
+       memset(&cp, 0, sizeof(cp));
+       cp.handle = cpu_to_le16(conn->handle);
+
+       /* If the key enc_size is already known, use it as conn->enc_key_size,
+        * otherwise use hdev->min_enc_key_size so the likes of
+        * l2cap_check_enc_key_size don't fail while waiting for
+        * HCI_OP_READ_ENC_KEY_SIZE response.
+        */
+       if (key_enc_size && *key_enc_size)
+               conn->enc_key_size = *key_enc_size;
+       else
+               conn->enc_key_size = hdev->min_enc_key_size;
+
+       return hci_send_cmd(hdev, HCI_OP_READ_ENC_KEY_SIZE, sizeof(cp), &cp);
+}
+
 static void hci_conn_complete_evt(struct hci_dev *hdev, void *data,
                                  struct sk_buff *skb)
 {
@@ -3157,23 +3196,11 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data,
                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 (!read_key_size_capable(hdev)) {
-                                       conn->enc_key_size = HCI_LINK_KEY_SIZE;
-                               } else {
-                                       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;
-                                       }
-                               }
-
+                               hci_read_enc_key_size(hdev, conn);
                                hci_encrypt_cfm(conn, ev->status);
                        }
                }
@@ -3612,24 +3639,8 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, void *data,
 
        /* Try reading the encryption key size for encrypted ACL links */
        if (!ev->status && ev->encrypt && conn->type == ACL_LINK) {
-               struct hci_cp_read_enc_key_size cp;
-
-               /* Only send HCI_Read_Encryption_Key_Size if the
-                * controller really supports it. If it doesn't, assume
-                * the default size (16).
-                */
-               if (!read_key_size_capable(hdev)) {
-                       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;
+               if (hci_read_enc_key_size(hdev, conn))
                        goto notify;
-               }
 
                goto unlock;
        }