2011-01-12 12:08:31

by Matti J. Aaltonen

[permalink] [raw]
Subject: [PATCH] Bluetooth: Check authorization when sec_level goes high.

Initiate authorization check also in cases where the
security level of an existing connection changes to
BT_SECURITY_HIGH.

This patch fixes a bug which makes commands bluez-test-device and
bluez-simple-agent fail, if the latter is given before the connection
created by bluez-test-device has expired.

Signed-off-by: Matti J. Aaltonen <[email protected]>
---
net/bluetooth/hci_conn.c | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 0b1e460..5df232b 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -380,8 +380,19 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
acl->auth_type = auth_type;
hci_acl_connect(acl);
} else {
- if (acl->sec_level < sec_level)
+ if (acl->sec_level < sec_level) {
acl->sec_level = sec_level;
+ if (acl->sec_level == BT_SECURITY_HIGH) {
+ struct hci_cp_auth_requested ar;
+
+ acl->state = BT_CONFIG;
+ memset(&ar, 0, sizeof(ar));
+ ar.handle = cpu_to_le16(acl->handle);
+ hci_send_cmd(hdev, HCI_OP_AUTH_REQUESTED,
+ sizeof(ar), &ar);
+ }
+ }
+
if (acl->auth_type < auth_type)
acl->auth_type = auth_type;
}
--
1.6.1.3


2011-01-13 08:08:31

by Matti J. Aaltonen

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Check authorization when sec_level goes high.

Hello

On Thu, 2011-01-13 at 00:31 +0200, ext Luiz Augusto von Dentz wrote:
> Hi,
>
> On Wed, Jan 12, 2011 at 2:08 PM, Matti J. Aaltonen
> <[email protected]> wrote:
> > Initiate authorization check also in cases where the
> > security level of an existing connection changes to
> > BT_SECURITY_HIGH.
> >
> > This patch fixes a bug which makes commands bluez-test-device and
> > bluez-simple-agent fail, if the latter is given before the connection
> > created by bluez-test-device has expired.
> >
> I don't think this fixes the problem properly, the authentication
> request could have been sent before because other levels might require
> it, also Im afraid this can break sockets using the same link since
> the state got reseted to BT_CONFIG when it could previously be
> BT_CONNECTED, what happens if there a socket in use and the link is
> set back to BT_CONFIG after connected?

Yes. One of our bluetooth specialists suggested a simpler and probably
better fix for this. He'll probably send his patch shortly...

Thanks,
Matti

2011-01-12 22:31:50

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Check authorization when sec_level goes high.

Hi,

On Wed, Jan 12, 2011 at 2:08 PM, Matti J. Aaltonen
<[email protected]> wrote:
> Initiate authorization check also in cases where the
> security level of an existing connection changes to
> BT_SECURITY_HIGH.
>
> This patch fixes a bug which makes commands bluez-test-device and
> bluez-simple-agent fail, if the latter is given before the connection
> created by bluez-test-device has expired.
>
> Signed-off-by: Matti J. Aaltonen <[email protected]>
> ---
> =A0net/bluetooth/hci_conn.c | =A0 13 ++++++++++++-
> =A01 files changed, 12 insertions(+), 1 deletions(-)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 0b1e460..5df232b 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -380,8 +380,19 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, i=
nt type, bdaddr_t *dst, __u8
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0acl->auth_type =3D auth_type;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0hci_acl_connect(acl);
> =A0 =A0 =A0 =A0} else {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (acl->sec_level < sec_level)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (acl->sec_level < sec_level) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0acl->sec_level =3D sec_lev=
el;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (acl->sec_level =3D=3D B=
T_SECURITY_HIGH) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct hci_=
cp_auth_requested ar;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 acl->state =
=3D BT_CONFIG;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 memset(&ar,=
0, sizeof(ar));
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ar.handle =
=3D cpu_to_le16(acl->handle);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 hci_send_cm=
d(hdev, HCI_OP_AUTH_REQUESTED,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 =A0 =A0 =A0sizeof(ar), &ar);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> +
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (acl->auth_type < auth_type)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0acl->auth_type =3D auth_ty=
pe;
> =A0 =A0 =A0 =A0}

I don't think this fixes the problem properly, the authentication
request could have been sent before because other levels might require
it, also Im afraid this can break sockets using the same link since
the state got reseted to BT_CONFIG when it could previously be
BT_CONNECTED, what happens if there a socket in use and the link is
set back to BT_CONFIG after connected?