All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/1] Bluetooth: Fix Just-Works re-pairing
@ 2021-02-08 22:05 Matias Karhumaa
  2021-02-08 22:05 ` [PATCH v2 1/1] " Matias Karhumaa
  0 siblings, 1 reply; 4+ messages in thread
From: Matias Karhumaa @ 2021-02-08 22:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz

Hi maintainers,

While updating our CI machines to 5.8 series kernel, we noticed some
regression in how Bluetooth LE Just-Works pairing works. In case Linux
acts as responder and another device tries to re-pair using Just-Works,
pairing fails due to DHKey check failure. This appears to be regression
from eed467b517e8 ("Bluetooth: fix passkey uninitialized when used").

Best regards,
Matias Karhumaa

v2: More detailed commit message, improved readability of if-clause.

Matias Karhumaa (1):
  Bluetooth: Fix Just-Works re-pairing

 net/bluetooth/smp.c | 42 +++++++++++++-----------------------------
 1 file changed, 13 insertions(+), 29 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 1/1] Bluetooth: Fix Just-Works re-pairing
  2021-02-08 22:05 [PATCH v2 0/1] Bluetooth: Fix Just-Works re-pairing Matias Karhumaa
@ 2021-02-08 22:05 ` Matias Karhumaa
  2021-02-08 23:13   ` bluez.test.bot
  2021-02-09  3:09   ` [PATCH v2 1/1] " Marcel Holtmann
  0 siblings, 2 replies; 4+ messages in thread
From: Matias Karhumaa @ 2021-02-08 22:05 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz, Ari Timonen

Fix Just-Works pairing responder role in case where LTK already exists.

Currently when another device that was previously paired with Linux
device and lost the LTK for some reason, tries to pair again using
NoInputNoOutout IO capability, pairing fails due to DHKey check failure.

Btmon snippet from failing pairing attempt, Linux side already has the
LTK:

< ACL Data TX: Handle 3585 flags 0x00 dlen 6               #12 [hci0] 38.872591
      SMP: Security Request (0x0b) len 1
        Authentication requirement: Bonding, MITM, SC, No Keypresses, CT2 (0x2d)
= bluetoothd: No cache for DE:C7:3E:59:CE:8B                          38.873677
> HCI Event: Number of Completed Packets (0x13) plen 5     #13 [hci0] 38.972258
        Num handles: 1
        Handle: 3585
        Count: 1
> HCI Event: Number of Completed Packets (0x13) plen 5     #14 [hci0] 39.072201
        Num handles: 1
        Handle: 3585
        Count: 1
> ACL Data RX: Handle 3585 flags 0x02 dlen 11              #16 [hci0] 39.171956
      SMP: Pairing Request (0x01) len 6
        IO capability: NoInputNoOutput (0x03)
        OOB data: Authentication data not present (0x00)
        Authentication requirement: Bonding, No MITM, SC, No Keypresses (0x09)
        Max encryption key size: 16
        Initiator key distribution: IdKey Sign LinkKey (0x0e)
        Responder key distribution: IdKey Sign LinkKey (0x0e)
< ACL Data TX: Handle 3585 flags 0x00 dlen 11              #17 [hci0] 39.172070
      SMP: Pairing Response (0x02) len 6
        IO capability: KeyboardDisplay (0x04)
        OOB data: Authentication data not present (0x00)
        Authentication requirement: Bonding, No MITM, SC, No Keypresses (0x09)
        Max encryption key size: 16
        Initiator key distribution: IdKey Sign LinkKey (0x0e)
        Responder key distribution: Sign LinkKey (0x0c)
> ACL Data RX: Handle 3585 flags 0x02 dlen 27              #18 [hci0] 39.371260
> ACL Data RX: Handle 3585 flags 0x01 dlen 27              #19 [hci0] 39.371550
> HCI Event: Number of Completed Packets (0x13) plen 5     #20 [hci0] 39.371891
        Num handles: 1
        Handle: 3585
        Count: 1
> ACL Data RX: Handle 3585 flags 0x01 dlen 15              #21 [hci0] 39.372120
      SMP: Pairing Public Key (0x0c) len 64
        X: ca5cb38db1955168537666917f6769235c16684dd5015b29d1f02040178a5e36
        Y: 59e440e4fe49cffb4a1d5abfd0392c088412b19a21c8799ed940e88bb1b7a844
