Bluetooth fixes for 2.6.27

Bluetooth fixes for 2.6.27

[Marcel Holtmann]

Hi Dave,

I do have three additional fixes for the Bluetooth subsystem that should
go into the 2.6.27 release. After the heated discussion, I really wanted
not to bother you until the next merge window, but from my perspective
all three patches fall in the regression or security fix category. That
is my personal view on these and feel free to disagree.

The first patch is a clear regression that got introduced with
2.6.27-rc1 when adding Simple Pairing support. I forgot to decrease the
reference count on an incoming ACL link. This patch actually makes the
code simpler.

The second patch fixes the authentication requirements. We do have to
separate between service discovery and actual profile channels. This is
a clear requirement of the Bluetooth Security Mode 4 introduced with the
addition of the Simple Pairing support. Not fixing this will result in
broken behavior when doing service discovery with Simple Pairing enabled
devices.

The third patch rejects insecure incoming connections. This is a clear
security issues since we can't rely on the initiator doing the right
thing and establishing an encrypted link. Malicious devices would just
skip that step and in that case we have to reject connection attempt
without going into the connection phase at all.

I tried to put detailed information into the commit messages to make it
clear why I submitted these patches in the -rc phase.

We do have one suspend/resume regression with the btusb driver that I
like to see fixed, but the fix would require new USB core functionality.
I am not sure if that happens in the 2.6.27 timeframe. I will check with
Oliver if we can come up with a patch that works with the current
kernel.

Regards

Marcel


Please pull from

    git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-2.6.git master

This will update the following files:

 include/net/bluetooth/hci_core.h |    3 ++-
 net/bluetooth/af_bluetooth.c     |    2 +-
 net/bluetooth/hci_conn.c         |   21 ++++++++++++++++++---
 net/bluetooth/hci_event.c        |   11 ++++-------
 net/bluetooth/l2cap.c            |   34 ++++++++++++++++++++++++++++------
 net/bluetooth/sco.c              |    2 +-
 6 files changed, 54 insertions(+), 19 deletions(-)

through these ChangeSets:

Marcel Holtmann (3):
    [Bluetooth] Fix reference counting during ACL config stage
    [Bluetooth] Enforce correct authentication requirements
    [Bluetooth] Reject L2CAP connections on an insecure ACL link

Re: Bluetooth fixes for 2.6.27

[David Miller]

From: Marcel Holtmann <marcel@holtmann.org>
Date: Mon, 08 Sep 2008 23:53:57 +0200

> The first patch is a clear regression that got introduced with
> 2.6.27-rc1 when adding Simple Pairing support. I forgot to decrease the
> reference count on an incoming ACL link. This patch actually makes the
> code simpler.

This is OK.

> The second patch fixes the authentication requirements. We do have to
> separate between service discovery and actual profile channels. This is
> a clear requirement of the Bluetooth Security Mode 4 introduced with the
> addition of the Simple Pairing support. Not fixing this will result in
> broken behavior when doing service discovery with Simple Pairing enabled
> devices.

What regression reported by a user is fixed by this?

This does not look like it is appropriate outside of the merge window.

> The third patch rejects insecure incoming connections. This is a clear
> security issues since we can't rely on the initiator doing the right
> thing and establishing an encrypted link. Malicious devices would just
> skip that step and in that case we have to reject connection attempt
> without going into the connection phase at all.

This is OK.

Re: Bluetooth fixes for 2.6.27

[Marcel Holtmann]

Hi Dave,

>> The second patch fixes the authentication requirements. We do have to
>> separate between service discovery and actual profile channels.  
>> This is
>> a clear requirement of the Bluetooth Security Mode 4 introduced  
>> with the
>> addition of the Simple Pairing support. Not fixing this will result  
>> in
>> broken behavior when doing service discovery with Simple Pairing  
>> enabled
>> devices.
>
> What regression reported by a user is fixed by this?
>
> This does not look like it is appropriate outside of the merge window.

the regression is that with Bluetooth 2.0 and before we always were  
allowing service discovery (SDP) connection without any bonding  
requirement. This is still true, but with Bluetooth 2.1 enabled  
devices it can now happen that we have to do a full pairing procedure.  
For SDP it should be at least using pairing with a just-works model  
with the no bonding requirement, while the other PSM channels (RFCOMM,  
BNEP etc.) should use generic bonding. We would now always use generic  
bonding (even for SDP).

