2023-10-05 21:23:42

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH 1/2] Bluetooth: hci_event: Fix using memcmp when comparing keys

From: Luiz Augusto von Dentz <[email protected]>

memcmp is not consider safe to use with cryptographic secrets:

'Do not use memcmp() to compare security critical data, such as
cryptographic secrets, because the required CPU time depends on the
number of equal bytes.'

While usage of memcmp for ZERO_KEY may not be considered a security
critical data, it can lead to more usage of memcmp with pairing keys
which could introduce more security problems.

Fixes: fe7a9da4fa54 ("Bluetooth: hci_event: Ignore NULL link key")
Signed-off-by: Luiz Augusto von Dentz <[email protected]>
---
net/bluetooth/hci_event.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 43ed691d0d90..d9c1bfb3082f 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -26,6 +26,8 @@
/* Bluetooth HCI event handling. */

#include <asm/unaligned.h>
+#include <linux/crypto.h>
+#include <crypto/algapi.h>

#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
@@ -4754,7 +4756,7 @@ static void hci_link_key_notify_evt(struct hci_dev *hdev, void *data,
goto unlock;

/* Ignore NULL link key against CVE-2020-26555 */
- if (!memcmp(ev->link_key, ZERO_KEY, HCI_LINK_KEY_SIZE)) {
+ if (!crypto_memneq(ev->link_key, ZERO_KEY, HCI_LINK_KEY_SIZE)) {
bt_dev_dbg(hdev, "Ignore NULL link key (ZERO KEY) for %pMR",
&ev->bdaddr);
hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE);
@@ -5294,8 +5296,8 @@ static u8 bredr_oob_data_present(struct hci_conn *conn)
* available, then do not declare that OOB data is
* present.
*/
- if (!memcmp(data->rand256, ZERO_KEY, 16) ||
- !memcmp(data->hash256, ZERO_KEY, 16))
+ if (!crypto_memneq(data->rand256, ZERO_KEY, 16) ||
+ !crypto_memneq(data->hash256, ZERO_KEY, 16))
return 0x00;

return 0x02;
@@ -5305,8 +5307,8 @@ static u8 bredr_oob_data_present(struct hci_conn *conn)
* not supported by the hardware, then check that if
* P-192 data values are present.
*/
- if (!memcmp(data->rand192, ZERO_KEY, 16) ||
- !memcmp(data->hash192, ZERO_KEY, 16))
+ if (!crypto_memneq(data->rand192, ZERO_KEY, 16) ||
+ !crypto_memneq(data->hash192, ZERO_KEY, 16))
return 0x00;

return 0x01;
--
2.41.0


2023-10-05 21:47:54

by bluez.test.bot

[permalink] [raw]
Subject: RE: [1/2] Bluetooth: hci_event: Fix using memcmp when comparing keys

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=790428

---Test result---

Test Summary:
CheckPatch PASS 1.49 seconds
GitLint FAIL 0.98 seconds
SubjectPrefix PASS 0.26 seconds
BuildKernel PASS 35.31 seconds
CheckAllWarning PASS 37.84 seconds
CheckSparse WARNING 43.53 seconds
CheckSmatch WARNING 116.97 seconds
BuildKernel32 PASS 33.67 seconds
TestRunnerSetup FAIL 41.23 seconds
TestRunner_l2cap-tester FAIL 0.14 seconds
TestRunner_iso-tester FAIL 0.14 seconds
TestRunner_bnep-tester FAIL 0.14 seconds
TestRunner_mgmt-tester FAIL 0.14 seconds
TestRunner_rfcomm-tester FAIL 0.14 seconds
TestRunner_sco-tester FAIL 0.13 seconds
TestRunner_ioctl-tester FAIL 0.13 seconds
TestRunner_mesh-tester FAIL 0.14 seconds
TestRunner_smp-tester FAIL 0.13 seconds
TestRunner_userchan-tester FAIL 0.14 seconds
IncrementalBuild PASS 44.86 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[2/2] Bluetooth: hci_event: Fix coding style

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
8: B3 Line contains hard tab characters (\t): "+ if (!bacmp(&hdev->bdaddr, &ev->bdaddr))"
9: B3 Line contains hard tab characters (\t): "+ {"
##############################
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):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):net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):
##############################
Test: TestRunnerSetup - FAIL
Desc: Setup kernel and bluez for test-runner
Output:
Bluez:
make[1]: *** No rule to make target 'unit/test-micp.c', needed by 'unit/test-micp.o'. Stop.
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4582: all] Error 2
##############################
Test: TestRunner_l2cap-tester - FAIL
Desc: Run l2cap-tester with test-runner
Output:
No tester found
##############################
Test: TestRunner_iso-tester - FAIL
Desc: Run iso-tester with test-runner
Output:
No tester found
##############################
Test: TestRunner_bnep-tester - FAIL
Desc: Run bnep-tester with test-runner
Output:
No tester found
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
No tester found
##############################
Test: TestRunner_rfcomm-tester - FAIL
Desc: Run rfcomm-tester with test-runner
Output:
No tester found
##############################
Test: TestRunner_sco-tester - FAIL
Desc: Run sco-tester with test-runner
Output:
No tester found
##############################
Test: TestRunner_ioctl-tester - FAIL
Desc: Run ioctl-tester with test-runner
Output:
No tester found
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
No tester found
##############################
Test: TestRunner_smp-tester - FAIL
Desc: Run smp-tester with test-runner
Output:
No tester found
##############################
Test: TestRunner_userchan-tester - FAIL
Desc: Run userchan-tester with test-runner
Output:
No tester found


---
Regards,
Linux Bluetooth

2023-10-07 01:00:50

by patchwork-bot+bluetooth

[permalink] [raw]
Subject: Re: [PATCH 1/2] Bluetooth: hci_event: Fix using memcmp when comparing keys

Hello:

This series was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:

On Thu, 5 Oct 2023 14:23:21 -0700 you wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> memcmp is not consider safe to use with cryptographic secrets:
>
> 'Do not use memcmp() to compare security critical data, such as
> cryptographic secrets, because the required CPU time depends on the
> number of equal bytes.'
>
> [...]

Here is the summary with links:
- [1/2] Bluetooth: hci_event: Fix using memcmp when comparing keys
https://git.kernel.org/bluetooth/bluetooth-next/c/0ce9e7726323
- [2/2] Bluetooth: hci_event: Fix coding style
https://git.kernel.org/bluetooth/bluetooth-next/c/f1b0dea09e3e

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html