[PATCH BlueZ 0/8] Proper support for pairing LE devices

[PATCH BlueZ 0/8] Proper support for pairing LE devices

[Vinicius Costa Gomes]

Hi,

This adds proper support for pairing LE devices.

--

Vinicius Costa Gomes (8):
  Add link_type information to the mgmt "Device Connected" event
  Fix doing SDP service discovery for LE devices
  Set the Paired property of a device when restoring from storage
  Fix not using the same way for pairing LE devices
  Fix not using the "bonded" property for new bondings
  Fix connecting to the local adapter when receiving new keys
  Remove the default security level from btio
  Fix btio users to not expect a default security level

 audio/avdtp.c        |    1 +
 audio/control.c      |    1 +
 audio/gateway.c      |    2 ++
 audio/headset.c      |    1 +
 btio/btio.c          |    1 -
 doc/mgmt-api.txt     |    1 +
 input/device.c       |    1 +
 lib/hci.h            |    2 ++
 lib/mgmt.h           |    1 +
 network/connection.c |    1 +
 plugins/hciops.c     |    4 ++--
 plugins/mgmtops.c    |    2 +-
 serial/port.c        |    2 ++
 src/adapter.c        |   10 +---------
 src/device.c         |   29 ++++++++++++-----------------
 src/event.c          |   22 ++++++++++++++++++----
 src/event.h          |    2 +-
 tools/hcitool.c      |    3 ---
 18 files changed, 48 insertions(+), 38 deletions(-)

--
1.7.6

[PATCH BlueZ 1/8] Add link_type information to the mgmt "Device Connected" event

[Vinicius Costa Gomes]

In the case of incomming connections we have to know the type of
the device that connected to us.

Through hciops we have the LE Connection Complete event, that
information was lost when Management interface was being used.

The documentation is updated to reflect this.
---
 doc/mgmt-api.txt  |    1 +
 lib/hci.h         |    2 ++
 lib/mgmt.h        |    1 +
 plugins/hciops.c  |    4 ++--
 plugins/mgmtops.c |    2 +-
 src/event.c       |    5 ++++-
 src/event.h       |    2 +-
 tools/hcitool.c   |    3 ---
 8 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
index 127af94..e700366 100644
--- a/doc/mgmt-api.txt
+++ b/doc/mgmt-api.txt
@@ -498,6 +498,7 @@ Device Connected Event
 Event Code		0x000B
 Controller Index:	<controller id>
 Event Parameters	Address (6 Octets)
+			Link Type (1 Octet)
 
 Device Disconnected Event
 =========================