< ACL Data TX: Handle 3585 flags 0x00 dlen 27              #22 [hci0] 39.382192
< ACL Data TX: Handle 3585 flags 0x01 dlen 27              #23 [hci0] 39.382197
< ACL Data TX: Handle 3585 flags 0x01 dlen 15              #24 [hci0] 39.382199
      SMP: Pairing Public Key (0x0c) len 64
        X: c19a87e4b8a77a38b5737aad34022cfb339ac421596e66405d0f7e4439598520
        Y: b1293924e8476082639900ea5241c9138842550b2757427b03d43be67a448409
< ACL Data TX: Handle 3585 flags 0x00 dlen 21              #25 [hci0] 39.382200
      SMP: Pairing Confirm (0x03) len 16
        Confim value: 34cb38b22d23b3a9e80f4bbc90f8efe0
> HCI Event: Number of Completed Packets (0x13) plen 5     #30 [hci0] 39.471989
        Num handles: 1
        Handle: 3585
        Count: 1
> HCI Event: Number of Completed Packets (0x13) plen 5     #31 [hci0] 39.472933
        Num handles: 1
        Handle: 3585
        Count: 1
> HCI Event: Number of Completed Packets (0x13) plen 5     #32 [hci0] 39.473930
        Num handles: 1
        Handle: 3585
        Count: 1
> ACL Data RX: Handle 3585 flags 0x02 dlen 21              #33 [hci0] 39.571354
      SMP: Pairing Random (0x04) len 16
        Random value: cccccccccccccccccccccccccccccccc
@ MGMT Event: User Confirmation R.. (0x000f) plen 12  {0x0002} [hci0] 39.571462
        LE Address: DE:C7:3E:59:CE:8B (Static)
        Confirm hint: 0x01
        Value: 0x00000000
@ MGMT Event: User Confirmation R.. (0x000f) plen 12  {0x0001} [hci0] 39.571462
        LE Address: DE:C7:3E:59:CE:8B (Static)
        Confirm hint: 0x01
        Value: 0x00000000
< ACL Data TX: Handle 3585 flags 0x00 dlen 21              #34 [hci0] 39.571482
      SMP: Pairing Random (0x04) len 16
        Random value: c57bf6866a97bfa184657f89c3c644e5
> HCI Event: Number of Completed Packets (0x13) plen 5     #35 [hci0] 39.571752
        Num handles: 1
        Handle: 3585
        Count: 1
> ACL Data RX: Handle 3585 flags 0x02 dlen 21              #37 [hci0] 39.721325
      SMP: Pairing DHKey Check (0x0d) len 16
        E: 7a264e8fa19c835ff0db5db07bec23f6
@ MGMT Event: Authentication Failed (0x0011) plen 8   {0x0002} [hci0] 39.721440
        LE Address: DE:C7:3E:59:CE:8B (Static)
        Status: Authentication Failed (0x05)
@ MGMT Event: Authentication Failed (0x0011) plen 8   {0x0001} [hci0] 39.721440
        LE Address: DE:C7:3E:59:CE:8B (Static)
        Status: Authentication Failed (0x05)
< ACL Data TX: Handle 3585 flags 0x00 dlen 6               #38 [hci0] 39.721463
      SMP: Pairing Failed (0x05) len 1
        Reason: DHKey check failed (0x0b)

DHKey check fails because one of the inputs of DHKey calculation
function smp_f6() is mackey and it is not calculated at all in this
scenario.

Commit introducing this bug was meant just for fixing uninitialized
use of passkey variable and the bug looks like accidental side effect.
The commit adds "goto confirm" statement that skips mackey calculation
in smp_cmd_pairing_random() function.

With this fix mackey is calculated in a similar way also in the case
that Linux responder already has the LTK. Mackey is calculated right
before requesting confirmation for Just-Works pairing from userspace
which in turn fixes the DHKey calculation.

Fixes: eed467b517e8 ("Bluetooth: fix passkey uninitialized when used")
Reported-by: Ari Timonen <ari.timonen@synopsys.com>
Signed-off-by: Matias Karhumaa <matias.karhumaa@gmail.com>
---
 net/bluetooth/smp.c | 42 +++++++++++++-----------------------------
 1 file changed, 13 insertions(+), 29 deletions(-)

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index b0c1ee110eff..b68fc1975e31 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -2122,7 +2122,7 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
 	struct smp_chan *smp = chan->data;
 	struct hci_conn *hcon = conn->hcon;
 	u8 *pkax, *pkbx, *na, *nb, confirm_hint;
