2013-01-03 13:05:28

by Jaganath Kanakkassery

[permalink] [raw]
Subject: [PATCH 1/2] Bluetooth: Move hci_outgoing_auth_needed() to hci_conn.c

This is done so that other files can use this function

Signed-off-by: Jaganath Kanakkassery <[email protected]>
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_conn.c | 17 +++++++++++++++++
net/bluetooth/hci_event.c | 18 ------------------
3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 014a2ea..2e1897c 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -706,6 +706,7 @@ int hci_get_dev_info(void __user *arg);
int hci_get_conn_list(void __user *arg);
int hci_get_conn_info(struct hci_dev *hdev, void __user *arg);
int hci_get_auth_info(struct hci_dev *hdev, void __user *arg);
+int hci_outgoing_auth_needed(struct hci_dev *hdev, struct hci_conn *conn);
int hci_inquiry(void __user *arg);

struct bdaddr_list *hci_blacklist_lookup(struct hci_dev *hdev,
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 25bfce0..7fbabae 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -1027,3 +1027,20 @@ struct hci_chan *hci_chan_lookup_handle(struct hci_dev *hdev, __u16 handle)

return hchan;
}
+
+int hci_outgoing_auth_needed(struct hci_dev *hdev, struct hci_conn *conn)
+{
+ if (conn->state != BT_CONFIG || !conn->out)
+ return 0;
+
+ if (conn->pending_sec_level == BT_SECURITY_SDP)
+ return 0;
+
+ /* Only request authentication for SSP connections or non-SSP
+ * devices with sec_level HIGH or if MITM protection is requested */
+ if (!hci_conn_ssp_enabled(conn) && !(conn->auth_type & 0x01) &&
+ conn->pending_sec_level != BT_SECURITY_HIGH)
+ return 0;
+
+ return 1;
+}
\ No newline at end of file
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 705078a..720d5ec 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1509,24 +1509,6 @@ static void hci_cs_set_conn_encrypt(struct hci_dev *hdev, __u8 status)
hci_dev_unlock(hdev);
}

