diff mbox series

[v1] adapter: Manage device state of cross-transport SMP keys

Message ID 20240826070438.557107-1-quic_chejiang@quicinc.com (mailing list archive)
State New
Headers show
Series [v1] adapter: Manage device state of cross-transport SMP keys | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch success CheckPatch PASS
tedd_an/GitLint success Gitlint PASS
tedd_an/BuildEll success Build ELL PASS
tedd_an/BluezMake success Bluez Make PASS
tedd_an/MakeCheck success Bluez Make Check PASS
tedd_an/MakeDistcheck success Make Distcheck PASS
tedd_an/CheckValgrind success Check Valgrind PASS
tedd_an/CheckSmatch success CheckSparse PASS
tedd_an/bluezmakeextell success Make External ELL PASS
tedd_an/IncrementalBuild success Incremental Build PASS
tedd_an/ScanBuild success Scan Build PASS

Commit Message

Cheng Jiang Aug. 26, 2024, 7:04 a.m. UTC
Cross-transport derived ltk/csrk/irk are reported to bluetoothd from
kernel with BR/EDR address type, and bluetoothd doesn't treat it as LE
paired/bonded. In this case, bluetoothd won't send remove bond operation
with LE address type to kernel when executing unpair, so SMP keys are
retained in kernel list. Then RPA is getting resolved since we still
have irk which was not deleted when unpair device is done because only
link key is deleted since addr type is bredr.

What’s more, pair LE of the same address will always fail in kernel
for ltk existence, and send back AlreadyExists error, but device state
is still unpaired/unbonded in bluetoothd.

So bluetoothd needs to consider LE paired/bonded when receiving SMP keys
even if they are cross-transport derived.
---
 src/adapter.c | 53 ++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 40 insertions(+), 13 deletions(-)

Comments

bluez.test.bot@gmail.com Aug. 26, 2024, 8:42 a.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=883189

---Test result---

Test Summary:
CheckPatch                    PASS      0.50 seconds
GitLint                       PASS      0.32 seconds
BuildEll                      PASS      24.74 seconds
BluezMake                     PASS      1699.99 seconds
MakeCheck                     PASS      13.24 seconds
MakeDistcheck                 PASS      177.91 seconds
CheckValgrind                 PASS      252.14 seconds
CheckSmatch                   PASS      355.88 seconds
bluezmakeextell               PASS      119.64 seconds
IncrementalBuild              PASS      1581.67 seconds
ScanBuild                     PASS      1003.40 seconds



---
Regards,
Linux Bluetooth
Luiz Augusto von Dentz Aug. 26, 2024, 1:56 p.m. UTC | #2
Hi Cheng,

