[PATCH 1/7] doc: Update settings-storage.txt

[PATCH 1/7] doc: Update settings-storage.txt

[Frédéric Danis]

Link key and Long term key for a device should be saved in
keys file under storage device directory
---
 doc/settings-storage.txt |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/doc/settings-storage.txt b/doc/settings-storage.txt
index 3fdcb03..c0171e9 100644
--- a/doc/settings-storage.txt
+++ b/doc/settings-storage.txt
@@ -30,6 +30,7 @@ contains:
  - one directory per remote device, named by remote device address, which
    contains:
     - an info file
+    - a keys file
     - an attributes file containing attributes of remote LE services
 
 So the directory structure is:
@@ -42,9 +43,11 @@ So the directory structure is:
             ...
         ./<remote device address>/
             ./info
+            ./keys
             ./attributes
         ./<remote device address>/
             ./info
+            ./keys
             ./attributes
         ...
 
@@ -127,8 +130,8 @@ This general group contains:
 Info file format
 ================
 
-Info file may includes multiple groups (General, Device ID, Link key and
-Long term key) related to a remote device.
+Info file may includes multiple groups (General and Device ID) related to
+a remote device.
 
 [General] group contains:
 
@@ -165,6 +168,12 @@ Long term key) related to a remote device.
   Version		Integer		Device version
 
 
+Keys file format
+================
+
+Keys file may includes multiple groups (Link key and Long term key) related
+to a remote device.
+
 [LinkKey] group contains:
 
   Key			String		Key in hexadecimal format
-- 
1.7.9.5

[PATCH 2/7] adapter: Load devices from new storage architecture

[Frédéric Danis]

---
 src/adapter.c |   30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index 163360f..3f184fc 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -34,6 +34,7 @@
 #include <stdbool.h>
 #include <sys/ioctl.h>
 #include <sys/file.h>
+#include <dirent.h>
 
 #include <bluetooth/bluetooth.h>
 #include <bluetooth/uuid.h>
@@ -1996,6 +1997,8 @@ static void load_devices(struct btd_adapter *adapter)
 	char srcaddr[18];
 	struct adapter_keys keys = { adapter, NULL };
 	int err;
+	DIR *dir;
+	struct dirent *entry;
 
 	ba2str(&adapter->bdaddr, srcaddr);
 
@@ -2031,6 +2034,33 @@ static void load_devices(struct btd_adapter *adapter)
 
 	create_name(filename, PATH_MAX, STORAGEDIR, srcaddr, "blocked");
 	textfile_foreach(filename, create_stored_device_from_blocked, adapter);
+
+	snprintf(filename, PATH_MAX, STORAGEDIR "/%s", srcaddr);
+	filename[PATH_MAX] = '\0';
+
+	dir = opendir(filename);
+	if (!dir) {
+		error("Unable to open adapter storage directory: %s", filename);
+		return;
+	}
+
+	while ((entry = readdir(dir)) != NULL) {
+		struct btd_device *device;
+
+		if (entry->d_type != DT_DIR || bachk(entry->d_name) < 0)
+			continue;
+
+		if (g_slist_find_custom(adapter->devices, entry->d_name,
+					(GCompareFunc) device_address_cmp))
+			continue;
+
+		device = device_create(adapter, entry->d_name, BDADDR_BREDR);
+		if (!device)
+			continue;
+
+		device_set_temporary(device, FALSE);
+		adapter->devices = g_slist_append(adapter->devices, device);
+	}
 }
 
 int btd_adapter_block_address(struct btd_adapter *adapter,
-- 
1.7.9.5

[PATCH 3/7] adapter: Add destination file to convert_file()

[Frédéric Danis]

In order to convert keys and longtermkeys, we will need to use
different target file names.
---
 src/adapter.c |   18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 3f184fc..9d6554a 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2556,6 +2556,7 @@ static void convert_names_entry(char *key, char *value, void *user_data)
 struct device_converter {
 	char *address;
 	void (*cb)(GKeyFile *key_file, void *value);
+	char *filename;
 };
 
 static void convert_aliases_entry(GKeyFile *key_file, void *value)
@@ -2631,8 +2632,8 @@ static void convert_entry(char *key, char *value, void *user_data)
 	if (bachk(key) != 0)
 		return;
 
-	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s/info",
-			converter->address, key);
+	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s/%s",
+			converter->address, key, converter->filename);
 	filename[PATH_MAX] = '\0';
 	create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 
