linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: MGMT/SMP: Fix address type when using smp over BREDR
@ 2023-12-05 11:49 Xiao Yao
  2023-12-05 12:40 ` bluez.test.bot
  2023-12-05 18:40 ` [PATCH] " Luiz Augusto von Dentz
  0 siblings, 2 replies; 3+ messages in thread
From: Xiao Yao @ 2023-12-05 11:49 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: xiaoyao

From: Xiao Yao <xiaoyao@rock-chips.com>

When using SMP over BREDR, the identity address(irk/ltk/csrk) is
distributed during the SMP key distribution phase.

> ACL Data RX: Handle 11 flags 0x02 dlen 12
	BR/EDR SMP: Identity Address Information (0x09) len 7
	Address: F8:7D:76:XX:XX:XX (OUI F8-7D-76)
@ MGMT Event: New Identity Resolving Key (0x0018) plen 30
	Store hint: Yes (0x01)
	Random address: 00:00:00:00:00:00 (Non-Resolvable)
	BR/EDR Address: F8:7D:76:XX:XX:XX (OUI F8-7D-76)
	Key: 30cac11ec2bbc046a3658f62xxxxxxxx
@ MGMT Event: New Long Term Key (0x000a) plen 37
        Store hint: Yes (0x01)
        LE Address: F8:7D:76:XX:XX:XX (OUI F8-7D-76)
        Key type: Authenticated key from P-256 (0x03)
        Central: 0x00
        Encryption size: 16
        Diversifier: 0000
        Randomizer: 0000000000000000
        Key: a3ca3f9e06cfa8c512edc13axxxxxxxx

In the mgmt_new_irk/ltk/csrk() function, addr.type is forcefully converted
to BDADDR_LE_PUBLIC using link_to_bdaddr(LE_LINK, irk/ltk/csrk->addr_type).
However, the actual address type should be BDADDR_BREDR. Therefore, let's
pass the actual link type to determine the address type.

Signed-off-by: Xiao Yao <xiaoyao@rock-chips.com>
---
 include/net/bluetooth/hci_core.h |  8 +++++---
 net/bluetooth/mgmt.c             | 14 ++++++++------
 net/bluetooth/smp.c              | 10 +++++-----
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index eed6c9f37b12..5088fbf4c7f5 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -2278,10 +2278,12 @@ void mgmt_suspending(struct hci_dev *hdev, u8 state);
 void mgmt_resuming(struct hci_dev *hdev, u8 reason, bdaddr_t *bdaddr,
 		   u8 addr_type);
 bool mgmt_powering_down(struct hci_dev *hdev);
-void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, bool persistent);
-void mgmt_new_irk(struct hci_dev *hdev, struct smp_irk *irk, bool persistent);
+void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, bool persistent,
+		  u8 link_type);
+void mgmt_new_irk(struct hci_dev *hdev, struct smp_irk *irk, bool persistent,
+		  u8 link_type);
 void mgmt_new_csrk(struct hci_dev *hdev, struct smp_csrk *csrk,
-		   bool persistent);
+		   bool persistent, u8 link_type);
 void mgmt_new_conn_param(struct hci_dev *hdev, bdaddr_t *bdaddr,
 			 u8 bdaddr_type, u8 store_hint, u16 min_interval,
 			 u16 max_interval, u16 latency, u16 timeout);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index da79a2369dd7..fdfb395e29ba 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -9550,7 +9550,8 @@ static u8 mgmt_ltk_type(struct smp_ltk *ltk)
 	return MGMT_LTK_UNAUTHENTICATED;
 }
 
-void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, bool persistent)
+void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, bool persistent,
+		  u8 link_type)
 {
 	struct mgmt_ev_new_long_term_key ev;
 
@@ -9574,7 +9575,7 @@ void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, bool persistent)
 		ev.store_hint = persistent;
 
 	bacpy(&ev.key.addr.bdaddr, &key->bdaddr);
-	ev.key.addr.type = link_to_bdaddr(LE_LINK, key->bdaddr_type);
+	ev.key.addr.type = link_to_bdaddr(link_type, key->bdaddr_type);
 	ev.key.type = mgmt_ltk_type(key);
 	ev.key.enc_size = key->enc_size;
 	ev.key.ediv = key->ediv;
