Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SMP timeout leading disconnection with Android 13 #567

Open
nitin-rjadhav opened this issue Jul 31, 2023 · 7 comments
Open

SMP timeout leading disconnection with Android 13 #567

nitin-rjadhav opened this issue Jul 31, 2023 · 7 comments

Comments

@nitin-rjadhav
Copy link
Contributor

Description:

Advertise from : Mobile Phone
Scan from : Linux Bluez DUT (bluetoothctl)

Android 13 phones are connected as master during pairing process, and this conflicts with the keys exchange process that leads to SMP timeout. In comparison, Android 12 or iPhone act as slaves during the pairing process; hence the issue is not observed with them.

As per Bluetooth specs, an Initiator(master) sends smp pairing request command (in which keys that the initiator requests to share and keys it demands from the responder/slave are mentioned).
image

In return, the responder sends smp pairing response (in which it sends keys that shall be shared by the initiator and keys which the responder shall share).
image

According to the specs, after the pairing response is sent, the responder/slave shall send its keys first (whichever is mentioned in the responder key distribution field of the pairing response).

Till android 12, all phones were connecting as slaves when the connection was initiated from the DUT, and the link was successful as phones were not changing their roles to master until all the SMP commands and key exchanges were complete. Starting from Android 13, phones sent role change orders after the DUT initiated the connection and became master.

Analysis and Fix

Analysis

  • All SMP-related tasks (handling SMP commands and keys generation/distribution) are handled in the Linux kernel smp.c file.
  • In which it is found that code only checks whether the connection is initiated from its side (hcon->out == 1), or it is an incoming connection(hcon->out == 0) from the other side.
  • As the connection is initiated from the DUT side, it expects itself as master as there is no check for its role, and it waits for keys from the phone despite being in a slave role, and an SMP timeout happens.

Please find the code screenshot for reference: - (linux-kernel/net/bluetooth/smp.c)
image

Fix:
After putting a check for the role, the connection is maintained and keys are appropriately distributed according to specs (checked with pixel 7 phones), and the connection is maintained.
image

Consequence of fix:

On Samsung phones with Android 13, we are facing issues during first-time connection, even with this patch. Later, the connection is sustained.

@Vudentz
Copy link
Contributor

Vudentz commented Aug 1, 2023

From the looks of it we were assuming hcon->out to mean central, but the SMP never says the central to correspond to initiator:

BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 3, Part H
page 1557:

Figure 2.1: LE pairing phases

So we need to check who actually sends the Pairing Request and properly mark is as iniatiator, that said it is probably recommended that only one side start the pairing procedure in which case the central probably needs to have priority over this, so when we act as peripheral we shall just send a Security_Request.

@NJNXP Can you please include the HCI/btmon traces as well?

@nitin-rjadhav
Copy link
Contributor Author

Hello Luiz,

PFA the btmon logs for the issue.

For quick reference following our log analysis:

Initiated connection from Bluez to Android 13 Phone. As we can see the Android 13 changes its role to Master after connection.
image

As Android 13 is Master hence it will send Pairing request.
image

  • All SMP-related tasks (handling SMP commands and keys generation/distribution) are handled in the Linux kernel smp.c file
  • In which it is found that code only checks whether the connection is initiated from its side (hcon->out == 1), or it is an incoming connection(hcon->out == 0) from the other side.
  • As the connection is initiated from the Bluez side, it expects the keys to be sent from the phone side.
  • Whereas Android 13 is Master and is initiating Pairing request hence it also waits for Keys from Bluez side. According to the specs, after the pairing response is sent, the responder/slave shall send its keys first (whichever is mentioned in the responder key distribution field of the pairing response).

This causes SMP Timeout issue and device get disconnected, and Phone removes the device from its paired list.
image

Thanks,
Warm Regards,
Nitin Jadhav

Pixel7_with_bluez_5-66.txt

@nitin-rjadhav
Copy link
Contributor Author

Hello @Vudentz,
In reference to the previous mentioned logs can you please further guide on this issue.

Thanks in advance.

Warm Regards
Nitin Jadhav

intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Aug 30, 2023
SMP initiator role shall be considered the one that initiates the
pairing procedure with SMP_CMD_PAIRING_REQ:

BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 3, Part H
page 1557:

Figure 2.1: LE pairing phases

Note that by sending SMP_CMD_SECURITY_REQ it doesn't change the role to
be Initiator.

Link: bluez/bluez#567
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
github-actions bot pushed a commit to BluezTestBot/bluetooth-next that referenced this issue Aug 30, 2023
SMP initiator role shall be considered the one that initiates the
pairing procedure with SMP_CMD_PAIRING_REQ:

BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 3, Part H
page 1557:

Figure 2.1: LE pairing phases

Note that by sending SMP_CMD_SECURITY_REQ it doesn't change the role to
be Initiator.

Link: bluez/bluez#567
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
github-actions bot pushed a commit to tedd-an/bluetooth-next that referenced this issue Aug 30, 2023
SMP initiator role shall be considered the one that initiates the
pairing procedure with SMP_CMD_PAIRING_REQ:

BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 3, Part H
page 1557:

Figure 2.1: LE pairing phases

Note that by sending SMP_CMD_SECURITY_REQ it doesn't change the role to
be Initiator.

Link: bluez/bluez#567
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
@Vudentz
Copy link
Contributor

Vudentz commented Aug 30, 2023

@NJNXP have a look if the patch above fixes the problem, we may need to double check if there isn't any test on SMP testing that affects this change before we merge it though.

@purendra-singh
Copy link

Hi @Vudentz,

I tested the changes done and following are the observations:-

  1. Pixel Android 14:-
    a. Without fix, issue was seen on this phone too and behavior was same as Android 13, keys were not getting exchanged.
    b. With fix, issue was not seen and all the keys during SMP pairing procedure were exchanged.

  2. Iphone 14:- Issue wasn't seen with or without fix.

  3. Samsung S20 FE Android 13:-
    a. Without fix, issue was seen with this phone, keys were not getting exchanged.
    b. With Fix, pairing is done, while connect command we are getting error as "Failed to connect: org.bluez.Error.Failed br-connection-create-socket" and then connection fails.
    c. We get connect request then from phone side and popup to Authorize service in bluetooth and when we authorize the service connection persists.

Attaching logs for all three devices which contains bluetoothctl, bluetoothd and btmon.

Thanks,
Purendra Singh

With_Luiz_Patch.zip

@Vudentz
Copy link
Contributor

Vudentz commented Sep 8, 2023

@purendra-singh thanks for testing, we may need to run SMP PTS tests just to be safe, can you do that?

@purendra-singh
Copy link

purendra-singh commented Sep 27, 2023

Hi @Vudentz,

Currently dont have compatible dongle to perform the SMP PTS test.

Could you please provide us the link to the dongle which is used to do the PTS test for BREDR, so that we can procure it and use it for future test cases if required.

Thanks,
Purendra Singh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants