Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230718034337.23502-1-jlee@suse.com>
In-Reply-To: <20230718034337.23502-1-jlee@suse.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Tue, 18 Jul 2023 10:22:26 -0700
Message-ID: <CABBYNZJ97UMyZ7yX1YAGbuU4XwNDdoFewKNwbd=51_L9aNrrCQ@mail.gmail.com>
Subject: Re: [PATCH v2] Bluetooth: hci_event: Ignore NULL link key
To:     "Lee, Chun-Yi" <joeyli.kernel@gmail.com>
Cc:     Marcel Holtmann <marcel@holtmann.org>,
        Johan Hedberg <johan.hedberg@gmail.com>,
        "David S . Miller" <davem@davemloft.net>,
        linux-kernel@vger.kernel.org,
        Markus Elfring <Markus.Elfring@web.de>,
        Dan Carpenter <dan.carpenter@linaro.org>,
        linux-bluetooth@vger.kernel.org, "Lee, Chun-Yi" <jlee@suse.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Chun-Yi,

On Mon, Jul 17, 2023 at 8:43=E2=80=AFPM Lee, Chun-Yi <joeyli.kernel@gmail.c=
om> wrote:
>
> This change is used to relieve CVE-2020-26555. The description of the
> CVE:
>
> Bluetooth legacy BR/EDR PIN code pairing in Bluetooth Core Specification
> 1.0B through 5.2 may permit an unauthenticated nearby device to spoof
> the BD_ADDR of the peer device to complete pairing without knowledge
> of the PIN. [1]

Btw, it is probably worth mentioning that in BR/EDR the key generation
is actually handled in the controller, below HCI.

> The detail of this attack is in IEEE paper:
> BlueMirror: Reflections on Bluetooth Pairing and Provisioning Protocols
> [2]
>
> It's a reflection attack. Base on the paper, attacker can induce the
> attacked target to generate null link key (zero key) without PIN code.
>
> We can ignore null link key in the handler of "Link Key Notification
> event" to relieve the attack. A similar implementation also shows in
> btstack project. [3]

Perhaps we could clarify this statement by stating that if we ignore
the link key it means the stack will not consider the device is bonded
and will not persist the link key, that said the controller will still
consider it as paired, so I perhaps we should go one step forward and
disconnect if we detect such a key is being used.

> v2:
> - Used Link: tag instead of Closes:
> - Used bt_dev_dbg instead of BT_DBG
> - Added Fixes: tag
>
> Fixes: 55ed8ca10f35 ("Bluetooth: Implement link key handling for the mana=
gement interface")
> Link: https://cve.mitre.org/cgi-bin/cvename.cgi?name=3DCVE-2020-26555 [1]
> Link: https://ieeexplore.ieee.org/abstract/document/9474325/authors#autho=
rs [2]
> Link: https://github.com/bluekitchen/btstack/blob/master/src/hci.c#L3722 =
[3]
> Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
> ---
>  net/bluetooth/hci_event.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 95816a938cea..ff0c331f53d6 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -4684,6 +4684,12 @@ static void hci_link_key_notify_evt(struct hci_dev=
 *hdev, void *data,
>         bool persistent;
>         u8 pin_len =3D 0;
>
> +       /* Ignore NULL link key against CVE-2020-26555 */
> +       if (!memcmp(ev->link_key, ZERO_KEY, HCI_LINK_KEY_SIZE)) {
> +               bt_dev_dbg(hdev, "Ignore NULL link key (ZERO KEY) for %pM=
R", &ev->bdaddr);
> +               return;
> +       }
> +
>         bt_dev_dbg(hdev, "");
>
>         hci_dev_lock(hdev);
> --
> 2.35.3
>


--=20
Luiz Augusto von Dentz