[PATCH] bluetooth: Fix for security block issue.

Discussion:

Lukasz Rymanowski

2011-01-25 14:27:19 UTC

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 <lukasz.rymanowski-++***@public.gmane.org>
---
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

Marcel Holtmann

2011-01-25 16:13:10 UTC

Permalink

Hi Lukasz,

Post by Lukasz Rymanowski
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

Antti Julku

2011-04-28 09:39:37 UTC

Permalink

Hi,

Post by Marcel Holtmann
Hi Lukasz,

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.

Post by Marcel Holtmann
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

Post by Marcel Holtmann
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

Ville Tervo

2011-04-28 09:51:39 UTC

Permalink

Hi,

Post by Antti Julku
Hi,

Post by Marcel Holtmann
Hi Lukasz,

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.

Post by Marcel Holtmann
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

Post by Marcel Holtmann
HCI Event: Encrypt Change (0x08) plen 4

status 0x00 handle 42 encrypt 0x01
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

Mika Linnanoja

2011-04-29 07:49:43 UTC

Permalink

Post by Ville Tervo
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

L***@public.gmane.org

2011-05-04 07:53:17 UTC

Permalink

Post by Antti Julku
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.

Post by Antti Julku
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.

Br
Lukasz

Luiz Augusto von Dentz

2011-05-04 15:03:19 UTC

Permalink

Hi,

Post by L***@public.gmane.org

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

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

Gustavo F. Padovan

2011-05-05 19:09:48 UTC

Permalink

Hi Lukasz,

Post by L***@public.gmane.org

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

Antti Julku

2011-09-21 07:22:17 UTC

Permalink

Hi Gustavo,

Post by Gustavo F. Padovan

Post by L***@public.gmane.org

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

Marcel Holtmann

2011-09-21 08:01:56 UTC

Permalink

Hi Antti,

Post by Antti Julku

Post by Gustavo F. Padovan

Post by L***@public.gmane.org

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

Gustavo F. Padovan

2011-05-05 19:02:30 UTC

Permalink

Hi Mika,

Post by Mika Linnanoja

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

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

Mika Linnanoja

2011-05-06 06:10:08 UTC

Permalink

Post by Gustavo F. Padovan

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

Mika Linnanoja

2011-05-06 07:28:23 UTC

Permalink

Post by Mika Linnanoja
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/