[PATCH BlueZ v2 0/9] Permanent storage of SMP keys

[PATCH BlueZ v2 0/9] Permanent storage of SMP keys

[Vinicius Costa Gomes]

Changes from last version:
 - Fixed a memory leak in the way that the core loads the keys;

--

Vinicius Costa Gomes (9):
  Add stubs for storing SMP keys
  Add support for storing SMP keys
  Add support for creating the device from the SMP keys
  Define macros for link key types
  Fix using "magic" values for key types
  Fix memory leak when loading keys
  Add support for getting the Encryption Key Size via btio
  Add support for passing the CID to btiotest
  Add support for btiotest to returning the key size

 btio/btio.c       |   21 ++++++++++++++
 btio/btio.h       |    1 +
 doc/mgmt-api.txt  |    2 +
 lib/bluetooth.h   |    1 +
 lib/hci.h         |   13 ++++++++
 lib/mgmt.h        |    2 +
 plugins/hciops.c  |   26 +++++++++++++----
 plugins/mgmtops.c |   33 +++++++++++++++------
 src/adapter.c     |   81 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 src/adapter.h     |    2 +
 src/event.c       |   61 +++++++++++++++++++++++++++++++++++++--
 src/event.h       |    2 +-
 src/storage.c     |   28 ++++++++++++++++++
 src/storage.h     |    2 +
 test/btiotest.c   |   24 ++++++++++++++--
 15 files changed, 273 insertions(+), 26 deletions(-)

--
1.7.6

[PATCH BlueZ v2 1/9] Add stubs for storing SMP keys

[Vinicius Costa Gomes]

As we begin to support pairing over LE links, we need to store those
keys in permanent storage.

This patch changes the data types according to the current kernel interface
and adds new methods that will be used to store those keys. The documentation
of the Management API is updated to reflect this.
---
 doc/mgmt-api.txt  |    2 ++
 lib/mgmt.h        |    2 ++
 plugins/hciops.c  |    2 +-
 plugins/mgmtops.c |    5 +++--
 src/adapter.h     |    2 ++
 src/event.c       |   10 ++++++----
 src/event.h       |    2 +-
 src/storage.c     |    5 +++++
 src/storage.h     |    1 +
 9 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
index d89467c..eea460d 100644
--- a/doc/mgmt-api.txt
+++ b/doc/mgmt-api.txt
@@ -172,6 +172,8 @@ Load Keys Command
 					Type (1 Octet)
 					Value (16 Octets)
 					PIN_Length (1 Octet)
+					Data Length (1 Octet)
+					Data (Data Length Octets)
 				}
 				Key2 { }
 				...
diff --git a/lib/mgmt.h b/lib/mgmt.h
index f22434e..dd0334c 100644
--- a/lib/mgmt.h
+++ b/lib/mgmt.h
@@ -111,6 +111,8 @@ struct mgmt_key_info {
 	uint8_t type;
 	uint8_t val[16];
 	uint8_t pin_len;
+	uint8_t dlen;
+	uint8_t data[0];
 } __packed;
 
 #define MGMT_OP_LOAD_KEYS		0x000D
diff --git a/plugins/hciops.c b/plugins/hciops.c
index ecc0e86..3daccac 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -994,7 +994,7 @@ static void link_key_notify(int index, void *ptr)
 
 		err = btd_event_link_key_notify(&dev->bdaddr, dba,
 						evt->link_key, key_type,
-						dev->pin_length);
+						dev->pin_length, 0, NULL);
 
 		if (err == -ENODEV)
 			status = HCI_OE_LOW_RESOURCES;
diff --git a/plugins/mgmtops.c b/plugins/mgmtops.c
index 3cdb97e..6b5422e 100644
--- a/plugins/mgmtops.c
+++ b/plugins/mgmtops.c
@@ -407,8 +407,9 @@ static void mgmt_new_key(int sk, uint16_t index, void *buf, size_t len)
 
 	if (ev->store_hint)
 		btd_event_link_key_notify(&info->bdaddr, &ev->key.bdaddr,
-						ev->key.val, ev->key.type,
-						ev->key.pin_len);
+					ev->key.val, ev->key.type,
+					ev->key.pin_len, ev->key.dlen,
+					ev->key.data);
 
 	btd_event_bonding_complete(&info->bdaddr, &ev->key.bdaddr, 0);
 }
diff --git a/src/adapter.h b/src/adapter.h
index 38ea3ca..5250341 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -61,6 +61,8 @@ struct link_key_info {
 	unsigned char key[16];
 	uint8_t type;
 	uint8_t pin_len;
+	uint8_t dlen;
+	uint8_t data[0];
 };
 
 struct remote_dev_info {
diff --git a/src/event.c b/src/event.c
index 86a413e..3f0f454 100644
--- a/src/event.c
+++ b/src/event.c
@@ -395,9 +395,8 @@ proceed:
 	adapter_set_state(adapter, STATE_IDLE);
 }
 
