2011-06-10 06:50:24

by Luiz Augusto von Dentz

Subject: [PATCH] Bluetooth: Fix L2CAP security check

From: Luiz Augusto von Dentz <[email protected]>

With older userspace versions (using hciops) it might not have the
key type to check if the key has sufficient security for any security
level so it is necessary to check the return of hci_conn_auth to make
sure the connection is authenticated

Signed-off-by: Luiz Augusto von Dentz <[email protected]>
Acked-by: Johan Hedberg <[email protected]>
---
net/bluetooth/hci_conn.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 3163330..efda44c 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -611,8 +611,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)
--
1.7.5.2

2011-06-13 12:36:45

by Luiz Augusto von Dentz

Subject: Re: [PATCH] Bluetooth: Fix L2CAP security check

Hi,

On Mon, Jun 13, 2011 at 9:24 PM, Ilia, Kolominsky <[email protected]> wrote:
>
>> -----Original Message-----
>> From: [email protected] [mailto:linux-bluetooth-
>> [email protected]] On Behalf Of Luiz Augusto von Dentz
>> Sent: Friday, June 10, 2011 9:50 AM
>> To: [email protected]
>> Subject: [PATCH] Bluetooth: Fix L2CAP security check
>>
>> From: Luiz Augusto von Dentz <[email protected]>
>>
>> With older userspace versions (using hciops) it might not have the
>> key type to check if the key has sufficient security for any security
>> level so it is necessary to check the return of hci_conn_auth to make
>> sure the connection is authenticated
>>
>> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
>> Acked-by: Johan Hedberg <[email protected]>
>> ---
>> ?net/bluetooth/hci_conn.c | ? ?4 ++--
>> ?1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>> index 3163330..efda44c 100644
>> --- a/net/bluetooth/hci_conn.c
>> +++ b/net/bluetooth/hci_conn.c
>> @@ -611,8 +611,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)
>> --
>> 1.7.5.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-
>> bluetooth" in
>> the body of a message to [email protected]
>> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>
> I have also come upon this bug - the unconditional "return 0"
> broke connection establishment of A2DP streams both in android
> and PTS tests. I think it is better to apply it ASAP.
> Regards.


I will send a v2 removing the extra parenthesis in a moment.

--
Luiz Augusto von Dentz
Computer Engineer

2011-06-13 12:24:18

by Ilia, Kolominsky

Subject: RE: [PATCH] Bluetooth: Fix L2CAP security check

> -----Original Message-----
> From: [email protected] [mailto:linux-bluetooth-
> [email protected]] On Behalf Of Luiz Augusto von Dentz
> Sent: Friday, June 10, 2011 9:50 AM
> To: [email protected]
> Subject: [PATCH] Bluetooth: Fix L2CAP security check
>
> From: Luiz Augusto von Dentz <[email protected]>
>
> With older userspace versions (using hciops) it might not have the
> key type to check if the key has sufficient security for any security
> level so it is necessary to check the return of hci_conn_auth to make
> sure the connection is authenticated
>
> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
> Acked-by: Johan Hedberg <[email protected]>
> ---
> net/bluetooth/hci_conn.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 3163330..efda44c 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -611,8 +611,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)
> --
> 1.7.5.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

I have also come upon this bug - the unconditional "return 0"
broke connection establishment of A2DP streams both in android
and PTS tests. I think it is better to apply it ASAP.
Regards.


Ilia Kolominsky
[email protected]
Direct: +972(9)7906231
Mobile: +972(54)909009

2011-06-10 14:26:49

by Anderson Lizardo

Subject: Re: [PATCH] Bluetooth: Fix L2CAP security check

Hi Luiz,

On Fri, Jun 10, 2011 at 2:50 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 3163330..efda44c 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -611,8 +611,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;

Too much nitpicking, but why the extra parenthesis?

Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil