Return-Path: MIME-Version: 1.0 In-Reply-To: <1868573.jZNrvtjvi1@ix> References: <1476448183-8630-1-git-send-email-jiangbo.wu@intel.com> <1880358.RakREcZFtR@ix> <20161018102338.GB25770@wujiangbo-ubuntu> <1868573.jZNrvtjvi1@ix> From: Szymon Janc Date: Sat, 22 Oct 2016 11:17:21 +0200 Message-ID: Subject: Re: [PATCH] Bluetooth: Add conn type to identify addr type with SMP over BR/EDR To: "Wu,Jiangbo" Cc: Johan Hedberg , Marcel Holtmann , martin.xu@intel.com, linux-bluetooth@vger.kernel.org Content-Type: text/plain; charset=UTF-8 List-ID: Hi Jiangbo, On 18 October 2016 at 22:32, Szymon Janc 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