-static int hci_outgoing_auth_needed(struct hci_dev *hdev,
- struct hci_conn *conn)
-{
- if (conn->state != BT_CONFIG || !conn->out)
- return 0;
-
- if (conn->pending_sec_level == BT_SECURITY_SDP)
- return 0;
-
- /* Only request authentication for SSP connections or non-SSP
- * devices with sec_level HIGH or if MITM protection is requested */
- if (!hci_conn_ssp_enabled(conn) && !(conn->auth_type & 0x01) &&
- conn->pending_sec_level != BT_SECURITY_HIGH)
- return 0;
-
- return 1;
-}
-
static int hci_resolve_name(struct hci_dev *hdev,
struct inquiry_entry *e)
{
--
1.7.9.5



2013-01-04 05:57:35

by Jaganath Kanakkassery

[permalink] [raw]
Subject: Re: [PATCH 2/2] Bluetooth: Fix authentication if acl data comes before remote feature evt

Hi Johan,

--------------------------------------------------
From: "Johan Hedberg" <[email protected]>
Sent: Thursday, January 03, 2013 7:30 PM
To: "Jaganath Kanakkassery" <[email protected]>
Cc: <[email protected]>
Subject: Re: [PATCH 2/2] Bluetooth: Fix authentication if acl data comes
before remote feature evt

> Hi Jaganath,
>
> On Thu, Jan 03, 2013, Jaganath Kanakkassery wrote:
>> If remote device sends l2cap info request before read_remote_ext_feature
>> completes then mgmt_connected will be sent in hci_acldata_packet() and
>> remote name request wont be sent and eventually authentication wont
>> happen
>>
>> Hcidump log of the issue
>>
>> < HCI Command: Create Connection (0x01|0x0005) plen 13
>> bdaddr BC:85:1F:74:7F:29 ptype 0xcc18 rswitch 0x01 clkoffset 0x4bf7
>> (valid)
>> Packet type: DM1 DM3 DM5 DH1 DH3 DH5
>> > HCI Event: Command Status (0x0f) plen 4
>> Create Connection (0x01|0x0005) status 0x00 ncmd 1
>> > HCI Event: Connect Complete (0x03) plen 11
>> status 0x00 handle 12 bdaddr BC:85:1F:74:7F:29 type ACL encrypt 0x00
>> < HCI Command: Read Remote Supported Features (0x01|0x001b) plen 2
>> handle 12
>> > HCI Event: Command Status (0x0f) plen 4
>> Read Remote Supported Features (0x01|0x001b) status 0x00 ncmd 1
>> > HCI Event: Read Remote Supported Features (0x0b) plen 11
>> status 0x00 handle 12
>> Features: 0xbf 0xfe 0xcf 0xfe 0xdb 0xff 0x7b 0x87
>> > HCI Event: Max Slots Change (0x1b) plen 3
>> handle 12 slots 5
>> < HCI Command: Read Remote Extended Features (0x01|0x001c) plen 3
>> handle 12 page 1
>> > HCI Event: Command Status (0x0f) plen 4
>> Read Remote Extended Features (0x01|0x001c) status 0x00 ncmd 1
>> > ACL data: handle 12 flags 0x02 dlen 10
>> L2CAP(s): Info req: type 2
>> < ACL data: handle 12 flags 0x00 dlen 16
>> L2CAP(s): Info rsp: type 2 result 0
>> Extended feature mask 0x00b8
>> Enhanced Retransmission mode
>> Streaming mode
>> FCS Option
>> Fixed Channels
>> > HCI Event: Read Remote Extended Features (0x23) plen 13
>> status 0x00 handle 12 page 1 max 1
>> Features: 0x01 0x00 0x00 0x00 0x00 0x00 0x00 0x00
>> > ACL data: handle 12 flags 0x02 dlen 10
>> L2CAP(s): Info req: type 3
>> < ACL data: handle 12 flags 0x00 dlen 20
>> L2CAP(s): Info rsp: type 3 result 0
>> Fixed channel list 0x00000002
>> L2CAP Signalling Channel
>> > HCI Event: Number of Completed Packets (0x13) plen 5
>> handle 12 packets 2
>>
>> Signed-off-by: Jaganath Kanakkassery <[email protected]>
>> ---
>> net/bluetooth/hci_core.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index 596660d..c14def9 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -2812,6 +2812,7 @@ static void hci_acldata_packet(struct hci_dev
>> *hdev, struct sk_buff *skb)
>>
>> hci_dev_lock(hdev);
>> if (test_bit(HCI_MGMT, &hdev->dev_flags) &&
>> + !hci_outgoing_auth_needed(hdev, conn) &&
>> !test_and_set_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags))
>> mgmt_device_connected(hdev, &conn->dst, conn->type,
>> conn->dst_type, 0, NULL, 0,
>
> I'm not completely sure if this is the right way or even the right place
> to fix the issue. The reason why this if-clause is here is so that we
> don't get a too late mgmt_connected event in case the remote device is
> fast in sending an L2CAP Connect Request. Maybe if-clause needs to be
> made L2CAP Connect request specific (and moved to an L2CAP specific
> location) or then something added to the code path taken for the info
> request?

If the reason for mgmt_connected in acl_data() is to handle early l2cap
connect request from remote then I think it is better to move it to
l2cap connect request as you said.

So I will add mgmt_connected in l2cap_connect_req() before sending
l2cap connect response?

This will solve the authentication issue as well.

Thanks,
Jaganath


2013-01-03 14:00:26

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 2/2] Bluetooth: Fix authentication if acl data comes before remote feature evt

Hi Jaganath,