-int btd_event_link_key_notify(bdaddr_t *local, bdaddr_t *peer,
-				uint8_t *key, uint8_t key_type,
-				uint8_t pin_length)
+int btd_event_link_key_notify(bdaddr_t *local, bdaddr_t *peer, uint8_t *key,
+		uint8_t key_type, uint8_t pin_length, int dlen, uint8_t *data)
 {
 	struct btd_adapter *adapter;
 	struct btd_device *device;
@@ -408,7 +407,10 @@ int btd_event_link_key_notify(bdaddr_t *local, bdaddr_t *peer,
 
 	DBG("storing link key of type 0x%02x", key_type);
 
-	ret = write_link_key(local, peer, key, key_type, pin_length);
+	if (key_type < 0x10)
+		ret = write_link_key(local, peer, key, key_type, pin_length);
+	else
+		ret = write_longtermkeys(local, peer, NULL);
 
 	if (ret == 0 && device_is_temporary(device))
 		device_set_temporary(device, FALSE);
diff --git a/src/event.h b/src/event.h
index 1268edf..7dabd44 100644
--- a/src/event.h
+++ b/src/event.h
@@ -39,4 +39,4 @@ int btd_event_user_confirm(bdaddr_t *sba, bdaddr_t *dba, uint32_t passkey);
 int btd_event_user_passkey(bdaddr_t *sba, bdaddr_t *dba);
 int btd_event_user_notify(bdaddr_t *sba, bdaddr_t *dba, uint32_t passkey);
 int btd_event_link_key_notify(bdaddr_t *local, bdaddr_t *peer, uint8_t *key,
-					uint8_t key_type, uint8_t pin_length);
+		uint8_t key_type, uint8_t pin_length, int dlen, uint8_t *data);
diff --git a/src/storage.c b/src/storage.c
index 73bbc36..fb7561b 100644
--- a/src/storage.c
+++ b/src/storage.c
@@ -1341,3 +1341,8 @@ device_type_t read_device_type(const bdaddr_t *sba, const bdaddr_t *dba)
 
 	return type;
 }
+
+int write_longtermkeys(bdaddr_t *local, bdaddr_t *peer, const char *key)
+{
+	return 0;
+}
diff --git a/src/storage.h b/src/storage.h
index 6929ada..923e819 100644
--- a/src/storage.h
+++ b/src/storage.h
@@ -87,6 +87,7 @@ int read_device_attributes(const bdaddr_t *sba, textfile_cb func, void *data);
 int write_device_type(const bdaddr_t *sba, const bdaddr_t *dba,
 						device_type_t type);
 device_type_t read_device_type(const bdaddr_t *sba, const bdaddr_t *dba);
+int write_longtermkeys(bdaddr_t *local, bdaddr_t *peer, const char *key);
 
 #define PNP_UUID		"00001200-0000-1000-8000-00805f9b34fb"
 
-- 
1.7.6

[PATCH BlueZ v2 2/9] Add support for storing SMP keys

[Vinicius Costa Gomes]

---
 plugins/mgmtops.c |    4 ++--
 src/event.c       |   44 +++++++++++++++++++++++++++++++++++++++++---
 src/storage.c     |   25 ++++++++++++++++++++++++-
 src/storage.h     |    1 +
 4 files changed, 68 insertions(+), 6 deletions(-)

diff --git a/plugins/mgmtops.c b/plugins/mgmtops.c
index 6b5422e..f11ff50 100644
--- a/plugins/mgmtops.c
+++ b/plugins/mgmtops.c
@@ -383,8 +383,8 @@ static void mgmt_new_key(int sk, uint16_t index, void *buf, size_t len)
 	struct mgmt_ev_new_key *ev = buf;
 	struct controller_info *info;
 