diff --git a/lib/hci.h b/lib/hci.h
index d6f79f2..451e134 100644
--- a/lib/hci.h
+++ b/lib/hci.h
@@ -212,6 +212,8 @@ enum {
 #define SCO_LINK	0x00
 #define ACL_LINK	0x01
 #define ESCO_LINK	0x02
+/* Low Energy links do not have defined link type. Use invented one */
+#define LE_LINK		0x80
 
 /* LMP features */
 #define LMP_3SLOT	0x01
diff --git a/lib/mgmt.h b/lib/mgmt.h
index 0c52850..b0c2a5e 100644
--- a/lib/mgmt.h
+++ b/lib/mgmt.h
@@ -282,6 +282,7 @@ struct mgmt_ev_new_key {
 #define MGMT_EV_DEVICE_CONNECTED	0x000B
 struct mgmt_ev_device_connected {
 	bdaddr_t bdaddr;
+	uint8_t link_type;
 } __packed;
 
 #define MGMT_EV_DEVICE_DISCONNECTED	0x000C
diff --git a/plugins/hciops.c b/plugins/hciops.c
index 03eec82..5d7b0db 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -2060,7 +2060,7 @@ static inline void conn_complete(int index, void *ptr)
 	conn = get_connection(dev, &evt->bdaddr);
 	conn->handle = btohs(evt->handle);
 
-	btd_event_conn_complete(&dev->bdaddr, &evt->bdaddr);
+	btd_event_conn_complete(&dev->bdaddr, &evt->bdaddr, evt->link_type);
 
 	if (conn->secmode3)
 		bonding_complete(dev, conn, 0);
@@ -2096,7 +2096,7 @@ static inline void le_conn_complete(int index, void *ptr)
 	conn = get_connection(dev, &evt->peer_bdaddr);
 	conn->handle = btohs(evt->handle);
 
-	btd_event_conn_complete(&dev->bdaddr, &evt->peer_bdaddr);
+	btd_event_conn_complete(&dev->bdaddr, &evt->peer_bdaddr, LE_LINK);
 
 	/* check if the remote version needs be requested */
 	ba2str(&dev->bdaddr, local_addr);
diff --git a/plugins/mgmtops.c b/plugins/mgmtops.c
index 3fd1b3f..e6f5a4f 100644
--- a/plugins/mgmtops.c
+++ b/plugins/mgmtops.c
@@ -436,7 +436,7 @@ static void mgmt_device_connected(int sk, uint16_t index, void *buf, size_t len)
 
 	info = &controllers[index];
 
-	btd_event_conn_complete(&info->bdaddr, &ev->bdaddr);
+	btd_event_conn_complete(&info->bdaddr, &ev->bdaddr, ev->link_type);
 }
 
 static void mgmt_device_disconnected(int sk, uint16_t index, void *buf,
diff --git a/src/event.c b/src/event.c
index ade882e..6d276f4 100644
--- a/src/event.c
+++ b/src/event.c
@@ -466,7 +466,7 @@ int btd_event_link_key_notify(bdaddr_t *local, bdaddr_t *peer, uint8_t *key,
 	return ret;
 }
 
-void btd_event_conn_complete(bdaddr_t *local, bdaddr_t *peer)
+void btd_event_conn_complete(bdaddr_t *local, bdaddr_t *peer, uint8_t link_type)
 {
 	struct btd_adapter *adapter;
 	struct btd_device *device;
@@ -476,6 +476,9 @@ void btd_event_conn_complete(bdaddr_t *local, bdaddr_t *peer)
 
 	update_lastused(local, peer);
 
+	if (link_type == LE_LINK)
+		device_set_type(device, DEVICE_TYPE_LE);
+
 	adapter_add_connection(adapter, device);
 }
 
diff --git a/src/event.h b/src/event.h
index 5047db1..3dd25f1 100644
--- a/src/event.h
+++ b/src/event.h
@@ -28,7 +28,7 @@ void btd_event_device_found(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
 void btd_event_set_legacy_pairing(bdaddr_t *local, bdaddr_t *peer, gboolean legacy);
 void btd_event_remote_class(bdaddr_t *local, bdaddr_t *peer, uint32_t class);
 void btd_event_remote_name(bdaddr_t *local, bdaddr_t *peer, uint8_t status, char *name);
-void btd_event_conn_complete(bdaddr_t *local, bdaddr_t *peer);
+void btd_event_conn_complete(bdaddr_t *local, bdaddr_t *peer, uint8_t link_type);
 void btd_event_conn_failed(bdaddr_t *local, bdaddr_t *peer, uint8_t status);
 void btd_event_disconn_complete(bdaddr_t *local, bdaddr_t *peer);
 void btd_event_bonding_complete(bdaddr_t *local, bdaddr_t *peer,
diff --git a/tools/hcitool.c b/tools/hcitool.c
index ece187c..dd13ced 100644
--- a/tools/hcitool.c
+++ b/tools/hcitool.c
@@ -46,9 +46,6 @@
 #include "textfile.h"
 #include "oui.h"
 
-/* Unofficial value, might still change */
-#define LE_LINK		0x03
-
 #define FLAGS_AD_TYPE 0x01
 #define FLAGS_LIMITED_MODE_BIT 0x01
 #define FLAGS_GENERAL_MODE_BIT 0x02
-- 
1.7.6

[PATCH BlueZ 2/8] Fix doing SDP service discovery for LE devices

[Vinicius Costa Gomes]

After the bonding is complete, the SDP service discovery was being made
not matter the type of the device. Now, we check the type of the device
and do the correct type of Service discovery.
---
 src/device.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/device.c b/src/device.c
index 1a333de..44d2586 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2217,8 +2217,12 @@ void device_bonding_complete(struct btd_device *device, uint8_t status)
 			device->discov_timer = 0;
 		}
 
-		device_browse_sdp(device, bonding->conn, bonding->msg,
-				NULL, FALSE);
+		if (device->type != DEVICE_TYPE_LE)
+			device_browse_sdp(device, bonding->conn, bonding->msg,
+								NULL, FALSE);
+		else
+			device_browse_primary(device, bonding->conn,
+							bonding->msg, FALSE);
 
 		bonding_request_free(bonding);
 	} else {
-- 
1.7.6

[PATCH BlueZ 3/8] Set the Paired property of a device when restoring from storage

[Vinicius Costa Gomes]

---
 src/device.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/src/device.c b/src/device.c
index 44d2586..b0adac0 100644
--- a/src/device.c
+++ b/src/device.c
@@ -945,6 +945,11 @@ struct btd_device *device_create(DBusConnection *conn,
 		device_set_bonded(device, TRUE);
 	}
 
+	if (type == DEVICE_TYPE_LE && has_longtermkeys(&src, &device->bdaddr)) {
+		device_set_paired(device, TRUE);
+		device_set_bonded(device, TRUE);
+	}
+
 	return btd_device_ref(device);
 }
 
-- 
1.7.6

[PATCH BlueZ 4/8] Fix not using the same way for pairing LE devices

[Vinicius Costa Gomes]

Now that both backends (hciops and mgmtops) are capable
of pairing LE devices, we don't have to force the procedure.
---
 src/adapter.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index cfa4ddc..df0e92a 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1672,15 +1672,7 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
 			return btd_error_failed(msg, strerror(-err));
 	}
 
-	if (device_get_type(device) != DEVICE_TYPE_LE)
-		return device_create_bonding(device, conn, msg,
-							agent_path, cap);
-
-	err = device_browse_primary(device, conn, msg, TRUE);
-	if (err < 0)
-		return btd_error_failed(msg, strerror(-err));
-
-	return NULL;
+	return device_create_bonding(device, conn, msg, agent_path, cap);
 }
 
 static gint device_path_cmp(struct btd_device *device, const gchar *path)