On Thu, Jan 03, 2013, Jaganath Kanakkassery wrote:
> If remote device sends l2cap info request before read_remote_ext_feature
> completes then mgmt_connected will be sent in hci_acldata_packet() and
> remote name request wont be sent and eventually authentication wont happen
>
> Hcidump log of the issue
>
> < HCI Command: Create Connection (0x01|0x0005) plen 13
> bdaddr BC:85:1F:74:7F:29 ptype 0xcc18 rswitch 0x01 clkoffset 0x4bf7 (valid)
> Packet type: DM1 DM3 DM5 DH1 DH3 DH5
> > HCI Event: Command Status (0x0f) plen 4
> Create Connection (0x01|0x0005) status 0x00 ncmd 1
> > HCI Event: Connect Complete (0x03) plen 11
> status 0x00 handle 12 bdaddr BC:85:1F:74:7F:29 type ACL encrypt 0x00
> < HCI Command: Read Remote Supported Features (0x01|0x001b) plen 2
> handle 12
> > HCI Event: Command Status (0x0f) plen 4
> Read Remote Supported Features (0x01|0x001b) status 0x00 ncmd 1
> > HCI Event: Read Remote Supported Features (0x0b) plen 11
> status 0x00 handle 12
> Features: 0xbf 0xfe 0xcf 0xfe 0xdb 0xff 0x7b 0x87
> > HCI Event: Max Slots Change (0x1b) plen 3
> handle 12 slots 5
> < HCI Command: Read Remote Extended Features (0x01|0x001c) plen 3
> handle 12 page 1
> > HCI Event: Command Status (0x0f) plen 4
> Read Remote Extended Features (0x01|0x001c) status 0x00 ncmd 1
> > ACL data: handle 12 flags 0x02 dlen 10
> L2CAP(s): Info req: type 2
> < ACL data: handle 12 flags 0x00 dlen 16
> L2CAP(s): Info rsp: type 2 result 0
> Extended feature mask 0x00b8
> Enhanced Retransmission mode
> Streaming mode
> FCS Option
> Fixed Channels
> > HCI Event: Read Remote Extended Features (0x23) plen 13
> status 0x00 handle 12 page 1 max 1
> Features: 0x01 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> > ACL data: handle 12 flags 0x02 dlen 10
> L2CAP(s): Info req: type 3
> < ACL data: handle 12 flags 0x00 dlen 20
> L2CAP(s): Info rsp: type 3 result 0
> Fixed channel list 0x00000002
> L2CAP Signalling Channel
> > HCI Event: Number of Completed Packets (0x13) plen 5
> handle 12 packets 2
>
> Signed-off-by: Jaganath Kanakkassery <[email protected]>
> ---
> net/bluetooth/hci_core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 596660d..c14def9 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2812,6 +2812,7 @@ static void hci_acldata_packet(struct hci_dev *hdev, struct sk_buff *skb)
>
> hci_dev_lock(hdev);
> if (test_bit(HCI_MGMT, &hdev->dev_flags) &&
> + !hci_outgoing_auth_needed(hdev, conn) &&
> !test_and_set_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags))
> mgmt_device_connected(hdev, &conn->dst, conn->type,
> conn->dst_type, 0, NULL, 0,

I'm not completely sure if this is the right way or even the right place
to fix the issue. The reason why this if-clause is here is so that we
don't get a too late mgmt_connected event in case the remote device is
fast in sending an L2CAP Connect Request. Maybe if-clause needs to be
made L2CAP Connect request specific (and moved to an L2CAP specific
location) or then something added to the code path taken for the info
request?

Johan

2013-01-03 13:51:45

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 1/2] Bluetooth: Move hci_outgoing_auth_needed() to hci_conn.c

Hi Jaganath,

On Thu, Jan 03, 2013, Jaganath Kanakkassery wrote:
> This is done so that other files can use this function
>
> Signed-off-by: Jaganath Kanakkassery <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_conn.c | 17 +++++++++++++++++
> net/bluetooth/hci_event.c | 18 ------------------
> 3 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 014a2ea..2e1897c 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -706,6 +706,7 @@ int hci_get_dev_info(void __user *arg);
> int hci_get_conn_list(void __user *arg);
> int hci_get_conn_info(struct hci_dev *hdev, void __user *arg);
> int hci_get_auth_info(struct hci_dev *hdev, void __user *arg);
> +int hci_outgoing_auth_needed(struct hci_dev *hdev, struct hci_conn *conn);
> int hci_inquiry(void __user *arg);
>
> struct bdaddr_list *hci_blacklist_lookup(struct hci_dev *hdev,
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 25bfce0..7fbabae 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -1027,3 +1027,20 @@ struct hci_chan *hci_chan_lookup_handle(struct hci_dev *hdev, __u16 handle)
>
> return hchan;
> }
> +
> +int hci_outgoing_auth_needed(struct hci_dev *hdev, struct hci_conn *conn)
> +{
> + if (conn->state != BT_CONFIG || !conn->out)
> + return 0;
> +
> + if (conn->pending_sec_level == BT_SECURITY_SDP)
> + return 0;
> +
> + /* Only request authentication for SSP connections or non-SSP
> + * devices with sec_level HIGH or if MITM protection is requested */
> + if (!hci_conn_ssp_enabled(conn) && !(conn->auth_type & 0x01) &&
> + conn->pending_sec_level != BT_SECURITY_HIGH)
> + return 0;
> +
> + return 1;
> +}