@@ -2647,7 +2648,7 @@ static void convert_entry(char *key, char *value, void *user_data)
 	g_key_file_free(key_file);
 }
 
-static void convert_file(char *file, char *address,
+static void convert_file(char *file, char *address, char *dest_file,
 				void (*cb)(GKeyFile *key_file, void *value))
 {
 	char filename[PATH_MAX + 1];
@@ -2663,6 +2664,7 @@ static void convert_file(char *file, char *address,
 	} else {
 		converter.address = address;
 		converter.cb = cb;
+		converter.filename = dest_file;
 
 		textfile_foreach(filename, convert_entry, &converter);
 		textfile_put(filename, "converted", "yes");
@@ -2692,19 +2694,19 @@ static void convert_device_storage(struct btd_adapter *adapter)
 	free(str);
 
 	/* Convert aliases */
-	convert_file("aliases", address, convert_aliases_entry);
+	convert_file("aliases", address, "info", convert_aliases_entry);
 
 	/* Convert trusts */
-	convert_file("trusts", address, convert_trusts_entry);
+	convert_file("trusts", address, "info", convert_trusts_entry);
 
 	/* Convert classes */
-	convert_file("classes", address, convert_classes_entry);
+	convert_file("classes", address, "info", convert_classes_entry);
 
 	/* Convert blocked */
-	convert_file("blocked", address, convert_blocked_entry);
+	convert_file("blocked", address, "info", convert_blocked_entry);
 
 	/* Convert device ids */
-	convert_file("did", address, convert_did_entry);
+	convert_file("did", address, "info", convert_did_entry);
 }
 
 static void convert_config(struct btd_adapter *adapter, const char *filename,
-- 
1.7.9.5

[PATCH 4/7] adapter: Convert storage linkkeys file

[Frédéric Danis]

---
 src/adapter.c |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index 9d6554a..32d3930 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2618,6 +2618,34 @@ static void convert_did_entry(GKeyFile *key_file, void *value)
 	g_key_file_set_integer(key_file, "DeviceID", "Version", val);
 }
 
+static void convert_linkkey_entry(GKeyFile *key_file, void *value)
+{
+	char *type_str, *length_str, *str;
+	gint val;
+
+	type_str = strchr(value, ' ');
+	if (!type_str)
+		return;
+
+	*(type_str++) = 0;
+
+	length_str = strchr(type_str, ' ');
+	if (!length_str)
+		return;
+
+	*(length_str++) = 0;
+
+	str = g_strconcat("0x", value, NULL);
+	g_key_file_set_string(key_file, "LinkKey", "Key", str);
+	g_free(str);
+
+	val = strtol(type_str, NULL, 16);
+	g_key_file_set_integer(key_file, "LinkKey", "Type", val);
+
+	val = strtol(length_str, NULL, 16);
+	g_key_file_set_integer(key_file, "LinkKey", "PINLength", val);
+}
+
 static void convert_entry(char *key, char *value, void *user_data)
 {
 	struct device_converter *converter = user_data;
@@ -2707,6 +2735,9 @@ static void convert_device_storage(struct btd_adapter *adapter)
 
 	/* Convert device ids */
 	convert_file("did", address, "info", convert_did_entry);
+
+	/* Convert linkkeys */
+	convert_file("linkkeys", address, "keys", convert_linkkey_entry);
 }
 
 static void convert_config(struct btd_adapter *adapter, const char *filename,
-- 
1.7.9.5

[PATCH 5/7] device: Remove keys file in device_remove_stored()

[Frédéric Danis]

---
 src/device.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/device.c b/src/device.c
index a99ca34..6b64c5d 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2014,6 +2014,11 @@ static void device_remove_stored(struct btd_device *device)
 	filename[PATH_MAX] = '\0';
 	remove(filename);
 
+	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s/keys", adapter_addr,
+			device_addr);
+	filename[PATH_MAX] = '\0';
+	remove(filename);
+
 	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s", adapter_addr,
 			device_addr);
 	filename[PATH_MAX] = '\0';
-- 
1.7.9.5

[PATCH 6/7] adapter: Upload link keys from new storage

[Frédéric Danis]

Remove read_link_key() from device_create, this moves to load_devices.
---
 src/adapter.c |  109 +++++++++++++++++++++++++--------------------------------
 src/device.c  |    6 ----
 2 files changed, 48 insertions(+), 67 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 32d3930..d473508 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1765,31 +1765,35 @@ static int str2buf(const char *str, uint8_t *buf, size_t blen)
 	return 0;
 }
 
