Bluetooth: hci_event: Ignore NULL link key
Subsystems (3)
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
@@ -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
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
Luiz Augusto von Dentz
maintainerHi 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_ADDRof 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
Signed-off-by "Lee, Chun-Yi" <[email protected]>--- net/bluetooth/hci_event.c | 6 ++++++ 1 file changed, 6 insertions(+)net/bluetooth/hci_event.c +0
-0
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
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_ADDRof 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
Signed-off-by "Lee, Chun-Yi" <[email protected]>--- net/bluetooth/hci_event.c | 6 ++++++ 1 file changed, 6 insertions(+)net/bluetooth/hci_event.c +0
-0@@ -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_dbginstead.
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]
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?
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]
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
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
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