Since you're moving this to hci_conn.c I'd prefix the function with
hci_conn_*. You should also remove the hdev parameter since it's not
used in the function (I believe the only reason it was there was for
consistency in hci_core.c).

Johan

2013-01-03 13:05:29

by Jaganath Kanakkassery

[permalink] [raw]
Subject: [PATCH 2/2] Bluetooth: Fix authentication if acl data comes before remote feature evt

If remote device sends l2cap info request before read_remote_ext_feature
completes then mgmt_connected will be sent in hci_acldata_packet() and
remote name request wont be sent and eventually authentication wont happen

Hcidump log of the issue

< HCI Command: Create Connection (0x01|0x0005) plen 13
bdaddr BC:85:1F:74:7F:29 ptype 0xcc18 rswitch 0x01 clkoffset 0x4bf7 (valid)
Packet type: DM1 DM3 DM5 DH1 DH3 DH5
> HCI Event: Command Status (0x0f) plen 4
Create Connection (0x01|0x0005) status 0x00 ncmd 1
> HCI Event: Connect Complete (0x03) plen 11
status 0x00 handle 12 bdaddr BC:85:1F:74:7F:29 type ACL encrypt 0x00
< HCI Command: Read Remote Supported Features (0x01|0x001b) plen 2
handle 12
> HCI Event: Command Status (0x0f) plen 4
Read Remote Supported Features (0x01|0x001b) status 0x00 ncmd 1
> HCI Event: Read Remote Supported Features (0x0b) plen 11
status 0x00 handle 12
Features: 0xbf 0xfe 0xcf 0xfe 0xdb 0xff 0x7b 0x87
> HCI Event: Max Slots Change (0x1b) plen 3
handle 12 slots 5
< HCI Command: Read Remote Extended Features (0x01|0x001c) plen 3
handle 12 page 1
> HCI Event: Command Status (0x0f) plen 4
Read Remote Extended Features (0x01|0x001c) status 0x00 ncmd 1
> ACL data: handle 12 flags 0x02 dlen 10
L2CAP(s): Info req: type 2
< ACL data: handle 12 flags 0x00 dlen 16
L2CAP(s): Info rsp: type 2 result 0
Extended feature mask 0x00b8
Enhanced Retransmission mode
Streaming mode
FCS Option
Fixed Channels
> HCI Event: Read Remote Extended Features (0x23) plen 13
status 0x00 handle 12 page 1 max 1
Features: 0x01 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> ACL data: handle 12 flags 0x02 dlen 10
L2CAP(s): Info req: type 3
< ACL data: handle 12 flags 0x00 dlen 20
L2CAP(s): Info rsp: type 3 result 0
Fixed channel list 0x00000002
L2CAP Signalling Channel
> HCI Event: Number of Completed Packets (0x13) plen 5
handle 12 packets 2

Signed-off-by: Jaganath Kanakkassery <[email protected]>
---
net/bluetooth/hci_core.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 596660d..c14def9 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2812,6 +2812,7 @@ static void hci_acldata_packet(struct hci_dev *hdev, struct sk_buff *skb)

hci_dev_lock(hdev);
if (test_bit(HCI_MGMT, &hdev->dev_flags) &&
+ !hci_outgoing_auth_needed(hdev, conn) &&
!test_and_set_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags))
mgmt_device_connected(hdev, &conn->dst, conn->type,
conn->dst_type, 0, NULL, 0,
--
1.7.9.5