The number of users are still limited to a few people actually testing  
with 2.1 hardware. These are mainly people working on 2.1 enabled  
products. However with the new MacBooks and the EeePC 901 we do have  
devices with Bluetooth 2.1 chips available for everybody.

This is clearly an oversight on my hand when developing the initial  
Simple Pairing patches that I submitted for 2.6.27-rc1 and I only  
found it when we tested against the official Bluetooth 2.1 test system.

This patch also fixes the issue when we for example wanna connect a  
BNEP connection with a 2.1 device. The requirement is that this has to  
be secure. However without this patch it pairs it during ACL  
connection setup and then again to ensure the man-in-the-middle  
protection. This decreases the connection setup time since the 2.1 key  
generation procedure takes 3-5 seconds and doing this twice is an  
significant penalty compared to 2.0 and before devices.

I think it is appropriate outside the merge window, but if you don't  
think so, I gonna pull that patch and only submit the other two.

Regards

Marcel

Re: Bluetooth fixes for 2.6.27

[David Miller]

From: Marcel Holtmann <marcel@holtmann.org>
Date: Tue, 9 Sep 2008 04:04:02 +0200

[ Please type enter every 80 columns... please! ]

> >> The second patch fixes the authentication requirements. We do have to
> >> separate between service discovery and actual profile channels. This is
> >> a clear requirement of the Bluetooth Security Mode 4 introduced with the
> >> addition of the Simple Pairing support. Not fixing this will result in
> >> broken behavior when doing service discovery with Simple Pairing enabled
> >> devices.
> >
> > What regression reported by a user is fixed by this?
> >
> > This does not look like it is appropriate outside of the merge window.
>
> the regression is that with Bluetooth 2.0 and before we always were
> allowing service discovery (SDP) connection without any bonding
> requirement. This is still true, but with Bluetooth 2.1 enabled
> devices it can now happen that we have to do a full pairing
> procedure. For SDP it should be at least using pairing with a
> just-works model with the no bonding requirement, while the other
> PSM channels (RFCOMM, BNEP etc.) should use generic bonding. We
> would now always use generic bonding (even for SDP).
>
> The number of users are still limited to a few people actually
> testing with 2.1 hardware. These are mainly people working on 2.1
> enabled products. However with the new MacBooks and the EeePC 901 we
> do have devices with Bluetooth 2.1 chips available for everybody.
>
> This is clearly an oversight on my hand when developing the initial
> Simple Pairing patches that I submitted for 2.6.27-rc1 and I only
> found it when we tested against the official Bluetooth 2.1 test
> system.

This is the core issue, if it regresses from 2.6.26, and if so you should
mention that somewhere.  Best would be in the commit message itself.

Then I wouldn't have had to ask you anything.

But instead you're having to describe it for me here in this email, which
nobody can see when scanning the GIT commit messages, so it essentially
might as well not even exist.

Re: Bluetooth fixes for 2.6.27

[Marcel Holtmann]

Hi Dave,

> [ Please type enter every 80 columns... please! ]

sorry, but sometimes I am just sitting their with my MacBook running the
Apple mail program and that one just does what it wants.

> > >> The second patch fixes the authentication requirements. We do have to
> > >> separate between service discovery and actual profile channels. This is
> > >> a clear requirement of the Bluetooth Security Mode 4 introduced with the
> > >> addition of the Simple Pairing support. Not fixing this will result in
> > >> broken behavior when doing service discovery with Simple Pairing enabled
> > >> devices.
> > >
> > > What regression reported by a user is fixed by this?
> > >
> > > This does not look like it is appropriate outside of the merge window.
> >
> > the regression is that with Bluetooth 2.0 and before we always were
> > allowing service discovery (SDP) connection without any bonding
> > requirement. This is still true, but with Bluetooth 2.1 enabled
> > devices it can now happen that we have to do a full pairing
> > procedure. For SDP it should be at least using pairing with a
> > just-works model with the no bonding requirement, while the other
> > PSM channels (RFCOMM, BNEP etc.) should use generic bonding. We
> > would now always use generic bonding (even for SDP).
> >
> > The number of users are still limited to a few people actually
> > testing with 2.1 hardware. These are mainly people working on 2.1
> > enabled products. However with the new MacBooks and the EeePC 901 we
> > do have devices with Bluetooth 2.1 chips available for everybody.
> >
> > This is clearly an oversight on my hand when developing the initial
> > Simple Pairing patches that I submitted for 2.6.27-rc1 and I only
> > found it when we tested against the official Bluetooth 2.1 test
> > system.
> 
> This is the core issue, if it regresses from 2.6.26, and if so you should
> mention that somewhere.  Best would be in the commit message itself.
> 
> Then I wouldn't have had to ask you anything.
> 
> But instead you're having to describe it for me here in this email, which
> nobody can see when scanning the GIT commit messages, so it essentially
> might as well not even exist.