-	if (len != sizeof(*ev)) {
-		error("new_key event size mismatch (%zu != %zu)",
+	if (len < sizeof(*ev)) {
+		error("new_key event size mismatch (%zu < %zu)",
 							len, sizeof(*ev));
 		return;
 	}
diff --git a/src/event.c b/src/event.c
index 3f0f454..9cdb5e1 100644
--- a/src/event.c
+++ b/src/event.c
@@ -395,6 +395,43 @@ proceed:
 	adapter_set_state(adapter, STATE_IDLE);
 }
 
+static gchar *buf2str(uint8_t *data, int datalen)
+{
+	gchar *buf;
+	int i;
+
+	buf = g_new0(gchar, (datalen * 2) + 1);
+
+	for (i = 0; i < datalen; i++)
+		sprintf(buf + (i * 2), "%2.2x", data[i]);
+
+	return buf;
+}
+
+static int store_longtermkey(bdaddr_t *local, bdaddr_t *peer, unsigned char *key,
+			uint8_t type, int length, int dlen, uint8_t *data)
+{
+	GString *newkey;
+	char *val, *str;
+	int err;
+
+	val = buf2str(key, 16);
+	newkey = g_string_new(val);
+	g_free(val);
+
+	g_string_append_printf(newkey, " %d %d %d ", type, length, dlen);
+
+	str = buf2str(data, dlen);
+	newkey = g_string_append(newkey, str);
+	g_free(str);
+
+	err = write_longtermkeys(local, peer, newkey->str);
+
+	g_string_free(newkey, TRUE);
+
+	return err;
+}
+
 int btd_event_link_key_notify(bdaddr_t *local, bdaddr_t *peer, uint8_t *key,
 		uint8_t key_type, uint8_t pin_length, int dlen, uint8_t *data)
 {
@@ -407,10 +444,11 @@ int btd_event_link_key_notify(bdaddr_t *local, bdaddr_t *peer, uint8_t *key,
 
 	DBG("storing link key of type 0x%02x", key_type);
 
-	if (key_type < 0x10)
-		ret = write_link_key(local, peer, key, key_type, pin_length);
+	if (key_type & 0x10)
+		ret = store_longtermkey(local, peer, key, key_type, pin_length,
+								dlen, data);
 	else
-		ret = write_longtermkeys(local, peer, NULL);
+		ret = write_link_key(local, peer, key, key_type, pin_length);
 
 	if (ret == 0 && device_is_temporary(device))
 		device_set_temporary(device, FALSE);
diff --git a/src/storage.c b/src/storage.c
index fb7561b..3d95adb 100644
--- a/src/storage.c
+++ b/src/storage.c
@@ -1344,5 +1344,28 @@ device_type_t read_device_type(const bdaddr_t *sba, const bdaddr_t *dba)
 
 int write_longtermkeys(bdaddr_t *local, bdaddr_t *peer, const char *key)
 {
-	return 0;
+	char filename[PATH_MAX + 1], addr[18];
+
+	if (!key)
+		return -EINVAL;
+
+	create_filename(filename, PATH_MAX, local, "longtermkeys");
+
+	create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+
+	ba2str(peer, addr);
+	return textfile_put(filename, addr, key);
+}
+
+char *read_longtermkeys(bdaddr_t *local, bdaddr_t *peer)
+{
+	char filename[PATH_MAX + 1], addr[18];
+
+	create_filename(filename, PATH_MAX, local, "longtermkeys");
+
+	create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+
+	ba2str(peer, addr);
+
+	return textfile_caseget(filename, addr);
 }
diff --git a/src/storage.h b/src/storage.h
index 923e819..23d9ab6 100644
--- a/src/storage.h
+++ b/src/storage.h
@@ -88,6 +88,7 @@ int write_device_type(const bdaddr_t *sba, const bdaddr_t *dba,
 						device_type_t type);
 device_type_t read_device_type(const bdaddr_t *sba, const bdaddr_t *dba);
 int write_longtermkeys(bdaddr_t *local, bdaddr_t *peer, const char *key);
+char *read_longtermkeys(bdaddr_t *local, bdaddr_t *peer);
 
 #define PNP_UUID		"00001200-0000-1000-8000-00805f9b34fb"
 
-- 
1.7.6

Re: [PATCH BlueZ v2 2/9] Add support for storing SMP keys

[Marcel Holtmann]

Hi Vinicius,

>  plugins/mgmtops.c |    4 ++--
>  src/event.c       |   44 +++++++++++++++++++++++++++++++++++++++++---
>  src/storage.c     |   25 ++++++++++++++++++++++++-
>  src/storage.h     |    1 +
>  4 files changed, 68 insertions(+), 6 deletions(-)
> 
> diff --git a/plugins/mgmtops.c b/plugins/mgmtops.c
> index 6b5422e..f11ff50 100644
> --- a/plugins/mgmtops.c
> +++ b/plugins/mgmtops.c
> @@ -383,8 +383,8 @@ static void mgmt_new_key(int sk, uint16_t index, void *buf, size_t len)
>  	struct mgmt_ev_new_key *ev = buf;
>  	struct controller_info *info;
>  
> -	if (len != sizeof(*ev)) {
> -		error("new_key event size mismatch (%zu != %zu)",
> +	if (len < sizeof(*ev)) {
> +		error("new_key event size mismatch (%zu < %zu)",
>  							len, sizeof(*ev));
>  		return;
>  	}
> diff --git a/src/event.c b/src/event.c
> index 3f0f454..9cdb5e1 100644
> --- a/src/event.c
> +++ b/src/event.c
> @@ -395,6 +395,43 @@ proceed:
>  	adapter_set_state(adapter, STATE_IDLE);
>  }
>  
> +static gchar *buf2str(uint8_t *data, int datalen)
> +{
> +	gchar *buf;
> +	int i;

please that not use more gchar anymore. Just use plain char.

> +	buf = g_new0(gchar, (datalen * 2) + 1);

Is it okay to not use g_try_new0.

> +
> +	for (i = 0; i < datalen; i++)
> +		sprintf(buf + (i * 2), "%2.2x", data[i]);
> +
> +	return buf;
> +}

Regards

Marcel

[PATCH BlueZ v2 3/9] Add support for creating the device from the SMP keys

[Vinicius Costa Gomes]

This adds support for creating the device from the SMP keys (for now
just the LTK) previously stored.
---
 plugins/mgmtops.c |   24 +++++++++++++---
 src/adapter.c     |   75 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+), 5 deletions(-)

diff --git a/plugins/mgmtops.c b/plugins/mgmtops.c
index f11ff50..4b4ac6f 100644
--- a/plugins/mgmtops.c
+++ b/plugins/mgmtops.c
@@ -1866,13 +1866,20 @@ static int mgmt_load_keys(int index, GSList *keys, gboolean debug_keys)
 	struct mgmt_key_info *key;
 	size_t key_count, cp_size;
 	GSList *l;
-	int err;
+	int err, i;
 
-	key_count = g_slist_length(keys);
+	key_count = 0;
+	cp_size = sizeof(*cp);
+	for (l = keys; l; l = g_slist_next(l)) {
+		struct link_key_info *info = l->data;
+		key_count++;
+		cp_size += sizeof(struct mgmt_key_info) + info->dlen;
+	}
 
-	DBG("index %d keys %zu debug_keys %d", index, key_count, debug_keys);
+	if (key_count == 0)
+		return 0;
 
-	cp_size = sizeof(*cp) + (key_count * sizeof(*key));
+	DBG("index %d keys %zu debug_keys %d size %zd", index, key_count, debug_keys, cp_size);
 
 	buf = g_try_malloc0(sizeof(*hdr) + cp_size);
 	if (buf == NULL)
@@ -1889,13 +1896,20 @@ static int mgmt_load_keys(int index, GSList *keys, gboolean debug_keys)
 	cp->debug_keys = debug_keys;
 	cp->key_count = htobs(key_count);
 
-	for (l = keys, key = cp->keys; l != NULL; l = g_slist_next(l), key++) {
+	i = 0;
+	for (l = keys; l != NULL; l = g_slist_next(l)) {
 		struct link_key_info *info = l->data;
 
+		key = ((void *) cp->keys) + i;
+
 		bacpy(&key->bdaddr, &info->bdaddr);
 		key->type = info->type;
 		memcpy(key->val, info->key, 16);
 		key->pin_len = info->pin_len;
+		memcpy(key->data, info->data, info->dlen);
+		key->dlen = info->dlen;
+
+		i += sizeof(struct mgmt_key_info) + info->dlen;
 	}
 
 	if (write(mgmt_sock, buf, sizeof(*hdr) + cp_size) < 0)
diff --git a/src/adapter.c b/src/adapter.c
index 0909a22..85979f5 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1924,6 +1924,47 @@ static struct link_key_info *get_key_info(const char *addr, const char *value)
 	return info;
 }
 
+static struct link_key_info *get_ltk_info(const char *addr, const char *value)
+{
+	struct link_key_info *info;
+	char tmp[3], *ptr;
+	int i, ret;
+
+	if (strlen(value) < 36) {
+		error("Unexpectedly short (%zu) link key line", strlen(value));
+		return NULL;
+	}
+
+	info = g_new0(struct link_key_info, 1);
+
+	str2ba(addr, &info->bdaddr);
+
+	memset(tmp, 0, sizeof(tmp));
+
+	for (i = 0; i < 16; i++) {
+		memcpy(tmp, value + (i * 2), 2);
+		info->key[i] = (uint8_t) strtol(tmp, NULL, 16);
+	}
+
+	ptr = (char *) value + 33;
+
+	ret = sscanf(ptr, "%hhd %hhd %hhd %n", &info->type, &info->pin_len,
+							&info->dlen, &i);
+	if (ret < 3) {
+		g_free(info);
+		return NULL;
+	}
+	ptr += i;
+
+	info = g_realloc(info, sizeof(struct link_key_info) + info->dlen);
+	for (i = 0; i < info->dlen; i++) {
+		memcpy(tmp, ptr + (i * 2), 2);
+		info->data[i] = (uint8_t) strtol(tmp, NULL, 16);
+	}
+
+	return info;
+}
+
 static void create_stored_device_from_linkkeys(char *key, char *value,
 							void *user_data)
 {
@@ -1947,6 +1988,37 @@ static void create_stored_device_from_linkkeys(char *key, char *value,
 	}
 }
 
+static void create_stored_device_from_ltks(char *key, char *value,
+							void *user_data)
+{
+	struct adapter_keys *keys = user_data;
+	struct btd_adapter *adapter = keys->adapter;
+	struct btd_device *device;
+	struct link_key_info *info;
+	char srcaddr[18];
+	bdaddr_t src;
+
+	info = get_ltk_info(key, value);
+	if (info)
+		keys->keys = g_slist_append(keys->keys, info);
+
+	if (g_slist_find_custom(adapter->devices, key,
+					(GCompareFunc) device_address_cmp))
+		return;
+
+	adapter_get_address(adapter, &src);
+	ba2str(&src, srcaddr);
+
+	if (g_strcmp0(srcaddr, key) == 0)
+		return;
+
+	device = device_create(connection, adapter, key, DEVICE_TYPE_LE);
+	if (device) {
+		device_set_temporary(device, FALSE);
+		adapter->devices = g_slist_append(adapter->devices, device);
+	}
+}
+
 static void create_stored_device_from_blocked(char *key, char *value,
 							void *user_data)
 {
@@ -2077,6 +2149,9 @@ static void load_devices(struct btd_adapter *adapter)
 	create_name(filename, PATH_MAX, STORAGEDIR, srcaddr, "linkkeys");
 	textfile_foreach(filename, create_stored_device_from_linkkeys, &keys);
 
+	create_name(filename, PATH_MAX, STORAGEDIR, srcaddr, "longtermkeys");
+	textfile_foreach(filename, create_stored_device_from_ltks, &keys);
+
 	err = adapter_ops->load_keys(adapter->dev_id, keys.keys,
 							main_opts.debug_keys);
 	if (err < 0) {
-- 
1.7.6

[PATCH BlueZ v2 4/9] Define macros for link key types

[Vinicius Costa Gomes]

---
 lib/hci.h |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/lib/hci.h b/lib/hci.h
index 51184ee..dc4b36b 100644
--- a/lib/hci.h
+++ b/lib/hci.h
@@ -293,6 +293,19 @@ enum {
 #define HCI_LM_RELIABLE	0x0010
 #define HCI_LM_SECURE	0x0020
 
+/* Link Key types */
+#define HCI_LK_COMBINATION		0x00
+#define HCI_LK_LOCAL_UNIT		0x01
+#define HCI_LK_REMOTE_UNIT		0x02
+#define HCI_LK_DEBUG_COMBINATION	0x03
+#define HCI_LK_UNAUTH_COMBINATION	0x04
+#define HCI_LK_AUTH_COMBINATION		0x05
+#define HCI_LK_CHANGED_COMBINATION	0x06
+/* The spec doesn't define types for SMP keys */
+#define HCI_LK_SMP_LTK			0x81
+#define HCI_LK_SMP_IRK			0x82
+#define HCI_LK_SMP_CSRK			0x83
+
 /* -----  HCI Commands ----- */
 
 /* Link Control */
-- 
1.7.6

[PATCH BlueZ v2 5/9] Fix using "magic" values for key types

[Vinicius Costa Gomes]

Now that we have macros for the different type of keys we should
use them to make the code clearer.
---
 plugins/hciops.c |   10 ++++++----
 src/event.c      |   19 ++++++++++++++++---
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/plugins/hciops.c b/plugins/hciops.c
index 3daccac..207e187 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -955,12 +955,12 @@ static void link_key_notify(int index, void *ptr)
 	DBG("local auth 0x%02x and remote auth 0x%02x",
 					conn->loc_auth, conn->rem_auth);
 
-	if (key_type == 0x06) {
+	if (key_type == HCI_LK_CHANGED_COMBINATION) {
 		/* Some buggy controller combinations generate a changed
 		 * combination key for legacy pairing even when there's no
 		 * previous key */
 		if (conn->rem_auth == 0xff && old_key_type == 0xff)
-			key_type = 0x00;
+			key_type = HCI_LK_COMBINATION;
 		else if (old_key_type != 0xff)
 			key_type = old_key_type;
 		else
@@ -972,7 +972,7 @@ static void link_key_notify(int index, void *ptr)
 	key_info->type = key_type;
 
 	/* Skip the storage check if this is a debug key */
-	if (key_type == 0x03)
+	if (key_type == HCI_LK_DEBUG_COMBINATION)
 		goto done;
 
 	/* Store the link key persistently if one of the following is true:
@@ -986,7 +986,9 @@ static void link_key_notify(int index, void *ptr)
 	 * If none of the above match only keep the link key around for
 	 * this connection and set the temporary flag for the device.
 	 */
-	if (key_type < 0x03 || (key_type == 0x06 && old_key_type != 0xff) ||
+	if (key_type < HCI_LK_DEBUG_COMBINATION ||
+			(key_type == HCI_LK_CHANGED_COMBINATION
+					&& old_key_type != 0xff) ||
 			(conn->loc_auth > 0x01 && conn->rem_auth > 0x01) ||
 			(conn->loc_auth == 0x02 || conn->loc_auth == 0x03) ||
 			(conn->rem_auth == 0x02 || conn->rem_auth == 0x03)) {
diff --git a/src/event.c b/src/event.c
index 9cdb5e1..2d7958b 100644
--- a/src/event.c
+++ b/src/event.c
@@ -444,11 +444,24 @@ int btd_event_link_key_notify(bdaddr_t *local, bdaddr_t *peer, uint8_t *key,
 
 	DBG("storing link key of type 0x%02x", key_type);
 
-	if (key_type & 0x10)
+	switch (key_type) {
+	case HCI_LK_COMBINATION:
+	case HCI_LK_LOCAL_UNIT:
+	case HCI_LK_REMOTE_UNIT:
+	case HCI_LK_DEBUG_COMBINATION:
+	case HCI_LK_UNAUTH_COMBINATION:
+	case HCI_LK_AUTH_COMBINATION:
+	case HCI_LK_CHANGED_COMBINATION:
+		ret = write_link_key(local, peer, key, key_type, pin_length);
+		break;
+	case HCI_LK_SMP_LTK:
 		ret = store_longtermkey(local, peer, key, key_type, pin_length,
 								dlen, data);
-	else
-		ret = write_link_key(local, peer, key, key_type, pin_length);
+		break;
+	default:
+		ret = -ENOTSUP;
+		break;
+	}
 
 	if (ret == 0 && device_is_temporary(device))
 		device_set_temporary(device, FALSE);
-- 
1.7.6

[PATCH BlueZ v2 6/9] Fix memory leak when loading keys

[Vinicius Costa Gomes]

Core bluetoothd doesn't need to have a reference to the link key list,
so we pass a reference to the list and free it as soon as possible. If any
user need to keep a copy of that list around, they need to copy it. This makes
the memory management more consistent.
---
 plugins/hciops.c |   14 +++++++++++++-
 src/adapter.c    |    6 +++---
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/plugins/hciops.c b/plugins/hciops.c
index 207e187..56daf0d 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -3450,6 +3450,7 @@ static int hciops_restore_powered(int index)
 static int hciops_load_keys(int index, GSList *keys, gboolean debug_keys)
 {
 	struct dev_info *dev = &devs[index];
+	GSList *l, *n, *new;
 
 	DBG("hci%d keys %d debug_keys %d", index, g_slist_length(keys),
 								debug_keys);
@@ -3457,7 +3458,18 @@ static int hciops_load_keys(int index, GSList *keys, gboolean debug_keys)
 	if (dev->keys != NULL)
 		return -EEXIST;
 
-	dev->keys = keys;
+	new = g_slist_copy(keys);
+
+	for (n = new, l = keys; l && n; l = l->next, n = n->next) {
+		struct link_key_info *orig;
+
+		orig = l->data;
+
+		n->data = g_malloc0(sizeof(*orig) + orig->dlen);
+		memcpy(n->data, orig, sizeof(*orig) + orig->dlen);
+	}
+
+	dev->keys = new;
 	dev->debug_keys = debug_keys;
 
 	return 0;
diff --git a/src/adapter.c b/src/adapter.c
index 85979f5..cde0244 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2154,11 +2154,11 @@ static void load_devices(struct btd_adapter *adapter)
 
 	err = adapter_ops->load_keys(adapter->dev_id, keys.keys,
 							main_opts.debug_keys);
-	if (err < 0) {
+	if (err < 0)
 		error("Unable to load keys to adapter_ops: %s (%d)",
 							strerror(-err), -err);
-		g_slist_free_full(keys.keys, g_free);
-	}
+
+	g_slist_free_full(keys.keys, g_free);
 
 	create_name(filename, PATH_MAX, STORAGEDIR, srcaddr, "blocked");
 	textfile_foreach(filename, create_stored_device_from_blocked, adapter);
-- 
1.7.6

Re: [PATCH BlueZ v2 6/9] Fix memory leak when loading keys

[Marcel Holtmann]

Hi Vincius.

> Core bluetoothd doesn't need to have a reference to the link key list,
> so we pass a reference to the list and free it as soon as possible. If any
> user need to keep a copy of that list around, they need to copy it. This makes
> the memory management more consistent.
> ---
>  plugins/hciops.c |   14 +++++++++++++-
>  src/adapter.c    |    6 +++---
>  2 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/plugins/hciops.c b/plugins/hciops.c
> index 207e187..56daf0d 100644
> --- a/plugins/hciops.c
> +++ b/plugins/hciops.c
> @@ -3450,6 +3450,7 @@ static int hciops_restore_powered(int index)
>  static int hciops_load_keys(int index, GSList *keys, gboolean debug_keys)
>  {
>  	struct dev_info *dev = &devs[index];
> +	GSList *l, *n, *new;
>  
>  	DBG("hci%d keys %d debug_keys %d", index, g_slist_length(keys),
>  								debug_keys);
> @@ -3457,7 +3458,18 @@ static int hciops_load_keys(int index, GSList *keys, gboolean debug_keys)
>  	if (dev->keys != NULL)
>  		return -EEXIST;
>  
> -	dev->keys = keys;
> +	new = g_slist_copy(keys);
> +
> +	for (n = new, l = keys; l && n; l = l->next, n = n->next) {
> +		struct link_key_info *orig;
> +
> +		orig = l->data;
> +
> +		n->data = g_malloc0(sizeof(*orig) + orig->dlen);
> +		memcpy(n->data, orig, sizeof(*orig) + orig->dlen);

you don't need to add a memset via g_malloc0 here. And do we really not
wanna check the memory allocation here?

Regards

Marcel

Re: [PATCH BlueZ v2 6/9] Fix memory leak when loading keys

[Anderson Lizardo]

Hi Marcel,

On Thu, Jul 7, 2011 at 4:35 AM, Marcel Holtmann <marcel@holtmann.org> wrote=
:
>> + =A0 =A0 =A0 =A0 =A0 =A0 n->data =3D g_malloc0(sizeof(*orig) + orig->dl=
en);
>> + =A0 =A0 =A0 =A0 =A0 =A0 memcpy(n->data, orig, sizeof(*orig) + orig->dl=
en);
>
> you don't need to add a memset via g_malloc0 here. And do we really not
> wanna check the memory allocation here?

What about using g_memdup() ? it does the equivalent of both calls.

Unfortunately, there is no "g_try_memdup"...

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

[PATCH BlueZ v2 7/9] Add support for getting the Encryption Key Size via btio

[Vinicius Costa Gomes]

Some profiles specify some restriction depending on the length
of the key used to encrypt the link, this adds an way to retrieve
that value from the kernel.

Current kernels don't provide this information so the size is
always zero.
---
 btio/btio.c     |   21 +++++++++++++++++++++
 btio/btio.h     |    1 +
 lib/bluetooth.h |    1 +
 3 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/btio/btio.c b/btio/btio.c
index 2cc9082..337dcb9 100644
--- a/btio/btio.c
+++ b/btio/btio.c
@@ -502,6 +502,22 @@ static int l2cap_set_flushable(int sock, gboolean flushable)
 	return 0;
 }
 
+static gboolean get_key_size(int sock, BtIOType type, int *size,
+								GError **err)
+{
+	struct bt_security sec;
+	socklen_t len;
+
+	memset(&sec, 0, sizeof(sec));
+	len = sizeof(sec);
+	if (getsockopt(sock, SOL_BLUETOOTH, BT_SECURITY, &sec, &len) == 0) {
+		*size = sec.key_size;
+		return TRUE;
+	}
+
+	return FALSE;
+}
+
 static gboolean l2cap_set(int sock, int sec_level, uint16_t imtu,
 				uint16_t omtu, uint8_t mode, int master,
 				int flushable, GError **err)
@@ -848,6 +864,11 @@ static gboolean l2cap_get(int sock, GError **err, BtIOOption opt1,
 						va_arg(args, int *), err))
 				return FALSE;
 			break;
+		case BT_IO_OPT_KEY_SIZE:
+			if (!get_key_size(sock, BT_IO_L2CAP,
+						va_arg(args, int *), err))
+				return FALSE;
+			break;
 		case BT_IO_OPT_PSM:
 			*(va_arg(args, uint16_t *)) = src.l2_psm ?
 						src.l2_psm : dst.l2_psm;
diff --git a/btio/btio.h b/btio/btio.h
index c6b736f..eaa9874 100644
--- a/btio/btio.h
+++ b/btio/btio.h
@@ -52,6 +52,7 @@ typedef enum {
 	BT_IO_OPT_DEST_BDADDR,
 	BT_IO_OPT_DEFER_TIMEOUT,
 	BT_IO_OPT_SEC_LEVEL,
+	BT_IO_OPT_KEY_SIZE,
 	BT_IO_OPT_CHANNEL,
 	BT_IO_OPT_SOURCE_CHANNEL,
 	BT_IO_OPT_DEST_CHANNEL,
diff --git a/lib/bluetooth.h b/lib/bluetooth.h
index 738e07a..1492139 100644
--- a/lib/bluetooth.h
+++ b/lib/bluetooth.h
@@ -63,6 +63,7 @@ extern "C" {
 #define BT_SECURITY	4
 struct bt_security {
 	uint8_t level;
+	uint8_t key_size;
 };
 #define BT_SECURITY_SDP		0
 #define BT_SECURITY_LOW		1
-- 
1.7.6

[PATCH BlueZ v2 8/9] Add support for passing the CID to btiotest

[Vinicius Costa Gomes]

---
 test/btiotest.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/test/btiotest.c b/test/btiotest.c
index c02a25a..c4d8f4f 100644
--- a/test/btiotest.c
+++ b/test/btiotest.c
@@ -225,7 +225,7 @@ static void confirm_cb(GIOChannel *io, gpointer user_data)
 }
 
 static void l2cap_connect(const char *src, const char *dst, uint16_t psm,
-						gint disconn, gint sec)
+					uint16_t cid, gint disconn, gint sec)
 {
 	struct io_data *data;
 	GError *err = NULL;
@@ -241,6 +241,7 @@ static void l2cap_connect(const char *src, const char *dst, uint16_t psm,
 						BT_IO_OPT_SOURCE, src,
 						BT_IO_OPT_DEST, dst,
 						BT_IO_OPT_PSM, psm,
+						BT_IO_OPT_CID, cid,
 						BT_IO_OPT_SEC_LEVEL, sec,
 						BT_IO_OPT_INVALID);
 	else
@@ -249,6 +250,7 @@ static void l2cap_connect(const char *src, const char *dst, uint16_t psm,
 						&err,
 						BT_IO_OPT_DEST, dst,
 						BT_IO_OPT_PSM, psm,
+						BT_IO_OPT_CID, cid,
 						BT_IO_OPT_SEC_LEVEL, sec,
 						BT_IO_OPT_INVALID);
 
@@ -466,6 +468,7 @@ static gint opt_disconn = -1;
 static gint opt_accept = DEFAULT_ACCEPT_TIMEOUT;
 static gint opt_sec = 0;
 static gboolean opt_master = FALSE;
+static gint opt_cid = 0;
 
 static GMainLoop *main_loop;
 
@@ -474,6 +477,8 @@ static GOptionEntry options[] = {
 				"RFCOMM channel" },
 	{ "psm", 'p', 0, G_OPTION_ARG_INT, &opt_psm,
 				"L2CAP PSM" },
+	{ "cid", 'j', 0, G_OPTION_ARG_INT, &opt_cid,
+				"L2CAP CID" },
 	{ "sco", 's', 0, G_OPTION_ARG_NONE, &opt_sco,
 				"Use SCO" },
 	{ "defer", 'd', 0, G_OPTION_ARG_NONE, &opt_defer,
@@ -513,9 +518,9 @@ int main(int argc, char *argv[])
 	printf("accept=%d, reject=%d, discon=%d, defer=%d, sec=%d\n",
 		opt_accept, opt_reject, opt_disconn, opt_defer, opt_sec);
 
-	if (opt_psm) {
+	if (opt_psm || opt_cid) {
 		if (argc > 1)
-			l2cap_connect(opt_dev, argv[1], opt_psm,
+			l2cap_connect(opt_dev, argv[1], opt_psm, opt_cid,
 							opt_disconn, opt_sec);
 		else
 			l2cap_listen(opt_dev, opt_psm, opt_defer, opt_reject,
-- 
1.7.6

[PATCH BlueZ v2 9/9] Add support for btiotest to returning the key size

[Vinicius Costa Gomes]

---
 test/btiotest.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/test/btiotest.c b/test/btiotest.c
index c4d8f4f..fdf2234 100644
--- a/test/btiotest.c
+++ b/test/btiotest.c
@@ -135,6 +135,19 @@ static void connect_cb(GIOChannel *io, GError *err, gpointer user_data)
 			printf("imtu=%u, omtu=%u\n", imtu, omtu);
 	}
 
+	if (data->type == BT_IO_L2CAP) {
+		uint8_t key_size;
+
+		if (!bt_io_get(io, data->type, &err,
+					BT_IO_OPT_KEY_SIZE, &key_size,
+					BT_IO_OPT_INVALID)) {
+			printf("Unable to get L2CAP Key size: %s\n",
+								err->message);
+			g_clear_error(&err);
+		} else
+			printf("key_size=%u\n", key_size);
+	}
+
 	if (data->disconn == 0) {
 		g_io_channel_shutdown(io, TRUE, NULL);
 		printf("Disconnected\n");
-- 
1.7.6