From: Jiangbo Wu <[email protected]>
SMP over BR/EDR distributes keys when encryption key changed. It should
use correct address type with link.
Signed-off-by: Jiangbo Wu <[email protected]>
---
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 f00bf66..caa8254 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1509,9 +1509,11 @@ void mgmt_remote_name(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
u8 addr_type, s8 rssi, u8 *name, u8 name_len);
void mgmt_discovering(struct hci_dev *hdev, u8 discovering);
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_csrk(struct hci_dev *hdev, struct smp_csrk *csrk,
+void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, u8 link_type,
+ bool persistent);
+void mgmt_new_irk(struct hci_dev *hdev, struct smp_irk *irk, u8 link_type,
+ bool persistent);
+void mgmt_new_csrk(struct hci_dev *hdev, struct smp_csrk *csrk, u8 link_type,
bool persistent);
void mgmt_new_conn_param(struct hci_dev *hdev, bdaddr_t *bdaddr,
u8 bdaddr_type, u8 store_hint, u16 min_interval,
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 19b8a5e..2b12b72 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -6757,7 +6757,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, u8 link_type,
+ bool persistent)
{
struct mgmt_ev_new_long_term_key ev;
@@ -6781,7 +6782,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;
@@ -6800,7 +6801,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, u8 link_type,
+ bool persistent)
{
struct mgmt_ev_new_irk ev;
@@ -6810,13 +6812,13 @@ 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,
+void mgmt_new_csrk(struct hci_dev *hdev, struct smp_csrk *csrk, u8 link_type,
bool persistent)
{
struct mgmt_ev_new_csrk ev;
@@ -6839,7 +6841,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 43faf2a..58412d3 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -1043,7 +1043,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, hcon->type, persistent);
/* Now that user space can be considered to know the
* identity address track the connection based on it
@@ -1059,25 +1059,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, hcon->type, persistent);
}
if (smp->slave_csrk) {
smp->slave_csrk->bdaddr_type = hcon->dst_type;
bacpy(&smp->slave_csrk->bdaddr, &hcon->dst);
- mgmt_new_csrk(hdev, smp->slave_csrk, persistent);
+ mgmt_new_csrk(hdev, smp->slave_csrk, hcon->type, persistent);
}
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, hcon->type, persistent);
}
if (smp->slave_ltk) {
smp->slave_ltk->bdaddr_type = hcon->dst_type;
bacpy(&smp->slave_ltk->bdaddr, &hcon->dst);
- mgmt_new_ltk(hdev, smp->slave_ltk, persistent);
+ mgmt_new_ltk(hdev, smp->slave_ltk, hcon->type, persistent);
}
if (smp->link_key) {
--
1.9.1
Hi Jiangbo,
>>>>>>>>>> If pair a device that unpair firstly that remove encryption
>>>>>>>>>> key,
>>>>>>>>>> encryption key event will be emitted. kernel will receive
>>>>>>>>>> 'L2CAP_CID_SMP_BREDR' frame, and then it will use SMP to
>>>>>>>>>> distribute
>>>>>>>>>> key. SMP would like to use LTK, IRK and CRSK to notify user.
>>>>>>>>>> If it
>>>>>>>>>> don't identify device by which conn type they are, only marks
>>>>>>>>>> LE as
>>>>>>>>>> the device type,
>>>>>>>>>
>>>>>>>>> Why would that happen? Before SMP over BR/EDR happens pairing
>>>>>>>>> would
>>>>>>>>> have
>>>>>>>>> happened over BR/EDR, so bluetoothd should know that BR/EDR is
>>>>>>>>> supported
>>>>>>>>> as well (it would even be aware of an existing BR/EDR
>>>>>>>>> connection). Are
>>>>>>>>> you perhaps trying to work around some bluetoothd bug with all
>>>>>>>>> this?
>>>>>>>>
>>>>>>>> I use upstream bluez source code without change.
>>>>>>>>
>>>>>>>> Yes, bluetoothd scan will find device type is BR/EDR or LE. As my
>>>>>>>> case,
>>>>>>>> device is BR/EDR. But if kernel report CRSK notify, bluetoothd will
>>>>>>>> change
>>>>>>>>
>>>>>>>> the device type to LE. The code you can see:
>>>>>>>> new_csrk_callback -> btd_adapter_get_device ->
>>>>>
>>>>> btd_adapter_find_device
>>>>>
>>>>>>>> if (bdaddr_type == BDADDR_BREDR)
>>>>>>>>
>>>>>>>> device_set_bredr_support(device);
>>>>>>>>
>>>>>>>> else
>>>>>>>>
>>>>>>>> device_set_le_support(device, bdaddr_type);
>>>>>>>>
>>>>>>>> As Marcel mentioned before, LTK, IRK and CRSK are only valid for LE
>>>>>>>> link.
>>>>>>>> So the rootcause is why remote start to pair a BR/EDR device, the
>>>>>>>> kernel
>>>>>>>> will receive CRSK event.
>>>>>>>>
>>>>>>>> This is the first pair, and it will pair success even if receive
>>>>>>>> CRSK
>>>>>>>> notify. And the second and the next all pair will be failed with
>>>>>>>> remote
>>>>>>>> device unpair and then pair again.
>>>>>>>>
>>>>>>>>>> while Bluetoothd will use this 'addr' and 'addr type' to reply
>>>>>>>>>> the
>>>>>>>>>> comfirm to kernel.
>>>>>>>>>
>>>>>>>>> What reply are you talking about? There's no user interaction
>>>>>>>>> involved
>>>>>>>>> with SMP over BR/EDR - that would already have occurred when SSP
>>>>>>>>> over
>>>>>>>>> BR/EDR happened.
>>>>>>>>
>>>>>>>> Sorry to confuse the case, the pairing failed coming with next pair
>>>>>>>> procedure. Because at the last pair with CRSK notify, device type
>>>>>>>> will
>>>>>>>> be
>>>>>>>> changed to LE, following is the failed scenario after last success
>>>>>>>> with
>>>>>>>> CRSK notify. Remote unpair and pair again.
>>>>>>>>
>>>>>>>> This reply is SPP, user confirm passkey reply. When pairing
>>>>>>>> proceduce,
>>>>>>>> User
>>>>>>>> confirm the pairing request through bluetoothd, that will send mgmt
>>>>>>>> op
>>>>>>>> 'MGMT_OP_USER_CONFIRM_REPLY' with device address and device type in
>>>>>>>> mgmt_cp_user_confirm_reply. Kernel use the device address and type
>>>>>>>> to
>>>>>>>> lookup hci conn. Unfortunately, it will lookup hci_conn from LE
>>>>>>>> hashtable, that don't include hci conn. So spp reply couldn't send
>>>>>>>> to
>>>>>>>> remote, caused pair failed.
>>>>>>>>
>>>>>>>>>> At the same time kernel always uses them to lookup hci_conn in
>>>>>>>>>> LE
>>>>>>>>>> hashtable firstly, because addr type always marks as LE.
>>>>>>>>>> Obviously
>>>>>>>>>> it
>>>>>>>>>> will failed with SMP over BR/EDR.
>>>>>>>>>
>>>>>>>>> I don't follow this either since there shouldn't have been any
>>>>>>>>> "reply"
>>>>>>>>> from user space for SMP over BR/EDR. All there should be are
>>>>>>>>> events
>>>>>>>>> from
>>>>>>>>> the kernel for the generated LE keys.
>>>>>>>>>
>>>>>>>>>> Actually, SPM is only for LE in SPEC,
>>>>>>>>>
>>>>>>>>> That's not true. SMP is specified both for LE-U and ACL-U.
>>>>>>>>>
>>>>>>>>>> but kernel already support and use SMP over BR/EDR. if BR/EDR
>>>>>>>>>> exchanges key with SMP, it will never reply pairing response to
>>>>>>>>>> remote, in other words it will be never paired, that is
>>>>>>>>>> happened in
>>>>>>>>>> our products.
>>>>>>>>>
>>>>>>>>> Szymon recently implemented SMP over BR/EDR for Zephyr and used
>>>>>>>>> Linux/BlueZ as a reference for testing. He didn't report any
>>>>>>>>> issues
>>>>>>>>> like
>>>>>>>>> this. It might help if you could provide some logs (particularly
>>>>>>>>> HCI/btmon but also from bluetoothd) to understand what's the
>>>>>>>>> actual
>>>>>>>>> issue you're seeing.
>>>>>>>>>
>>>>>>>>> Johan
>>>>>>>>
>>>>>>>> Sorry to confuse this issue, the log is not in my hand right now,
>>>>>>>> so it maybe later.
>>>>>>>
>>>>>>> So I was able to reproduce this issue. This is bluetoothd bug and not
>>>>>>> kernel one. This bug is no specific to cross-transport pairing. It
>>>>>>> can
>>>>>>> happen with any dual-mode device that is doing BR/EDR pairing while
>>>>>>> being
>>>>>>> known as dual mode by bluetoothd when agent replies with passkey or
>>>>>>> confirmation.
>>>>>>>
>>>>>>> To fix this we probably need to hold extra information in
>>>>>>> 'struct authentication_req' in device.c about type of pairing (LE or
>>>>>>> BR/EDR). This is not a one-liner-fix so I don't have a patch ready
>>>>>>> yet.
>>>>>>
>>>>>> Totally agree with you about dual-mode device pairing known as dual
>>>>>> mode.
>>>>>> But i want to known is that reasonable about device is to do BR/EDR
>>>>>> pairing
>>>>>> will generate CRSK notify? I'm very intersting about this fixing, this
>>>>>> bug
>>>>>> is hight priority in our product. In my opinion hold extra informatin
>>>>>> in
>>>>>> 'struct authentication_req' may not fix this bug. Because if CRSK event
>>>>>> is
>>>>>> still report, then bluetoothd will change the device type to LE even if
>>>>>> we
>>>>>> pair device that is scaned with BR/EDR. So i think the rootcase is find
>>>>>> does CRSK event make sense in BR/EDR pairing, and how to handle CRSK
>>>>>> events
>>>>>> in BR/EDR pairing if it make sense. I'm confuse with those.
>>>>>
>>>>> It doesn't change the device to LE but to dual mode device. This is
>>>>> *cross-transport* pairing so keys for other transport are generated.
>>>>> baddr_type specify only LE address type, not BR/EDR since there is no
>>>>> address type for BR/EDR. This is mostly true but few places in
>>>>> bluetoothd seem to asusme that for device supporting BR/EDR type is
>>>>> equal 0. Which is not true if device is dual mode.
>>>>>
>>>>> You should be able to reproduce this bug with dual-mode devices that
>>>>> don't
>>>>> support cross-transport pairing: enable advertising, scan from linux,
>>>>> when
>>>>> device is found stop advertising and make device discoverable over
>>>>> BR/EDR
>>>>> (inquiry). When device is found over BR/EDR stop scanning and start
>>>>> pairing.> >
>>>>>> I noticed that if quikly reply the passkey confirm, this bug always be
>>>>>> reproduced, but if wait for 2~3s to reply the passkey confirm, it works
>>>>>> well every time. In terms of code, wait for 2~3s will cause l2cap chan
>>>>>> timeout for info timer that created by HCI_EV_REMOTE_EXT_FEATURES
>>>>>> event,
>>>>>> and timeout will change l2cap chan to BT_CONNECTED. So next SMP
>>>>>> resume/ready don't distribute key also CRSK events.
>>>>>>
>>>>>> It can't reproduce with btmgmt, because it reply passkey confirm always
>>>>>> only use BR/EDR in 'struct mgmt_cp_user_confirm_reply' not use device
>>>>>> relation type.
>>>>>>
>>>>>> bluetoothd.log and btmon.log are attached. It records two pair request
>>>>>> sequence, one is pair success that have CRSK event, another is next
>>>>>> pair
>>>>>> reqeust don't success any, hope those maybe help you to annlyze this
>>>>>> bug.
>>>>
>>>> I've sent a patch "[RFC] core: Fix BR/EDR pairing for dual mode devices".
>>>> Please check if this solves issue you are seeing.
>>>
>>> Thanks for your patch. Maybe it can resolve the issue, but it will cause
>>> other issues. For example, some operations also use device->bdaddr as
>>> parameter in MGMT operations, unpair is the same. If kernel hold the device
>>> as BR/EDR type in hdev->conn_hash, unpair operator won't find the hci conn,
>>> so it couldn't terminal the link. but the link is exist at the moment. MGMT
>>> also complete when don't terminal the link. So bluetoothd and the user
>>> don't feel the different. But is that we would like? The code implies we
>>> should terminate the connection if it is exist.
>>>
>>> The patch use auth_req with BDADDR_BREDR to handle pairing request. It could
>>> resolve the pairing procedure. But kernel hold the device as BR/EDR, even
>>> if cross-tranport is generated on BR/EDR hci conn. Meanwhile bluetoothd
>>> will set device->bdaddr_type to BDADDR_LE_PUBLIC with new_csrk_callback
>>> that generated by cross-transport. I mean, the user-space hold the device
>>> with BDADDR_LE_PUBLIC (yes, device->bredr and device->le are true, but
>>> addr-type is BDADDR_LE_PUBLIC), the kernel hold the device with
>>> BDADDR_BREDR. Whenever user-space use couple {addr, addr-type} to send
>>> request to kernel. It maybe failed.
>>
>> bdaddr_type is used only with LE address (not with BR/EDR address) so I'm not
>> sure what other issues you are talking about. Could you provide some logs?
>>
>>>
>>> As the end. In my case, i don't do the steps you mentioned(enable
>>> advertising, scan from linux, stop advertising). I only start BR/EDR
>>> discovering with discovery filter, pair device and unpair device to
>>> reproduce this bug. I don't start/stop LE advertising.
>>
>> So you still see your issue with this patch?
>
> You can see in my case, all of them are BR/EDR. I use D-Bus method 'SetDiscoveryFilter'
> to enable bredr scan only. So scanning is BR/EDR, pairing is BR/EDR, HCI conn is
> created with BR/EDR. Even if crsk is generated, it's also transport SMP over BR/EDR.
SMP over BR/EDR is for transporting LE keys. It is not for transporting BR/EDR keys. Full stop here. Please understand what I wrote initially. The IRK and CSRK transported via SMP over BR/EDR are LE keys. If they are transported, your remote device offers cross-transport pairing and it is a dual-mode device.
That you discover only BR/EDR devices has nothing to do with when pairing them. If you get keys for both BR/EDR and LE, then that is because BlueZ supports cross-transport pairing and so does the remote side.
> bluetoothd/kernel all consider peers is BR/EDR.
>
> Your case enable LE advertising, so you think peers is LE. But it's not correct
> in my case. I only use BR/EDR. So your patch can resolve the pairing procedure,
> but what about other operations? They all hold the addr type is BR/EDR, not LE.
> I maintain my submission patch locally, and it works fine in our product.
Do _not_ maintain the kernel patch you send initially. It is _wrong_ and it will make it specification non-compliant.
If the remote side offers cross-transport pairing, then the remote side is a dual-mode device. It is plain simple as that. It means it will allow you to also connect via LE since all the keys are exchanged. LE related keys like LTK, IRK and CSRK are send with the LE address type. That is meant to be this way.
Regards
Marcel
On Mon, Oct 24, 2016 at 08:40:18PM +0200, Szymon Janc wrote:
> Hi Jiangbo,
>
> On Monday, 24 October 2016 15:30:34 CEST Wu,Jiangbo wrote:
> > Hi Szymon,
> >
> > On Sat, Oct 22, 2016 at 11:17:21AM +0200, Szymon Janc wrote:
> > > Hi Jiangbo,
> > >
> > > On 18 October 2016 at 22:32, Szymon Janc <[email protected]> wrote:
> > > > Hi Jiangbo,
> > > >
> > > > On Tuesday, 18 October 2016 18:23:38 CEST Wu,Jiangbo wrote:
> > > >> Hi,
> > > >>
> > > >> On Mon, Oct 17, 2016 at 11:05:33PM +0200, Szymon Janc wrote:
> > > >> > Hi,
> > > >> >
> > > >> > On Saturday, 15 October 2016 00:43:13 CEST wujiangbo wrote:
> > > >> > > On Fri, Oct 14, 2016 at 05:19:38PM +0300, Johan Hedberg wrote:
> > > >> > > > Hi Jiangbo,
> > > >> > > >
> > > >> > > > Please don't top-post on this list.
> > > >> > > >
> > > >> > > > On Fri, Oct 14, 2016, Wu, Jiangbo wrote:
> > > >> > > > > If pair a device that unpair firstly that remove encryption
> > > >> > > > > key,
> > > >> > > > > encryption key event will be emitted. kernel will receive
> > > >> > > > > 'L2CAP_CID_SMP_BREDR' frame, and then it will use SMP to
> > > >> > > > > distribute
> > > >> > > > > key. SMP would like to use LTK, IRK and CRSK to notify user.
> > > >> > > > > If it
> > > >> > > > > don't identify device by which conn type they are, only marks
> > > >> > > > > LE as
> > > >> > > > > the device type,
> > > >> > > >
> > > >> > > > Why would that happen? Before SMP over BR/EDR happens pairing
> > > >> > > > would
> > > >> > > > have
> > > >> > > > happened over BR/EDR, so bluetoothd should know that BR/EDR is
> > > >> > > > supported
> > > >> > > > as well (it would even be aware of an existing BR/EDR
> > > >> > > > connection). Are
> > > >> > > > you perhaps trying to work around some bluetoothd bug with all
> > > >> > > > this?
> > > >> > >
> > > >> > > I use upstream bluez source code without change.
> > > >> > >
> > > >> > > Yes, bluetoothd scan will find device type is BR/EDR or LE. As my
> > > >> > > case,
> > > >> > > device is BR/EDR. But if kernel report CRSK notify, bluetoothd will
> > > >> > > change
> > > >> > >
> > > >> > > the device type to LE. The code you can see:
> > > >> > > new_csrk_callback -> btd_adapter_get_device ->
> > > >
> > > > btd_adapter_find_device
> > > >
> > > >> > > if (bdaddr_type == BDADDR_BREDR)
> > > >> > >
> > > >> > > device_set_bredr_support(device);
> > > >> > >
> > > >> > > else
> > > >> > >
> > > >> > > device_set_le_support(device, bdaddr_type);
> > > >> > >
> > > >> > > As Marcel mentioned before, LTK, IRK and CRSK are only valid for LE
> > > >> > > link.
> > > >> > > So the rootcause is why remote start to pair a BR/EDR device, the
> > > >> > > kernel
> > > >> > > will receive CRSK event.
> > > >> > >
> > > >> > > This is the first pair, and it will pair success even if receive
> > > >> > > CRSK
> > > >> > > notify. And the second and the next all pair will be failed with
> > > >> > > remote
> > > >> > > device unpair and then pair again.
> > > >> > >
> > > >> > > > > while Bluetoothd will use this 'addr' and 'addr type' to reply
> > > >> > > > > the
> > > >> > > > > comfirm to kernel.
> > > >> > > >
> > > >> > > > What reply are you talking about? There's no user interaction
> > > >> > > > involved
> > > >> > > > with SMP over BR/EDR - that would already have occurred when SSP
> > > >> > > > over
> > > >> > > > BR/EDR happened.
> > > >> > >
> > > >> > > Sorry to confuse the case, the pairing failed coming with next pair
> > > >> > > procedure. Because at the last pair with CRSK notify, device type
> > > >> > > will
> > > >> > > be
> > > >> > > changed to LE, following is the failed scenario after last success
> > > >> > > with
> > > >> > > CRSK notify. Remote unpair and pair again.
> > > >> > >
> > > >> > > This reply is SPP, user confirm passkey reply. When pairing
> > > >> > > proceduce,
> > > >> > > User
> > > >> > > confirm the pairing request through bluetoothd, that will send mgmt
> > > >> > > op
> > > >> > > 'MGMT_OP_USER_CONFIRM_REPLY' with device address and device type in
> > > >> > > mgmt_cp_user_confirm_reply. Kernel use the device address and type
> > > >> > > to
> > > >> > > lookup hci conn. Unfortunately, it will lookup hci_conn from LE
> > > >> > > hashtable, that don't include hci conn. So spp reply couldn't send
> > > >> > > to
> > > >> > > remote, caused pair failed.
> > > >> > >
> > > >> > > > > At the same time kernel always uses them to lookup hci_conn in
> > > >> > > > > LE
> > > >> > > > > hashtable firstly, because addr type always marks as LE.
> > > >> > > > > Obviously
> > > >> > > > > it
> > > >> > > > > will failed with SMP over BR/EDR.
> > > >> > > >
> > > >> > > > I don't follow this either since there shouldn't have been any
> > > >> > > > "reply"
> > > >> > > > from user space for SMP over BR/EDR. All there should be are
> > > >> > > > events
> > > >> > > > from
> > > >> > > > the kernel for the generated LE keys.
> > > >> > > >
> > > >> > > > > Actually, SPM is only for LE in SPEC,
> > > >> > > >
> > > >> > > > That's not true. SMP is specified both for LE-U and ACL-U.
> > > >> > > >
> > > >> > > > > but kernel already support and use SMP over BR/EDR. if BR/EDR
> > > >> > > > > exchanges key with SMP, it will never reply pairing response to
> > > >> > > > > remote, in other words it will be never paired, that is
> > > >> > > > > happened in
> > > >> > > > > our products.
> > > >> > > >
> > > >> > > > Szymon recently implemented SMP over BR/EDR for Zephyr and used
> > > >> > > > Linux/BlueZ as a reference for testing. He didn't report any
> > > >> > > > issues
> > > >> > > > like
> > > >> > > > this. It might help if you could provide some logs (particularly
> > > >> > > > HCI/btmon but also from bluetoothd) to understand what's the
> > > >> > > > actual
> > > >> > > > issue you're seeing.
> > > >> > > >
> > > >> > > > Johan
> > > >> > >
> > > >> > > Sorry to confuse this issue, the log is not in my hand right now,
> > > >> > > so it maybe later.
> > > >> >
> > > >> > So I was able to reproduce this issue. This is bluetoothd bug and not
> > > >> > kernel one. This bug is no specific to cross-transport pairing. It
> > > >> > can
> > > >> > happen with any dual-mode device that is doing BR/EDR pairing while
> > > >> > being
> > > >> > known as dual mode by bluetoothd when agent replies with passkey or
> > > >> > confirmation.
> > > >> >
> > > >> > To fix this we probably need to hold extra information in
> > > >> > 'struct authentication_req' in device.c about type of pairing (LE or
> > > >> > BR/EDR). This is not a one-liner-fix so I don't have a patch ready
> > > >> > yet.
> > > >>
> > > >> Totally agree with you about dual-mode device pairing known as dual
> > > >> mode.
> > > >> But i want to known is that reasonable about device is to do BR/EDR
> > > >> pairing
> > > >> will generate CRSK notify? I'm very intersting about this fixing, this
> > > >> bug
> > > >> is hight priority in our product. In my opinion hold extra informatin
> > > >> in
> > > >> 'struct authentication_req' may not fix this bug. Because if CRSK event
> > > >> is
> > > >> still report, then bluetoothd will change the device type to LE even if
> > > >> we
> > > >> pair device that is scaned with BR/EDR. So i think the rootcase is find
> > > >> does CRSK event make sense in BR/EDR pairing, and how to handle CRSK
> > > >> events
> > > >> in BR/EDR pairing if it make sense. I'm confuse with those.
> > > >
> > > > It doesn't change the device to LE but to dual mode device. This is
> > > > *cross-transport* pairing so keys for other transport are generated.
> > > > baddr_type specify only LE address type, not BR/EDR since there is no
> > > > address type for BR/EDR. This is mostly true but few places in
> > > > bluetoothd seem to asusme that for device supporting BR/EDR type is
> > > > equal 0. Which is not true if device is dual mode.
> > > >
> > > > You should be able to reproduce this bug with dual-mode devices that
> > > > don't
> > > > support cross-transport pairing: enable advertising, scan from linux,
> > > > when
> > > > device is found stop advertising and make device discoverable over
> > > > BR/EDR
> > > > (inquiry). When device is found over BR/EDR stop scanning and start
> > > > pairing.> >
> > > >> I noticed that if quikly reply the passkey confirm, this bug always be
> > > >> reproduced, but if wait for 2~3s to reply the passkey confirm, it works
> > > >> well every time. In terms of code, wait for 2~3s will cause l2cap chan
> > > >> timeout for info timer that created by HCI_EV_REMOTE_EXT_FEATURES
> > > >> event,
> > > >> and timeout will change l2cap chan to BT_CONNECTED. So next SMP
> > > >> resume/ready don't distribute key also CRSK events.
> > > >>
> > > >> It can't reproduce with btmgmt, because it reply passkey confirm always
> > > >> only use BR/EDR in 'struct mgmt_cp_user_confirm_reply' not use device
> > > >> relation type.
> > > >>
> > > >> bluetoothd.log and btmon.log are attached. It records two pair request
> > > >> sequence, one is pair success that have CRSK event, another is next
> > > >> pair
> > > >> reqeust don't success any, hope those maybe help you to annlyze this
> > > >> bug.
> > >
> > > I've sent a patch "[RFC] core: Fix BR/EDR pairing for dual mode devices".
> > > Please check if this solves issue you are seeing.
> >
> > Thanks for your patch. Maybe it can resolve the issue, but it will cause
> > other issues. For example, some operations also use device->bdaddr as
> > parameter in MGMT operations, unpair is the same. If kernel hold the device
> > as BR/EDR type in hdev->conn_hash, unpair operator won't find the hci conn,
> > so it couldn't terminal the link. but the link is exist at the moment. MGMT
> > also complete when don't terminal the link. So bluetoothd and the user
> > don't feel the different. But is that we would like? The code implies we
> > should terminate the connection if it is exist.
> >
> > The patch use auth_req with BDADDR_BREDR to handle pairing request. It could
> > resolve the pairing procedure. But kernel hold the device as BR/EDR, even
> > if cross-tranport is generated on BR/EDR hci conn. Meanwhile bluetoothd
> > will set device->bdaddr_type to BDADDR_LE_PUBLIC with new_csrk_callback
> > that generated by cross-transport. I mean, the user-space hold the device
> > with BDADDR_LE_PUBLIC (yes, device->bredr and device->le are true, but
> > addr-type is BDADDR_LE_PUBLIC), the kernel hold the device with
> > BDADDR_BREDR. Whenever user-space use couple {addr, addr-type} to send
> > request to kernel. It maybe failed.
>
> bdaddr_type is used only with LE address (not with BR/EDR address) so I'm not
> sure what other issues you are talking about. Could you provide some logs?
>
> >
> > As the end. In my case, i don't do the steps you mentioned(enable
> > advertising, scan from linux, stop advertising). I only start BR/EDR
> > discovering with discovery filter, pair device and unpair device to
> > reproduce this bug. I don't start/stop LE advertising.
>
> So you still see your issue with this patch?
You can see in my case, all of them are BR/EDR. I use D-Bus method 'SetDiscoveryFilter'
to enable bredr scan only. So scanning is BR/EDR, pairing is BR/EDR, HCI conn is
created with BR/EDR. Even if crsk is generated, it's also transport SMP over BR/EDR.
bluetoothd/kernel all consider peers is BR/EDR.
Your case enable LE advertising, so you think peers is LE. But it's not correct
in my case. I only use BR/EDR. So your patch can resolve the pairing procedure,
but what about other operations? They all hold the addr type is BR/EDR, not LE.
I maintain my submission patch locally, and it works fine in our product.
Also thanks for your attention.
Thanks
Jiangbo
>
> --
> pozdrawiam
> Szymon Janc
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Jiangbo,
On Monday, 24 October 2016 15:30:34 CEST Wu,Jiangbo wrote:
> Hi Szymon,
>
> On Sat, Oct 22, 2016 at 11:17:21AM +0200, Szymon Janc wrote:
> > Hi Jiangbo,
> >
> > On 18 October 2016 at 22:32, Szymon Janc <[email protected]> wrote:
> > > Hi Jiangbo,
> > >
> > > On Tuesday, 18 October 2016 18:23:38 CEST Wu,Jiangbo wrote:
> > >> Hi,
> > >>
> > >> On Mon, Oct 17, 2016 at 11:05:33PM +0200, Szymon Janc wrote:
> > >> > Hi,
> > >> >
> > >> > On Saturday, 15 October 2016 00:43:13 CEST wujiangbo wrote:
> > >> > > On Fri, Oct 14, 2016 at 05:19:38PM +0300, Johan Hedberg wrote:
> > >> > > > Hi Jiangbo,
> > >> > > >
> > >> > > > Please don't top-post on this list.
> > >> > > >
> > >> > > > On Fri, Oct 14, 2016, Wu, Jiangbo wrote:
> > >> > > > > If pair a device that unpair firstly that remove encryption
> > >> > > > > key,
> > >> > > > > encryption key event will be emitted. kernel will receive
> > >> > > > > 'L2CAP_CID_SMP_BREDR' frame, and then it will use SMP to
> > >> > > > > distribute
> > >> > > > > key. SMP would like to use LTK, IRK and CRSK to notify user.
> > >> > > > > If it
> > >> > > > > don't identify device by which conn type they are, only marks
> > >> > > > > LE as
> > >> > > > > the device type,
> > >> > > >
> > >> > > > Why would that happen? Before SMP over BR/EDR happens pairing
> > >> > > > would
> > >> > > > have
> > >> > > > happened over BR/EDR, so bluetoothd should know that BR/EDR is
> > >> > > > supported
> > >> > > > as well (it would even be aware of an existing BR/EDR
> > >> > > > connection). Are
> > >> > > > you perhaps trying to work around some bluetoothd bug with all
> > >> > > > this?
> > >> > >
> > >> > > I use upstream bluez source code without change.
> > >> > >
> > >> > > Yes, bluetoothd scan will find device type is BR/EDR or LE. As my
> > >> > > case,
> > >> > > device is BR/EDR. But if kernel report CRSK notify, bluetoothd will
> > >> > > change
> > >> > >
> > >> > > the device type to LE. The code you can see:
> > >> > > new_csrk_callback -> btd_adapter_get_device ->
> > >
> > > btd_adapter_find_device
> > >
> > >> > > if (bdaddr_type == BDADDR_BREDR)
> > >> > >
> > >> > > device_set_bredr_support(device);
> > >> > >
> > >> > > else
> > >> > >
> > >> > > device_set_le_support(device, bdaddr_type);
> > >> > >
> > >> > > As Marcel mentioned before, LTK, IRK and CRSK are only valid for LE
> > >> > > link.
> > >> > > So the rootcause is why remote start to pair a BR/EDR device, the
> > >> > > kernel
> > >> > > will receive CRSK event.
> > >> > >
> > >> > > This is the first pair, and it will pair success even if receive
> > >> > > CRSK
> > >> > > notify. And the second and the next all pair will be failed with
> > >> > > remote
> > >> > > device unpair and then pair again.
> > >> > >
> > >> > > > > while Bluetoothd will use this 'addr' and 'addr type' to reply
> > >> > > > > the
> > >> > > > > comfirm to kernel.
> > >> > > >
> > >> > > > What reply are you talking about? There's no user interaction
> > >> > > > involved
> > >> > > > with SMP over BR/EDR - that would already have occurred when SSP
> > >> > > > over
> > >> > > > BR/EDR happened.
> > >> > >
> > >> > > Sorry to confuse the case, the pairing failed coming with next pair
> > >> > > procedure. Because at the last pair with CRSK notify, device type
> > >> > > will
> > >> > > be
> > >> > > changed to LE, following is the failed scenario after last success
> > >> > > with
> > >> > > CRSK notify. Remote unpair and pair again.
> > >> > >
> > >> > > This reply is SPP, user confirm passkey reply. When pairing
> > >> > > proceduce,
> > >> > > User
> > >> > > confirm the pairing request through bluetoothd, that will send mgmt
> > >> > > op
> > >> > > 'MGMT_OP_USER_CONFIRM_REPLY' with device address and device type in
> > >> > > mgmt_cp_user_confirm_reply. Kernel use the device address and type
> > >> > > to
> > >> > > lookup hci conn. Unfortunately, it will lookup hci_conn from LE
> > >> > > hashtable, that don't include hci conn. So spp reply couldn't send
> > >> > > to
> > >> > > remote, caused pair failed.
> > >> > >
> > >> > > > > At the same time kernel always uses them to lookup hci_conn in
> > >> > > > > LE
> > >> > > > > hashtable firstly, because addr type always marks as LE.
> > >> > > > > Obviously
> > >> > > > > it
> > >> > > > > will failed with SMP over BR/EDR.
> > >> > > >
> > >> > > > I don't follow this either since there shouldn't have been any
> > >> > > > "reply"
> > >> > > > from user space for SMP over BR/EDR. All there should be are
> > >> > > > events
> > >> > > > from
> > >> > > > the kernel for the generated LE keys.
> > >> > > >
> > >> > > > > Actually, SPM is only for LE in SPEC,
> > >> > > >
> > >> > > > That's not true. SMP is specified both for LE-U and ACL-U.
> > >> > > >
> > >> > > > > but kernel already support and use SMP over BR/EDR. if BR/EDR
> > >> > > > > exchanges key with SMP, it will never reply pairing response to
> > >> > > > > remote, in other words it will be never paired, that is
> > >> > > > > happened in
> > >> > > > > our products.
> > >> > > >
> > >> > > > Szymon recently implemented SMP over BR/EDR for Zephyr and used
> > >> > > > Linux/BlueZ as a reference for testing. He didn't report any
> > >> > > > issues
> > >> > > > like
> > >> > > > this. It might help if you could provide some logs (particularly
> > >> > > > HCI/btmon but also from bluetoothd) to understand what's the
> > >> > > > actual
> > >> > > > issue you're seeing.
> > >> > > >
> > >> > > > Johan
> > >> > >
> > >> > > Sorry to confuse this issue, the log is not in my hand right now,
> > >> > > so it maybe later.
> > >> >
> > >> > So I was able to reproduce this issue. This is bluetoothd bug and not
> > >> > kernel one. This bug is no specific to cross-transport pairing. It
> > >> > can
> > >> > happen with any dual-mode device that is doing BR/EDR pairing while
> > >> > being
> > >> > known as dual mode by bluetoothd when agent replies with passkey or
> > >> > confirmation.
> > >> >
> > >> > To fix this we probably need to hold extra information in
> > >> > 'struct authentication_req' in device.c about type of pairing (LE or
> > >> > BR/EDR). This is not a one-liner-fix so I don't have a patch ready
> > >> > yet.
> > >>
> > >> Totally agree with you about dual-mode device pairing known as dual
> > >> mode.
> > >> But i want to known is that reasonable about device is to do BR/EDR
> > >> pairing
> > >> will generate CRSK notify? I'm very intersting about this fixing, this
> > >> bug
> > >> is hight priority in our product. In my opinion hold extra informatin
> > >> in
> > >> 'struct authentication_req' may not fix this bug. Because if CRSK event
> > >> is
> > >> still report, then bluetoothd will change the device type to LE even if
> > >> we
> > >> pair device that is scaned with BR/EDR. So i think the rootcase is find
> > >> does CRSK event make sense in BR/EDR pairing, and how to handle CRSK
> > >> events
> > >> in BR/EDR pairing if it make sense. I'm confuse with those.
> > >
> > > It doesn't change the device to LE but to dual mode device. This is
> > > *cross-transport* pairing so keys for other transport are generated.
> > > baddr_type specify only LE address type, not BR/EDR since there is no
> > > address type for BR/EDR. This is mostly true but few places in
> > > bluetoothd seem to asusme that for device supporting BR/EDR type is
> > > equal 0. Which is not true if device is dual mode.
> > >
> > > You should be able to reproduce this bug with dual-mode devices that
> > > don't
> > > support cross-transport pairing: enable advertising, scan from linux,
> > > when
> > > device is found stop advertising and make device discoverable over
> > > BR/EDR
> > > (inquiry). When device is found over BR/EDR stop scanning and start
> > > pairing.> >
> > >> I noticed that if quikly reply the passkey confirm, this bug always be
> > >> reproduced, but if wait for 2~3s to reply the passkey confirm, it works
> > >> well every time. In terms of code, wait for 2~3s will cause l2cap chan
> > >> timeout for info timer that created by HCI_EV_REMOTE_EXT_FEATURES
> > >> event,
> > >> and timeout will change l2cap chan to BT_CONNECTED. So next SMP
> > >> resume/ready don't distribute key also CRSK events.
> > >>
> > >> It can't reproduce with btmgmt, because it reply passkey confirm always
> > >> only use BR/EDR in 'struct mgmt_cp_user_confirm_reply' not use device
> > >> relation type.
> > >>
> > >> bluetoothd.log and btmon.log are attached. It records two pair request
> > >> sequence, one is pair success that have CRSK event, another is next
> > >> pair
> > >> reqeust don't success any, hope those maybe help you to annlyze this
> > >> bug.
> >
> > I've sent a patch "[RFC] core: Fix BR/EDR pairing for dual mode devices".
> > Please check if this solves issue you are seeing.
>
> Thanks for your patch. Maybe it can resolve the issue, but it will cause
> other issues. For example, some operations also use device->bdaddr as
> parameter in MGMT operations, unpair is the same. If kernel hold the device
> as BR/EDR type in hdev->conn_hash, unpair operator won't find the hci conn,
> so it couldn't terminal the link. but the link is exist at the moment. MGMT
> also complete when don't terminal the link. So bluetoothd and the user
> don't feel the different. But is that we would like? The code implies we
> should terminate the connection if it is exist.
>
> The patch use auth_req with BDADDR_BREDR to handle pairing request. It could
> resolve the pairing procedure. But kernel hold the device as BR/EDR, even
> if cross-tranport is generated on BR/EDR hci conn. Meanwhile bluetoothd
> will set device->bdaddr_type to BDADDR_LE_PUBLIC with new_csrk_callback
> that generated by cross-transport. I mean, the user-space hold the device
> with BDADDR_LE_PUBLIC (yes, device->bredr and device->le are true, but
> addr-type is BDADDR_LE_PUBLIC), the kernel hold the device with
> BDADDR_BREDR. Whenever user-space use couple {addr, addr-type} to send
> request to kernel. It maybe failed.
bdaddr_type is used only with LE address (not with BR/EDR address) so I'm not
sure what other issues you are talking about. Could you provide some logs?
>
> As the end. In my case, i don't do the steps you mentioned(enable
> advertising, scan from linux, stop advertising). I only start BR/EDR
> discovering with discovery filter, pair device and unpair device to
> reproduce this bug. I don't start/stop LE advertising.
So you still see your issue with this patch?
--
pozdrawiam
Szymon Janc
Hi Szymon,
On Sat, Oct 22, 2016 at 11:17:21AM +0200, Szymon Janc wrote:
> Hi Jiangbo,
>
> On 18 October 2016 at 22:32, Szymon Janc <[email protected]> wrote:
> > Hi Jiangbo,
> >
> > On Tuesday, 18 October 2016 18:23:38 CEST Wu,Jiangbo wrote:
> >> Hi,
> >>
> >> On Mon, Oct 17, 2016 at 11:05:33PM +0200, Szymon Janc wrote:
> >> > Hi,
> >> >
> >> > On Saturday, 15 October 2016 00:43:13 CEST wujiangbo wrote:
> >> > > On Fri, Oct 14, 2016 at 05:19:38PM +0300, Johan Hedberg wrote:
> >> > > > Hi Jiangbo,
> >> > > >
> >> > > > Please don't top-post on this list.
> >> > > >
> >> > > > On Fri, Oct 14, 2016, Wu, Jiangbo wrote:
> >> > > > > If pair a device that unpair firstly that remove encryption key,
> >> > > > > encryption key event will be emitted. kernel will receive
> >> > > > > 'L2CAP_CID_SMP_BREDR' frame, and then it will use SMP to distribute
> >> > > > > key. SMP would like to use LTK, IRK and CRSK to notify user. If it
> >> > > > > don't identify device by which conn type they are, only marks LE as
> >> > > > > the device type,
> >> > > >
> >> > > > Why would that happen? Before SMP over BR/EDR happens pairing would
> >> > > > have
> >> > > > happened over BR/EDR, so bluetoothd should know that BR/EDR is
> >> > > > supported
> >> > > > as well (it would even be aware of an existing BR/EDR connection). Are
> >> > > > you perhaps trying to work around some bluetoothd bug with all this?
> >> > >
> >> > > I use upstream bluez source code without change.
> >> > >
> >> > > Yes, bluetoothd scan will find device type is BR/EDR or LE. As my case,
> >> > > device is BR/EDR. But if kernel report CRSK notify, bluetoothd will
> >> > > change
> >> > >
> >> > > the device type to LE. The code you can see:
> >> > > new_csrk_callback -> btd_adapter_get_device ->
> > btd_adapter_find_device
> >> > >
> >> > > if (bdaddr_type == BDADDR_BREDR)
> >> > >
> >> > > device_set_bredr_support(device);
> >> > >
> >> > > else
> >> > >
> >> > > device_set_le_support(device, bdaddr_type);
> >> > >
> >> > > As Marcel mentioned before, LTK, IRK and CRSK are only valid for LE
> >> > > link.
> >> > > So the rootcause is why remote start to pair a BR/EDR device, the kernel
> >> > > will receive CRSK event.
> >> > >
> >> > > This is the first pair, and it will pair success even if receive CRSK
> >> > > notify. And the second and the next all pair will be failed with remote
> >> > > device unpair and then pair again.
> >> > >
> >> > > > > while Bluetoothd will use this 'addr' and 'addr type' to reply the
> >> > > > > comfirm to kernel.
> >> > > >
> >> > > > What reply are you talking about? There's no user interaction involved
> >> > > > with SMP over BR/EDR - that would already have occurred when SSP over
> >> > > > BR/EDR happened.
> >> > >
> >> > > Sorry to confuse the case, the pairing failed coming with next pair
> >> > > procedure. Because at the last pair with CRSK notify, device type will
> >> > > be
> >> > > changed to LE, following is the failed scenario after last success with
> >> > > CRSK notify. Remote unpair and pair again.
> >> > >
> >> > > This reply is SPP, user confirm passkey reply. When pairing proceduce,
> >> > > User
> >> > > confirm the pairing request through bluetoothd, that will send mgmt op
> >> > > 'MGMT_OP_USER_CONFIRM_REPLY' with device address and device type in
> >> > > mgmt_cp_user_confirm_reply. Kernel use the device address and type to
> >> > > lookup hci conn. Unfortunately, it will lookup hci_conn from LE
> >> > > hashtable, that don't include hci conn. So spp reply couldn't send to
> >> > > remote, caused pair failed.
> >> > >
> >> > > > > At the same time kernel always uses them to lookup hci_conn in LE
> >> > > > > hashtable firstly, because addr type always marks as LE. Obviously
> >> > > > > it
> >> > > > > will failed with SMP over BR/EDR.
> >> > > >
> >> > > > I don't follow this either since there shouldn't have been any "reply"
> >> > > > from user space for SMP over BR/EDR. All there should be are events
> >> > > > from
> >> > > > the kernel for the generated LE keys.
> >> > > >
> >> > > > > Actually, SPM is only for LE in SPEC,
> >> > > >
> >> > > > That's not true. SMP is specified both for LE-U and ACL-U.
> >> > > >
> >> > > > > but kernel already support and use SMP over BR/EDR. if BR/EDR
> >> > > > > exchanges key with SMP, it will never reply pairing response to
> >> > > > > remote, in other words it will be never paired, that is happened in
> >> > > > > our products.
> >> > > >
> >> > > > Szymon recently implemented SMP over BR/EDR for Zephyr and used
> >> > > > Linux/BlueZ as a reference for testing. He didn't report any issues
> >> > > > like
> >> > > > this. It might help if you could provide some logs (particularly
> >> > > > HCI/btmon but also from bluetoothd) to understand what's the actual
> >> > > > issue you're seeing.
> >> > > >
> >> > > > Johan
> >> > >
> >> > > Sorry to confuse this issue, the log is not in my hand right now,
> >> > > so it maybe later.
> >> >
> >> > So I was able to reproduce this issue. This is bluetoothd bug and not
> >> > kernel one. This bug is no specific to cross-transport pairing. It can
> >> > happen with any dual-mode device that is doing BR/EDR pairing while being
> >> > known as dual mode by bluetoothd when agent replies with passkey or
> >> > confirmation.
> >> >
> >> > To fix this we probably need to hold extra information in
> >> > 'struct authentication_req' in device.c about type of pairing (LE or
> >> > BR/EDR). This is not a one-liner-fix so I don't have a patch ready yet.
> >>
> >> Totally agree with you about dual-mode device pairing known as dual mode.
> >> But i want to known is that reasonable about device is to do BR/EDR pairing
> >> will generate CRSK notify? I'm very intersting about this fixing, this bug
> >> is hight priority in our product. In my opinion hold extra informatin in
> >> 'struct authentication_req' may not fix this bug. Because if CRSK event is
> >> still report, then bluetoothd will change the device type to LE even if we
> >> pair device that is scaned with BR/EDR. So i think the rootcase is find
> >> does CRSK event make sense in BR/EDR pairing, and how to handle CRSK events
> >> in BR/EDR pairing if it make sense. I'm confuse with those.
> >
> > It doesn't change the device to LE but to dual mode device. This is
> > *cross-transport* pairing so keys for other transport are generated.
> > baddr_type specify only LE address type, not BR/EDR since there is no address
> > type for BR/EDR. This is mostly true but few places in bluetoothd seem to
> > asusme that for device supporting BR/EDR type is equal 0. Which is not true if
> > device is dual mode.
> >
> > You should be able to reproduce this bug with dual-mode devices that don't
> > support cross-transport pairing: enable advertising, scan from linux, when
> > device is found stop advertising and make device discoverable over BR/EDR
> > (inquiry). When device is found over BR/EDR stop scanning and start pairing.
> >
> >>
> >> I noticed that if quikly reply the passkey confirm, this bug always be
> >> reproduced, but if wait for 2~3s to reply the passkey confirm, it works
> >> well every time. In terms of code, wait for 2~3s will cause l2cap chan
> >> timeout for info timer that created by HCI_EV_REMOTE_EXT_FEATURES event,
> >> and timeout will change l2cap chan to BT_CONNECTED. So next SMP
> >> resume/ready don't distribute key also CRSK events.
> >>
> >> It can't reproduce with btmgmt, because it reply passkey confirm always only
> >> use BR/EDR in 'struct mgmt_cp_user_confirm_reply' not use device relation
> >> type.
> >>
> >> bluetoothd.log and btmon.log are attached. It records two pair request
> >> sequence, one is pair success that have CRSK event, another is next pair
> >> reqeust don't success any, hope those maybe help you to annlyze this bug.
>
> I've sent a patch "[RFC] core: Fix BR/EDR pairing for dual mode devices".
> Please check if this solves issue you are seeing.
>
Thanks for your patch. Maybe it can resolve the issue, but it will cause other
issues. For example, some operations also use device->bdaddr as parameter in
MGMT operations, unpair is the same. If kernel hold the device as BR/EDR type
in hdev->conn_hash, unpair operator won't find the hci conn, so it couldn't
terminal the link. but the link is exist at the moment. MGMT also complete
when don't terminal the link. So bluetoothd and the user don't feel the
different. But is that we would like? The code implies we should terminate the
connection if it is exist.
The patch use auth_req with BDADDR_BREDR to handle pairing request. It could
resolve the pairing procedure. But kernel hold the device as BR/EDR, even if
cross-tranport is generated on BR/EDR hci conn. Meanwhile bluetoothd will set
device->bdaddr_type to BDADDR_LE_PUBLIC with new_csrk_callback that generated
by cross-transport. I mean, the user-space hold the device with BDADDR_LE_PUBLIC
(yes, device->bredr and device->le are true, but addr-type is BDADDR_LE_PUBLIC),
the kernel hold the device with BDADDR_BREDR. Whenever user-space use couple
{addr, addr-type} to send request to kernel. It maybe failed.
As the end. In my case, i don't do the steps you mentioned(enable advertising,
scan from linux, stop advertising). I only start BR/EDR discovering with discovery
filter, pair device and unpair device to reproduce this bug. I don't start/stop
LE advertising.
> --
> pozdrawiam
> Szymon K. Janc
Hi Jiangbo,
On 18 October 2016 at 22:32, Szymon Janc <[email protected]> wrote:
> Hi Jiangbo,
>
> On Tuesday, 18 October 2016 18:23:38 CEST Wu,Jiangbo wrote:
>> Hi,
>>
>> On Mon, Oct 17, 2016 at 11:05:33PM +0200, Szymon Janc wrote:
>> > Hi,
>> >
>> > On Saturday, 15 October 2016 00:43:13 CEST wujiangbo wrote:
>> > > On Fri, Oct 14, 2016 at 05:19:38PM +0300, Johan Hedberg wrote:
>> > > > Hi Jiangbo,
>> > > >
>> > > > Please don't top-post on this list.
>> > > >
>> > > > On Fri, Oct 14, 2016, Wu, Jiangbo wrote:
>> > > > > If pair a device that unpair firstly that remove encryption key,
>> > > > > encryption key event will be emitted. kernel will receive
>> > > > > 'L2CAP_CID_SMP_BREDR' frame, and then it will use SMP to distribute
>> > > > > key. SMP would like to use LTK, IRK and CRSK to notify user. If it
>> > > > > don't identify device by which conn type they are, only marks LE as
>> > > > > the device type,
>> > > >
>> > > > Why would that happen? Before SMP over BR/EDR happens pairing would
>> > > > have
>> > > > happened over BR/EDR, so bluetoothd should know that BR/EDR is
>> > > > supported
>> > > > as well (it would even be aware of an existing BR/EDR connection). Are
>> > > > you perhaps trying to work around some bluetoothd bug with all this?
>> > >
>> > > I use upstream bluez source code without change.
>> > >
>> > > Yes, bluetoothd scan will find device type is BR/EDR or LE. As my case,
>> > > device is BR/EDR. But if kernel report CRSK notify, bluetoothd will
>> > > change
>> > >
>> > > the device type to LE. The code you can see:
>> > > new_csrk_callback -> btd_adapter_get_device ->
> btd_adapter_find_device
>> > >
>> > > if (bdaddr_type == BDADDR_BREDR)
>> > >
>> > > device_set_bredr_support(device);
>> > >
>> > > else
>> > >
>> > > device_set_le_support(device, bdaddr_type);
>> > >
>> > > As Marcel mentioned before, LTK, IRK and CRSK are only valid for LE
>> > > link.
>> > > So the rootcause is why remote start to pair a BR/EDR device, the kernel
>> > > will receive CRSK event.
>> > >
>> > > This is the first pair, and it will pair success even if receive CRSK
>> > > notify. And the second and the next all pair will be failed with remote
>> > > device unpair and then pair again.
>> > >
>> > > > > while Bluetoothd will use this 'addr' and 'addr type' to reply the
>> > > > > comfirm to kernel.
>> > > >
>> > > > What reply are you talking about? There's no user interaction involved
>> > > > with SMP over BR/EDR - that would already have occurred when SSP over
>> > > > BR/EDR happened.
>> > >
>> > > Sorry to confuse the case, the pairing failed coming with next pair
>> > > procedure. Because at the last pair with CRSK notify, device type will
>> > > be
>> > > changed to LE, following is the failed scenario after last success with
>> > > CRSK notify. Remote unpair and pair again.
>> > >
>> > > This reply is SPP, user confirm passkey reply. When pairing proceduce,
>> > > User
>> > > confirm the pairing request through bluetoothd, that will send mgmt op
>> > > 'MGMT_OP_USER_CONFIRM_REPLY' with device address and device type in
>> > > mgmt_cp_user_confirm_reply. Kernel use the device address and type to
>> > > lookup hci conn. Unfortunately, it will lookup hci_conn from LE
>> > > hashtable, that don't include hci conn. So spp reply couldn't send to
>> > > remote, caused pair failed.
>> > >
>> > > > > At the same time kernel always uses them to lookup hci_conn in LE
>> > > > > hashtable firstly, because addr type always marks as LE. Obviously
>> > > > > it
>> > > > > will failed with SMP over BR/EDR.
>> > > >
>> > > > I don't follow this either since there shouldn't have been any "reply"
>> > > > from user space for SMP over BR/EDR. All there should be are events
>> > > > from
>> > > > the kernel for the generated LE keys.
>> > > >
>> > > > > Actually, SPM is only for LE in SPEC,
>> > > >
>> > > > That's not true. SMP is specified both for LE-U and ACL-U.
>> > > >
>> > > > > but kernel already support and use SMP over BR/EDR. if BR/EDR
>> > > > > exchanges key with SMP, it will never reply pairing response to
>> > > > > remote, in other words it will be never paired, that is happened in
>> > > > > our products.
>> > > >
>> > > > Szymon recently implemented SMP over BR/EDR for Zephyr and used
>> > > > Linux/BlueZ as a reference for testing. He didn't report any issues
>> > > > like
>> > > > this. It might help if you could provide some logs (particularly
>> > > > HCI/btmon but also from bluetoothd) to understand what's the actual
>> > > > issue you're seeing.
>> > > >
>> > > > Johan
>> > >
>> > > Sorry to confuse this issue, the log is not in my hand right now,
>> > > so it maybe later.
>> >
>> > So I was able to reproduce this issue. This is bluetoothd bug and not
>> > kernel one. This bug is no specific to cross-transport pairing. It can
>> > happen with any dual-mode device that is doing BR/EDR pairing while being
>> > known as dual mode by bluetoothd when agent replies with passkey or
>> > confirmation.
>> >
>> > To fix this we probably need to hold extra information in
>> > 'struct authentication_req' in device.c about type of pairing (LE or
>> > BR/EDR). This is not a one-liner-fix so I don't have a patch ready yet.
>>
>> Totally agree with you about dual-mode device pairing known as dual mode.
>> But i want to known is that reasonable about device is to do BR/EDR pairing
>> will generate CRSK notify? I'm very intersting about this fixing, this bug
>> is hight priority in our product. In my opinion hold extra informatin in
>> 'struct authentication_req' may not fix this bug. Because if CRSK event is
>> still report, then bluetoothd will change the device type to LE even if we
>> pair device that is scaned with BR/EDR. So i think the rootcase is find
>> does CRSK event make sense in BR/EDR pairing, and how to handle CRSK events
>> in BR/EDR pairing if it make sense. I'm confuse with those.
>
> It doesn't change the device to LE but to dual mode device. This is
> *cross-transport* pairing so keys for other transport are generated.
> baddr_type specify only LE address type, not BR/EDR since there is no address
> type for BR/EDR. This is mostly true but few places in bluetoothd seem to
> asusme that for device supporting BR/EDR type is equal 0. Which is not true if
> device is dual mode.
>
> You should be able to reproduce this bug with dual-mode devices that don't
> support cross-transport pairing: enable advertising, scan from linux, when
> device is found stop advertising and make device discoverable over BR/EDR
> (inquiry). When device is found over BR/EDR stop scanning and start pairing.
>
>>
>> I noticed that if quikly reply the passkey confirm, this bug always be
>> reproduced, but if wait for 2~3s to reply the passkey confirm, it works
>> well every time. In terms of code, wait for 2~3s will cause l2cap chan
>> timeout for info timer that created by HCI_EV_REMOTE_EXT_FEATURES event,
>> and timeout will change l2cap chan to BT_CONNECTED. So next SMP
>> resume/ready don't distribute key also CRSK events.
>>
>> It can't reproduce with btmgmt, because it reply passkey confirm always only
>> use BR/EDR in 'struct mgmt_cp_user_confirm_reply' not use device relation
>> type.
>>
>> bluetoothd.log and btmon.log are attached. It records two pair request
>> sequence, one is pair success that have CRSK event, another is next pair
>> reqeust don't success any, hope those maybe help you to annlyze this bug.
I've sent a patch "[RFC] core: Fix BR/EDR pairing for dual mode devices".
Please check if this solves issue you are seeing.
--
pozdrawiam
Szymon K. Janc
Hi Jiangbo,
On Tuesday, 18 October 2016 18:23:38 CEST Wu,Jiangbo wrote:
> Hi,
>
> On Mon, Oct 17, 2016 at 11:05:33PM +0200, Szymon Janc wrote:
> > Hi,
> >
> > On Saturday, 15 October 2016 00:43:13 CEST wujiangbo wrote:
> > > On Fri, Oct 14, 2016 at 05:19:38PM +0300, Johan Hedberg wrote:
> > > > Hi Jiangbo,
> > > >
> > > > Please don't top-post on this list.
> > > >
> > > > On Fri, Oct 14, 2016, Wu, Jiangbo wrote:
> > > > > If pair a device that unpair firstly that remove encryption key,
> > > > > encryption key event will be emitted. kernel will receive
> > > > > 'L2CAP_CID_SMP_BREDR' frame, and then it will use SMP to distribute
> > > > > key. SMP would like to use LTK, IRK and CRSK to notify user. If it
> > > > > don't identify device by which conn type they are, only marks LE as
> > > > > the device type,
> > > >
> > > > Why would that happen? Before SMP over BR/EDR happens pairing would
> > > > have
> > > > happened over BR/EDR, so bluetoothd should know that BR/EDR is
> > > > supported
> > > > as well (it would even be aware of an existing BR/EDR connection). Are
> > > > you perhaps trying to work around some bluetoothd bug with all this?
> > >
> > > I use upstream bluez source code without change.
> > >
> > > Yes, bluetoothd scan will find device type is BR/EDR or LE. As my case,
> > > device is BR/EDR. But if kernel report CRSK notify, bluetoothd will
> > > change
> > >
> > > the device type to LE. The code you can see:
> > > new_csrk_callback -> btd_adapter_get_device ->
btd_adapter_find_device
> > >
> > > if (bdaddr_type == BDADDR_BREDR)
> > >
> > > device_set_bredr_support(device);
> > >
> > > else
> > >
> > > device_set_le_support(device, bdaddr_type);
> > >
> > > As Marcel mentioned before, LTK, IRK and CRSK are only valid for LE
> > > link.
> > > So the rootcause is why remote start to pair a BR/EDR device, the kernel
> > > will receive CRSK event.
> > >
> > > This is the first pair, and it will pair success even if receive CRSK
> > > notify. And the second and the next all pair will be failed with remote
> > > device unpair and then pair again.
> > >
> > > > > while Bluetoothd will use this 'addr' and 'addr type' to reply the
> > > > > comfirm to kernel.
> > > >
> > > > What reply are you talking about? There's no user interaction involved
> > > > with SMP over BR/EDR - that would already have occurred when SSP over
> > > > BR/EDR happened.
> > >
> > > Sorry to confuse the case, the pairing failed coming with next pair
> > > procedure. Because at the last pair with CRSK notify, device type will
> > > be
> > > changed to LE, following is the failed scenario after last success with
> > > CRSK notify. Remote unpair and pair again.
> > >
> > > This reply is SPP, user confirm passkey reply. When pairing proceduce,
> > > User
> > > confirm the pairing request through bluetoothd, that will send mgmt op
> > > 'MGMT_OP_USER_CONFIRM_REPLY' with device address and device type in
> > > mgmt_cp_user_confirm_reply. Kernel use the device address and type to
> > > lookup hci conn. Unfortunately, it will lookup hci_conn from LE
> > > hashtable, that don't include hci conn. So spp reply couldn't send to
> > > remote, caused pair failed.
> > >
> > > > > At the same time kernel always uses them to lookup hci_conn in LE
> > > > > hashtable firstly, because addr type always marks as LE. Obviously
> > > > > it
> > > > > will failed with SMP over BR/EDR.
> > > >
> > > > I don't follow this either since there shouldn't have been any "reply"
> > > > from user space for SMP over BR/EDR. All there should be are events
> > > > from
> > > > the kernel for the generated LE keys.
> > > >
> > > > > Actually, SPM is only for LE in SPEC,
> > > >
> > > > That's not true. SMP is specified both for LE-U and ACL-U.
> > > >
> > > > > but kernel already support and use SMP over BR/EDR. if BR/EDR
> > > > > exchanges key with SMP, it will never reply pairing response to
> > > > > remote, in other words it will be never paired, that is happened in
> > > > > our products.
> > > >
> > > > Szymon recently implemented SMP over BR/EDR for Zephyr and used
> > > > Linux/BlueZ as a reference for testing. He didn't report any issues
> > > > like
> > > > this. It might help if you could provide some logs (particularly
> > > > HCI/btmon but also from bluetoothd) to understand what's the actual
> > > > issue you're seeing.
> > > >
> > > > Johan
> > >
> > > Sorry to confuse this issue, the log is not in my hand right now,
> > > so it maybe later.
> >
> > So I was able to reproduce this issue. This is bluetoothd bug and not
> > kernel one. This bug is no specific to cross-transport pairing. It can
> > happen with any dual-mode device that is doing BR/EDR pairing while being
> > known as dual mode by bluetoothd when agent replies with passkey or
> > confirmation.
> >
> > To fix this we probably need to hold extra information in
> > 'struct authentication_req' in device.c about type of pairing (LE or
> > BR/EDR). This is not a one-liner-fix so I don't have a patch ready yet.
>
> Totally agree with you about dual-mode device pairing known as dual mode.
> But i want to known is that reasonable about device is to do BR/EDR pairing
> will generate CRSK notify? I'm very intersting about this fixing, this bug
> is hight priority in our product. In my opinion hold extra informatin in
> 'struct authentication_req' may not fix this bug. Because if CRSK event is
> still report, then bluetoothd will change the device type to LE even if we
> pair device that is scaned with BR/EDR. So i think the rootcase is find
> does CRSK event make sense in BR/EDR pairing, and how to handle CRSK events
> in BR/EDR pairing if it make sense. I'm confuse with those.
It doesn't change the device to LE but to dual mode device. This is
*cross-transport* pairing so keys for other transport are generated.
baddr_type specify only LE address type, not BR/EDR since there is no address
type for BR/EDR. This is mostly true but few places in bluetoothd seem to
asusme that for device supporting BR/EDR type is equal 0. Which is not true if
device is dual mode.
You should be able to reproduce this bug with dual-mode devices that don't
support cross-transport pairing: enable advertising, scan from linux, when
device is found stop advertising and make device discoverable over BR/EDR
(inquiry). When device is found over BR/EDR stop scanning and start pairing.
>
> I noticed that if quikly reply the passkey confirm, this bug always be
> reproduced, but if wait for 2~3s to reply the passkey confirm, it works
> well every time. In terms of code, wait for 2~3s will cause l2cap chan
> timeout for info timer that created by HCI_EV_REMOTE_EXT_FEATURES event,
> and timeout will change l2cap chan to BT_CONNECTED. So next SMP
> resume/ready don't distribute key also CRSK events.
>
> It can't reproduce with btmgmt, because it reply passkey confirm always only
> use BR/EDR in 'struct mgmt_cp_user_confirm_reply' not use device relation
> type.
>
> bluetoothd.log and btmon.log are attached. It records two pair request
> sequence, one is pair success that have CRSK event, another is next pair
> reqeust don't success any, hope those maybe help you to annlyze this bug.
>
> Thanks
> Jiangbo
--
pozdrawiam
Szymon Janc
Hi,
On Mon, Oct 17, 2016 at 11:05:33PM +0200, Szymon Janc wrote:
> Hi,
>
> On Saturday, 15 October 2016 00:43:13 CEST wujiangbo wrote:
> > On Fri, Oct 14, 2016 at 05:19:38PM +0300, Johan Hedberg wrote:
> > > Hi Jiangbo,
> > >
> > > Please don't top-post on this list.
> > >
> > > On Fri, Oct 14, 2016, Wu, Jiangbo wrote:
> > > > If pair a device that unpair firstly that remove encryption key,
> > > > encryption key event will be emitted. kernel will receive
> > > > 'L2CAP_CID_SMP_BREDR' frame, and then it will use SMP to distribute
> > > > key. SMP would like to use LTK, IRK and CRSK to notify user. If it
> > > > don't identify device by which conn type they are, only marks LE as
> > > > the device type,
> > >
> > > Why would that happen? Before SMP over BR/EDR happens pairing would have
> > > happened over BR/EDR, so bluetoothd should know that BR/EDR is supported
> > > as well (it would even be aware of an existing BR/EDR connection). Are
> > > you perhaps trying to work around some bluetoothd bug with all this?
> >
> > I use upstream bluez source code without change.
> >
> > Yes, bluetoothd scan will find device type is BR/EDR or LE. As my case,
> > device is BR/EDR. But if kernel report CRSK notify, bluetoothd will change
> > the device type to LE. The code you can see:
> > new_csrk_callback -> btd_adapter_get_device -> btd_adapter_find_device
> > if (bdaddr_type == BDADDR_BREDR)
> > device_set_bredr_support(device);
> > else
> > device_set_le_support(device, bdaddr_type);
> > As Marcel mentioned before, LTK, IRK and CRSK are only valid for LE link.
> > So the rootcause is why remote start to pair a BR/EDR device, the kernel
> > will receive CRSK event.
> >
> > This is the first pair, and it will pair success even if receive CRSK
> > notify. And the second and the next all pair will be failed with remote
> > device unpair and then pair again.
> >
> > > > while Bluetoothd will use this 'addr' and 'addr type' to reply the
> > > > comfirm to kernel.
> > >
> > > What reply are you talking about? There's no user interaction involved
> > > with SMP over BR/EDR - that would already have occurred when SSP over
> > > BR/EDR happened.
> >
> > Sorry to confuse the case, the pairing failed coming with next pair
> > procedure. Because at the last pair with CRSK notify, device type will be
> > changed to LE, following is the failed scenario after last success with
> > CRSK notify. Remote unpair and pair again.
> >
> > This reply is SPP, user confirm passkey reply. When pairing proceduce, User
> > confirm the pairing request through bluetoothd, that will send mgmt op
> > 'MGMT_OP_USER_CONFIRM_REPLY' with device address and device type in
> > mgmt_cp_user_confirm_reply. Kernel use the device address and type to lookup
> > hci conn. Unfortunately, it will lookup hci_conn from LE hashtable, that
> > don't include hci conn. So spp reply couldn't send to remote, caused pair
> > failed.
> > > > At the same time kernel always uses them to lookup hci_conn in LE
> > > > hashtable firstly, because addr type always marks as LE. Obviously it
> > > > will failed with SMP over BR/EDR.
> > >
> > > I don't follow this either since there shouldn't have been any "reply"
> > > from user space for SMP over BR/EDR. All there should be are events from
> > > the kernel for the generated LE keys.
> > >
> > > > Actually, SPM is only for LE in SPEC,
> > >
> > > That's not true. SMP is specified both for LE-U and ACL-U.
> > >
> > > > but kernel already support and use SMP over BR/EDR. if BR/EDR
> > > > exchanges key with SMP, it will never reply pairing response to
> > > > remote, in other words it will be never paired, that is happened in
> > > > our products.
> > >
> > > Szymon recently implemented SMP over BR/EDR for Zephyr and used
> > > Linux/BlueZ as a reference for testing. He didn't report any issues like
> > > this. It might help if you could provide some logs (particularly
> > > HCI/btmon but also from bluetoothd) to understand what's the actual
> > > issue you're seeing.
> > >
> > > Johan
> >
> > Sorry to confuse this issue, the log is not in my hand right now,
> > so it maybe later.
>
> So I was able to reproduce this issue. This is bluetoothd bug and not kernel
> one. This bug is no specific to cross-transport pairing. It can happen with
> any dual-mode device that is doing BR/EDR pairing while being known as dual
> mode by bluetoothd when agent replies with passkey or confirmation.
>
> To fix this we probably need to hold extra information in
> 'struct authentication_req' in device.c about type of pairing (LE or BR/EDR).
> This is not a one-liner-fix so I don't have a patch ready yet.
>
Totally agree with you about dual-mode device pairing known as dual mode. But i
want to known is that reasonable about device is to do BR/EDR pairing will
generate CRSK notify? I'm very intersting about this fixing, this bug is hight
priority in our product. In my opinion hold extra informatin in
'struct authentication_req' may not fix this bug. Because if CRSK event is still
report, then bluetoothd will change the device type to LE even if we pair device
that is scaned with BR/EDR. So i think the rootcase is find does CRSK event make
sense in BR/EDR pairing, and how to handle CRSK events in BR/EDR pairing if it
make sense. I'm confuse with those.
I noticed that if quikly reply the passkey confirm, this bug always be reproduced,
but if wait for 2~3s to reply the passkey confirm, it works well every time.
In terms of code, wait for 2~3s will cause l2cap chan timeout for info timer that
created by HCI_EV_REMOTE_EXT_FEATURES event, and timeout will change l2cap chan
to BT_CONNECTED. So next SMP resume/ready don't distribute key also CRSK events.
It can't reproduce with btmgmt, because it reply passkey confirm always only use
BR/EDR in 'struct mgmt_cp_user_confirm_reply' not use device relation type.
bluetoothd.log and btmon.log are attached. It records two pair request sequence,
one is pair success that have CRSK event, another is next pair reqeust don't
success any, hope those maybe help you to annlyze this bug.
Thanks
Jiangbo
> --
> pozdrawiam
> Szymon Janc
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi,
On Saturday, 15 October 2016 00:43:13 CEST wujiangbo wrote:
> On Fri, Oct 14, 2016 at 05:19:38PM +0300, Johan Hedberg wrote:
> > Hi Jiangbo,
> >
> > Please don't top-post on this list.
> >
> > On Fri, Oct 14, 2016, Wu, Jiangbo wrote:
> > > If pair a device that unpair firstly that remove encryption key,
> > > encryption key event will be emitted. kernel will receive
> > > 'L2CAP_CID_SMP_BREDR' frame, and then it will use SMP to distribute
> > > key. SMP would like to use LTK, IRK and CRSK to notify user. If it
> > > don't identify device by which conn type they are, only marks LE as
> > > the device type,
> >
> > Why would that happen? Before SMP over BR/EDR happens pairing would have
> > happened over BR/EDR, so bluetoothd should know that BR/EDR is supported
> > as well (it would even be aware of an existing BR/EDR connection). Are
> > you perhaps trying to work around some bluetoothd bug with all this?
>
> I use upstream bluez source code without change.
>
> Yes, bluetoothd scan will find device type is BR/EDR or LE. As my case,
> device is BR/EDR. But if kernel report CRSK notify, bluetoothd will change
> the device type to LE. The code you can see:
> new_csrk_callback -> btd_adapter_get_device -> btd_adapter_find_device
> if (bdaddr_type == BDADDR_BREDR)
> device_set_bredr_support(device);
> else
> device_set_le_support(device, bdaddr_type);
> As Marcel mentioned before, LTK, IRK and CRSK are only valid for LE link.
> So the rootcause is why remote start to pair a BR/EDR device, the kernel
> will receive CRSK event.
>
> This is the first pair, and it will pair success even if receive CRSK
> notify. And the second and the next all pair will be failed with remote
> device unpair and then pair again.
>
> > > while Bluetoothd will use this 'addr' and 'addr type' to reply the
> > > comfirm to kernel.
> >
> > What reply are you talking about? There's no user interaction involved
> > with SMP over BR/EDR - that would already have occurred when SSP over
> > BR/EDR happened.
>
> Sorry to confuse the case, the pairing failed coming with next pair
> procedure. Because at the last pair with CRSK notify, device type will be
> changed to LE, following is the failed scenario after last success with
> CRSK notify. Remote unpair and pair again.
>
> This reply is SPP, user confirm passkey reply. When pairing proceduce, User
> confirm the pairing request through bluetoothd, that will send mgmt op
> 'MGMT_OP_USER_CONFIRM_REPLY' with device address and device type in
> mgmt_cp_user_confirm_reply. Kernel use the device address and type to lookup
> hci conn. Unfortunately, it will lookup hci_conn from LE hashtable, that
> don't include hci conn. So spp reply couldn't send to remote, caused pair
> failed.
> > > At the same time kernel always uses them to lookup hci_conn in LE
> > > hashtable firstly, because addr type always marks as LE. Obviously it
> > > will failed with SMP over BR/EDR.
> >
> > I don't follow this either since there shouldn't have been any "reply"
> > from user space for SMP over BR/EDR. All there should be are events from
> > the kernel for the generated LE keys.
> >
> > > Actually, SPM is only for LE in SPEC,
> >
> > That's not true. SMP is specified both for LE-U and ACL-U.
> >
> > > but kernel already support and use SMP over BR/EDR. if BR/EDR
> > > exchanges key with SMP, it will never reply pairing response to
> > > remote, in other words it will be never paired, that is happened in
> > > our products.
> >
> > Szymon recently implemented SMP over BR/EDR for Zephyr and used
> > Linux/BlueZ as a reference for testing. He didn't report any issues like
> > this. It might help if you could provide some logs (particularly
> > HCI/btmon but also from bluetoothd) to understand what's the actual
> > issue you're seeing.
> >
> > Johan
>
> Sorry to confuse this issue, the log is not in my hand right now,
> so it maybe later.
So I was able to reproduce this issue. This is bluetoothd bug and not kernel
one. This bug is no specific to cross-transport pairing. It can happen with
any dual-mode device that is doing BR/EDR pairing while being known as dual
mode by bluetoothd when agent replies with passkey or confirmation.
To fix this we probably need to hold extra information in
'struct authentication_req' in device.c about type of pairing (LE or BR/EDR).
This is not a one-liner-fix so I don't have a patch ready yet.
--
pozdrawiam
Szymon Janc
On Fri, Oct 14, 2016 at 05:19:38PM +0300, Johan Hedberg wrote:
> Hi Jiangbo,
>
> Please don't top-post on this list.
>
> On Fri, Oct 14, 2016, Wu, Jiangbo wrote:
> > If pair a device that unpair firstly that remove encryption key,
> > encryption key event will be emitted. kernel will receive
> > 'L2CAP_CID_SMP_BREDR' frame, and then it will use SMP to distribute
> > key. SMP would like to use LTK, IRK and CRSK to notify user. If it
> > don't identify device by which conn type they are, only marks LE as
> > the device type,
>
> Why would that happen? Before SMP over BR/EDR happens pairing would have
> happened over BR/EDR, so bluetoothd should know that BR/EDR is supported
> as well (it would even be aware of an existing BR/EDR connection). Are
> you perhaps trying to work around some bluetoothd bug with all this?
>
I use upstream bluez source code without change.
Yes, bluetoothd scan will find device type is BR/EDR or LE. As my case, device
is BR/EDR. But if kernel report CRSK notify, bluetoothd will change the device
type to LE. The code you can see:
new_csrk_callback -> btd_adapter_get_device -> btd_adapter_find_device
if (bdaddr_type == BDADDR_BREDR)
device_set_bredr_support(device);
else
device_set_le_support(device, bdaddr_type);
As Marcel mentioned before, LTK, IRK and CRSK are only valid for LE link.
So the rootcause is why remote start to pair a BR/EDR device, the kernel will
receive CRSK event.
This is the first pair, and it will pair success even if receive CRSK notify.
And the second and the next all pair will be failed with remote device unpair
and then pair again.
> > while Bluetoothd will use this 'addr' and 'addr type' to reply the
> > comfirm to kernel.
>
> What reply are you talking about? There's no user interaction involved
> with SMP over BR/EDR - that would already have occurred when SSP over
> BR/EDR happened.
>
Sorry to confuse the case, the pairing failed coming with next pair procedure.
Because at the last pair with CRSK notify, device type will be changed to LE,
following is the failed scenario after last success with CRSK notify. Remote
unpair and pair again.
This reply is SPP, user confirm passkey reply. When pairing proceduce, User
confirm the pairing request through bluetoothd, that will send mgmt op
'MGMT_OP_USER_CONFIRM_REPLY' with device address and device type in
mgmt_cp_user_confirm_reply. Kernel use the device address and type to lookup
hci conn. Unfortunately, it will lookup hci_conn from LE hashtable, that don't
include hci conn. So spp reply couldn't send to remote, caused pair failed.
> > At the same time kernel always uses them to lookup hci_conn in LE
> > hashtable firstly, because addr type always marks as LE. Obviously it
> > will failed with SMP over BR/EDR.
>
> I don't follow this either since there shouldn't have been any "reply"
> from user space for SMP over BR/EDR. All there should be are events from
> the kernel for the generated LE keys.
>
> > Actually, SPM is only for LE in SPEC,
>
> That's not true. SMP is specified both for LE-U and ACL-U.
>
> > but kernel already support and use SMP over BR/EDR. if BR/EDR
> > exchanges key with SMP, it will never reply pairing response to
> > remote, in other words it will be never paired, that is happened in
> > our products.
>
> Szymon recently implemented SMP over BR/EDR for Zephyr and used
> Linux/BlueZ as a reference for testing. He didn't report any issues like
> this. It might help if you could provide some logs (particularly
> HCI/btmon but also from bluetoothd) to understand what's the actual
> issue you're seeing.
>
> Johan
Sorry to confuse this issue, the log is not in my hand right now,
so it maybe later.
Jiangbo
Hi Jiangbo,
Please don't top-post on this list.
On Fri, Oct 14, 2016, Wu, Jiangbo wrote:
> If pair a device that unpair firstly that remove encryption key,
> encryption key event will be emitted. kernel will receive
> 'L2CAP_CID_SMP_BREDR' frame, and then it will use SMP to distribute
> key. SMP would like to use LTK, IRK and CRSK to notify user. If it
> don't identify device by which conn type they are, only marks LE as
> the device type,
Why would that happen? Before SMP over BR/EDR happens pairing would have
happened over BR/EDR, so bluetoothd should know that BR/EDR is supported
as well (it would even be aware of an existing BR/EDR connection). Are
you perhaps trying to work around some bluetoothd bug with all this?
> while Bluetoothd will use this 'addr' and 'addr type' to reply the
> comfirm to kernel.
What reply are you talking about? There's no user interaction involved
with SMP over BR/EDR - that would already have occurred when SSP over
BR/EDR happened.
> At the same time kernel always uses them to lookup hci_conn in LE
> hashtable firstly, because addr type always marks as LE. Obviously it
> will failed with SMP over BR/EDR.
I don't follow this either since there shouldn't have been any "reply"
from user space for SMP over BR/EDR. All there should be are events from
the kernel for the generated LE keys.
> Actually, SPM is only for LE in SPEC,
That's not true. SMP is specified both for LE-U and ACL-U.
> but kernel already support and use SMP over BR/EDR. if BR/EDR
> exchanges key with SMP, it will never reply pairing response to
> remote, in other words it will be never paired, that is happened in
> our products.
Szymon recently implemented SMP over BR/EDR for Zephyr and used
Linux/BlueZ as a reference for testing. He didn't report any issues like
this. It might help if you could provide some logs (particularly
HCI/btmon but also from bluetoothd) to understand what's the actual
issue you're seeing.
Johan
If pair a device that unpair firstly that remove encryption key, encryption=
key event will be emitted. kernel will receive 'L2CAP_CID_SMP_BREDR' frame=
, and then it will use SMP to distribute key. SMP would like to use LTK, I=
RK and CRSK to notify user. If it don't identify device by which conn type =
they are, only marks LE as the device type, while Bluetoothd will use this =
'addr' and 'addr type' to reply the comfirm to kernel. At the same time ker=
nel always uses them to lookup hci_conn in LE hashtable firstly, because ad=
dr type always marks as LE. Obviously it will failed with SMP over BR/EDR.
Actually, SPM is only for LE in SPEC, but kernel already support and use SM=
P over BR/EDR. if BR/EDR exchanges key with SMP, it will never reply pairin=
g response to remote, in other words it will be never paired, that is happe=
ned in our products.
Thanks
Jiangbo
-----Original Message-----
From: [email protected] [mailto:linux-bluetooth-owner@v=
ger.kernel.org] On Behalf Of Marcel Holtmann
Sent: Friday, October 14, 2016 8:39 PM
To: Wu, Jiangbo <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] Bluetooth: Add conn type to identify addr type with SM=
P over BR/EDR
Hi Jiangbo,
> SMP over BR/EDR distributes keys when encryption key changed. It=20
> should use correct address type with link.
>=20
> Signed-off-by: Jiangbo Wu <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 8 +++++---
> net/bluetooth/mgmt.c | 14 ++++++++------
> net/bluetooth/smp.c | 10 +++++-----
> 3 files changed, 18 insertions(+), 14 deletions(-)
>=20
> diff --git a/include/net/bluetooth/hci_core.h=20
> b/include/net/bluetooth/hci_core.h
> index f00bf66..caa8254 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1509,9 +1509,11 @@ void mgmt_remote_name(struct hci_dev *hdev, bdaddr=
_t *bdaddr, u8 link_type,
> u8 addr_type, s8 rssi, u8 *name, u8 name_len); void=20
> mgmt_discovering(struct hci_dev *hdev, u8 discovering); bool=20
> mgmt_powering_down(struct hci_dev *hdev); -void mgmt_new_ltk(struct=20
> hci_dev *hdev, struct smp_ltk *key, bool persistent); -void=20
> mgmt_new_irk(struct hci_dev *hdev, struct smp_irk *irk, bool=20
> persistent); -void mgmt_new_csrk(struct hci_dev *hdev, struct smp_csrk=20
> *csrk,
> +void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, u8 link_typ=
e,
> + bool persistent);
> +void mgmt_new_irk(struct hci_dev *hdev, struct smp_irk *irk, u8 link_typ=
e,
> + bool persistent);
> +void mgmt_new_csrk(struct hci_dev *hdev, struct smp_csrk *csrk, u8=20
> +link_type,
> bool persistent);
> void mgmt_new_conn_param(struct hci_dev *hdev, bdaddr_t *bdaddr,
> u8 bdaddr_type, u8 store_hint, u16 min_interval, diff --git=20
> a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index 19b8a5e..2b12b72=20
> 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -6757,7 +6757,8 @@ static u8 mgmt_ltk_type(struct smp_ltk *ltk)
> return MGMT_LTK_UNAUTHENTICATED;
> }
>=20
> -void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, bool=20
> persistent)
> +void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, u8 link_typ=
e,
> + bool persistent)
> {
> struct mgmt_ev_new_long_term_key ev;
>=20
> @@ -6781,7 +6782,7 @@ void mgmt_new_ltk(struct hci_dev *hdev, struct smp_=
ltk *key, bool persistent)
> ev.store_hint =3D persistent;
>=20
> bacpy(&ev.key.addr.bdaddr, &key->bdaddr);
> - ev.key.addr.type =3D link_to_bdaddr(LE_LINK, key->bdaddr_type);
> + ev.key.addr.type =3D link_to_bdaddr(link_type, key->bdaddr_type);
what am I missing here. LTK, IRK and CRSK are only valid for LE links. So e=
ven if they are transported over BR/EDR, they are meant for the LE link and=
not BR/EDR link.
Regards
Marcel
Hi Jiangbo,
> SMP over BR/EDR distributes keys when encryption key changed. It should
> use correct address type with link.
>
> Signed-off-by: Jiangbo Wu <[email protected]>
> ---
> 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 f00bf66..caa8254 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1509,9 +1509,11 @@ void mgmt_remote_name(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
> u8 addr_type, s8 rssi, u8 *name, u8 name_len);
> void mgmt_discovering(struct hci_dev *hdev, u8 discovering);
> 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_csrk(struct hci_dev *hdev, struct smp_csrk *csrk,
> +void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, u8 link_type,
> + bool persistent);
> +void mgmt_new_irk(struct hci_dev *hdev, struct smp_irk *irk, u8 link_type,
> + bool persistent);
> +void mgmt_new_csrk(struct hci_dev *hdev, struct smp_csrk *csrk, u8 link_type,
> bool persistent);
> void mgmt_new_conn_param(struct hci_dev *hdev, bdaddr_t *bdaddr,
> u8 bdaddr_type, u8 store_hint, u16 min_interval,
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 19b8a5e..2b12b72 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -6757,7 +6757,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, u8 link_type,
> + bool persistent)
> {
> struct mgmt_ev_new_long_term_key ev;
>
> @@ -6781,7 +6782,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);
what am I missing here. LTK, IRK and CRSK are only valid for LE links. So even if they are transported over BR/EDR, they are meant for the LE link and not BR/EDR link.
Regards
Marcel