I thought that I did describe this properly in the commit message, but
it could also only be clear to me. Do you want me to fix up the commit
message with more details?

Regards

Marcel

Re: Bluetooth fixes for 2.6.27

[David Miller]

From: Marcel Holtmann <marcel@holtmann.org>
Date: Tue, 09 Sep 2008 06:28:01 +0200

> > > The number of users are still limited to a few people actually
> > > testing with 2.1 hardware. These are mainly people working on 2.1
> > > enabled products. However with the new MacBooks and the EeePC 901 we
> > > do have devices with Bluetooth 2.1 chips available for everybody.
> > >
> > > This is clearly an oversight on my hand when developing the initial
> > > Simple Pairing patches that I submitted for 2.6.27-rc1 and I only
> > > found it when we tested against the official Bluetooth 2.1 test
> > > system.
> > 
> > This is the core issue, if it regresses from 2.6.26, and if so you should
> > mention that somewhere.  Best would be in the commit message itself.
> > 
> > Then I wouldn't have had to ask you anything.
> > 
> > But instead you're having to describe it for me here in this email, which
> > nobody can see when scanning the GIT commit messages, so it essentially
> > might as well not even exist.
> 
> I thought that I did describe this properly in the commit message, but
> it could also only be clear to me. Do you want me to fix up the commit
> message with more details?

Where in your commit message did you specifically state and explain
that this problem is specifically a regression against 2.6.26?  I didn't
catch that part.

Re: Bluetooth fixes for 2.6.27

[Marcel Holtmann]

Hi Dave,

> > > > The number of users are still limited to a few people actually
> > > > testing with 2.1 hardware. These are mainly people working on 2.1
> > > > enabled products. However with the new MacBooks and the EeePC 901 we
> > > > do have devices with Bluetooth 2.1 chips available for everybody.
> > > >
> > > > This is clearly an oversight on my hand when developing the initial
> > > > Simple Pairing patches that I submitted for 2.6.27-rc1 and I only
> > > > found it when we tested against the official Bluetooth 2.1 test
> > > > system.
> > > 
> > > This is the core issue, if it regresses from 2.6.26, and if so you should
> > > mention that somewhere.  Best would be in the commit message itself.
> > > 
> > > Then I wouldn't have had to ask you anything.
> > > 
> > > But instead you're having to describe it for me here in this email, which
> > > nobody can see when scanning the GIT commit messages, so it essentially
> > > might as well not even exist.
> > 
> > I thought that I did describe this properly in the commit message, but
> > it could also only be clear to me. Do you want me to fix up the commit
> > message with more details?
> 
> Where in your commit message did you specifically state and explain
> that this problem is specifically a regression against 2.6.26?  I didn't
> catch that part.

it was the "... L2CAP PSM 1 (used by SDP) the just-works model ..." and
I admit, after you pointing my nose on it, that it is not clear to
anybody not deeply into this Bluetooth 2.1 stuff.

Let me fix up the tree and have this commit with a better explanation
what it is all about.

Regards

Marcel

Re: Bluetooth fixes for 2.6.27

[David Miller]

From: Marcel Holtmann <marcel@holtmann.org>
Date: Tue, 09 Sep 2008 06:46:01 +0200

> it was the "... L2CAP PSM 1 (used by SDP) the just-works model ..." and
> I admit, after you pointing my nose on it, that it is not clear to
> anybody not deeply into this Bluetooth 2.1 stuff.
> 
> Let me fix up the tree and have this commit with a better explanation
> what it is all about.

Thanks a lot.

Re: Bluetooth fixes for 2.6.27

[Marcel Holtmann]

Hi Dave,

> > it was the "... L2CAP PSM 1 (used by SDP) the just-works model ..." and
> > I admit, after you pointing my nose on it, that it is not clear to
> > anybody not deeply into this Bluetooth 2.1 stuff.
> > 
> > Let me fix up the tree and have this commit with a better explanation
> > what it is all about.
> 
> Thanks a lot.

I re-pushed the three patches and made sure that the second one has a
more detailed description on what is going on. Let me know what you
think.