-	u32 passkey;
+	u32 passkey = 0;
 	int err;
 
 	BT_DBG("conn %p", conn);
@@ -2174,24 +2174,6 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
 		smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(smp->prnd),
 			     smp->prnd);
 		SMP_ALLOW_CMD(smp, SMP_CMD_DHKEY_CHECK);
-
-		/* Only Just-Works pairing requires extra checks */
-		if (smp->method != JUST_WORKS)
-			goto mackey_and_ltk;
-
-		/* If there already exists long term key in local host, leave
-		 * the decision to user space since the remote device could
-		 * be legitimate or malicious.
-		 */
-		if (hci_find_ltk(hcon->hdev, &hcon->dst, hcon->dst_type,
-				 hcon->role)) {
-			/* Set passkey to 0. The value can be any number since
-			 * it'll be ignored anyway.
-			 */
-			passkey = 0;
-			confirm_hint = 1;
-			goto confirm;
-		}
 	}
 
 mackey_and_ltk:
@@ -2206,17 +2188,19 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
 			SMP_ALLOW_CMD(smp, SMP_CMD_DHKEY_CHECK);
 		}
 		return 0;
-	}
-
-	err = smp_g2(smp->tfm_cmac, pkax, pkbx, na, nb, &passkey);
-	if (err)
-		return SMP_UNSPECIFIED;
-
-	confirm_hint = 0;
-
-confirm:
-	if (smp->method == JUST_WORKS)
+	} else if (smp->method == JUST_WORKS) {
+		/* Leave the decision to user space since the remote device could
+		 * be legitimate or malicious.
+		 */
 		confirm_hint = 1;
+	} else {
+		/* Generate passkey for numeric comparison. */
+		err = smp_g2(smp->tfm_cmac, pkax, pkbx, na, nb, &passkey);
+		if (err)
+			return SMP_UNSPECIFIED;
+
+		confirm_hint = 0;
+	}
 
 	err = mgmt_user_confirm_request(hcon->hdev, &hcon->dst, hcon->type,
 					hcon->dst_type, passkey, confirm_hint);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* RE: Bluetooth: Fix Just-Works re-pairing
  2021-02-08 22:05 ` [PATCH v2 1/1] " Matias Karhumaa
@ 2021-02-08 23:13   ` bluez.test.bot
  2021-02-09  3:09   ` [PATCH v2 1/1] " Marcel Holtmann
  1 sibling, 0 replies; 4+ messages in thread
From: bluez.test.bot @ 2021-02-08 23:13 UTC (permalink / raw)
  To: linux-bluetooth, matias.karhumaa

[-- Attachment #1: Type: text/plain, Size: 2366 bytes --]

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

---Test result---

##############################
    Test: CheckPatch - FAIL
    Bluetooth: Fix Just-Works re-pairing
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#15: 
< ACL Data TX: Handle 3585 flags 0x00 dlen 6               #12 [hci0] 38.872591

WARNING: Unknown commit id 'eed467b517e8', maybe rebased or not pulled?
#119: 
Fixes: eed467b517e8 ("Bluetooth: fix passkey uninitialized when used")

total: 0 errors, 2 warnings, 0 checks, 61 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.

"[PATCH] Bluetooth: Fix Just-Works re-pairing" has style problems, please review.

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


    ##############################
    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 - PASS
    Total: 416, Passed: 402 (96.6%), Failed: 0, 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


[-- Attachment #2: l2cap-tester.log --]
[-- Type: application/octet-stream, Size: 43340 bytes --]

[-- Attachment #3: bnep-tester.log --]
[-- Type: application/octet-stream, Size: 3531 bytes --]

[-- Attachment #4: mgmt-tester.log --]
[-- Type: application/octet-stream, Size: 546677 bytes --]

[-- Attachment #5: rfcomm-tester.log --]
[-- Type: application/octet-stream, Size: 11651 bytes --]

[-- Attachment #6: sco-tester.log --]
[-- Type: application/octet-stream, Size: 9887 bytes --]

[-- Attachment #7: smp-tester.log --]
[-- Type: application/octet-stream, Size: 11798 bytes --]

[-- Attachment #8: userchan-tester.log --]
[-- Type: application/octet-stream, Size: 5428 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/1] Bluetooth: Fix Just-Works re-pairing
  2021-02-08 22:05 ` [PATCH v2 1/1] " Matias Karhumaa
  2021-02-08 23:13   ` bluez.test.bot
@ 2021-02-09  3:09   ` Marcel Holtmann
  1 sibling, 0 replies; 4+ messages in thread
