2011-06-09 07:59:49

by Rymarkiewicz Waldemar

[permalink] [raw]
Subject: RE: Regression caused by "Bluetooth: Map sec_level to link key requirements"

Hi Keith,

>Patch 13d39315c22b128f4796fc008b04914a7c32bb1a is causing a
>regression From 2.6.39. I cannot communicate with my Nokia
>N900 using either the SyncML or DUN RFCOMM services. I get a
>connection reset error shortly after startup.
>
>I've reverted this patch on top of something past -rc2 (to be
>precise, I'm branching from ef2398019b305827ea7130ebaf7bf521b444530e).
>With the following fix-up to make things build again, my
>bluetooth works again.

Logs from kernel and bluez would be much appreciated to see what happend.
As I remember this was pretty well tested with BITE test by Johan.

Thanks,
Waldek

2011-06-09 08:04:40

by Rymarkiewicz Waldemar

[permalink] [raw]
Subject: RE: Regression caused by "Bluetooth: Map sec_level to link key requirements"


>Patch 13d39315c22b128f4796fc008b04914a7c32bb1a is causing a
>regression From 2.6.39. I cannot communicate with my Nokia
>N900 using either the SyncML or DUN RFCOMM services. I get a
>connection reset error shortly after startup.
>
>I've reverted this patch on top of something past -rc2 (to be
>precise, I'm branching from ef2398019b305827ea7130ebaf7bf521b444530e).
>With the following fix-up to make things build again, my
>bluetooth works again.
>

hcidump -Rt > foo.log from your test session will help as well


Waldek

2011-06-09 08:34:06

by Johan Hedberg

[permalink] [raw]
Subject: Re: Regression caused by "Bluetooth: Map sec_level to link key requirements"

Hi,

On Thu, Jun 09, 2011, [email protected] wrote:
> >Patch 13d39315c22b128f4796fc008b04914a7c32bb1a is causing a
> >regression From 2.6.39. I cannot communicate with my Nokia
> >N900 using either the SyncML or DUN RFCOMM services. I get a
> >connection reset error shortly after startup.
> >
> >I've reverted this patch on top of something past -rc2 (to be
> >precise, I'm branching from ef2398019b305827ea7130ebaf7bf521b444530e).
> >With the following fix-up to make things build again, my
> >bluetooth works again.
>
> Logs from kernel and bluez would be much appreciated to see what happend.
> As I remember this was pretty well tested with BITE test by Johan.

The patch was not only fine but actually *needed* by the BITE. So
reverting it will make some qualification tests fail.

There's no DUN support in the N900 by default and the only package I'm
aware that provides it uses the command line rfcomm tool with high
security on the socket to block just-works SSP pairing (since the rfcomm
tool doesn't use the authorization framework to guarantee a user pop-up
dialog). The SyncML code I can't comment on since I haven't seen it.

So potentially this might be limited to high security sockets.
Speculating further, if the device connecting to the N900 has a pre-2.1
Bluetooth controller this could simply be about not having a 16-digit
PIN which high security services require. So could whoever is able to
reproduce the issue try repairing and entering a 16-digit PIN to see if
the problem goes away? And if this is in fact the case then the kernel
code is working exactly as it should; the only issue is that these
services should really be using medium security level instead of high.

Johan

2011-06-09 17:11:38

by Rymarkiewicz Waldemar

[permalink] [raw]
Subject: RE: Regression caused by "Bluetooth: Map sec_level to link key requirements"

Hi Keith,

>I re-pair'ed using a manually entered a 16 digit pin, but now
>DUN setup doesn't succeed at all.

Did you try to remove all stored link keys and start again?
If you already have a pincode which is insecure you have to remove it manually.

Try 'hciconfig hciX noauth noencrypt ' before.

Check syslog and switch debug on in the kernel first.


Waldek

2011-06-10 05:56:08

by Johan Hedberg

[permalink] [raw]
Subject: Re: Regression caused by "Bluetooth: Map sec_level to link key requirements"

Hi,

On Thu, Jun 09, 2011, [email protected] wrote:
> >I re-pair'ed using a manually entered a 16 digit pin, but now
> >DUN setup doesn't succeed at all.
>
> Did you try to remove all stored link keys and start again?
> If you already have a pincode which is insecure you have to remove it manually.
>
> Try 'hciconfig hciX noauth noencrypt ' before.
>
> Check syslog and switch debug on in the kernel first.

So I just realized that the BITE tests were only done for mgmtops.c and
not hciops.c. One critical difference is that conn->key_type will not be
set (or actually set to 0xff) for connections which happen after the
initial pairing has occurred. We're right now testing a one-line patch
which might fix the issue. Luiz will send it soon if it turns out to
work fine.