Regards

Marcel


Please pull from

    git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-2.6.git master

This will update the following files:

 include/net/bluetooth/hci_core.h |    3 ++-
 net/bluetooth/af_bluetooth.c     |    2 +-
 net/bluetooth/hci_conn.c         |   21 ++++++++++++++++++---
 net/bluetooth/hci_event.c        |   11 ++++-------
 net/bluetooth/l2cap.c            |   34 ++++++++++++++++++++++++++++------
 net/bluetooth/sco.c              |    2 +-
 6 files changed, 54 insertions(+), 19 deletions(-)

through these ChangeSets:

Marcel Holtmann (3):
    [Bluetooth] Fix reference counting during ACL config stage
    [Bluetooth] Enforce correct authentication requirements
    [Bluetooth] Reject L2CAP connections on an insecure ACL link

Re: Bluetooth fixes for 2.6.27

[David Miller]

From: Marcel Holtmann <marcel@holtmann.org>
Date: Tue, 09 Sep 2008 08:08:24 +0200

> I re-pushed the three patches and made sure that the second one has a
> more detailed description on what is going on. Let me know what you
> think.

Pulled and pushed back out to net-2.6, thanks a lot.

Bluetooth fixes for 2.6.27

[Marcel Holtmann]

Hi Dave,

here are three fixes for the Bluetooth subsystem.

The removal of the /sys/class/bluetooth symlinks broke userspace
assumption. This has been fixed now in a way we should have done it a
long time ago.

For the btusb driver this adds the promised SCO support. The btusb
driver is a new driver and will eventually replace hci_usb. Adding SCO
support was the last missing piece. All distributions are using the
hci_usb driver at the moment and you can only select one of them. So
this can't introduce any regression. With this change the distributions
are now able to select the new driver if they really want to.

I have one pending optimization for this driver, but that depends on the
USB patches. The usb_unlink_anchored_urbs() is currently broken and thus
we can't use it. I will submit this patch once the USB fixes got merged.

In addition, I finally cleaned out the MAINTAINERS file and the
MODULE_AUTHOR tags to reflect maintainer information.

Regards

Marcel


Please pull from

    git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-2.6.git master

This will update the following files:

 MAINTAINERS                   |   87 +---------
 drivers/bluetooth/Kconfig     |   10 +-
 drivers/bluetooth/bt3c_cs.c   |    2 +-
 drivers/bluetooth/btusb.c     |  282 +++++++++++++++++++++++++++++--
 drivers/bluetooth/hci_ldisc.c |    2 +-
 drivers/bluetooth/hci_usb.c   |    2 +-
 drivers/bluetooth/hci_vhci.c  |    2 +-
 net/bluetooth/af_bluetooth.c  |    2 +-
 net/bluetooth/bnep/core.c     |    2 +-
 net/bluetooth/hci_sysfs.c     |  376 +++++++++++++++++++++--------------------
 net/bluetooth/l2cap.c         |    2 +-
 net/bluetooth/rfcomm/core.c   |    2 +-
 net/bluetooth/sco.c           |    2 +-
 13 files changed, 476 insertions(+), 297 deletions(-)

through these ChangeSets:

Marcel Holtmann (3):
    [Bluetooth] Add SCO support to btusb driver
    [Bluetooth] Fix userspace breakage due missing class links
    [Bluetooth] Consolidate maintainers information

Re: Bluetooth fixes for 2.6.27

[David Miller]

From: Marcel Holtmann <marcel@holtmann.org>
Date: Tue, 19 Aug 2008 10:23:27 +0200

> The removal of the /sys/class/bluetooth symlinks broke userspace
> assumption. This has been fixed now in a way we should have done it a
> long time ago.
> 
> For the btusb driver this adds the promised SCO support. The btusb
> driver is a new driver and will eventually replace hci_usb. Adding SCO
> support was the last missing piece. All distributions are using the
> hci_usb driver at the moment and you can only select one of them. So
> this can't introduce any regression. With this change the distributions
> are now able to select the new driver if they really want to.
> 
> I have one pending optimization for this driver, but that depends on the
> USB patches. The usb_unlink_anchored_urbs() is currently broken and thus
> we can't use it. I will submit this patch once the USB fixes got merged.
> 
> In addition, I finally cleaned out the MAINTAINERS file and the
> MODULE_AUTHOR tags to reflect maintainer information.

Pulled, thanks a lot Marcel.