Skip to contentSkip to conversation

Bluetooth: hci_event: Ignore NULL link key

PATCH Superseded Lee, Chun-Yi 5 participants 8 messages Jul 14, 2023 – Jul 17, 2023 2 messages not in the archive

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Bluetooth: hci_event: Ignore NULL link key

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]

---
 net/bluetooth/hci_event.c | 6 ++++++
 1 file changed, 6 insertions(+)
net/bluetooth/hci_event.c +6 -0
index 95816a938cea..e81b8d6c13ba 100644
@@ -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);
+		return;
+	}
+
 	bt_dev_dbg(hdev, "");
 
 	hci_dev_lock(hdev);
Signature · 1 line
2.35.3
#1 lore raw

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=765780

---Test result---

Test Summary:

CheckPatch                    FAIL      0.94 seconds
GitLint                       FAIL      0.54 seconds
SubjectPrefix                 PASS      0.09 seconds
BuildKernel                   PASS      33.91 seconds
CheckAllWarning               PASS      36.79 seconds
CheckSparse                   WARNING   41.59 seconds
CheckSmatch                   WARNING   113.49 seconds
BuildKernel32                 PASS      32.55 seconds
TestRunnerSetup               PASS      496.37 seconds
TestRunner_l2cap-tester       PASS      23.35 seconds
TestRunner_iso-tester         PASS      41.73 seconds
TestRunner_bnep-tester        PASS      10.61 seconds
TestRunner_mgmt-tester        PASS      217.91 seconds
TestRunner_rfcomm-tester      PASS      16.03 seconds
TestRunner_sco-tester         PASS      16.84 seconds
TestRunner_ioctl-tester       PASS      17.97 seconds
TestRunner_mesh-tester        PASS      13.39 seconds
TestRunner_smp-tester         PASS      14.57 seconds
TestRunner_userchan-tester    PASS      11.50 seconds
IncrementalBuild              PASS      31.19 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
Bluetooth: hci_event: Ignore NULL link key
WARNING: From:/Signed-off-by: email address mismatch: 'From: "Lee, Chun-Yi" <[email protected]>' != 'Signed-off-by: "Lee, Chun-Yi" <[email protected]>'

total: 0 errors, 1 warnings, 0 checks, 12 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/src/13313835.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
Bluetooth: hci_event: Ignore NULL link key

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
23: B1 Line exceeds max length (81>80): "Closes: https://ieeexplore.ieee.org/abstract/document/9474325/authors#authors [2]"
##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):

---
Regards,
Linux Bluetooth

#2 lore raw
In reply to Lee, Chun-Yi

Hi Chun-Yi,

Quoted 16 lines · Chun-Yi

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.

Quoted 7 lines

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]

Shouldn't the last 2 be using Link: instead?

Quoted 10 lines
---
 net/bluetooth/hci_event.c | 6 ++++++
 1 file changed, 6 insertions(+)
net/bluetooth/hci_event.c +0 -0
index 95816a938cea..e81b8d6c13ba 100644

net/bluetooth/hci_event.c static void hci_link_key_notify_evt(struct hci_dev *hdev, void *data, · line 4689

Quoted 6 lines
net/bluetooth/hci_event.c +3 -0
@@ -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.

Quoted 9 lines
net/bluetooth/hci_event.c +3 -1
+               return;
+       }
+
        bt_dev_dbg(hdev, "");
 
        hci_dev_lock(hdev);
--

2.35.3

Signature · 1 line
Luiz Augusto von Dentz
#3 lore raw

Hi Luiz Augusto von Dentz,

First, thanks for your review!

Quoted 23 lines · Luiz Augusto von Dentz

Hi Chun-Yi,

Quoted 20 lines · Chun-Yi

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]

Quoted 6 lines
Quoted 3 lines

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

Quoted 13 lines
Quoted 13 lines
---
 net/bluetooth/hci_event.c | 6 ++++++
 1 file changed, 6 insertions(+)
