2011-01-25 14:27:19

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH] bluetooth: Fix for security block issue.

It can happen that controller will schedule ACL data
containing L2CAP connect request to host just before
encryption change event, even though link is encrypted on
LMP level before L2CAP connect request come.
With this fix, L2CAP layer will handle such scenario.

Signed-off-by: Lukasz Rymanowski <[email protected]>
---
include/net/bluetooth/l2cap.h | 8 +++++++
net/bluetooth/l2cap.c | 46 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 7f88a87..f1a5bd8 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -262,6 +262,11 @@ struct l2cap_chan_list {
long num;
};

+struct l2cap_pend_conn_req {
+ struct l2cap_cmd_hdr cmd;
+ struct l2cap_conn_req conn_req;
+};
+
struct l2cap_conn {
struct hci_conn *hcon;

@@ -276,6 +281,9 @@ struct l2cap_conn {
__u8 info_ident;

struct timer_list info_timer;
+ struct timer_list encrypt_timer;
+
+ struct l2cap_pend_conn_req *p_req;

spinlock_t lock;

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index d99b6b7..09181c7 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -57,6 +57,7 @@

#define VERSION "2.15"

+#define ENCRYPT_TIMEOUT (20) /* 20 ms */
static int disable_ertm;

static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN;
@@ -75,6 +76,7 @@ static void l2cap_busy_work(struct work_struct *work);
static void __l2cap_sock_close(struct sock *sk, int reason);
static void l2cap_sock_close(struct sock *sk);
static void l2cap_sock_kill(struct sock *sk);
+static void l2cap_encrypt_timeout(unsigned long arg);

static int l2cap_build_conf_req(struct sock *sk, void *data);
static struct sk_buff *l2cap_build_cmd(struct l2cap_conn *conn,
@@ -707,6 +709,8 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon, u8 status)
setup_timer(&conn->info_timer, l2cap_info_timeout,
(unsigned long) conn);

+ setup_timer(&conn->encrypt_timer, l2cap_encrypt_timeout,
+ (unsigned long) conn);
conn->disc_reason = 0x13;

return conn;
@@ -735,6 +739,11 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_SENT)
del_timer_sync(&conn->info_timer);

+ if (conn->p_req)
+ del_timer_sync(&conn->encrypt_timer);
+
+ kfree(conn->p_req);
+
hcon->l2cap_data = NULL;
kfree(conn);
}
@@ -2982,6 +2991,28 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
/* Check if the ACL is secure enough (if not SDP) */
if (psm != cpu_to_le16(0x0001) &&
!hci_conn_check_link_mode(conn->hcon)) {
+ /* Let's give a chance to Encryption Change Evt.*/
+ if (!conn->p_req) {
+ conn->p_req = kzalloc(sizeof(*conn->p_req), GFP_KERNEL);
+ if (conn->p_req) {
+ BT_DBG("Create pending connection req %p.",
+ conn->p_req);
+
+ memcpy(&conn->p_req->cmd, cmd, sizeof(*cmd));
+ memcpy(&conn->p_req->conn_req, req,
+ sizeof(*req));
+
+ mod_timer(&conn->encrypt_timer, jiffies +
+ msecs_to_jiffies(ENCRYPT_TIMEOUT));
+
+ /*
+ * We will restart l2cap_conn_req in
+ * l2cap_encrypt_timeout.
+ */
+ bh_unlock_sock(parent);
+ return 0;
+ }
+ }
conn->disc_reason = 0x05;
result = L2CAP_CR_SEC_BLOCK;
goto response;
@@ -3147,6 +3178,21 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd
return 0;
}