-- 
1.7.6

[PATCH BlueZ 5/8] Fix not using the "bonded" property for new bondings

[Vinicius Costa Gomes]

When checking if a device was already bonded, we should
use the paired property instead of reading the link key
from storage. This method will work for LE links, also.
---
 src/device.c |   16 +---------------
 1 files changed, 1 insertions(+), 15 deletions(-)

diff --git a/src/device.c b/src/device.c
index b0adac0..04469d7 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2125,29 +2125,15 @@ DBusMessage *device_create_bonding(struct btd_device *device,
 					const char *agent_path,
 					uint8_t capability)
 {
-	char filename[PATH_MAX + 1];
-	char *str, srcaddr[18], dstaddr[18];
 	struct btd_adapter *adapter = device->adapter;
 	struct bonding_req *bonding;
-	bdaddr_t src;
 	int err;
 
-	adapter_get_address(adapter, &src);
-	ba2str(&src, srcaddr);
-	ba2str(&device->bdaddr, dstaddr);
-
 	if (device->bonding)
 		return btd_error_in_progress(msg);
 
-	/* check if a link key already exists */
-	create_name(filename, PATH_MAX, STORAGEDIR, srcaddr,
-			"linkkeys");
-
-	str = textfile_caseget(filename, dstaddr);
-	if (str) {
-		free(str);
+	if (device_is_bonded(device))
 		return btd_error_already_exists(msg);
-	}
 
 	err = adapter_create_bonding(adapter, &device->bdaddr, capability);
 	if (err < 0)
-- 
1.7.6

[PATCH BlueZ 6/8] Fix connecting to the local adapter when receiving new keys

[Vinicius Costa Gomes]

In some cases, we receive a key for when the local adapter is
the responder and one key for when the local adapter is
the initiator of the connection, so one key might have both address
equal to the local adapter.
---
 src/event.c |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/event.c b/src/event.c
index 6d276f4..fb7d102 100644
--- a/src/event.c
+++ b/src/event.c
@@ -215,6 +215,9 @@ void btd_event_bonding_complete(bdaddr_t *local, bdaddr_t *peer,
 
 	create = status ? FALSE : TRUE;
 
+	if (bacmp(local, peer) == 0)
+		return;
+
 	if (!get_adapter_and_device(local, peer, &adapter, &device, create))
 		return;
 
@@ -430,9 +433,17 @@ int btd_event_link_key_notify(bdaddr_t *local, bdaddr_t *peer, uint8_t *key,
 {
 	struct btd_adapter *adapter;
 	struct btd_device *device;
-	int ret;
+	int ret, diff;
 
-	if (!get_adapter_and_device(local, peer, &adapter, &device, TRUE))
+	diff = bacmp(local, peer);
+
+	/*
+	 * If local and peer addresses are equal it means that this key is a
+	 * "slave" key, and it should be stored, but the device must not be
+	 * created.
+	 */
+	if (diff && !get_adapter_and_device(local, peer,
+						&adapter, &device, TRUE))
 		return -ENODEV;
 
 	DBG("storing link key of type 0x%02x", key_type);
@@ -456,7 +467,7 @@ int btd_event_link_key_notify(bdaddr_t *local, bdaddr_t *peer, uint8_t *key,
 		break;
 	}
 
-	if (ret == 0) {
+	if (ret == 0 && diff) {
 		device_set_bonded(device, TRUE);
 
 		if (device_is_temporary(device))
-- 
1.7.6

[PATCH BlueZ 7/8] Remove the default security level from btio

[Vinicius Costa Gomes]

The default value of sec_level when setting *any* option
using bt_io_set() was BT_SECURITY_MEDIUM. This was causing
the security procedure being started in some situations that
it should not.
---
 btio/btio.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/btio/btio.c b/btio/btio.c
index 337dcb9..fbe6e56 100644
--- a/btio/btio.c
+++ b/btio/btio.c
@@ -682,7 +682,6 @@ static gboolean parse_set_opts(struct set_opts *opts, GError **err,
 	/* Set defaults */
 	opts->defer = DEFAULT_DEFER_TIMEOUT;
 	opts->master = -1;
-	opts->sec_level = BT_IO_SEC_MEDIUM;
 	opts->mode = L2CAP_MODE_BASIC;
 	opts->flushable = -1;
 
-- 
1.7.6

[PATCH BlueZ 8/8] Fix btio users to not expect a default security level

[Vinicius Costa Gomes]

The users of btio should not expect that btio will set the security
level to medium if it wasn't specified. Now, all the users specfify
the security level needed.
---
 audio/avdtp.c        |    1 +
 audio/control.c      |    1 +
 audio/gateway.c      |    2 ++
 audio/headset.c      |    1 +
 input/device.c       |    1 +
 network/connection.c |    1 +
 serial/port.c        |    2 ++
 7 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/audio/avdtp.c b/audio/avdtp.c
index 11f6515..d825dd3 100644
--- a/audio/avdtp.c
+++ b/audio/avdtp.c
@@ -2518,6 +2518,7 @@ static GIOChannel *l2cap_connect(struct avdtp *session)
 				BT_IO_OPT_SOURCE_BDADDR, &session->server->src,
 				BT_IO_OPT_DEST_BDADDR, &session->dst,
 				BT_IO_OPT_PSM, AVDTP_PSM,
+				BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
 				BT_IO_OPT_INVALID);
 	if (!io) {
 		error("%s", err->message);
diff --git a/audio/control.c b/audio/control.c
index 882c9fb..53b7876 100644
--- a/audio/control.c
+++ b/audio/control.c
@@ -2012,6 +2012,7 @@ gboolean avrcp_connect(struct audio_device *dev)
 				BT_IO_OPT_SOURCE_BDADDR, &dev->src,
 				BT_IO_OPT_DEST_BDADDR, &dev->dst,
 				BT_IO_OPT_PSM, AVCTP_PSM,
+				BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
 				BT_IO_OPT_INVALID);
 	if (err) {
 		avctp_set_state(control, AVCTP_STATE_DISCONNECTED);
diff --git a/audio/gateway.c b/audio/gateway.c
index e9485d0..afd2f94 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -421,6 +421,7 @@ static void get_record_cb(sdp_list_t *recs, int err, gpointer user_data)
 	io = bt_io_connect(BT_IO_RFCOMM, rfcomm_connect_cb, dev, NULL, &gerr,
 				BT_IO_OPT_SOURCE_BDADDR, &dev->src,
 				BT_IO_OPT_DEST_BDADDR, &dev->dst,
+				BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
 				BT_IO_OPT_CHANNEL, ch,
 				BT_IO_OPT_INVALID);
 	if (!io) {
@@ -751,6 +752,7 @@ gboolean gateway_request_stream(struct audio_device *dev,
 		io = bt_io_connect(BT_IO_SCO, sco_connect_cb, dev, NULL, &err,
 				BT_IO_OPT_SOURCE_BDADDR, &dev->src,
 				BT_IO_OPT_DEST_BDADDR, &dev->dst,
+				BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
 				BT_IO_OPT_INVALID);
 		if (!io) {
 			error("%s", err->message);
diff --git a/audio/headset.c b/audio/headset.c
index 8e63afc..d36e07c 100644
--- a/audio/headset.c
+++ b/audio/headset.c
@@ -1622,6 +1622,7 @@ static int rfcomm_connect(struct audio_device *dev, headset_stream_cb_t cb,
 					BT_IO_OPT_SOURCE_BDADDR, &dev->src,
 					BT_IO_OPT_DEST_BDADDR, &dev->dst,
 					BT_IO_OPT_CHANNEL, hs->rfcomm_ch,
+					BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
 					BT_IO_OPT_INVALID);
 
 	hs->rfcomm_ch = -1;
diff --git a/input/device.c b/input/device.c
index 70d0ba2..bde7236 100644
--- a/input/device.c
+++ b/input/device.c
@@ -368,6 +368,7 @@ static gboolean rfcomm_connect(struct input_conn *iconn, GError **err)
 				NULL, err,
 				BT_IO_OPT_SOURCE_BDADDR, &idev->src,
 				BT_IO_OPT_DEST_BDADDR, &idev->dst,
+				BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
 				BT_IO_OPT_INVALID);
 	if (!io)
 		return FALSE;
diff --git a/network/connection.c b/network/connection.c
index ac27cf2..c56368e 100644
--- a/network/connection.c
+++ b/network/connection.c
@@ -373,6 +373,7 @@ static DBusMessage *connection_connect(DBusConnection *conn,
 				BT_IO_OPT_SOURCE_BDADDR, &peer->src,
 				BT_IO_OPT_DEST_BDADDR, &peer->dst,
 				BT_IO_OPT_PSM, BNEP_PSM,
+				BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
 				BT_IO_OPT_OMTU, BNEP_MTU,
 				BT_IO_OPT_IMTU, BNEP_MTU,
 				BT_IO_OPT_INVALID);
diff --git a/serial/port.c b/serial/port.c
index d011084..ba2eb64 100644
--- a/serial/port.c
+++ b/serial/port.c
@@ -404,6 +404,7 @@ static void get_record_cb(sdp_list_t *recs, int err, gpointer user_data)
 				BT_IO_OPT_SOURCE_BDADDR, &device->src,
 				BT_IO_OPT_DEST_BDADDR, &device->dst,
 				BT_IO_OPT_CHANNEL, port->channel,
+				BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
 				BT_IO_OPT_INVALID);
 	if (!port->io) {
 		error("%s", gerr->message);
@@ -444,6 +445,7 @@ connect:
 				BT_IO_OPT_SOURCE_BDADDR, &device->src,
 				BT_IO_OPT_DEST_BDADDR, &device->dst,
 				BT_IO_OPT_CHANNEL, port->channel,
+				BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
 				BT_IO_OPT_INVALID);
 	if (port->io)
 		return 0;
-- 
1.7.6

Re: [PATCH BlueZ 1/8] Add link_type information to the mgmt "Device Connected" event

[Johan Hedberg]

Hi Vinicius,

On Fri, Aug 19, 2011, Vinicius Costa Gomes wrote:
> --- a/lib/hci.h
> +++ b/lib/hci.h
> @@ -212,6 +212,8 @@ enum {
>  #define SCO_LINK	0x00
>  #define ACL_LINK	0x01
>  #define ESCO_LINK	0x02
> +/* Low Energy links do not have defined link type. Use invented one */
> +#define LE_LINK		0x80

I remember Marcel being against any self-invented additions like this.
Have you discussed this with him?

Johan

Re: [PATCH BlueZ 1/8] Add link_type information to the mgmt "Device Connected" event

[Luiz Augusto von Dentz]

Hi,

On Wed, Aug 31, 2011 at 10:51 AM, Johan Hedberg <johan.hedberg@gmail.com> w=
rote:
> Hi Vinicius,
>
> On Fri, Aug 19, 2011, Vinicius Costa Gomes wrote:
>> --- a/lib/hci.h
>> +++ b/lib/hci.h
>> @@ -212,6 +212,8 @@ enum {
>> =A0#define SCO_LINK =A0 =A0 0x00
>> =A0#define ACL_LINK =A0 =A0 0x01
>> =A0#define ESCO_LINK =A0 =A00x02
>> +/* Low Energy links do not have defined link type. Use invented one */
>> +#define LE_LINK =A0 =A0 =A0 =A0 =A0 =A0 =A00x80
>
> I remember Marcel being against any self-invented additions like this.
> Have you discussed this with him?

Actually this follows what kernel has being doing:

include/net/bluetooth/hci.h:175:#define LE_LINK		0x80

--=20
Luiz Augusto von Dentz

Re: [PATCH BlueZ 1/8] Add link_type information to the mgmt "Device Connected" event

[Johan Hedberg]

Hi Luiz,

On Wed, Aug 31, 2011, Luiz Augusto von Dentz wrote:
> >> +/* Low Energy links do not have defined link type. Use invented one */
> >> +#define LE_LINK              0x80
> >
> > I remember Marcel being against any self-invented additions like this.
> > Have you discussed this with him?
> 
> Actually this follows what kernel has being doing:
> 
> include/net/bluetooth/hci.h:175:#define LE_LINK		0x80

Indeed it does. Also, the patch (from Ville) that introduced this to the
kernel has Marcel's Ack on it, so I guess it's fine then.

Johan