[RFC v0 0/3] auth & enrypt cleanup attempt

[RFC v0 0/3] auth & enrypt cleanup attempt

[Daniel Wagner]

From: Daniel Wagner <daniel.wagner@bmw-carit.de>

Hi,

Luiz and I had a discussion on IRC and he suggested
that it might make sense to remove duplicated code around
auth & enrypt in hci_event.c. I found it hard to 
really simply the code there. There are a lot of side
conditions.

Maybe using hci_conn_auth() makes sense since it will
also ask to encrypt the connection. 

cheers,
daniel

Daniel Wagner (3):
  Bluetooth: Make hci_conn_auth/encrypt public functions
  Bluetooth: Use hci_conn_auth() instead of open coded version
  Bluetooth: Use hci_conn_encrypt() instead of open coded version

 include/net/bluetooth/hci_core.h |    2 ++
 net/bluetooth/hci_conn.c         |    4 ++--
 net/bluetooth/hci_event.c        |   24 ++++--------------------
 3 files changed, 8 insertions(+), 22 deletions(-)

-- 
1.7.8.2.302.g17b4

[RFC v0 1/3] Bluetooth: Make hci_conn_auth/encrypt public functions

[Daniel Wagner]

From: Daniel Wagner <daniel.wagner@bmw-carit.de>

Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
---
 include/net/bluetooth/hci_core.h |    2 ++
 net/bluetooth/hci_conn.c         |    4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ea9231f..301b86b 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -527,6 +527,8 @@ void hci_chan_list_flush(struct hci_conn *conn);
 struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
 						__u8 sec_level, __u8 auth_type);
 int hci_conn_check_link_mode(struct hci_conn *conn);
+int hci_conn_auth(struct hci_conn *conn, __u8 sec_level, __u8 auth_type);
+void hci_conn_encrypt(struct hci_conn *conn);
 int hci_conn_check_secure(struct hci_conn *conn, __u8 sec_level);
 int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type);
 int hci_conn_change_link_key(struct hci_conn *conn);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 07bc69e..da49b9c 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -616,7 +616,7 @@ int hci_conn_check_link_mode(struct hci_conn *conn)
 EXPORT_SYMBOL(hci_conn_check_link_mode);
 
 /* Authenticate remote device */
-static int hci_conn_auth(struct hci_conn *conn, __u8 sec_level, __u8 auth_type)
+int hci_conn_auth(struct hci_conn *conn, __u8 sec_level, __u8 auth_type)
 {
 	BT_DBG("conn %p", conn);
 
@@ -650,7 +650,7 @@ static int hci_conn_auth(struct hci_conn *conn, __u8 sec_level, __u8 auth_type)
 }
 
 /* Encrypt the the link */