Johan

2011-06-10 05:58:54

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: Regression caused by "Bluetooth: Map sec_level to link key requirements"

Hi Keith,

On Fri, Jun 10, 2011 at 2:55 PM, Johan Hedberg <[email protected]> wrote:
> Hi,
>
> On Thu, Jun 09, 2011, [email protected] wrote:
>> >I re-pair'ed using a manually entered a 16 digit pin, but now
>> >DUN setup doesn't succeed at all.
>>
>> Did you try to remove all stored link keys and start again?
>> If you already have a pincode which is insecure you have to remove it manually.
>>
>> Try 'hciconfig hciX noauth noencrypt ' before.
>>
>> Check syslog and switch debug on in the kernel first.
>
> So I just realized that the BITE tests were only done for mgmtops.c and
> not hciops.c. One critical difference is that conn->key_type will not be
> set (or actually set to 0xff) for connections which happen after the
> initial pairing has occurred. We're right now testing a one-line patch
> which might fix the issue. Luiz will send it soon if it turns out to
> work fine.

So the fix we were testing looks like this:

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 2f5ae53..b309f84 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -673,8 +673,8 @@ auth:
if (test_and_set_bit(HCI_CONN_ENCRYPT_PEND, &conn->pend))
return 0;

- hci_conn_auth(conn, sec_level, auth_type);
- return 0;
+ if (!(hci_conn_auth(conn, sec_level, auth_type)))
+ return 0;

encrypt:
if (conn->link_mode & HCI_LM_ENCRYPT)

--
Luiz Augusto von Dentz
Computer Engineer

2011-06-19 17:59:49

by Johan Hedberg

[permalink] [raw]
Subject: Re: Regression caused by "Bluetooth: Map sec_level to link key requirements"

Hi Keith,

On Sun, Jun 19, 2011, Keith Packard wrote:
> On Fri, 10 Jun 2011 14:58:50 +0900, Luiz Augusto von Dentz <[email protected]> wrote:
>
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index 2f5ae53..b309f84 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -673,8 +673,8 @@ auth:
> > if (test_and_set_bit(HCI_CONN_ENCRYPT_PEND, &conn->pend))
> > return 0;
> >
> > - hci_conn_auth(conn, sec_level, auth_type);
> > - return 0;
> > + if (!(hci_conn_auth(conn, sec_level, auth_type)))
> > + return 0;
> >
> > encrypt:
> > if (conn->link_mode & HCI_LM_ENCRYPT)
>
> That makes the first connection afer boot succeed, but subsequent
> connections fail in the same way.

Did you try also the following patch (which should also be making it to
3.0):
http://git.kernel.org/?p=linux/kernel/git/padovan/bluetooth-2.6.git;a=commitdiff;h=330605423ca6eafafb8dcc27502bce1c585d1b06

I think you'll need both of them to fix the pairing/security issues.

Johan

2011-06-25 05:45:10

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: Regression caused by "Bluetooth: Map sec_level to link key requirements"

On 06/19/2011 12:01 PM, Keith Packard wrote:
>> I think you'll need both of them to fix the pairing/security issues.
> Yup, with both of those patches, I get syncml and dun working again.
>
> (sorry for the delayed testing, I'm traveling)

I just bisected the problems I had reconnecting my Nokia BH-905i
headphones to the same change that Keith identified, and merging
git://git.kernel.org/pub/scm/linux/kernel/git/padovan/bluetooth-2.6.git
into my tree fixed it.

So +1 from me to get this into mainline soon.

Thanks,
J

2011-06-25 19:40:10

by Gustavo Padovan

[permalink] [raw]
Subject: Re: Regression caused by "Bluetooth: Map sec_level to link key requirements"

* Jeremy Fitzhardinge <[email protected]> [2011-06-24 22:45:02 -0700]:

> On 06/19/2011 12:01 PM, Keith Packard wrote:
> >> I think you'll need both of them to fix the pairing/security issues.
> > Yup, with both of those patches, I get syncml and dun working again.
> >
> > (sorry for the delayed testing, I'm traveling)
>
> I just bisected the problems I had reconnecting my Nokia BH-905i
> headphones to the same change that Keith identified, and merging
> git://git.kernel.org/pub/scm/linux/kernel/git/padovan/bluetooth-2.6.git
> into my tree fixed it.
>
> So +1 from me to get this into mainline soon.

It's on the way. Right now patches are on David Miller' tree.

Gustavo