net/bluetooth/hci_event.c +0 -0
index 95816a938cea..e81b8d6c13ba 100644
@@ -4684,6 +4684,12 @@ static void hci_link_key_notify_evt(struct hci_dev *hdev, void *data,
        bool persistent;
        u8 pin_len = 0;
 
Quoted 6 lines

net/bluetooth/hci_event.c static void hci_link_key_notify_evt(struct hci_dev *hdev, void *data, · line 4689

Quoted 3 lines
net/bluetooth/hci_event.c +3 -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.

Quoted 9 lines
Quoted 9 lines
net/bluetooth/hci_event.c +3 -1
+               return;
+       }
+
        bt_dev_dbg(hdev, "");
 
        hci_dev_lock(hdev);
--

2.35.3

Thanks a lot!
Joey Lee

A message that is not in the archive (sent off-list or not yet received): [email protected]

#5 lore raw

Re: [PATCH] Bluetooth: hci_event: Ignore NULL link key in hci_link_key_notify_evt()

Hi Markus,

Thanks for your review!

Quoted 4 lines · Markus Elfring

…

Quoted 2 lines

We can ignore null link key in the handler of "Link Key Notification event" to relieve the attack. …

Quoted 9 lines

Are imperative change descriptions still preferred?

See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.5-rc1#n94

How do you think about to add the tag “Fixes” because of an added case distinction?

Sorry for I didn't capture your point. The "Link Key Notification event" is a term in bluetooth-core spec. What should I change in my patch description?

Thanks a lot!
Joey Le

A message that is not in the archive (sent off-list or not yet received): [email protected]

#7 lore raw

Re: [PATCH] Bluetooth: hci_event: Ignore NULL link key in hci_link_key_notify_evt()

Hi Markus,

Quoted 8 lines · Markus Elfring
Quoted 3 lines
Quoted 3 lines

…

Quoted 2 lines

We can ignore null link key in the handler of "Link Key Notification event" to relieve the attack. …

…

Quoted 1 line

Sorry for I didn't capture your point.

Did you provide sufficient justification for a possible addition of the tag “Fixes”?

This patch is against a CVE. The issue is not introduced by any old kernel patch. So I think it doesn't need Fixes: tag.

Quoted 7 lines
Quoted 2 lines

What should I change in my patch description?

I hope that corresponding imperative wordings can become more helpful also according to another Linux development requirement.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.5-rc1#n94

Thanks a lot!
Joey LEe

#8 lore raw

Re: [PATCH] Bluetooth: hci_event: Ignore NULL link key in hci_link_key_notify_evt()

Quoted 6 lines · joeyli

Hi Markus,

Quoted 3 lines · Markus Elfring
Quoted 3 lines
Quoted 3 lines

…

Quoted 2 lines

We can ignore null link key in the handler of "Link Key Notification event" to relieve the attack. …

Quoted 8 lines
Quoted 5 lines

…

Quoted 1 line

Sorry for I didn't capture your point.

Did you provide sufficient justification for a possible addition of the tag “Fixes”?

This patch is against a CVE. The issue is not introduced by any old kernel patch. So I think it doesn't need Fixes: tag.

You should probably put a Fixes tag against when the feature was introduced. (Kernel's prior to that were not affected by the CVE).

regards,
dan carpenter

#9 lore raw

Re: [PATCH] Bluetooth: hci_event: Ignore NULL link key in hci_link_key_notify_evt()

Hi Dan,

Quoted 10 lines · Dan Carpenter
Quoted 9 lines · joeyli

Hi Markus,

Quoted 6 lines · Markus Elfring
Quoted 3 lines
Quoted 3 lines

…

Quoted 2 lines

We can ignore null link key in the handler of "Link Key Notification event" to relieve the attack. …

…

Quoted 1 line

Sorry for I didn't capture your point.

Quoted 9 lines
Quoted 5 lines
Quoted 2 lines

Did you provide sufficient justification for a possible addition of the tag “Fixes”?

This patch is against a CVE. The issue is not introduced by any old kernel patch. So I think it doesn't need Fixes: tag.

You should probably put a Fixes tag against when the feature was introduced. (Kernel's prior to that were not affected by the CVE).

OK! I see.

I have digged that the link key stored function be introduced by 55ed8ca10f35 since v2.6.39-rc1:

commit 55ed8ca10f3530de8edbbf138acb50992bf5005b

Author: Johan Hedberg <[email protected]>
Date:   Mon Jan 17 14:41:05 2011 +0200

Bluetooth: Implement link key handling for the management interface

I will add Fixes: 55ed8ca10f35 ("Bluetooth: Implement link key handling for the management interface") in next version.

Thanks for your and Markus's reminder.

Joey Lee