-static struct link_key_info *get_key_info(const char *addr, const char *value)
+static struct link_key_info *get_key_info(const char *local,
+						const char *peer)
 {
-	struct link_key_info *info;
-	char tmp[3];
-	long int l;
+	struct link_key_info *info = NULL;
+	char filename[PATH_MAX + 1];
+	GKeyFile *key_file;
+	char *str;
 
-	if (strlen(value) < 36) {
-		error("Unexpectedly short (%zu) link key line", strlen(value));
-		return NULL;
-	}
+	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s/keys", local, peer);
 
-	info = g_new0(struct link_key_info, 1);
+	key_file = g_key_file_new();
+	g_key_file_load_from_file(key_file, filename, 0, NULL);
 
-	str2ba(addr, &info->bdaddr);
+	str = g_key_file_get_string(key_file, "LinkKey", "Key", NULL);
+	if (!str || strlen(str) != 34)
+		goto failed;
 
-	str2buf(value, info->key, sizeof(info->key));
+	info = g_new0(struct link_key_info, 1);
+
+	str2ba(peer, &info->bdaddr);
+	str2buf(&str[2], info->key, sizeof(info->key));
 
-	memcpy(tmp, value + 33, 2);
-	info->type = (uint8_t) strtol(tmp, NULL, 10);
+	info->type = g_key_file_get_integer(key_file, "LinkKey", "Type", NULL);
+	info->pin_len = g_key_file_get_integer(key_file, "LinkKey", "PINLength",
+						NULL);
 
-	memcpy(tmp, value + 35, 2);
-	l = strtol(tmp, NULL, 10);
-	if (l < 0)
-		l = 0;
-	info->pin_len = l;
+failed:
+	g_free(str);
+	g_key_file_free(key_file);
 
 	return info;
 }
@@ -1830,34 +1834,6 @@ static struct smp_ltk_info *get_ltk_info(const char *addr, uint8_t bdaddr_type,
 	return ltk;
 }
 