+static void l2cap_encrypt_timeout(unsigned long arg)
+{
+ struct l2cap_conn *conn = (void *) arg;
+
+ BUG_ON(conn->p_req == NULL);
+
+ BT_DBG("conn %p, p_req %p", conn, conn->p_req);
+
+ (void)l2cap_connect_req(conn, &conn->p_req->cmd,
+ (u8 *)&conn->p_req->conn_req);
+
+ kfree(conn->p_req);
+ conn->p_req = NULL;
+}
+
static inline void set_default_fcs(struct l2cap_pinfo *pi)
{
/* FCS is enabled only in ERTM or streaming mode, if one or both
--
1.7.0.4

/Lukasz
on behalf of STEricsson


2011-01-25 16:13:10

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] bluetooth: Fix for security block issue.

Hi Lukasz,

> It can happen that controller will schedule ACL data
> containing L2CAP connect request to host just before
> encryption change event, even though link is encrypted on
> LMP level before L2CAP connect request come.
> With this fix, L2CAP layer will handle such scenario.

I really don't like to have a work around for this. It is clearly a bug
in the controller.

Regards

Marcel



2011-04-29 07:49:43

by Mika Linnanoja

[permalink] [raw]
Subject: Re: [PATCH] bluetooth: Fix for security block issue.

On 04/28/2011 12:51 PM, ext Ville Tervo wrote:
> Could the actual reason be some change in usb stack? Could it have lower
> priority for event pipe than for data pipe? In that case event for security
> change might arrive to bt stack too late.
>
> At lest I haven't seen this kind of behaviour with serial attached chips. So I
> think this is something USB specific.

Happens on Ubuntu 11.04 (released yesterday; bluez 4.91 & kernel 2.6.38-8) as
well, tested with White PTS 2.1 dongle (CSR chip) on laptop x61s. Sending
party (OPP with obex-client in a loop) was a phone.

Cheers,
Mika

2011-04-28 09:51:39

by Ville Tervo

[permalink] [raw]
Subject: Re: [PATCH] bluetooth: Fix for security block issue.

Hi,

On Thu, Apr 28, 2011 at 12:39:37PM +0300, Antti Julku wrote:
>
> Hi,
>
> On 01/25/2011 06:13 PM, ext Marcel Holtmann wrote:
> >Hi Lukasz,
> >
> >>It can happen that controller will schedule ACL data
> >>containing L2CAP connect request to host just before
> >>encryption change event, even though link is encrypted on
> >>LMP level before L2CAP connect request come.
> >>With this fix, L2CAP layer will handle such scenario.
> >
> >I really don't like to have a work around for this. It is clearly a bug
> >in the controller.
>
> We see this security block issue all the time in our automated
> testing at Nokia. RFCOMM connections to an Ubuntu PC fail randomly
> because of security block, for example when sending files over OPP.
>
> Hcidump always shows L2CAP before Encrypt Change:
> > ACL data: handle 42 flags 0x02 dlen 12
> L2CAP(s): Connect req: psm 3 scid 0x0041
> < ACL data: handle 42 flags 0x02 dlen 16
> L2CAP(s): Connect rsp: dcid 0x0000 scid 0x0041 result 3 status 0
> Connection refused - security block
> > HCI Event: Encrypt Change (0x08) plen 4
> status 0x00 handle 42 encrypt 0x01
>
> It's easy to reproduce at least with these dongles:
>
> Alink BLUEUSB21 (BCM)
> Belkin BT2.1 F8T017 (BCM)
> DeLock 2.1 (CSR)
> PTS 2.1 (CSR)
>
> So most of our BT 2.1 dongles seem to be buggy. It would be nice to
> have a workaround since it happens with so many dongles.

Could the actual reason be some change in usb stack? Could it have lower
priority for event pipe than for data pipe? In that case event for security
change might arrive to bt stack too late.

At lest I haven't seen this kind of behaviour with serial attached chips. So I
think this is something USB specific.

--
Ville

2011-04-28 09:39:37

by Antti Julku

[permalink] [raw]
Subject: Re: [PATCH] bluetooth: Fix for security block issue.


Hi,

On 01/25/2011 06:13 PM, ext Marcel Holtmann wrote:
> Hi Lukasz,
>
>> It can happen that controller will schedule ACL data
>> containing L2CAP connect request to host just before
>> encryption change event, even though link is encrypted on
>> LMP level before L2CAP connect request come.
>> With this fix, L2CAP layer will handle such scenario.
>
> I really don't like to have a work around for this. It is clearly a bug
> in the controller.

We see this security block issue all the time in our automated testing
at Nokia. RFCOMM connections to an Ubuntu PC fail randomly because of
security block, for example when sending files over OPP.

Hcidump always shows L2CAP before Encrypt Change:
> ACL data: handle 42 flags 0x02 dlen 12
L2CAP(s): Connect req: psm 3 scid 0x0041
< ACL data: handle 42 flags 0x02 dlen 16
L2CAP(s): Connect rsp: dcid 0x0000 scid 0x0041 result 3 status 0
Connection refused - security block
> HCI Event: Encrypt Change (0x08) plen 4
status 0x00 handle 42 encrypt 0x01

It's easy to reproduce at least with these dongles:

Alink BLUEUSB21 (BCM)
Belkin BT2.1 F8T017 (BCM)
DeLock 2.1 (CSR)
PTS 2.1 (CSR)

So most of our BT 2.1 dongles seem to be buggy. It would be nice to have
a workaround since it happens with so many dongles.

Br,
Antti

2011-05-06 07:28:23

by Mika Linnanoja

[permalink] [raw]
Subject: Re: [PATCH] bluetooth: Fix for security block issue.

On 05/06/2011 09:10 AM, Mika Linnanoja wrote:
> Hi.
>
> Was not happening on ubuntu 9.04 (2.6.28.x) and ubuntu 9.10 (2.6.31.x).
>
> Since 10.04 (2.6.32.x) it has been happening to my knowledge.
>
> It's rather easy to reproduce also manually just by trying to send files in a
> loop via OPP between e.g. 2 ubuntu boxes when both have BT2.1 usb dongles
> (devices can be paired, doesn't help).

One another thing; this specific patch doesn't apply on top of your
linux-bluetooth or bluetooth-next anymore due to the refactoring work (.39 level).

Applying it on top of 2.6.38.x kernel[1] works, but after sending OPP towards
PC running with applied kernel I get kernel oops/crash like:

[ 143.503531] Call Trace:
[ 143.503531] [<c12eaff2>] acpi_idle_enter_simple+0x105/0x13d
[ 143.820013] [<c12eb114>] acpi_idle_enter_bm+0xd5/0x25e
[ 143.820017] [<c142000d>] cpuidle_idle_call+0x7d/0x160
[ 143.820020] [<c10092ca>] cpu_idle+0x8a/0xc0
[ 143.820023] [<c104021e>] ? complete+0x4e/0x60
[ 143.820026] [<c15055cd>] rest_init+0x5d/0x70
[ 143.820029] [<c17a87ee>] start_kernel+0x35f/0x366
[ 143.820032] [<c17a82a3>] ? pass_all_bootoptions+0x0/0x149
[ 143.820036] [<c17a80f1>] i386_start_kernel+0xe0/0xe8

After that trying "hciconfig hci0 -a" results in timeout. That trace doesn't
look like bt specific at all to my untrained eye.

Stock kernel without this patch doesn't cause such kind of issue. Of course it
could be built somehow differently, I'm not an expert, followed some blog's
instructions[2].

BR,
Mika

[1] git cloned git://kernel.ubuntu.com/ubuntu/ubuntu-natty.git
[2] http://parabing.com/2011/04/28/ubuntu-natty-a-custom-kernel-is-what-you-want/

2011-05-06 06:10:08

by Mika Linnanoja

[permalink] [raw]
Subject: Re: [PATCH] bluetooth: Fix for security block issue.

On 05/05/2011 10:02 PM, ext Gustavo F. Padovan wrote:
>> Happens on Ubuntu 11.04 (released yesterday; bluez 4.91& kernel 2.6.38-8) as
>> well, tested with White PTS 2.1 dongle (CSR chip) on laptop x61s. Sending
>> party (OPP with obex-client in a loop) was a phone.
>
> Is this an regression? Or did it never happen before?

Hi.

Was not happening on ubuntu 9.04 (2.6.28.x) and ubuntu 9.10 (2.6.31.x).

Since 10.04 (2.6.32.x) it has been happening to my knowledge.

It's rather easy to reproduce also manually just by trying to send files in a
loop via OPP between e.g. 2 ubuntu boxes when both have BT2.1 usb dongles
(devices can be paired, doesn't help).

Cheers,
Mika

2011-05-05 19:09:48

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH] bluetooth: Fix for security block issue.

Hi Lukasz,

* [email protected] <[email protected]> [2011-05-04 10:53:17 +0300]:

> On Thu, Apr 28, 2011 at 12:39:37PM +0300, Antti Julku wrote:
> > It's easy to reproduce at least with these dongles:
> >
> > Alink BLUEUSB21 (BCM)
> > Belkin BT2.1 F8T017 (BCM)
> > DeLock 2.1 (CSR)
> > PTS 2.1 (CSR)
> >
> > So most of our BT 2.1 dongles seem to be buggy. It would be nice to
> > have a workaround since it happens with so many dongles.
>
> If there is so many buggy devices I think Gustavo and Marcel could consider to take this patch.

The patch is too hackish, I don't wanna have such workaround in the tree.
It is a good idea check the USB stack for possible causes for this bug.

--
Gustavo F. Padovan
http://profusion.mobi

2011-05-05 19:02:30

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH] bluetooth: Fix for security block issue.

Hi Mika,

* Mika Linnanoja <[email protected]> [2011-04-29 10:49:43 +0300]:

> On 04/28/2011 12:51 PM, ext Ville Tervo wrote:
> > Could the actual reason be some change in usb stack? Could it have lower
> > priority for event pipe than for data pipe? In that case event for security
> > change might arrive to bt stack too late.
> >
> > At lest I haven't seen this kind of behaviour with serial attached chips. So I
> > think this is something USB specific.
>
> Happens on Ubuntu 11.04 (released yesterday; bluez 4.91 & kernel 2.6.38-8) as
> well, tested with White PTS 2.1 dongle (CSR chip) on laptop x61s. Sending
> party (OPP with obex-client in a loop) was a phone.

Is this an regression? Or did it never happen before?

--
Gustavo F. Padovan
http://profusion.mobi

2011-05-04 15:03:19

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] bluetooth: Fix for security block issue.

Hi,

On Wed, May 4, 2011 at 10:53 AM, <[email protected]> wrote:
> On Thu, Apr 28, 2011 at 12:39:37PM +0300, Antti Julku wrote:
>> It's easy to reproduce at least with these dongles:
>>
>> Alink BLUEUSB21 (BCM)
>> Belkin BT2.1 F8T017 (BCM)
>> DeLock 2.1 (CSR)
>> PTS 2.1 (CSR)
>>
>> So most of our BT 2.1 dongles seem to be buggy. It would be nice to
>> have a workaround since it happens with so many dongles.
>
> If there is so many buggy devices I think Gustavo and Marcel could consider to take this patch.
>
> On 04/28/2011 12:51 PM, ext Ville Tervo wrote:
>> Could the actual reason be some change in usb stack? Could it have
>> lower priority for event pipe than for data pipe? In that case event
>> for security change might arrive to bt stack too late.
>>
>> At lest I haven't seen this kind of behaviour with serial attached
>> chips. So I think this is something USB specific.
>
> I found the problem on serial attached chip.
> Before I received fix in the chip I used that patch for couple months without the problems.

I guess it worth checking if there is some priority inversion like
Ville suggested or it could be somehow related to RFCOMM, not long ago
I fixed a bug to the security level of the rfcomm socket to be applied
also to l2cap maybe it affects this.

Another thing that I noticed is that this check for psm 1 may be to
strict, there could be situation where an application want to
BT_SECURITY_SDP on some arbitrary/non-reserved psm, which currently is
possible with psm 3 (RFCOMM) since it only checks for sec.level >
BT_SECURITY_HIGH (see rfcomm_sock_setsockopt:710, bug?), but it can't
because of this security block will prevent any connection attempt.

--
Luiz Augusto von Dentz
Computer Engineer

2011-05-04 07:53:17

by Lukasz Rymanowski

[permalink] [raw]
Subject: RE: [PATCH] bluetooth: Fix for security block issue.

On Thu, Apr 28, 2011 at 12:39:37PM +0300, Antti Julku wrote:
> It's easy to reproduce at least with these dongles:
>=20
> Alink BLUEUSB21 (BCM)
> Belkin BT2.1 F8T017 (BCM)
> DeLock 2.1 (CSR)
> PTS 2.1 (CSR)
>=20
> So most of our BT 2.1 dongles seem to be buggy. It would be nice to=20
> have a workaround since it happens with so many dongles.

If there is so many buggy devices I think Gustavo and Marcel could consider=
to take this patch.

On 04/28/2011 12:51 PM, ext Ville Tervo wrote:
> Could the actual reason be some change in usb stack? Could it have=20
> lower priority for event pipe than for data pipe? In that case event=20
> for security change might arrive to bt stack too late.
>
> At lest I haven't seen this kind of behaviour with serial attached=20
> chips. So I think this is something USB specific.

I found the problem on serial attached chip.
Before I received fix in the chip I used that patch for couple months witho=
ut the problems.

Br
Lukasz

2011-09-21 08:01:56

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] bluetooth: Fix for security block issue.

Hi Antti,

> >>> It's easy to reproduce at least with these dongles:
> >>>
> >>> Alink BLUEUSB21 (BCM)
> >>> Belkin BT2.1 F8T017 (BCM)
> >>> DeLock 2.1 (CSR)
> >>> PTS 2.1 (CSR)
> >>>
> >>> So most of our BT 2.1 dongles seem to be buggy. It would be nice to
> >>> have a workaround since it happens with so many dongles.
> >>
> >> If there is so many buggy devices I think Gustavo and Marcel could consider to take this patch.
> >
> > The patch is too hackish, I don't wanna have such workaround in the tree.
> > It is a good idea check the USB stack for possible causes for this bug.
> >
>
> We checked this with USB sniffer. Data comes from USB in the same order
> as shown in hcidump, so USB seems to work correctly.

and this means that the Bluetooth controller is screwing this up.

If you take a real Bluetooth air sniffer then you see that the packet is
most likely encrypted. Problem is just that we do not know for sure. And
the Bluetooth specification is pretty clear on what to do in these
cases. There is even a special qualification test case for this.

The real problem is that the HCI event packets and HCI data packets are
on a different endpoint when you do this over USB. And there is no real
guarantee of ordering in USB when you go over different endpoints.

This is essentially the same problem when some controllers send the
first data packet before we received the Connect Complete event with the
connection handle.

Fact is that the chip manufactures need to start testing for this. And
BlueZ is faster with its event processing than other stacks. That is
something they have to deal with.

Regards

Marcel



2011-09-21 07:22:17

by Antti Julku

[permalink] [raw]
Subject: Re: [PATCH] bluetooth: Fix for security block issue.


Hi Gustavo,

On 05/05/2011 10:09 PM, ext Gustavo F. Padovan wrote:

>> On Thu, Apr 28, 2011 at 12:39:37PM +0300, Antti Julku wrote:
>>> It's easy to reproduce at least with these dongles:
>>>
>>> Alink BLUEUSB21 (BCM)
>>> Belkin BT2.1 F8T017 (BCM)
>>> DeLock 2.1 (CSR)
>>> PTS 2.1 (CSR)
>>>
>>> So most of our BT 2.1 dongles seem to be buggy. It would be nice to
>>> have a workaround since it happens with so many dongles.
>>
>> If there is so many buggy devices I think Gustavo and Marcel could consider to take this patch.
>
> The patch is too hackish, I don't wanna have such workaround in the tree.
> It is a good idea check the USB stack for possible causes for this bug.
>

We checked this with USB sniffer. Data comes from USB in the same order
as shown in hcidump, so USB seems to work correctly.

Br,
Antti