On Mon, Aug 26, 2024 at 3:05 AM Cheng Jiang <quic_chejiang@quicinc.com> wrote:
>
> Cross-transport derived ltk/csrk/irk are reported to bluetoothd from
> kernel with BR/EDR address type, and bluetoothd doesn't treat it as LE
> paired/bonded. In this case, bluetoothd won't send remove bond operation
> with LE address type to kernel when executing unpair, so SMP keys are
> retained in kernel list. Then RPA is getting resolved since we still
> have irk which was not deleted when unpair device is done because only
> link key is deleted since addr type is bredr.
>
> What’s more, pair LE of the same address will always fail in kernel
> for ltk existence, and send back AlreadyExists error, but device state
> is still unpaired/unbonded in bluetoothd.
>
> So bluetoothd needs to consider LE paired/bonded when receiving SMP keys
> even if they are cross-transport derived.
> ---
>  src/adapter.c | 53 ++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 40 insertions(+), 13 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index 245de4456..4e5af9579 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -8647,6 +8647,7 @@ static void new_link_key_callback(uint16_t index, uint16_t length,
>         struct btd_adapter *adapter = user_data;
>         struct btd_device *device;
>         char dst[18];
> +       uint8_t addr_type;
>
>         if (length < sizeof(*ev)) {
>                 btd_error(adapter->dev_id, "Too small new link key event");
> @@ -8666,7 +8667,13 @@ static void new_link_key_callback(uint16_t index, uint16_t length,
>                 return;
>         }
>
> -       device = btd_adapter_get_device(adapter, &addr->bdaddr, addr->type);
> +       /*
> +        * For LE public address, key here is cross-transport derived,
> +        * so consider it as BR/EDR public address.
> +        *
> +        */
> +       addr_type = addr->type == BDADDR_LE_PUBLIC ? BDADDR_BREDR : addr->type;
> +       device = btd_adapter_get_device(adapter, &addr->bdaddr, addr_type);
>         if (!device) {
>                 btd_error(adapter->dev_id,
>                                 "Unable to get device object for %s", dst);
> @@ -8682,7 +8689,7 @@ static void new_link_key_callback(uint16_t index, uint16_t length,
>                 device_set_bonded(device, BDADDR_BREDR);
>         }
>
> -       bonding_complete(adapter, &addr->bdaddr, addr->type, 0);
> +       bonding_complete(adapter, &addr->bdaddr, addr_type, 0);
>  }
>
>  static void store_ltk_group(struct btd_adapter *adapter, const bdaddr_t *peer,
> @@ -8773,6 +8780,7 @@ static void new_long_term_key_callback(uint16_t index, uint16_t length,
>         struct btd_device *device;
>         bool persistent;
>         char dst[18];
> +       uint8_t addr_type;
>
>         if (length < sizeof(*ev)) {
>                 btd_error(adapter->dev_id, "Too small long term key event");
> @@ -8784,7 +8792,13 @@ static void new_long_term_key_callback(uint16_t index, uint16_t length,
>         DBG("hci%u new LTK for %s type %u enc_size %u",
>                 adapter->dev_id, dst, ev->key.type, ev->key.enc_size);
>
> -       device = btd_adapter_get_device(adapter, &addr->bdaddr, addr->type);
> +       /*
> +        * For BR/EDR public address, key here is cross-transport derived,
> +        * so consider it as LE public address for SMP.
> +        *
> +        */

This is only the case if, an only if, the device is a dual-mode
device, so this probably need to be checked that we don't attempt to
do this regardless.

> +       addr_type = addr->type == BDADDR_BREDR ? BDADDR_LE_PUBLIC : addr->type;
> +       device = btd_adapter_get_device(adapter, &addr->bdaddr, addr_type);
>         if (!device) {
>                 btd_error(adapter->dev_id,
>                                 "Unable to get device object for %s", dst);
> @@ -8802,8 +8816,7 @@ static void new_long_term_key_callback(uint16_t index, uint16_t length,
>          * be persistently stored.
>          *
>          */
> -       if (addr->type == BDADDR_LE_RANDOM &&
> -                               (addr->bdaddr.b[5] & 0xc0) != 0xc0)
> +       if (addr_type == BDADDR_LE_RANDOM && (addr->bdaddr.b[5] & 0xc0) != 0xc0)
>                 persistent = false;
>         else
>                 persistent = !!ev->store_hint;
> @@ -8817,15 +8830,15 @@ static void new_long_term_key_callback(uint16_t index, uint16_t length,
>                 rand = le64_to_cpu(key->rand);
>
>                 store_longtermkey(adapter, &key->addr.bdaddr,
> -                                       key->addr.type, key->val, key->central,
> +                                       addr_type, key->val, key->central,
>                                         key->type, key->enc_size, ediv, rand);
>
> -               device_set_bonded(device, addr->type);
> +               device_set_bonded(device, addr_type);
>         }
>
>         device_set_ltk(device, ev->key.val, ev->key.central, ev->key.enc_size);
>
> -       bonding_complete(adapter, &addr->bdaddr, addr->type, 0);
> +       bonding_complete(adapter, &addr->bdaddr, addr_type, 0);
>  }
>
>  static void new_csrk_callback(uint16_t index, uint16_t length,
> @@ -8837,6 +8850,7 @@ static void new_csrk_callback(uint16_t index, uint16_t length,
>         struct btd_adapter *adapter = user_data;
>         struct btd_device *device;
>         char dst[18];
> +       uint8_t addr_type;
>
>         if (length < sizeof(*ev)) {
>                 btd_error(adapter->dev_id, "Too small CSRK event");
> @@ -8848,7 +8862,13 @@ static void new_csrk_callback(uint16_t index, uint16_t length,
>         DBG("hci%u new CSRK for %s type %u", adapter->dev_id, dst,
>                                                                 ev->key.type);
>
> -       device = btd_adapter_get_device(adapter, &addr->bdaddr, addr->type);
> +       /*
> +        * For BR/EDR public address, key here is cross-transport derived,
> +        * so consider it as LE public address for SMP.
> +        *
> +        */
> +       addr_type = addr->type == BDADDR_BREDR ? BDADDR_LE_PUBLIC : addr->type;

Ditto.

> +       device = btd_adapter_get_device(adapter, &addr->bdaddr, addr_type);
>         if (!device) {
>                 btd_error(adapter->dev_id,
>                                 "Unable to get device object for %s", dst);
> @@ -8911,6 +8931,7 @@ static void new_irk_callback(uint16_t index, uint16_t length,
>         struct btd_device *device, *duplicate;
>         bool persistent;
>         char dst[18], rpa[18];
> +       uint8_t addr_type;
>
>         if (length < sizeof(*ev)) {
>                 btd_error(adapter->dev_id, "Too small New IRK event");
> @@ -8922,16 +8943,22 @@ static void new_irk_callback(uint16_t index, uint16_t length,
>
>         DBG("hci%u new IRK for %s RPA %s", adapter->dev_id, dst, rpa);
>
> +       /*
> +        * For BR/EDR public address, key here is cross-transport derived,
> +        * so consider it as LE public address for SMP.
> +        *
> +        */
> +       addr_type = addr->type == BDADDR_BREDR ? BDADDR_LE_PUBLIC : addr->type;

Ditto.

>         if (bacmp(&ev->rpa, BDADDR_ANY)) {
>                 device = btd_adapter_get_device(adapter, &ev->rpa,
>                                                         BDADDR_LE_RANDOM);
>                 duplicate = btd_adapter_find_device(adapter, &addr->bdaddr,
> -                                                               addr->type);
> +                                                               addr_type);
>                 if (duplicate == device)
>                         duplicate = NULL;
>         } else {
>                 device = btd_adapter_get_device(adapter, &addr->bdaddr,
> -                                                               addr->type);
> +                                                               addr_type);
>                 duplicate = NULL;
>         }
>
> @@ -8941,7 +8968,7 @@ static void new_irk_callback(uint16_t index, uint16_t length,
>                 return;
>         }
>
> -       device_update_addr(device, &addr->bdaddr, addr->type);
> +       device_update_addr(device, &addr->bdaddr, addr_type);
>
>         if (duplicate)
>                 device_merge_duplicate(device, duplicate);
> @@ -8950,7 +8977,7 @@ static void new_irk_callback(uint16_t index, uint16_t length,
>         if (!persistent)
>                 return;
>
> -       store_irk(adapter, &addr->bdaddr, addr->type, irk->val);
> +       store_irk(adapter, &addr->bdaddr, addr_type, irk->val);
>
>         btd_device_set_temporary(device, false);
>  }
> --
> 2.25.1
>
>
Cheng Jiang Aug. 27, 2024, 2:20 a.m. UTC | #3
Hi Luiz, Please check the comment inline, Please correct me if I'm wrong.

On 8/26/2024 9:56 PM, Luiz Augusto von Dentz wrote:
> Hi Cheng,
> 
> On Mon, Aug 26, 2024 at 3:05 AM Cheng Jiang <quic_chejiang@quicinc.com> wrote:
>>
>> Cross-transport derived ltk/csrk/irk are reported to bluetoothd from
>> kernel with BR/EDR address type, and bluetoothd doesn't treat it as LE
>> paired/bonded. In this case, bluetoothd won't send remove bond operation
>> with LE address type to kernel when executing unpair, so SMP keys are
>> retained in kernel list. Then RPA is getting resolved since we still
>> have irk which was not deleted when unpair device is done because only
>> link key is deleted since addr type is bredr.
>>
>> What’s more, pair LE of the same address will always fail in kernel
>> for ltk existence, and send back AlreadyExists error, but device state
>> is still unpaired/unbonded in bluetoothd.
>>
>> So bluetoothd needs to consider LE paired/bonded when receiving SMP keys
>> even if they are cross-transport derived.
>> ---
>>  src/adapter.c | 53 ++++++++++++++++++++++++++++++++++++++-------------
>>  1 file changed, 40 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/adapter.c b/src/adapter.c
>> index 245de4456..4e5af9579 100644
>> --- a/src/adapter.c
>> +++ b/src/adapter.c
>> @@ -8647,6 +8647,7 @@ static void new_link_key_callback(uint16_t index, uint16_t length,
>>         struct btd_adapter *adapter = user_data;
>>         struct btd_device *device;
>>         char dst[18];
>> +       uint8_t addr_type;
>>
>>         if (length < sizeof(*ev)) {
>>                 btd_error(adapter->dev_id, "Too small new link key event");
>> @@ -8666,7 +8667,13 @@ static void new_link_key_callback(uint16_t index, uint16_t length,
>>                 return;
>>         }
>>
>> -       device = btd_adapter_get_device(adapter, &addr->bdaddr, addr->type);
>> +       /*
>> +        * For LE public address, key here is cross-transport derived,
>> +        * so consider it as BR/EDR public address.
>> +        *
>> +        */
>> +       addr_type = addr->type == BDADDR_LE_PUBLIC ? BDADDR_BREDR : addr->type;
>> +       device = btd_adapter_get_device(adapter, &addr->bdaddr, addr_type);
>>         if (!device) {
>>                 btd_error(adapter->dev_id,
>>                                 "Unable to get device object for %s", dst);
>> @@ -8682,7 +8689,7 @@ static void new_link_key_callback(uint16_t index, uint16_t length,
>>                 device_set_bonded(device, BDADDR_BREDR);
>>         }
>>
>> -       bonding_complete(adapter, &addr->bdaddr, addr->type, 0);
>> +       bonding_complete(adapter, &addr->bdaddr, addr_type, 0);
>>  }
>>
>>  static void store_ltk_group(struct btd_adapter *adapter, const bdaddr_t *peer,
>> @@ -8773,6 +8780,7 @@ static void new_long_term_key_callback(uint16_t index, uint16_t length,
>>         struct btd_device *device;
>>         bool persistent;
>>         char dst[18];
>> +       uint8_t addr_type;
>>
>>         if (length < sizeof(*ev)) {
>>                 btd_error(adapter->dev_id, "Too small long term key event");
>> @@ -8784,7 +8792,13 @@ static void new_long_term_key_callback(uint16_t index, uint16_t length,
>>         DBG("hci%u new LTK for %s type %u enc_size %u",
>>                 adapter->dev_id, dst, ev->key.type, ev->key.enc_size);
>>
>> -       device = btd_adapter_get_device(adapter, &addr->bdaddr, addr->type);
>> +       /*
>> +        * For BR/EDR public address, key here is cross-transport derived,
>> +        * so consider it as LE public address for SMP.
>> +        *
>> +        */
> 
> This is only the case if, an only if, the device is a dual-mode
> device, so this probably need to be checked that we don't attempt to
> do this regardless.
Only the dual-mode device supports Cross-transport derived Keys. From BLE keys to BR/EDR keys, vice versa. If
single mode device, the addr->type always is BDADDR_BREDR in link key callback. and BDADDR_LE_PUBLIC or
BDADDR_LE_RANDOM for LTK/CSRK/IRK callback. 

Link Key is for BR/EDR, so we check the address type is BDADDR_LE_PUBLIC or not, if yes, treated as BR/EDR,
it means the link key is derived from BLE bearer. Otherwise, use the original addr type. LTK, CSRK, IRK are
related to BLE, so checked the address type is BDADDR_BREDR or not, if yes, treated as BLE address, it means
the BLE related key are derived from BR/EDR bearer. 

The change seems not affect the current logic, could you please help to explain more what need to check? Thanks!
> 
>> +       addr_type = addr->type == BDADDR_BREDR ? BDADDR_LE_PUBLIC : addr->type;
>> +       device = btd_adapter_get_device(adapter, &addr->bdaddr, addr_type);
>>         if (!device) {
>>                 btd_error(adapter->dev_id,
>>                                 "Unable to get device object for %s", dst);
>> @@ -8802,8 +8816,7 @@ static void new_long_term_key_callback(uint16_t index, uint16_t length,
>>          * be persistently stored.
>>          *
>>          */
>> -       if (addr->type == BDADDR_LE_RANDOM &&
>> -                               (addr->bdaddr.b[5] & 0xc0) != 0xc0)
>> +       if (addr_type == BDADDR_LE_RANDOM && (addr->bdaddr.b[5] & 0xc0) != 0xc0)
>>                 persistent = false;
>>         else
>>                 persistent = !!ev->store_hint;
>> @@ -8817,15 +8830,15 @@ static void new_long_term_key_callback(uint16_t index, uint16_t length,
>>                 rand = le64_to_cpu(key->rand);
>>
>>                 store_longtermkey(adapter, &key->addr.bdaddr,
>> -                                       key->addr.type, key->val, key->central,
>> +                                       addr_type, key->val, key->central,
>>                                         key->type, key->enc_size, ediv, rand);
>>
>> -               device_set_bonded(device, addr->type);
>> +               device_set_bonded(device, addr_type);
>>         }
>>
>>         device_set_ltk(device, ev->key.val, ev->key.central, ev->key.enc_size);
>>
>> -       bonding_complete(adapter, &addr->bdaddr, addr->type, 0);
>> +       bonding_complete(adapter, &addr->bdaddr, addr_type, 0);
>>  }
>>
>>  static void new_csrk_callback(uint16_t index, uint16_t length,
>> @@ -8837,6 +8850,7 @@ static void new_csrk_callback(uint16_t index, uint16_t length,
>>         struct btd_adapter *adapter = user_data;
>>         struct btd_device *device;
>>         char dst[18];
>> +       uint8_t addr_type;
>>
>>         if (length < sizeof(*ev)) {
>>                 btd_error(adapter->dev_id, "Too small CSRK event");
>> @@ -8848,7 +8862,13 @@ static void new_csrk_callback(uint16_t index, uint16_t length,
>>         DBG("hci%u new CSRK for %s type %u", adapter->dev_id, dst,
>>                                                                 ev->key.type);
>>
>> -       device = btd_adapter_get_device(adapter, &addr->bdaddr, addr->type);
>> +       /*
>> +        * For BR/EDR public address, key here is cross-transport derived,
>> +        * so consider it as LE public address for SMP.
>> +        *
>> +        */
>> +       addr_type = addr->type == BDADDR_BREDR ? BDADDR_LE_PUBLIC : addr->type;
> 
> Ditto.
> 
>> +       device = btd_adapter_get_device(adapter, &addr->bdaddr, addr_type);
>>         if (!device) {
>>                 btd_error(adapter->dev_id,
>>                                 "Unable to get device object for %s", dst);
>> @@ -8911,6 +8931,7 @@ static void new_irk_callback(uint16_t index, uint16_t length,
>>         struct btd_device *device, *duplicate;
>>         bool persistent;
>>         char dst[18], rpa[18];
>> +       uint8_t addr_type;
>>
>>         if (length < sizeof(*ev)) {
>>                 btd_error(adapter->dev_id, "Too small New IRK event");
>> @@ -8922,16 +8943,22 @@ static void new_irk_callback(uint16_t index, uint16_t length,
>>
>>         DBG("hci%u new IRK for %s RPA %s", adapter->dev_id, dst, rpa);
>>
>> +       /*
>> +        * For BR/EDR public address, key here is cross-transport derived,
>> +        * so consider it as LE public address for SMP.
>> +        *
>> +        */
>> +       addr_type = addr->type == BDADDR_BREDR ? BDADDR_LE_PUBLIC : addr->type;
> 
> Ditto.
> 
>>         if (bacmp(&ev->rpa, BDADDR_ANY)) {
>>                 device = btd_adapter_get_device(adapter, &ev->rpa,
>>                                                         BDADDR_LE_RANDOM);
>>                 duplicate = btd_adapter_find_device(adapter, &addr->bdaddr,
>> -                                                               addr->type);
>> +                                                               addr_type);
>>                 if (duplicate == device)
>>                         duplicate = NULL;
>>         } else {
>>                 device = btd_adapter_get_device(adapter, &addr->bdaddr,
>> -                                                               addr->type);
>> +                                                               addr_type);
>>                 duplicate = NULL;
>>         }
>>
>> @@ -8941,7 +8968,7 @@ static void new_irk_callback(uint16_t index, uint16_t length,
>>                 return;
>>         }
>>
>> -       device_update_addr(device, &addr->bdaddr, addr->type);
>> +       device_update_addr(device, &addr->bdaddr, addr_type);
>>
>>         if (duplicate)
>>                 device_merge_duplicate(device, duplicate);
>> @@ -8950,7 +8977,7 @@ static void new_irk_callback(uint16_t index, uint16_t length,
>>         if (!persistent)
>>                 return;
>>
>> -       store_irk(adapter, &addr->bdaddr, addr->type, irk->val);
>> +       store_irk(adapter, &addr->bdaddr, addr_type, irk->val);
>>
>>         btd_device_set_temporary(device, false);
>>  }
>> --
>> 2.25.1
>>
>>
> 
>
Luiz Augusto von Dentz Aug. 27, 2024, 12:59 p.m. UTC | #4
Hi Cheng,

On Mon, Aug 26, 2024 at 10:20 PM Cheng Jiang <quic_chejiang@quicinc.com> wrote:
>
> Hi Luiz, Please check the comment inline, Please correct me if I'm wrong.
>
> On 8/26/2024 9:56 PM, Luiz Augusto von Dentz wrote:
> > Hi Cheng,
> >
> > On Mon, Aug 26, 2024 at 3:05 AM Cheng Jiang <quic_chejiang@quicinc.com> wrote:
> >>
> >> Cross-transport derived ltk/csrk/irk are reported to bluetoothd from
> >> kernel with BR/EDR address type, and bluetoothd doesn't treat it as LE
> >> paired/bonded. In this case, bluetoothd won't send remove bond operation
> >> with LE address type to kernel when executing unpair, so SMP keys are
> >> retained in kernel list. Then RPA is getting resolved since we still
> >> have irk which was not deleted when unpair device is done because only
> >> link key is deleted since addr type is bredr.
> >>
> >> What’s more, pair LE of the same address will always fail in kernel
> >> for ltk existence, and send back AlreadyExists error, but device state
> >> is still unpaired/unbonded in bluetoothd.
> >>
> >> So bluetoothd needs to consider LE paired/bonded when receiving SMP keys
> >> even if they are cross-transport derived.

Yeah, and we are chasing our own tail here:

https://github.com/bluez/bluetooth-next/commit/59b047bc98084f8af2c41483e4d68a5adf2fa7f7

So I think we better revert that since that is the culprit of the
problem and doesn't follow our very own documentation, too bad we
didn't realize it sooner because it is causing a lot of problems such:

https://github.com/bluez/bluez/issues/875

> >> ---
> >>  src/adapter.c | 53 ++++++++++++++++++++++++++++++++++++++-------------
> >>  1 file changed, 40 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/src/adapter.c b/src/adapter.c
> >> index 245de4456..4e5af9579 100644
> >> --- a/src/adapter.c
> >> +++ b/src/adapter.c
> >> @@ -8647,6 +8647,7 @@ static void new_link_key_callback(uint16_t index, uint16_t length,
> >>         struct btd_adapter *adapter = user_data;
> >>         struct btd_device *device;
> >>         char dst[18];
> >> +       uint8_t addr_type;
> >>
> >>         if (length < sizeof(*ev)) {
> >>                 btd_error(adapter->dev_id, "Too small new link key event");
> >> @@ -8666,7 +8667,13 @@ static void new_link_key_callback(uint16_t index, uint16_t length,
> >>                 return;
> >>         }
> >>
> >> -       device = btd_adapter_get_device(adapter, &addr->bdaddr, addr->type);
> >> +       /*
> >> +        * For LE public address, key here is cross-transport derived,
> >> +        * so consider it as BR/EDR public address.
> >> +        *
> >> +        */
> >> +       addr_type = addr->type == BDADDR_LE_PUBLIC ? BDADDR_BREDR : addr->type;
> >> +       device = btd_adapter_get_device(adapter, &addr->bdaddr, addr_type);
> >>         if (!device) {
> >>                 btd_error(adapter->dev_id,
> >>                                 "Unable to get device object for %s", dst);
> >> @@ -8682,7 +8689,7 @@ static void new_link_key_callback(uint16_t index, uint16_t length,
> >>                 device_set_bonded(device, BDADDR_BREDR);
> >>         }
> >>
> >> -       bonding_complete(adapter, &addr->bdaddr, addr->type, 0);
> >> +       bonding_complete(adapter, &addr->bdaddr, addr_type, 0);
> >>  }
> >>
> >>  static void store_ltk_group(struct btd_adapter *adapter, const bdaddr_t *peer,
> >> @@ -8773,6 +8780,7 @@ static void new_long_term_key_callback(uint16_t index, uint16_t length,
> >>         struct btd_device *device;
> >>         bool persistent;
> >>         char dst[18];
> >> +       uint8_t addr_type;
> >>
> >>         if (length < sizeof(*ev)) {
> >>                 btd_error(adapter->dev_id, "Too small long term key event");
> >> @@ -8784,7 +8792,13 @@ static void new_long_term_key_callback(uint16_t index, uint16_t length,
> >>         DBG("hci%u new LTK for %s type %u enc_size %u",
> >>                 adapter->dev_id, dst, ev->key.type, ev->key.enc_size);
> >>
> >> -       device = btd_adapter_get_device(adapter, &addr->bdaddr, addr->type);
> >> +       /*
> >> +        * For BR/EDR public address, key here is cross-transport derived,
> >> +        * so consider it as LE public address for SMP.
> >> +        *
> >> +        */
> >
> > This is only the case if, an only if, the device is a dual-mode
> > device, so this probably need to be checked that we don't attempt to
> > do this regardless.
> Only the dual-mode device supports Cross-transport derived Keys. From BLE keys to BR/EDR keys, vice versa. If
> single mode device, the addr->type always is BDADDR_BREDR in link key callback. and BDADDR_LE_PUBLIC or
> BDADDR_LE_RANDOM for LTK/CSRK/IRK callback.
>
> Link Key is for BR/EDR, so we check the address type is BDADDR_LE_PUBLIC or not, if yes, treated as BR/EDR,
> it means the link key is derived from BLE bearer. Otherwise, use the original addr type. LTK, CSRK, IRK are
> related to BLE, so checked the address type is BDADDR_BREDR or not, if yes, treated as BLE address, it means
> the BLE related key are derived from BR/EDR bearer.
>
> The change seems not affect the current logic, could you please help to explain more what need to check? Thanks!
> >
> >> +       addr_type = addr->type == BDADDR_BREDR ? BDADDR_LE_PUBLIC : addr->type;
> >> +       device = btd_adapter_get_device(adapter, &addr->bdaddr, addr_type);
> >>         if (!device) {
> >>                 btd_error(adapter->dev_id,
> >>                                 "Unable to get device object for %s", dst);
> >> @@ -8802,8 +8816,7 @@ static void new_long_term_key_callback(uint16_t index, uint16_t length,
> >>          * be persistently stored.
> >>          *
> >>          */
> >> -       if (addr->type == BDADDR_LE_RANDOM &&
> >> -                               (addr->bdaddr.b[5] & 0xc0) != 0xc0)
> >> +       if (addr_type == BDADDR_LE_RANDOM && (addr->bdaddr.b[5] & 0xc0) != 0xc0)
> >>                 persistent = false;
> >>         else
> >>                 persistent = !!ev->store_hint;
> >> @@ -8817,15 +8830,15 @@ static void new_long_term_key_callback(uint16_t index, uint16_t length,
> >>                 rand = le64_to_cpu(key->rand);
> >>
> >>                 store_longtermkey(adapter, &key->addr.bdaddr,
> >> -                                       key->addr.type, key->val, key->central,
> >> +                                       addr_type, key->val, key->central,
> >>                                         key->type, key->enc_size, ediv, rand);
> >>
> >> -               device_set_bonded(device, addr->type);
> >> +               device_set_bonded(device, addr_type);
> >>         }
> >>
> >>         device_set_ltk(device, ev->key.val, ev->key.central, ev->key.enc_size);
> >>
> >> -       bonding_complete(adapter, &addr->bdaddr, addr->type, 0);
> >> +       bonding_complete(adapter, &addr->bdaddr, addr_type, 0);
> >>  }
> >>
> >>  static void new_csrk_callback(uint16_t index, uint16_t length,
> >> @@ -8837,6 +8850,7 @@ static void new_csrk_callback(uint16_t index, uint16_t length,
> >>         struct btd_adapter *adapter = user_data;
> >>         struct btd_device *device;
> >>         char dst[18];
> >> +       uint8_t addr_type;
> >>
> >>         if (length < sizeof(*ev)) {
> >>                 btd_error(adapter->dev_id, "Too small CSRK event");
> >> @@ -8848,7 +8862,13 @@ static void new_csrk_callback(uint16_t index, uint16_t length,
> >>         DBG("hci%u new CSRK for %s type %u", adapter->dev_id, dst,
> >>                                                                 ev->key.type);
> >>
> >> -       device = btd_adapter_get_device(adapter, &addr->bdaddr, addr->type);
> >> +       /*
> >> +        * For BR/EDR public address, key here is cross-transport derived,
> >> +        * so consider it as LE public address for SMP.
> >> +        *
> >> +        */
> >> +       addr_type = addr->type == BDADDR_BREDR ? BDADDR_LE_PUBLIC : addr->type;
> >
> > Ditto.
> >
> >> +       device = btd_adapter_get_device(adapter, &addr->bdaddr, addr_type);
> >>         if (!device) {
> >>                 btd_error(adapter->dev_id,
> >>                                 "Unable to get device object for %s", dst);
> >> @@ -8911,6 +8931,7 @@ static void new_irk_callback(uint16_t index, uint16_t length,
> >>         struct btd_device *device, *duplicate;
> >>         bool persistent;
> >>         char dst[18], rpa[18];
> >> +       uint8_t addr_type;
> >>
> >>         if (length < sizeof(*ev)) {
> >>                 btd_error(adapter->dev_id, "Too small New IRK event");
> >> @@ -8922,16 +8943,22 @@ static void new_irk_callback(uint16_t index, uint16_t length,
> >>
> >>         DBG("hci%u new IRK for %s RPA %s", adapter->dev_id, dst, rpa);
> >>
> >> +       /*
> >> +        * For BR/EDR public address, key here is cross-transport derived,
> >> +        * so consider it as LE public address for SMP.
> >> +        *
> >> +        */
> >> +       addr_type = addr->type == BDADDR_BREDR ? BDADDR_LE_PUBLIC : addr->type;
> >
> > Ditto.
> >
> >>         if (bacmp(&ev->rpa, BDADDR_ANY)) {
> >>                 device = btd_adapter_get_device(adapter, &ev->rpa,
> >>                                                         BDADDR_LE_RANDOM);
> >>                 duplicate = btd_adapter_find_device(adapter, &addr->bdaddr,
> >> -                                                               addr->type);
> >> +                                                               addr_type);
> >>                 if (duplicate == device)
> >>                         duplicate = NULL;
> >>         } else {
> >>                 device = btd_adapter_get_device(adapter, &addr->bdaddr,
> >> -                                                               addr->type);
> >> +                                                               addr_type);
> >>                 duplicate = NULL;
> >>         }
> >>
> >> @@ -8941,7 +8968,7 @@ static void new_irk_callback(uint16_t index, uint16_t length,
> >>                 return;
> >>         }
> >>
> >> -       device_update_addr(device, &addr->bdaddr, addr->type);
> >> +       device_update_addr(device, &addr->bdaddr, addr_type);
> >>
> >>         if (duplicate)
> >>                 device_merge_duplicate(device, duplicate);
> >> @@ -8950,7 +8977,7 @@ static void new_irk_callback(uint16_t index, uint16_t length,
> >>         if (!persistent)
> >>                 return;
> >>
> >> -       store_irk(adapter, &addr->bdaddr, addr->type, irk->val);
> >> +       store_irk(adapter, &addr->bdaddr, addr_type, irk->val);
> >>
> >>         btd_device_set_temporary(device, false);
> >>  }
> >> --
> >> 2.25.1
> >>
> >>
> >
> >
>
diff mbox series

Patch

diff --git a/src/adapter.c b/src/adapter.c
index 245de4456..4e5af9579 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -8647,6 +8647,7 @@  static void new_link_key_callback(uint16_t index, uint16_t length,
 	struct btd_adapter *adapter = user_data;
 	struct btd_device *device;
 	char dst[18];
+	uint8_t addr_type;
 
 	if (length < sizeof(*ev)) {
 		btd_error(adapter->dev_id, "Too small new link key event");
@@ -8666,7 +8667,13 @@  static void new_link_key_callback(uint16_t index, uint16_t length,
 		return;
 	}
 
-	device = btd_adapter_get_device(adapter, &addr->bdaddr, addr->type);
+	/*
+	 * For LE public address, key here is cross-transport derived,
+	 * so consider it as BR/EDR public address.
+	 *
+	 */
+	addr_type = addr->type == BDADDR_LE_PUBLIC ? BDADDR_BREDR : addr->type;
+	device = btd_adapter_get_device(adapter, &addr->bdaddr, addr_type);
 	if (!device) {
 		btd_error(adapter->dev_id,
 				"Unable to get device object for %s", dst);
@@ -8682,7 +8689,7 @@  static void new_link_key_callback(uint16_t index, uint16_t length,
 		device_set_bonded(device, BDADDR_BREDR);
 	}
 
-	bonding_complete(adapter, &addr->bdaddr, addr->type, 0);
+	bonding_complete(adapter, &addr->bdaddr, addr_type, 0);
 }
 
 static void store_ltk_group(struct btd_adapter *adapter, const bdaddr_t *peer,
@@ -8773,6 +8780,7 @@  static void new_long_term_key_callback(uint16_t index, uint16_t length,
 	struct btd_device *device;
 	bool persistent;
 	char dst[18];
+	uint8_t addr_type;
 
 	if (length < sizeof(*ev)) {
 		btd_error(adapter->dev_id, "Too small long term key event");
@@ -8784,7 +8792,13 @@  static void new_long_term_key_callback(uint16_t index, uint16_t length,
 	DBG("hci%u new LTK for %s type %u enc_size %u",
 		adapter->dev_id, dst, ev->key.type, ev->key.enc_size);
 
-	device = btd_adapter_get_device(adapter, &addr->bdaddr, addr->type);
+	/*
+	 * For BR/EDR public address, key here is cross-transport derived,
+	 * so consider it as LE public address for SMP.
+	 *
+	 */
+	addr_type = addr->type == BDADDR_BREDR ? BDADDR_LE_PUBLIC : addr->type;
+	device = btd_adapter_get_device(adapter, &addr->bdaddr, addr_type);
 	if (!device) {
 		btd_error(adapter->dev_id,
 				"Unable to get device object for %s", dst);
@@ -8802,8 +8816,7 @@  static void new_long_term_key_callback(uint16_t index, uint16_t length,
 	 * be persistently stored.
 	 *
 	 */
-	if (addr->type == BDADDR_LE_RANDOM &&
-				(addr->bdaddr.b[5] & 0xc0) != 0xc0)
+	if (addr_type == BDADDR_LE_RANDOM && (addr->bdaddr.b[5] & 0xc0) != 0xc0)
 		persistent = false;
 	else
 		persistent = !!ev->store_hint;
@@ -8817,15 +8830,15 @@  static void new_long_term_key_callback(uint16_t index, uint16_t length,
 		rand = le64_to_cpu(key->rand);
 
 		store_longtermkey(adapter, &key->addr.bdaddr,
-					key->addr.type, key->val, key->central,
+					addr_type, key->val, key->central,
 					key->type, key->enc_size, ediv, rand);
 
-		device_set_bonded(device, addr->type);
+		device_set_bonded(device, addr_type);
 	}
 
 	device_set_ltk(device, ev->key.val, ev->key.central, ev->key.enc_size);
 
-	bonding_complete(adapter, &addr->bdaddr, addr->type, 0);
+	bonding_complete(adapter, &addr->bdaddr, addr_type, 0);
 }
 
 static void new_csrk_callback(uint16_t index, uint16_t length,
@@ -8837,6 +8850,7 @@  static void new_csrk_callback(uint16_t index, uint16_t length,
 	struct btd_adapter *adapter = user_data;
 	struct btd_device *device;
 	char dst[18];
+	uint8_t addr_type;
 
 	if (length < sizeof(*ev)) {
 		btd_error(adapter->dev_id, "Too small CSRK event");
@@ -8848,7 +8862,13 @@  static void new_csrk_callback(uint16_t index, uint16_t length,
 	DBG("hci%u new CSRK for %s type %u", adapter->dev_id, dst,
 								ev->key.type);
 
-	device = btd_adapter_get_device(adapter, &addr->bdaddr, addr->type);
+	/*
+	 * For BR/EDR public address, key here is cross-transport derived,
+	 * so consider it as LE public address for SMP.
+	 *
+	 */
+	addr_type = addr->type == BDADDR_BREDR ? BDADDR_LE_PUBLIC : addr->type;
+	device = btd_adapter_get_device(adapter, &addr->bdaddr, addr_type);
 	if (!device) {
 		btd_error(adapter->dev_id,
 				"Unable to get device object for %s", dst);
@@ -8911,6 +8931,7 @@  static void new_irk_callback(uint16_t index, uint16_t length,
 	struct btd_device *device, *duplicate;
 	bool persistent;
 	char dst[18], rpa[18];
+	uint8_t addr_type;
 
 	if (length < sizeof(*ev)) {
 		btd_error(adapter->dev_id, "Too small New IRK event");
@@ -8922,16 +8943,22 @@  static void new_irk_callback(uint16_t index, uint16_t length,
 
 	DBG("hci%u new IRK for %s RPA %s", adapter->dev_id, dst, rpa);
 
+	/*
+	 * For BR/EDR public address, key here is cross-transport derived,
+	 * so consider it as LE public address for SMP.
+	 *
+	 */
+	addr_type = addr->type == BDADDR_BREDR ? BDADDR_LE_PUBLIC : addr->type;
 	if (bacmp(&ev->rpa, BDADDR_ANY)) {
 		device = btd_adapter_get_device(adapter, &ev->rpa,
 							BDADDR_LE_RANDOM);
 		duplicate = btd_adapter_find_device(adapter, &addr->bdaddr,
-								addr->type);
+								addr_type);
 		if (duplicate == device)
 			duplicate = NULL;
 	} else {
 		device = btd_adapter_get_device(adapter, &addr->bdaddr,
-								addr->type);
+								addr_type);
 		duplicate = NULL;
 	}
 
@@ -8941,7 +8968,7 @@  static void new_irk_callback(uint16_t index, uint16_t length,
 		return;
 	}
 
-	device_update_addr(device, &addr->bdaddr, addr->type);
+	device_update_addr(device, &addr->bdaddr, addr_type);
 
 	if (duplicate)
 		device_merge_duplicate(device, duplicate);
@@ -8950,7 +8977,7 @@  static void new_irk_callback(uint16_t index, uint16_t length,
 	if (!persistent)
 		return;
 
-	store_irk(adapter, &addr->bdaddr, addr->type, irk->val);
+	store_irk(adapter, &addr->bdaddr, addr_type, irk->val);
 
 	btd_device_set_temporary(device, false);
 }