Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 17 Jul 2023 13:38:03 +0800
From:   joeyli <jlee@suse.com>
To:     Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc:     "Lee, Chun-Yi" <joeyli.kernel@gmail.com>,
        Marcel Holtmann <marcel@holtmann.org>,
        Johan Hedberg <johan.hedberg@gmail.com>,
        "David S . Miller" <davem@davemloft.net>,
        linux-kernel@vger.kernel.org, linux-bluetooth@vger.kernel.org
Subject: Re: [PATCH] Bluetooth: hci_event: Ignore NULL link key
Message-ID: <20230717053803.GN5866@linux-l9pv.suse>
References: <20230714161210.20969-1-jlee@suse.com>
 <CABBYNZJ46Lt-Y4OjT7AqXczaGyGoRTzEA0gpG4Z+91GugHwsNw@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CABBYNZJ46Lt-Y4OjT7AqXczaGyGoRTzEA0gpG4Z+91GugHwsNw@mail.gmail.com>
User-Agent: Mutt/1.11.4 (2019-03-13)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?WUNNbW5NUWNTZlJpL1BCM3pPWnJ5eVYrZnZnblZpV1hVSU9JUzBLK3Rta2V5?=
 =?utf-8?B?RXorWGtMODBVY1dGTkNwWlJPc3h0dFBSZ0tGME5FMWYvczlIajJkMEwrWEtJ?=
 =?utf-8?B?QWppL2ZQVy9DeWxPbW9PZFJjNnVvYWJ2c1N2N0kvQ2VDUUJraElIOXBsaHQv?=
 =?utf-8?B?c2RrRnNZK0lhYlJGRjUxajJBdGRzWnBUYTk0bmlQYThSVjgxSko5Vm54Y0Nj?=
 =?utf-8?B?YUJTR0Q4ZCt3SXg0em9jMEdkVFJZdmZURDYybWd4TDlCaWRNMm5iWENwVVJh?=
 =?utf-8?B?b0dSaVRjRkIwM2hxUFdETEd4cXFQb3NPRmZiZVdld211RFdQeFZEYWJNUVFl?=
 =?utf-8?B?MklVN3lxdnh4MmRXbUJEMWF1VXdwVzFPS1BTOGw1OVQya3hiS3F3TEkzVWxs?=
 =?utf-8?B?bzd4NEcyL1graGFkRXB1ODJGVndabzVLaEVKWSs1S1pZVUJEanNMdS9VWjJE?=
 =?utf-8?B?TG0zNEFsMDZ3Ump3T2gvME5XSHc3TGw4ejRlMnVoZ0JwZkIvR0RPNWh5VTFW?=
 =?utf-8?B?c2g2QTBXQkNFSm1QeU1lQWlDRG02SzBCbnRDbHRLQUEwQVovYTZIdFNSb24v?=
 =?utf-8?B?dGs4SXlZaThyeGVqTmRmdkkzUEFvZnpiYWJ4QjlvS3kyYWlSUGw4S2dFc2dV?=
 =?utf-8?B?SzBKOXZsOUNFWFR1cVhxMFFNaTQzL0swQnhBWDBmYzVZR1l0YXN3ZWIzcWVa?=
 =?utf-8?B?Ni9zaE5vbXFEZFI0QlAxRnBIb2NCcDY5bmg3TUxJeGJYcXYxZkFzQUJ6RThQ?=
 =?utf-8?B?WWhTclFSNnl3cEFYbmdiVE5tdWlhQ3cyVlBSdUxQbGhSRS91Y3JCOW03QkVp?=
 =?utf-8?B?V3Y1K3Bzb0NzS0orQVdobnQ0OFd4K2RDdC9yU3ZaRUtuUEFmOEs5RmZZQVVa?=
 =?utf-8?B?QmdVbXpDMVUyU050ejZ4YmNrVXFxVDMyckdldkpLTkQyUFZSVmpLa2hpNjNh?=
 =?utf-8?B?bzU0NzZ0Q0xXelJKSWg5cVk4MU93anNnR3JOU0IwLzlHSjJNTmdTQVY0MlRP?=
 =?utf-8?B?RXh5NWFSVldmcWo1WklLN2wycnFoakpWWE0vYUdNaVJvYzM0NnJGdjlpeU5Y?=
 =?utf-8?B?NjlLNXFkYmM1V1VSazNCQ2d2eXhFbGRGcG96d3JmZURwTUZXZEpnRFRKUjVX?=
 =?utf-8?B?U1g3RDlCVFlsRVZaR2JiN0pHUUVyZGZ0ZUhaR3MzZWp4K2ZRTXJCZGJhamNY?=
 =?utf-8?B?ZEFvMkNqUGUrMVBWdDd6dWJHK2xvL1VBNk43NHg5eS8rWjIveUtxUVM0d1Fr?=
 =?utf-8?B?cU53Unc5S1JyUEpsSnhOOEtCdnJUek13MlpMRXA2QmVJYjBIbkI0Um9hRkxp?=
 =?utf-8?B?NVVSSEwzS2hvWUFPQXhVRmowb1I4Z3ZoZUxsbXF4UlBiZy8zRlJWYkkrbkpn?=
 =?utf-8?B?UE9PR0pNY3h4N3Z2M1Y2ellGazhoditrbHRacHl5K3pzWjFhOE5TMHB5R3JW?=
 =?utf-8?B?VE5Hb0VmdFRpbi9GZExoSEwzS1IyYjE4b2NabTdRNjB5OXc3MjRLZ0FKK0tz?=
 =?utf-8?B?Y1pJSmhJbzVjL3JuWGNpZXVPMDFhaldaalk0aFpQVW81SFIxTzFhMGRscGg3?=
 =?utf-8?B?M2M5d20xeS92MWYyZ0tmcU1ycEI4eXMvdmVKODVMci9FSHhnbkFKMUIwNHBu?=
 =?utf-8?B?THo2UDhZY1BYS2FiRlJKUXcxNVIwNTBQTVJaTGxGalQxTFh6ZU1BNDZVWmg4?=
 =?utf-8?B?Q0RDOHpYbHQvR0cwR3MxWkdWSVBUelMvaXBvYnh3U1Y1OVJZc0dNbm1Oa2Zm?=
 =?utf-8?B?eE5adkZHWmNUOC9iNm5qNWNLeXJtWlBqN2U2Q1FwTmNMd3R3NHNJNzRITTh5?=
 =?utf-8?B?NGc2NHZCTWVMVmp6a21EajFwWXFqRDVGQ0hDRnoyMHBoLzFGMHdSeVlYTHly?=
 =?utf-8?B?Y2EvcGtRbG94d0xvbUxzdzhvWnIxcGdkZHJGVi9PTzM3alRYZWsva1JjWlVr?=
 =?utf-8?B?VkRSZndaMHBCVWMwQ2hFb00xNGJodGE2RStrRitPNndBZE15VjR5T0I3TEVQ?=
 =?utf-8?B?UjdLQUxJSGdwamtiYzJuSEQ5Z1Axd0hpYUdUQTRzUFcvanEvTU5pOXA5NmNZ?=
 =?utf-8?B?Mlc1dkRacS9Ca2syRzRBUklrOHEvWEtDTGtOYzBWTndNSkZBRlp2SFRGczg0?=
 =?utf-8?Q?i0QE=3D?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: deea9419-78b2-43a6-ee0c-08db8688046f
X-MS-Exchange-CrossTenant-AuthSource: DB8PR04MB7164.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Jul 2023 05:38:15.2437
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: E4gdL8Y2lfjHK0AtDHNtW5BFLJ+lEgiLw8rcjI4sIyxPoA7h+5zjRNCkaMqhITUz
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DU0PR04MB9417
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-bluetooth.vger.kernel.org>
X-Mailing-List: linux-bluetooth@vger.kernel.org

Hi Luiz Augusto von Dentz,

First, thanks for your review!

On Fri, Jul 14, 2023 at 11:44:28AM -0700, Luiz Augusto von Dentz wrote:
> Hi Chun-Yi,
> 
> On Fri, Jul 14, 2023 at 9:14 AM Lee, Chun-Yi <joeyli.kernel@gmail.com> 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]
> >
> > 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]
> >
> > Closes: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-26555 [1]
> > Closes: https://ieeexplore.ieee.org/abstract/document/9474325/authors#authors [2]
> > Closes: https://github.com/bluekitchen/btstack/blob/master/src/hci.c#L3722 [3]
> 
> Shouldn't the last 2 be using Link: instead?
> 

Sorry for I confused Link: with Closes:. I will change all of them to Link: tag

> > 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..e81b8d6c13ba 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 = 0;
> >
> > +       /* Ignore NULL link key against CVE-2020-26555 */
> > +       if (!memcmp(ev->link_key, ZERO_KEY, HCI_LINK_KEY_SIZE)) {
> > +               BT_DBG("Ignore NULL link key (ZERO KEY) for %pMR", &ev->bdaddr);
> 
> Please use bt_dev_dbg instead.
>

I see! I will use bt_dev_dbg.
 
> > +               return;
> > +       }
> > +
> >         bt_dev_dbg(hdev, "");
> >
> >         hci_dev_lock(hdev);
> > --
> > 2.35.3
> >

Thanks a lot!
Joey Lee