diff mbox series

Bluetooth: MGMT/SMP: Fix address type when using smp over BREDR

Message ID 20231205114924.834626-1-xiaokeqinhealth@126.com (mailing list archive)
State New, archived
Headers show
Series Bluetooth: MGMT/SMP: Fix address type when using smp over BREDR | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch success CheckPatch PASS
tedd_an/GitLint fail 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"
tedd_an/SubjectPrefix success Gitlint PASS
tedd_an/BuildKernel success BuildKernel PASS
tedd_an/CheckAllWarning success CheckAllWarning PASS
tedd_an/CheckSparse success CheckSparse PASS
tedd_an/CheckSmatch success CheckSparse PASS
tedd_an/BuildKernel32 success BuildKernel32 PASS
tedd_an/TestRunnerSetup success TestRunnerSetup PASS
tedd_an/TestRunner_l2cap-tester success TestRunner PASS
tedd_an/TestRunner_iso-tester success TestRunner PASS
tedd_an/TestRunner_bnep-tester success TestRunner PASS
tedd_an/TestRunner_mgmt-tester success TestRunner PASS
tedd_an/TestRunner_rfcomm-tester success TestRunner PASS
tedd_an/TestRunner_sco-tester success TestRunner PASS
tedd_an/TestRunner_ioctl-tester success TestRunner PASS
tedd_an/TestRunner_mesh-tester success TestRunner PASS
tedd_an/TestRunner_smp-tester success TestRunner PASS
tedd_an/TestRunner_userchan-tester success TestRunner PASS
tedd_an/IncrementalBuild success Incremental Build PASS

Commit Message

Yao Xiao Dec. 5, 2023, 11:49 a.m. UTC 
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(-)

Comments

bluez.test.bot@gmail.com Dec. 5, 2023, 12:40 p.m. UTC | #1 
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
Luiz Augusto von Dentz Dec. 5, 2023, 6:40 p.m. UTC | #2 
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
>
>
diff mbox series

Patch

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) {