[PATCH 1/2] Bluetooth: Create a unified authentication request function

[PATCH 1/2] Bluetooth: Create a unified authentication request function

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@nokia.com>

This patch adds a single function that's responsible for requesting
authentication for outgoing connections. This is preparation for the
next patch which will add automated name requests and thereby move the
authentication requests to a different location.

Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
---
 net/bluetooth/hci_event.c |   54 +++++++++++++++++++++++++++++++++-----------
 1 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 8430276..9c6d9bc 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -677,6 +677,29 @@ static void hci_cs_set_conn_encrypt(struct hci_dev *hdev, __u8 status)
 	hci_dev_unlock(hdev);
 }
 
+static int hci_request_outgoing_auth(struct hci_dev *hdev,
+						struct hci_conn *conn)
+{
+	struct hci_cp_auth_requested cp;
+
+	if (conn->state != BT_CONFIG || !conn->out)
+		return 0;
+
+	if (conn->sec_level == BT_SECURITY_SDP)
+		return 0;
+
+	/* Only request authentication for SSP connections or non-SSP
+	 * devices with sec_level HIGH */
+	if (!(hdev->ssp_mode > 0 && conn->ssp_mode > 0) &&
+					conn->sec_level != BT_SECURITY_HIGH)
+		return 0;
+
+	cp.handle = __cpu_to_le16(conn->handle);
+	hci_send_cmd(hdev, HCI_OP_AUTH_REQUESTED, sizeof(cp), &cp);
+
+	return 1;
+}
+
 static void hci_cs_remote_name_req(struct hci_dev *hdev, __u8 status)
 {
 	BT_DBG("%s status 0x%x", hdev->name, status);
@@ -1156,6 +1179,7 @@ static inline void hci_remote_features_evt(struct hci_dev *hdev, struct sk_buff
 {
 	struct hci_ev_remote_features *ev = (void *) skb->data;
 	struct hci_conn *conn;
+	int auth_requested;
 
 	BT_DBG("%s status %d", hdev->name, ev->status);
 
@@ -1177,12 +1201,15 @@ static inline void hci_remote_features_evt(struct hci_dev *hdev, struct sk_buff
 		cp.page = 0x01;
 		hci_send_cmd(hdev, HCI_OP_READ_REMOTE_EXT_FEATURES,
 							sizeof(cp), &cp);
-	} else if (!ev->status && conn->out &&
-			conn->sec_level == BT_SECURITY_HIGH) {
-		struct hci_cp_auth_requested cp;
-		cp.handle = ev->handle;
-		hci_send_cmd(hdev, HCI_OP_AUTH_REQUESTED, sizeof(cp), &cp);
-	} else {
+		goto unlock;
+	}
+
+	if (!ev->status)
+		auth_requested = hci_request_outgoing_auth(hdev, conn);
+	else
+		auth_requested = 0;
+
+	if (!auth_requested) {
 		conn->state = BT_CONNECTED;
 		hci_proto_connect_cfm(conn, ev->status);
 		hci_conn_put(conn);
@@ -1640,6 +1667,7 @@ static inline void hci_remote_ext_features_evt(struct hci_dev *hdev, struct sk_b
 {
 	struct hci_ev_remote_ext_features *ev = (void *) skb->data;
 	struct hci_conn *conn;
+	int auth_requested;
 
 	BT_DBG("%s", hdev->name);
 
@@ -1661,14 +1689,12 @@ static inline void hci_remote_ext_features_evt(struct hci_dev *hdev, struct sk_b
 	if (conn->state != BT_CONFIG)
 		goto unlock;
 
-	if (!ev->status && hdev->ssp_mode > 0 &&
-			conn->ssp_mode > 0 && conn->out &&
-			conn->sec_level != BT_SECURITY_SDP) {
-		struct hci_cp_auth_requested cp;
-		cp.handle = ev->handle;
-		hci_send_cmd(hdev, HCI_OP_AUTH_REQUESTED,
-				sizeof(cp), &cp);
-	} else {
+	if (!ev->status)
+		auth_requested = hci_request_outgoing_auth(hdev, conn);
+	else
+		auth_requested = 0;
+
+	if (!auth_requested) {
 		conn->state = BT_CONNECTED;
 		hci_proto_connect_cfm(conn, ev->status);
 		hci_conn_put(conn);
-- 
1.7.2.3

[PATCH 2/2] Bluetooth: Automate remote name requests

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@nokia.com>

In Bluetooth there are no automatic updates of remote device names when
they get changed on the remote side. Instead, it is a good idea to do a
manual name request when a new connection gets created (for whatever
reason) since at this point it is very cheap (no costly baseband
connection creation needed just for the sake of the name request).

So far userspace has been responsible for this extra name request but
tighter control is needed in order not to flood Bluetooth controllers
with two many commands during connection creation. It has been shown
that some controllers simply fail to function correctly if they get too
many (almost) simultaneous commands during connection creation. The
simplest way to acheive better control of these commands is to move
their sending completely to the kernel side.

This patch inserts name requests into the sequence of events that the
kernel performs during connection creation. It does this after the
remote features have been successfully requested and before any pending
authentication requests are performed. The code will work sub-optimally
with userspace versions that still do the name requesting themselves (it
shouldn't break anything though) so it is recommended to combine this
with a userspace software version that doesn't have automated name
requests.

Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
---
 net/bluetooth/hci_event.c |   72 +++++++++++++++++++++++++++++++++-----------
 1 files changed, 54 insertions(+), 18 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 9c6d9bc..4165895 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -677,11 +677,9 @@ static void hci_cs_set_conn_encrypt(struct hci_dev *hdev, __u8 status)
 	hci_dev_unlock(hdev);
 }
 
-static int hci_request_outgoing_auth(struct hci_dev *hdev,
+static int hci_outgoing_auth_needed(struct hci_dev *hdev,
 						struct hci_conn *conn)
 {
-	struct hci_cp_auth_requested cp;
-
 	if (conn->state != BT_CONFIG || !conn->out)
 		return 0;
 
@@ -694,15 +692,35 @@ static int hci_request_outgoing_auth(struct hci_dev *hdev,
 					conn->sec_level != BT_SECURITY_HIGH)
 		return 0;
 
-	cp.handle = __cpu_to_le16(conn->handle);
-	hci_send_cmd(hdev, HCI_OP_AUTH_REQUESTED, sizeof(cp), &cp);
-
 	return 1;
 }
 
 static void hci_cs_remote_name_req(struct hci_dev *hdev, __u8 status)
 {
+	struct hci_cp_remote_name_req *cp;
+	struct hci_conn *conn;
+
 	BT_DBG("%s status 0x%x", hdev->name, status);
+
+	/* If successful wait for the name req complete event before
+	 * checking for the need to do authentication */
+	if (!status)
+		return;
+
+	cp = hci_sent_cmd_data(hdev, HCI_OP_REMOTE_NAME_REQ);
+	if (!cp)
+		return;
+
+	hci_dev_lock(hdev);
+
+	conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &cp->bdaddr);
+	if (conn && hci_outgoing_auth_needed(hdev, conn)) {
+		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_dev_unlock(hdev);
 }
 
 static void hci_cs_read_remote_features(struct hci_dev *hdev, __u8 status)
@@ -1113,9 +1131,23 @@ static inline void hci_auth_complete_evt(struct hci_dev *hdev, struct sk_buff *s
 
 static inline void hci_remote_name_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
+	struct hci_ev_remote_name *ev = (void *) skb->data;
+	struct hci_conn *conn;
+
 	BT_DBG("%s", hdev->name);
 
 	hci_conn_check_pending(hdev);
+
+	hci_dev_lock(hdev);
+
+	conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
+	if (conn && hci_outgoing_auth_needed(hdev, conn)) {
+		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_dev_unlock(hdev);
 }
 
 static inline void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb)
@@ -1179,7 +1211,6 @@ static inline void hci_remote_features_evt(struct hci_dev *hdev, struct sk_buff
 {
 	struct hci_ev_remote_features *ev = (void *) skb->data;
 	struct hci_conn *conn;
-	int auth_requested;
 
 	BT_DBG("%s status %d", hdev->name, ev->status);
 
@@ -1204,12 +1235,15 @@ static inline void hci_remote_features_evt(struct hci_dev *hdev, struct sk_buff
 		goto unlock;
 	}
 
-	if (!ev->status)
-		auth_requested = hci_request_outgoing_auth(hdev, conn);
-	else
-		auth_requested = 0;
+	if (!ev->status) {
+		struct hci_cp_remote_name_req cp;
+		memset(&cp, 0, sizeof(cp));
+		bacpy(&cp.bdaddr, &conn->dst);
+		cp.pscan_rep_mode = 0x02;
+		hci_send_cmd(hdev, HCI_OP_REMOTE_NAME_REQ, sizeof(cp), &cp);
+	}
 
-	if (!auth_requested) {
+	if (!hci_outgoing_auth_needed(hdev, conn)) {
 		conn->state = BT_CONNECTED;
 		hci_proto_connect_cfm(conn, ev->status);
 		hci_conn_put(conn);
@@ -1667,7 +1701,6 @@ static inline void hci_remote_ext_features_evt(struct hci_dev *hdev, struct sk_b
 {
 	struct hci_ev_remote_ext_features *ev = (void *) skb->data;
 	struct hci_conn *conn;
-	int auth_requested;
 
 	BT_DBG("%s", hdev->name);
 
@@ -1689,12 +1722,15 @@ static inline void hci_remote_ext_features_evt(struct hci_dev *hdev, struct sk_b
 	if (conn->state != BT_CONFIG)
 		goto unlock;
 
-	if (!ev->status)
-		auth_requested = hci_request_outgoing_auth(hdev, conn);
-	else
-		auth_requested = 0;
+	if (!ev->status) {
+		struct hci_cp_remote_name_req cp;
+		memset(&cp, 0, sizeof(cp));
+		bacpy(&cp.bdaddr, &conn->dst);
+		cp.pscan_rep_mode = 0x02;
+		hci_send_cmd(hdev, HCI_OP_REMOTE_NAME_REQ, sizeof(cp), &cp);
+	}
 
-	if (!auth_requested) {
+	if (!hci_outgoing_auth_needed(hdev, conn)) {
 		conn->state = BT_CONNECTED;
 		hci_proto_connect_cfm(conn, ev->status);
 		hci_conn_put(conn);
-- 
1.7.2.3

Re: [PATCH 2/2] Bluetooth: Automate remote name requests

[Gustavo F. Padovan]

Hi Johan,

* johan.hedberg@gmail.com <johan.hedberg@gmail.com> [2010-11-18 22:22:29 +0200]:

> From: Johan Hedberg <johan.hedberg@nokia.com>
> 
> In Bluetooth there are no automatic updates of remote device names when
> they get changed on the remote side. Instead, it is a good idea to do a
> manual name request when a new connection gets created (for whatever
> reason) since at this point it is very cheap (no costly baseband
> connection creation needed just for the sake of the name request).
> 
> So far userspace has been responsible for this extra name request but
> tighter control is needed in order not to flood Bluetooth controllers
> with two many commands during connection creation. It has been shown
> that some controllers simply fail to function correctly if they get too
> many (almost) simultaneous commands during connection creation. The
> simplest way to acheive better control of these commands is to move
> their sending completely to the kernel side.
> 
> This patch inserts name requests into the sequence of events that the
> kernel performs during connection creation. It does this after the
> remote features have been successfully requested and before any pending
> authentication requests are performed. The code will work sub-optimally
> with userspace versions that still do the name requesting themselves (it
> shouldn't break anything though) so it is recommended to combine this
> with a userspace software version that doesn't have automated name
> requests.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
> ---
>  net/bluetooth/hci_event.c |   72 +++++++++++++++++++++++++++++++++-----------
>  1 files changed, 54 insertions(+), 18 deletions(-)

Both patches have been applied. Thanks.

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

Re: [PATCH 2/2] Bluetooth: Automate remote name requests

[Arun K. Singh]

Hello Johan,


> The code will work sub-optimally
> with userspace versions that still do the name requesting themselves (it
> shouldn't break anything though) so it is recommended to combine this
> with a userspace software version that doesn't have automated name
> requests.


Could you be bit more explicit about such user space versions that you
recommend? Would latest bluez versions such as 4.80 qualify for your
recommendation?


Best Regards,
Arun Kumar.

Re: [PATCH 2/2] Bluetooth: Automate remote name requests

[Johan Hedberg]

Hi Arun,

On Mon, Nov 22, 2010, Arun K. Singh wrote:
> > The code will work sub-optimally with userspace versions that still
> > do the name requesting themselves (it shouldn't break anything
> > though) so it is recommended to combine this with a userspace
> > software version that doesn't have automated name requests.
> 
> Could you be bit more explicit about such user space versions that you
> recommend? Would latest bluez versions such as 4.80 qualify for your
> recommendation?

The automated name requests upon "connect complete" events were removed
in 4.78 so any version from there onwards would qualify.

Johan

Re: [PATCH 2/2] Bluetooth: Automate remote name requests

[Luiz Augusto von Dentz]

Hi Johan,

On Mon, Nov 22, 2010 at 9:12 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Arun,
>
> On Mon, Nov 22, 2010, Arun K. Singh wrote:
>> > The code will work sub-optimally with userspace versions that still
>> > do the name requesting themselves (it shouldn't break anything
>> > though) so it is recommended to combine this with a userspace
>> > software version that doesn't have automated name requests.
>>
>> Could you be bit more explicit about such user space versions that you
>> recommend? Would latest bluez versions such as 4.80 qualify for your
>> recommendation?
>
> The automated name requests upon "connect complete" events were removed
> in 4.78 so any version from there onwards would qualify.

Maybe there is some way to detect that the kernel is not doing it, or
at least force bluetoothd to resolve once to make sure it can be used
with older kernel versions so it doesn't have to always update as it
was used to be but if we are to authenticate or authorize a device it
should have a name so the ui can display it to the user.

-- 
Luiz Augusto von Dentz
Computer Engineer

Re: [PATCH 2/2] Bluetooth: Automate remote name requests

[Johan Hedberg]

Hi Luiz,

On Mon, Nov 22, 2010, Luiz Augusto von Dentz wrote:
> >> Could you be bit more explicit about such user space versions that you
> >> recommend? Would latest bluez versions such as 4.80 qualify for your
> >> recommendation?
> >
> > The automated name requests upon "connect complete" events were removed
> > in 4.78 so any version from there onwards would qualify.
> 
> Maybe there is some way to detect that the kernel is not doing it, or
> at least force bluetoothd to resolve once to make sure it can be used
> with older kernel versions so it doesn't have to always update as it
> was used to be but if we are to authenticate or authorize a device it
> should have a name so the ui can display it to the user.

Regarding the kernel version detection, this was certainly considered
but there doesn't seem to be any simple way to do it. However, now that
you mention it, this can certainly be an inconvenient issue with
incoming connections for devices which we've never seen during a device
discovery. Therefore, some kind of a userspace workaround for this would
imho be desirable and I'd certainly be willing to accept a patch for it
as long as it doesn't get too complicated. I suppose it's sufficient if
the patch would be for hciops only since any system running mgmtops
would also have a kernel that does the name request.

Johan