From: Marcel Holtmann @ 2021-02-09  3:09 UTC (permalink / raw)
  To: Matias Karhumaa
  Cc: Bluetooth Kernel Mailing List, Johan Hedberg,
	Luiz Augusto von Dentz, Ari Timonen

Hi Matias,

> Fix Just-Works pairing responder role in case where LTK already exists.
> 
> Currently when another device that was previously paired with Linux
> device and lost the LTK for some reason, tries to pair again using
> NoInputNoOutout IO capability, pairing fails due to DHKey check failure.
> 
> Btmon snippet from failing pairing attempt, Linux side already has the
> LTK:
> 
> < ACL Data TX: Handle 3585 flags 0x00 dlen 6               #12 [hci0] 38.872591
>      SMP: Security Request (0x0b) len 1
>        Authentication requirement: Bonding, MITM, SC, No Keypresses, CT2 (0x2d)
> = bluetoothd: No cache for DE:C7:3E:59:CE:8B                          38.873677
>> HCI Event: Number of Completed Packets (0x13) plen 5     #13 [hci0] 38.972258
>        Num handles: 1
>        Handle: 3585
>        Count: 1
>> HCI Event: Number of Completed Packets (0x13) plen 5     #14 [hci0] 39.072201
>        Num handles: 1
>        Handle: 3585
>        Count: 1
>> ACL Data RX: Handle 3585 flags 0x02 dlen 11              #16 [hci0] 39.171956
>      SMP: Pairing Request (0x01) len 6
>        IO capability: NoInputNoOutput (0x03)
>        OOB data: Authentication data not present (0x00)
>        Authentication requirement: Bonding, No MITM, SC, No Keypresses (0x09)
>        Max encryption key size: 16
>        Initiator key distribution: IdKey Sign LinkKey (0x0e)
>        Responder key distribution: IdKey Sign LinkKey (0x0e)
> < ACL Data TX: Handle 3585 flags 0x00 dlen 11              #17 [hci0] 39.172070
>      SMP: Pairing Response (0x02) len 6
>        IO capability: KeyboardDisplay (0x04)
>        OOB data: Authentication data not present (0x00)
>        Authentication requirement: Bonding, No MITM, SC, No Keypresses (0x09)
>        Max encryption key size: 16
>        Initiator key distribution: IdKey Sign LinkKey (0x0e)
>        Responder key distribution: Sign LinkKey (0x0c)
>> ACL Data RX: Handle 3585 flags 0x02 dlen 27              #18 [hci0] 39.371260
>> ACL Data RX: Handle 3585 flags 0x01 dlen 27              #19 [hci0] 39.371550
>> HCI Event: Number of Completed Packets (0x13) plen 5     #20 [hci0] 39.371891
>        Num handles: 1
>        Handle: 3585
>        Count: 1
>> ACL Data RX: Handle 3585 flags 0x01 dlen 15              #21 [hci0] 39.372120
>      SMP: Pairing Public Key (0x0c) len 64
>        X: ca5cb38db1955168537666917f6769235c16684dd5015b29d1f02040178a5e36
>        Y: 59e440e4fe49cffb4a1d5abfd0392c088412b19a21c8799ed940e88bb1b7a844
> < ACL Data TX: Handle 3585 flags 0x00 dlen 27              #22 [hci0] 39.382192
> < ACL Data TX: Handle 3585 flags 0x01 dlen 27              #23 [hci0] 39.382197
> < ACL Data TX: Handle 3585 flags 0x01 dlen 15              #24 [hci0] 39.382199
>      SMP: Pairing Public Key (0x0c) len 64
>        X: c19a87e4b8a77a38b5737aad34022cfb339ac421596e66405d0f7e4439598520
>        Y: b1293924e8476082639900ea5241c9138842550b2757427b03d43be67a448409
> < ACL Data TX: Handle 3585 flags 0x00 dlen 21              #25 [hci0] 39.382200
>      SMP: Pairing Confirm (0x03) len 16
>        Confim value: 34cb38b22d23b3a9e80f4bbc90f8efe0
>> HCI Event: Number of Completed Packets (0x13) plen 5     #30 [hci0] 39.471989
>        Num handles: 1
>        Handle: 3585
>        Count: 1
>> HCI Event: Number of Completed Packets (0x13) plen 5     #31 [hci0] 39.472933
>        Num handles: 1
>        Handle: 3585
>        Count: 1
>> HCI Event: Number of Completed Packets (0x13) plen 5     #32 [hci0] 39.473930
>        Num handles: 1
>        Handle: 3585
>        Count: 1
>> ACL Data RX: Handle 3585 flags 0x02 dlen 21              #33 [hci0] 39.571354
>      SMP: Pairing Random (0x04) len 16
>        Random value: cccccccccccccccccccccccccccccccc
> @ MGMT Event: User Confirmation R.. (0x000f) plen 12  {0x0002} [hci0] 39.571462
>        LE Address: DE:C7:3E:59:CE:8B (Static)
>        Confirm hint: 0x01
>        Value: 0x00000000
> @ MGMT Event: User Confirmation R.. (0x000f) plen 12  {0x0001} [hci0] 39.571462
>        LE Address: DE:C7:3E:59:CE:8B (Static)
>        Confirm hint: 0x01
>        Value: 0x00000000
> < ACL Data TX: Handle 3585 flags 0x00 dlen 21              #34 [hci0] 39.571482
>      SMP: Pairing Random (0x04) len 16
>        Random value: c57bf6866a97bfa184657f89c3c644e5
>> HCI Event: Number of Completed Packets (0x13) plen 5     #35 [hci0] 39.571752
>        Num handles: 1
>        Handle: 3585
>        Count: 1
>> ACL Data RX: Handle 3585 flags 0x02 dlen 21              #37 [hci0] 39.721325
>      SMP: Pairing DHKey Check (0x0d) len 16
>        E: 7a264e8fa19c835ff0db5db07bec23f6
> @ MGMT Event: Authentication Failed (0x0011) plen 8   {0x0002} [hci0] 39.721440
>        LE Address: DE:C7:3E:59:CE:8B (Static)
>        Status: Authentication Failed (0x05)
> @ MGMT Event: Authentication Failed (0x0011) plen 8   {0x0001} [hci0] 39.721440
>        LE Address: DE:C7:3E:59:CE:8B (Static)
>        Status: Authentication Failed (0x05)
> < ACL Data TX: Handle 3585 flags 0x00 dlen 6               #38 [hci0] 39.721463
>      SMP: Pairing Failed (0x05) len 1
>        Reason: DHKey check failed (0x0b)
> 
> DHKey check fails because one of the inputs of DHKey calculation
> function smp_f6() is mackey and it is not calculated at all in this
> scenario.
> 
> Commit introducing this bug was meant just for fixing uninitialized
> use of passkey variable and the bug looks like accidental side effect.
> The commit adds "goto confirm" statement that skips mackey calculation
> in smp_cmd_pairing_random() function.
> 
> With this fix mackey is calculated in a similar way also in the case
> that Linux responder already has the LTK. Mackey is calculated right
> before requesting confirmation for Just-Works pairing from userspace
> which in turn fixes the DHKey calculation.
> 
> Fixes: eed467b517e8 ("Bluetooth: fix passkey uninitialized when used")
> Reported-by: Ari Timonen <ari.timonen@synopsys.com>
> Signed-off-by: Matias Karhumaa <matias.karhumaa@gmail.com>
> ---
> net/bluetooth/smp.c | 42 +++++++++++++-----------------------------
> 1 file changed, 13 insertions(+), 29 deletions(-)

thanks for the extra explanation. Now I just would prefer that we get a Tested-by and Reviewed-by tags added from at least one other person.

Regards

Marcel


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-02-09  3:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-08 22:05 [PATCH v2 0/1] Bluetooth: Fix Just-Works re-pairing Matias Karhumaa
2021-02-08 22:05 ` [PATCH v2 1/1] " Matias Karhumaa
2021-02-08 23:13   ` bluez.test.bot
2021-02-09  3:09   ` [PATCH v2 1/1] " Marcel Holtmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.