-static void hci_conn_encrypt(struct hci_conn *conn)
+void hci_conn_encrypt(struct hci_conn *conn)
 {
 	BT_DBG("conn %p", conn);
 
-- 
1.7.8.2.302.g17b4

[RFC v0 2/3] Bluetooth: Use hci_conn_auth() instead of open coded version

[Daniel Wagner]

From: Daniel Wagner <daniel.wagner@bmw-carit.de>

Note this will also enable the encryption which will
not happen in the open coded version.

Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
---
 net/bluetooth/hci_event.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index d37f5b2..453e483 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1296,11 +1296,7 @@ static void hci_cs_remote_name_req(struct hci_dev *hdev, __u8 status)
 	if (!hci_outgoing_auth_needed(hdev, conn))
 		goto unlock;
 
-	if (!test_and_set_bit(HCI_CONN_AUTH_PEND, &conn->pend)) {
-		struct hci_cp_auth_requested cp;
-		cp.handle = __cpu_to_le16(conn->handle);
-		hci_send_cmd(hdev, HCI_OP_AUTH_REQUESTED, sizeof(cp), &cp);
-	}
+	hci_conn_auth(conn, BT_SECURITY_HIGH, conn->auth_type);
 
 unlock:
 	hci_dev_unlock(hdev);
@@ -1814,11 +1810,7 @@ static inline void hci_remote_name_evt(struct hci_dev *hdev, struct sk_buff *skb
 	if (!hci_outgoing_auth_needed(hdev, conn))
 		goto unlock;
 
-	if (!test_and_set_bit(HCI_CONN_AUTH_PEND, &conn->pend)) {
-		struct hci_cp_auth_requested cp;
-		cp.handle = __cpu_to_le16(conn->handle);
-		hci_send_cmd(hdev, HCI_OP_AUTH_REQUESTED, sizeof(cp), &cp);
-	}
+	hci_conn_auth(conn, BT_SECURITY_HIGH, conn->auth_type);
 
 unlock:
 	hci_dev_unlock(hdev);
-- 
1.7.8.2.302.g17b4

[RFC v0 3/3] Bluetooth: Use hci_conn_encrypt() instead of open coded version

[Daniel Wagner]

From: Daniel Wagner <daniel.wagner@bmw-carit.de>

Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
---
 net/bluetooth/hci_event.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 453e483..7a4262d 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1754,11 +1754,7 @@ static inline void hci_auth_complete_evt(struct hci_dev *hdev, struct sk_buff *s
 
 	if (conn->state == BT_CONFIG) {
 		if (!ev->status && hdev->ssp_mode > 0 && conn->ssp_mode > 0) {
-			struct hci_cp_set_conn_encrypt cp;
-			cp.handle  = ev->handle;
-			cp.encrypt = 0x01;
-			hci_send_cmd(hdev, HCI_OP_SET_CONN_ENCRYPT, sizeof(cp),
-									&cp);
+			hci_conn_encrypt(conn);
 		} else {
 			conn->state = BT_CONNECTED;
 			hci_proto_connect_cfm(conn, ev->status);
@@ -1774,11 +1770,7 @@ static inline void hci_auth_complete_evt(struct hci_dev *hdev, struct sk_buff *s
 
 	if (test_bit(HCI_CONN_ENCRYPT_PEND, &conn->pend)) {
 		if (!ev->status) {
-			struct hci_cp_set_conn_encrypt cp;
-			cp.handle  = ev->handle;
-			cp.encrypt = 0x01;
-			hci_send_cmd(hdev, HCI_OP_SET_CONN_ENCRYPT, sizeof(cp),
-									&cp);
+			hci_conn_encrypt(conn);
 		} else {
 			clear_bit(HCI_CONN_ENCRYPT_PEND, &conn->pend);
 			hci_encrypt_cfm(conn, ev->status, 0x00);
-- 
1.7.8.2.302.g17b4

Re: [RFC v0 1/3] Bluetooth: Make hci_conn_auth/encrypt public functions

[Marcel Holtmann]

Hi Daniel,

> From: Daniel Wagner <daniel.wagner@bmw-carit.de>

so I expect a commit message with explanation why this got changed. I do
not wanna go through ten other patches to understand why this is the
correct change or not.

> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
> ---
>  include/net/bluetooth/hci_core.h |    2 ++
>  net/bluetooth/hci_conn.c         |    4 ++--
>  2 files changed, 4 insertions(+), 2 deletions(-)

Regards

Marcel

Re: [RFC v0 2/3] Bluetooth: Use hci_conn_auth() instead of open coded version

[Marcel Holtmann]

Hi Daniel,

> Note this will also enable the encryption which will
> not happen in the open coded version.
> 
> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
> ---
>  net/bluetooth/hci_event.c |   12 ++----------
>  1 files changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index d37f5b2..453e483 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1296,11 +1296,7 @@ static void hci_cs_remote_name_req(struct hci_dev *hdev, __u8 status)
>  	if (!hci_outgoing_auth_needed(hdev, conn))
>  		goto unlock;
>  
> -	if (!test_and_set_bit(HCI_CONN_AUTH_PEND, &conn->pend)) {
> -		struct hci_cp_auth_requested cp;
> -		cp.handle = __cpu_to_le16(conn->handle);
> -		hci_send_cmd(hdev, HCI_OP_AUTH_REQUESTED, sizeof(cp), &cp);
> -	}
> +	hci_conn_auth(conn, BT_SECURITY_HIGH, conn->auth_type);

this is not a replacement. Not even a little bit. You are currently
messing with powers that can break qualification.

Regards

Marcel

Re: [RFC v0 2/3] Bluetooth: Use hci_conn_auth() instead of open coded version

[Daniel Wagner]

Hi Marcel,

On 04.01.2012 16:51, Marcel Holtmann wrote:
> Hi Daniel,
> 
>> Note this will also enable the encryption which will
>> not happen in the open coded version.
>>
>> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
>> ---
>>  net/bluetooth/hci_event.c |   12 ++----------
>>  1 files changed, 2 insertions(+), 10 deletions(-)
>>
>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>> index d37f5b2..453e483 100644
>> --- a/net/bluetooth/hci_event.c
>> +++ b/net/bluetooth/hci_event.c
>> @@ -1296,11 +1296,7 @@ static void hci_cs_remote_name_req(struct hci_dev *hdev, __u8 status)
>>  	if (!hci_outgoing_auth_needed(hdev, conn))
>>  		goto unlock;
>>  
>> -	if (!test_and_set_bit(HCI_CONN_AUTH_PEND, &conn->pend)) {
>> -		struct hci_cp_auth_requested cp;
>> -		cp.handle = __cpu_to_le16(conn->handle);
>> -		hci_send_cmd(hdev, HCI_OP_AUTH_REQUESTED, sizeof(cp), &cp);
>> -	}
>> +	hci_conn_auth(conn, BT_SECURITY_HIGH, conn->auth_type);
> 
> this is not a replacement. Not even a little bit. You are currently
> messing with powers that can break qualification.

I suspected this :) In this case I better leave this stuff untouched.

cheers,
daniel