@@ -9593,7 +9594,8 @@ void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, bool persistent)
 	mgmt_event(MGMT_EV_NEW_LONG_TERM_KEY, hdev, &ev, sizeof(ev), NULL);
 }
 
-void mgmt_new_irk(struct hci_dev *hdev, struct smp_irk *irk, bool persistent)
+void mgmt_new_irk(struct hci_dev *hdev, struct smp_irk *irk, bool persistent,
+		  u8 link_type)
 {
 	struct mgmt_ev_new_irk ev;
 
@@ -9603,14 +9605,14 @@ void mgmt_new_irk(struct hci_dev *hdev, struct smp_irk *irk, bool persistent)
 
 	bacpy(&ev.rpa, &irk->rpa);
 	bacpy(&ev.irk.addr.bdaddr, &irk->bdaddr);
-	ev.irk.addr.type = link_to_bdaddr(LE_LINK, irk->addr_type);
+	ev.irk.addr.type = link_to_bdaddr(link_type, irk->addr_type);
 	memcpy(ev.irk.val, irk->val, sizeof(irk->val));
 
 	mgmt_event(MGMT_EV_NEW_IRK, hdev, &ev, sizeof(ev), NULL);
 }
 
 void mgmt_new_csrk(struct hci_dev *hdev, struct smp_csrk *csrk,
-		   bool persistent)
+		   bool persistent, u8 link_type)
 {
 	struct mgmt_ev_new_csrk ev;
 
@@ -9632,7 +9634,7 @@ void mgmt_new_csrk(struct hci_dev *hdev, struct smp_csrk *csrk,
 		ev.store_hint = persistent;
 
 	bacpy(&ev.key.addr.bdaddr, &csrk->bdaddr);
-	ev.key.addr.type = link_to_bdaddr(LE_LINK, csrk->bdaddr_type);
+	ev.key.addr.type = link_to_bdaddr(link_type, csrk->bdaddr_type);
 	ev.key.type = csrk->type;
 	memcpy(ev.key.val, csrk->val, sizeof(csrk->val));
 
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index f1a9fc0012f0..3f640741e07b 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -1060,7 +1060,7 @@ static void smp_notify_keys(struct l2cap_conn *conn)
 	}
 
 	if (smp->remote_irk) {
-		mgmt_new_irk(hdev, smp->remote_irk, persistent);
+		mgmt_new_irk(hdev, smp->remote_irk, persistent, hcon->type);
 
 		/* Now that user space can be considered to know the
 		 * identity address track the connection based on it
@@ -1081,25 +1081,25 @@ static void smp_notify_keys(struct l2cap_conn *conn)
 	if (smp->csrk) {
 		smp->csrk->bdaddr_type = hcon->dst_type;
 		bacpy(&smp->csrk->bdaddr, &hcon->dst);
-		mgmt_new_csrk(hdev, smp->csrk, persistent);
+		mgmt_new_csrk(hdev, smp->csrk, persistent, hcon->type);
 	}
 
 	if (smp->responder_csrk) {
 		smp->responder_csrk->bdaddr_type = hcon->dst_type;
 		bacpy(&smp->responder_csrk->bdaddr, &hcon->dst);
-		mgmt_new_csrk(hdev, smp->responder_csrk, persistent);
+		mgmt_new_csrk(hdev, smp->responder_csrk, persistent, hcon->type);
 	}
 
 	if (smp->ltk) {
 		smp->ltk->bdaddr_type = hcon->dst_type;
 		bacpy(&smp->ltk->bdaddr, &hcon->dst);
-		mgmt_new_ltk(hdev, smp->ltk, persistent);
+		mgmt_new_ltk(hdev, smp->ltk, persistent, hcon->type);
 	}
 
 	if (smp->responder_ltk) {
 		smp->responder_ltk->bdaddr_type = hcon->dst_type;
 		bacpy(&smp->responder_ltk->bdaddr, &hcon->dst);
-		mgmt_new_ltk(hdev, smp->responder_ltk, persistent);
+		mgmt_new_ltk(hdev, smp->responder_ltk, persistent, hcon->type);
 	}
 
 	if (smp->link_key) {
-- 
2.34.1


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

* RE: Bluetooth: MGMT/SMP: Fix address type when using smp over BREDR
  2023-12-05 11:49 [PATCH] Bluetooth: MGMT/SMP: Fix address type when using smp over BREDR Xiao Yao
@ 2023-12-05 12:40 ` bluez.test.bot
  2023-12-05 18:40 ` [PATCH] " Luiz Augusto von Dentz
  1 sibling, 0 replies; 3+ messages in thread
From: bluez.test.bot @ 2023-12-05 12:40 UTC (permalink / raw)
  To: linux-bluetooth, xiaokeqinhealth

[-- Attachment #1: Type: text/plain, Size: 2489 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=807009

---Test result---

Test Summary:
CheckPatch                    PASS      0.77 seconds
GitLint                       FAIL      1.58 seconds
SubjectPrefix                 PASS      0.16 seconds
BuildKernel                   PASS      27.52 seconds
CheckAllWarning               PASS      30.08 seconds
CheckSparse                   PASS      35.30 seconds
CheckSmatch                   PASS      97.90 seconds
BuildKernel32                 PASS      27.38 seconds
TestRunnerSetup               PASS      417.74 seconds
TestRunner_l2cap-tester       PASS      22.78 seconds
TestRunner_iso-tester         PASS      50.71 seconds
TestRunner_bnep-tester        PASS      6.93 seconds
TestRunner_mgmt-tester        PASS      163.21 seconds
TestRunner_rfcomm-tester      PASS      10.73 seconds
TestRunner_sco-tester         PASS      14.48 seconds
TestRunner_ioctl-tester       PASS      12.04 seconds
TestRunner_mesh-tester        PASS      8.76 seconds
TestRunner_smp-tester         PASS      9.73 seconds
TestRunner_userchan-tester    PASS      7.19 seconds
IncrementalBuild              PASS      25.45 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
Bluetooth: MGMT/SMP: Fix address type when using smp over BREDR

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
9: B3 Line contains hard tab characters (\t): "	BR/EDR SMP: Identity Address Information (0x09) len 7"
10: B3 Line contains hard tab characters (\t): "	Address: F8:7D:76:XX:XX:XX (OUI F8-7D-76)"
12: B3 Line contains hard tab characters (\t): "	Store hint: Yes (0x01)"
13: B3 Line contains hard tab characters (\t): "	Random address: 00:00:00:00:00:00 (Non-Resolvable)"
14: B3 Line contains hard tab characters (\t): "	BR/EDR Address: F8:7D:76:XX:XX:XX (OUI F8-7D-76)"
15: B3 Line contains hard tab characters (\t): "	Key: 30cac11ec2bbc046a3658f62xxxxxxxx"


---
Regards,
Linux Bluetooth


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

* Re: [PATCH] Bluetooth: MGMT/SMP: Fix address type when using smp over BREDR
  2023-12-05 11:49 [PATCH] Bluetooth: MGMT/SMP: Fix address type when using smp over BREDR Xiao Yao
  2023-12-05 12:40 ` bluez.test.bot
@ 2023-12-05 18:40 ` Luiz Augusto von Dentz
  1 sibling, 0 replies; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2023-12-05 18:40 UTC (permalink / raw)
  To: Xiao Yao; +Cc: linux-bluetooth, xiaoyao

Hi,

On Tue, Dec 5, 2023 at 7:05 AM Xiao Yao <xiaokeqinhealth@126.com> wrote:
>
> From: Xiao Yao <xiaoyao@rock-chips.com>
>
> When using SMP over BREDR, the identity address(irk/ltk/csrk) is
> distributed during the SMP key distribution phase.
>
> > ACL Data RX: Handle 11 flags 0x02 dlen 12
>         BR/EDR SMP: Identity Address Information (0x09) len 7
>         Address: F8:7D:76:XX:XX:XX (OUI F8-7D-76)
> @ MGMT Event: New Identity Resolving Key (0x0018) plen 30
>         Store hint: Yes (0x01)
>         Random address: 00:00:00:00:00:00 (Non-Resolvable)
>         BR/EDR Address: F8:7D:76:XX:XX:XX (OUI F8-7D-76)
>         Key: 30cac11ec2bbc046a3658f62xxxxxxxx
> @ MGMT Event: New Long Term Key (0x000a) plen 37
>         Store hint: Yes (0x01)
>         LE Address: F8:7D:76:XX:XX:XX (OUI F8-7D-76)

So I assume the above should change to BR/EDR after applying these
changes? It probably make sense to have the trace of before and after
just to be clearer.

>         Key type: Authenticated key from P-256 (0x03)
>         Central: 0x00
>         Encryption size: 16
>         Diversifier: 0000
>         Randomizer: 0000000000000000
>         Key: a3ca3f9e06cfa8c512edc13axxxxxxxx
>
> In the mgmt_new_irk/ltk/csrk() function, addr.type is forcefully converted
> to BDADDR_LE_PUBLIC using link_to_bdaddr(LE_LINK, irk/ltk/csrk->addr_type).
> However, the actual address type should be BDADDR_BREDR. Therefore, let's
> pass the actual link type to determine the address type.
>
> Signed-off-by: Xiao Yao <xiaoyao@rock-chips.com>
> ---
>  include/net/bluetooth/hci_core.h |  8 +++++---
>  net/bluetooth/mgmt.c             | 14 ++++++++------
>  net/bluetooth/smp.c              | 10 +++++-----
>  3 files changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index eed6c9f37b12..5088fbf4c7f5 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -2278,10 +2278,12 @@ void mgmt_suspending(struct hci_dev *hdev, u8 state);
>  void mgmt_resuming(struct hci_dev *hdev, u8 reason, bdaddr_t *bdaddr,
>                    u8 addr_type);
>  bool mgmt_powering_down(struct hci_dev *hdev);
> -void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, bool persistent);
> -void mgmt_new_irk(struct hci_dev *hdev, struct smp_irk *irk, bool persistent);
> +void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, bool persistent,
> +                 u8 link_type);
> +void mgmt_new_irk(struct hci_dev *hdev, struct smp_irk *irk, bool persistent,
> +                 u8 link_type);
>  void mgmt_new_csrk(struct hci_dev *hdev, struct smp_csrk *csrk,
> -                  bool persistent);
> +                  bool persistent, u8 link_type);
>  void mgmt_new_conn_param(struct hci_dev *hdev, bdaddr_t *bdaddr,
>                          u8 bdaddr_type, u8 store_hint, u16 min_interval,
>                          u16 max_interval, u16 latency, u16 timeout);
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index da79a2369dd7..fdfb395e29ba 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -9550,7 +9550,8 @@ static u8 mgmt_ltk_type(struct smp_ltk *ltk)
>         return MGMT_LTK_UNAUTHENTICATED;
>  }
>
> -void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, bool persistent)
> +void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, bool persistent,
> +                 u8 link_type)
>  {
>         struct mgmt_ev_new_long_term_key ev;
>
> @@ -9574,7 +9575,7 @@ void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, bool persistent)
>                 ev.store_hint = persistent;
>
>         bacpy(&ev.key.addr.bdaddr, &key->bdaddr);
> -       ev.key.addr.type = link_to_bdaddr(LE_LINK, key->bdaddr_type);
> +       ev.key.addr.type = link_to_bdaddr(link_type, key->bdaddr_type);
>         ev.key.type = mgmt_ltk_type(key);
>         ev.key.enc_size = key->enc_size;
>         ev.key.ediv = key->ediv;
> @@ -9593,7 +9594,8 @@ void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, bool persistent)
>         mgmt_event(MGMT_EV_NEW_LONG_TERM_KEY, hdev, &ev, sizeof(ev), NULL);
>  }
>
> -void mgmt_new_irk(struct hci_dev *hdev, struct smp_irk *irk, bool persistent)
> +void mgmt_new_irk(struct hci_dev *hdev, struct smp_irk *irk, bool persistent,
> +                 u8 link_type)
>  {
>         struct mgmt_ev_new_irk ev;
>
> @@ -9603,14 +9605,14 @@ void mgmt_new_irk(struct hci_dev *hdev, struct smp_irk *irk, bool persistent)
>
>         bacpy(&ev.rpa, &irk->rpa);
>         bacpy(&ev.irk.addr.bdaddr, &irk->bdaddr);
> -       ev.irk.addr.type = link_to_bdaddr(LE_LINK, irk->addr_type);
> +       ev.irk.addr.type = link_to_bdaddr(link_type, irk->addr_type);

Perhaps we should incorporate the link_type as part of irk, ltk, csrk,
etc, to guarantee this information is always available.

>         memcpy(ev.irk.val, irk->val, sizeof(irk->val));
>
>         mgmt_event(MGMT_EV_NEW_IRK, hdev, &ev, sizeof(ev), NULL);
>  }
>
>  void mgmt_new_csrk(struct hci_dev *hdev, struct smp_csrk *csrk,
> -                  bool persistent)
> +                  bool persistent, u8 link_type)
>  {
>         struct mgmt_ev_new_csrk ev;
>
> @@ -9632,7 +9634,7 @@ void mgmt_new_csrk(struct hci_dev *hdev, struct smp_csrk *csrk,
>                 ev.store_hint = persistent;
>
>         bacpy(&ev.key.addr.bdaddr, &csrk->bdaddr);
> -       ev.key.addr.type = link_to_bdaddr(LE_LINK, csrk->bdaddr_type);
> +       ev.key.addr.type = link_to_bdaddr(link_type, csrk->bdaddr_type);
>         ev.key.type = csrk->type;
>         memcpy(ev.key.val, csrk->val, sizeof(csrk->val));
>
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index f1a9fc0012f0..3f640741e07b 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -1060,7 +1060,7 @@ static void smp_notify_keys(struct l2cap_conn *conn)
>         }
>
>         if (smp->remote_irk) {
> -               mgmt_new_irk(hdev, smp->remote_irk, persistent);
> +               mgmt_new_irk(hdev, smp->remote_irk, persistent, hcon->type);
>
>                 /* Now that user space can be considered to know the
>                  * identity address track the connection based on it
> @@ -1081,25 +1081,25 @@ static void smp_notify_keys(struct l2cap_conn *conn)
>         if (smp->csrk) {
>                 smp->csrk->bdaddr_type = hcon->dst_type;
>                 bacpy(&smp->csrk->bdaddr, &hcon->dst);
> -               mgmt_new_csrk(hdev, smp->csrk, persistent);
> +               mgmt_new_csrk(hdev, smp->csrk, persistent, hcon->type);
>         }
>
>         if (smp->responder_csrk) {
>                 smp->responder_csrk->bdaddr_type = hcon->dst_type;
>                 bacpy(&smp->responder_csrk->bdaddr, &hcon->dst);
> -               mgmt_new_csrk(hdev, smp->responder_csrk, persistent);
> +               mgmt_new_csrk(hdev, smp->responder_csrk, persistent, hcon->type);
>         }
>
>         if (smp->ltk) {
>                 smp->ltk->bdaddr_type = hcon->dst_type;
>                 bacpy(&smp->ltk->bdaddr, &hcon->dst);
> -               mgmt_new_ltk(hdev, smp->ltk, persistent);
> +               mgmt_new_ltk(hdev, smp->ltk, persistent, hcon->type);
>         }
>
>         if (smp->responder_ltk) {
>                 smp->responder_ltk->bdaddr_type = hcon->dst_type;
>                 bacpy(&smp->responder_ltk->bdaddr, &hcon->dst);
> -               mgmt_new_ltk(hdev, smp->responder_ltk, persistent);
> +               mgmt_new_ltk(hdev, smp->responder_ltk, persistent, hcon->type);
>         }
>
>         if (smp->link_key) {
> --
> 2.34.1
>
>


-- 
Luiz Augusto von Dentz

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

end of thread, other threads:[~2023-12-05 18:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-05 11:49 [PATCH] Bluetooth: MGMT/SMP: Fix address type when using smp over BREDR Xiao Yao
2023-12-05 12:40 ` bluez.test.bot
2023-12-05 18:40 ` [PATCH] " Luiz Augusto von Dentz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).