-static void create_stored_device_from_linkkeys(char *key, char *value,
-							void *user_data)
-{
-	char address[18];
-	uint8_t bdaddr_type;
-	struct adapter_keys *keys = user_data;
-	struct btd_adapter *adapter = keys->adapter;
-	struct btd_device *device;
-	struct link_key_info *info;
-
-	if (sscanf(key, "%17s#%hhu", address, &bdaddr_type) < 2)
-		bdaddr_type = BDADDR_BREDR;
-
-	info = get_key_info(address, value);
-	if (info)
-		keys->keys = g_slist_append(keys->keys, info);
-
-	if (g_slist_find_custom(adapter->devices, address,
-					(GCompareFunc) device_address_cmp))
-		return;
-
-	device = device_create(adapter, address, bdaddr_type);
-	if (device) {
-		device_set_temporary(device, FALSE);
-		adapter->devices = g_slist_append(adapter->devices, device);
-	}
-}
-
 static void create_stored_device_from_ltks(char *key, char *value,
 							void *user_data)
 {
@@ -2010,18 +1986,6 @@ static void load_devices(struct btd_adapter *adapter)
 	textfile_foreach(filename, create_stored_device_from_primaries,
 								adapter);
 
-	create_name(filename, PATH_MAX, STORAGEDIR, srcaddr, "linkkeys");
-	textfile_foreach(filename, create_stored_device_from_linkkeys, &keys);
-
-	err = mgmt_load_link_keys(adapter->dev_id, keys.keys,
-							main_opts.debug_keys);
-	if (err < 0)
-		error("Unable to load link keys: %s (%d)",
-							strerror(-err), -err);
-
-	g_slist_free_full(keys.keys, g_free);
-	keys.keys = NULL;
-
 	create_name(filename, PATH_MAX, STORAGEDIR, srcaddr, "longtermkeys");
 	textfile_foreach(filename, create_stored_device_from_ltks, &keys);
 
@@ -2046,13 +2010,22 @@ static void load_devices(struct btd_adapter *adapter)
 
 	while ((entry = readdir(dir)) != NULL) {
 		struct btd_device *device;
+		struct link_key_info *key_info;
+		GSList *l;
 
 		if (entry->d_type != DT_DIR || bachk(entry->d_name) < 0)
 			continue;
 
-		if (g_slist_find_custom(adapter->devices, entry->d_name,
-					(GCompareFunc) device_address_cmp))
-			continue;
+		key_info = get_key_info(srcaddr, entry->d_name);
+		if (key_info)
+			keys.keys = g_slist_append(keys.keys, key_info);
+
+		l = g_slist_find_custom(adapter->devices, entry->d_name,
+					(GCompareFunc) device_address_cmp);
+		if (l) {
+			device = l->data;
+			goto device_exist;
+		}
 
 		device = device_create(adapter, entry->d_name, BDADDR_BREDR);
 		if (!device)
@@ -2060,7 +2033,21 @@ static void load_devices(struct btd_adapter *adapter)
 
 		device_set_temporary(device, FALSE);
 		adapter->devices = g_slist_append(adapter->devices, device);
+
+device_exist:
+		if (key_info) {
+			device_set_paired(device, TRUE);
+			device_set_bonded(device, TRUE);
+		}
 	}
+
+	err = mgmt_load_link_keys(adapter->dev_id, keys.keys,
+							main_opts.debug_keys);
+	if (err < 0)
+		error("Unable to load link keys: %s (%d)",
+							strerror(-err), -err);
+
+	g_slist_free_full(keys.keys, g_free);
 }
 
 int btd_adapter_block_address(struct btd_adapter *adapter,
diff --git a/src/device.c b/src/device.c
index 6b64c5d..d6780ee 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1897,12 +1897,6 @@ struct btd_device *device_create(struct btd_adapter *adapter,
 
 	load_info(device, srcaddr, address);
 
-	if (read_link_key(src, &device->bdaddr, device->bdaddr_type, NULL,
-								NULL) == 0) {
-		device_set_paired(device, TRUE);
-		device_set_bonded(device, TRUE);
-	}
-
 	if (device_is_le(device) && has_longtermkeys(src, &device->bdaddr,
 							device->bdaddr_type)) {
 		device_set_paired(device, TRUE);
-- 
1.7.9.5

[PATCH 7/7] event: Store link key infos in new keys file

[Frédéric Danis]

---
 src/event.c |   59 +++++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 47 insertions(+), 12 deletions(-)

diff --git a/src/event.c b/src/event.c
index 7fc8f02..59da740 100644
--- a/src/event.c
+++ b/src/event.c
@@ -354,33 +354,68 @@ static int store_longtermkey(bdaddr_t *local, bdaddr_t *peer,
 	return err;
 }
 
+static void store_link_key(struct btd_adapter *adapter,
+				struct btd_device *device, uint8_t *key,
+				uint8_t type, uint8_t pin_length)
+{
+	char adapter_addr[18];
+	char device_addr[18];
+	char filename[PATH_MAX + 1];
+	GKeyFile *key_file;
+	char key_str[35];
+	char *str;
+	int i;
+	gsize length = 0;
+
+	ba2str(adapter_get_address(adapter), adapter_addr);
+	ba2str(device_get_address(device), device_addr);
+
+	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s/keys", adapter_addr,
+								device_addr);
+	filename[PATH_MAX] = '\0';
+
+	key_file = g_key_file_new();
+	g_key_file_load_from_file(key_file, filename, 0, NULL);
+
+	key_str[0] = '0';
+	key_str[1] = 'x';
+	for (i = 0; i < 16; i++)
+		sprintf(key_str + 2 + (i * 2), "%2.2X", key[i]);
+
+	g_key_file_set_string(key_file, "LinkKey", "Key", key_str);
+
+	g_key_file_set_integer(key_file, "LinkKey", "Type", type);
+	g_key_file_set_integer(key_file, "LinkKey", "PINLength", pin_length);
+
+	create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+
+	str = g_key_file_to_data(key_file, &length, NULL);
+	g_file_set_contents(filename, str, length, NULL);
+	g_free(str);
+
+	g_key_file_free(key_file);
+}
+
 int btd_event_link_key_notify(bdaddr_t *local, bdaddr_t *peer,
 				uint8_t *key, uint8_t key_type,
 				uint8_t pin_length)
 {
 	struct btd_adapter *adapter;
 	struct btd_device *device;
-	uint8_t peer_type;
-	int ret;
 
 	if (!get_adapter_and_device(local, peer, &adapter, &device, TRUE))
 		return -ENODEV;
 
 	DBG("storing link key of type 0x%02x", key_type);
 
-	peer_type = device_get_addr_type(device);
+	store_link_key(adapter, device, key, key_type, pin_length);
 
-	ret = write_link_key(local, peer, peer_type, key, key_type,
-								pin_length);
+	device_set_bonded(device, TRUE);
 
-	if (ret == 0) {
-		device_set_bonded(device, TRUE);
-
-		if (device_is_temporary(device))
-			device_set_temporary(device, FALSE);
-	}
+	if (device_is_temporary(device))
+		device_set_temporary(device, FALSE);
 
-	return ret;
+	return 0;
 }
 
 int btd_event_ltk_notify(bdaddr_t *local, bdaddr_t *peer, uint8_t bdaddr_type,
-- 
1.7.9.5

Re: [PATCH 7/7] event: Store link key infos in new keys file

[Johan Hedberg]

Hi Frederic,

On Tue, Nov 27, 2012, Frédéric Danis wrote:
> ---
>  src/event.c |   59 +++++++++++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 47 insertions(+), 12 deletions(-)

After these patches there's the following leak:

==23252== 32,808 bytes in 1 blocks are definitely lost in loss record 210 of 210
==23252==    at 0x4A0883C: malloc (vg_replace_malloc.c:270)
==23252==    by 0x565E25F: __alloc_dir (opendir.c:199)
==23252==    by 0x175382: load_devices (adapter.c:2005)
==23252==    by 0x178E50: adapter_init (adapter.c:2896)
==23252==    by 0x17391E: btd_manager_register_adapter (manager.c:334)
==23252==    by 0x1883E1: mgmt_event.part.36 (mgmt.c:1081)
==23252==    by 0x4C78A74: g_main_context_dispatch (gmain.c:2715)
==23252==    by 0x4C78DA7: g_main_context_iterate.isra.24 (gmain.c:3290)
==23252==    by 0x4C791A1: g_main_loop_run (gmain.c:3484)
==23252==    by 0x121640: main (main.c:544)

That seems to be simple missing closedir() call. Didn't I already tell
you to always use valgrind?

Another problem with these is that I'm getting over a hundred unexpected
device objects which I didn't have before. It seems that this is because
you convert the classes file into the device storage instead of the
cache. It might be due to some other info as well though.

Johan