2021-03-09 18:42:28

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH] Bluetooth: SMP: Fail if remote and local public keys are identical

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

This fails the pairing procedure when both remote and local non-debug
public keys are identical.

Signed-off-by: Luiz Augusto von Dentz <[email protected]>
---
net/bluetooth/smp.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index b0c1ee110eff..9e7e3655e4ec 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -2732,6 +2732,15 @@ static int smp_cmd_public_key(struct l2cap_conn *conn, struct sk_buff *skb)
if (skb->len < sizeof(*key))
return SMP_INVALID_PARAMS;

+ /* Check if remote and local public keys are the same and debug key is
+ * not in use.
+ */
+ if (!test_bit(SMP_FLAG_DEBUG_KEY, &smp->flags) &&
+ !memcmp(key, smp->local_pk, 64)) {
+ BT_ERR("Remote and local public keys are identical");
+ return SMP_UNSPECIFIED;
+ }
+
memcpy(smp->remote_pk, key, 64);

if (test_bit(SMP_FLAG_REMOTE_OOB, &smp->flags)) {
--
2.29.2


2021-03-09 19:25:19

by bluez.test.bot

[permalink] [raw]
Subject: RE: Bluetooth: SMP: Fail if remote and local public keys are identical

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

---Test result---

##############################
Test: CheckPatch - PASS


##############################
Test: CheckGitLint - PASS


##############################
Test: CheckBuildK - PASS


##############################
Test: CheckTestRunner: Setup - PASS


##############################
Test: CheckTestRunner: l2cap-tester - PASS
Total: 40, Passed: 34 (85.0%), Failed: 0, Not Run: 6

##############################
Test: CheckTestRunner: bnep-tester - PASS
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: mgmt-tester - FAIL
Total: 416, Passed: 399 (95.9%), Failed: 3, Not Run: 14

##############################
Test: CheckTestRunner: rfcomm-tester - PASS
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: sco-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: smp-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: userchan-tester - PASS
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0



---
Regards,
Linux Bluetooth


Attachments:
l2cap-tester.log (42.33 kB)
bnep-tester.log (3.45 kB)
mgmt-tester.log (534.30 kB)
rfcomm-tester.log (11.38 kB)
sco-tester.log (9.66 kB)
smp-tester.log (11.52 kB)
userchan-tester.log (5.30 kB)
Download all attachments

2021-03-09 19:32:41

by Matias Karhumaa

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: SMP: Fail if remote and local public keys are identical

Hi Luiz,

ti 9. maalisk. 2021 klo 20.42 Luiz Augusto von Dentz
([email protected]) kirjoitti:
>
> From: Luiz Augusto von Dentz <[email protected]>
>
> This fails the pairing procedure when both remote and local non-debug
> public keys are identical.

Would you mind elaborating why we need to disallow identical public keys?
Do you have some specific attack scenario in your mind?

>
> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
> ---
> net/bluetooth/smp.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index b0c1ee110eff..9e7e3655e4ec 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -2732,6 +2732,15 @@ static int smp_cmd_public_key(struct l2cap_conn *conn, struct sk_buff *skb)
> if (skb->len < sizeof(*key))
> return SMP_INVALID_PARAMS;
>
> + /* Check if remote and local public keys are the same and debug key is
> + * not in use.
> + */
> + if (!test_bit(SMP_FLAG_DEBUG_KEY, &smp->flags) &&
> + !memcmp(key, smp->local_pk, 64)) {
> + BT_ERR("Remote and local public keys are identical");
> + return SMP_UNSPECIFIED;
> + }
> +
> memcpy(smp->remote_pk, key, 64);
>
> if (test_bit(SMP_FLAG_REMOTE_OOB, &smp->flags)) {
> --
> 2.29.2
>

Best regards,
Matias Karhumaa

2021-03-10 07:01:10

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: SMP: Fail if remote and local public keys are identical

Hi Luiz,

> This fails the pairing procedure when both remote and local non-debug
> public keys are identical.
>
> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
> ---
> net/bluetooth/smp.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index b0c1ee110eff..9e7e3655e4ec 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -2732,6 +2732,15 @@ static int smp_cmd_public_key(struct l2cap_conn *conn, struct sk_buff *skb)
> if (skb->len < sizeof(*key))
> return SMP_INVALID_PARAMS;
>
> + /* Check if remote and local public keys are the same and debug key is
> + * not in use.
> + */
> + if (!test_bit(SMP_FLAG_DEBUG_KEY, &smp->flags) &&
> + !memcmp(key, smp->local_pk, 64)) {

lets use crypto_memneq here.

> + BT_ERR("Remote and local public keys are identical");

And we need to start using bt_dev_err in SMP. I will apply v2, but then we need a cleanup patch to move this to bt_dev_{err,dbg,info}.

Regards

Marcel