[PATCH v2 00/12] android/gatt: Add support for write signature command

[PATCH v2 00/12] android/gatt: Add support for write signature command

[Lukasz Rymanowski]

This patch set add support for write signature command for android part.
* handling CSRK and signCounter in android/bluetooth - storage
* exchanging CSRK and signCounter between android/bluetooth and gatt
* handling incoming and outgoing write signature command.

Some common part has been updated (attrib, crypto)

In next patches error handling will be added.

Note: If you run on Nexus devices there is need to update kernel which
needs to support CRYPTO_CSRK, CRYPTO_USER_API, CRYPTO_USER_API_HASH,
CRYPTO_USER_API_SKCIPHER. All the information available on
https://code.google.com/p/aosp-bluez/

v2:
* handled Johan comments

Lukasz Rymanowski (12):
  shared/crypto: Extend bt_crypto_sign_att with sign counter
  attrib: Add helpers to enc and dec signed write command
  attrib/gatt: Add wrapper to send signed write command
  android/bluetooth: Expose API to get CSRK for device
  android/bluetooth: Expose API to update sign counter
  android/bluetooth: Add support to read CSRK from the kernel
  android/bluetooth: Store CSRK
  android/bluetooth: Read CSRK from the storage on startup
  android/bluetooth: Store sign counter needed for aes-cmac sign
  android/gatt: Add crypto needed for sign write
  android/gatt: Add support for signed write command
  android/gatt: Add handling signed write from remote device

 android/Android.mk  |   1 +
 android/Makefile.am |   1 +
 android/bluetooth.c | 185 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 android/bluetooth.h |  10 +++
 android/gatt.c      |  89 ++++++++++++++++++++++++-
 attrib/att.c        |  55 ++++++++++++++++
 attrib/att.h        |  11 ++++
 attrib/gatt.c       |  16 +++++
 attrib/gatt.h       |   5 ++
 src/shared/crypto.c |  24 +++++--
 src/shared/crypto.h |   1 +
 unit/test-crypto.c  |   2 +-
 12 files changed, 394 insertions(+), 6 deletions(-)

-- 
1.8.4

[PATCH v2 01/12] shared/crypto: Extend bt_crypto_sign_att with sign counter

[Lukasz Rymanowski]

Sign counter is use in two places during att signing:
1) Shall be concatenated with siging message before sigining: BT spec
4.1, Vol[3], Part H, chapter 2.4.5

2) Shall be a part of signature send in the att packet: BT spec 4.1
Vol[3], Part C, chapter 10.4.1

With this patch, bt_crypto_sign_att returns correct signature.

Note: For testing purpose it is possible to provide sign counter
less then 0.
---
 src/shared/crypto.c | 24 ++++++++++++++++++++----
 src/shared/crypto.h |  1 +
 unit/test-crypto.c  |  2 +-
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/src/shared/crypto.c b/src/shared/crypto.c
index 0aec373..fed8912 100644
--- a/src/shared/crypto.c
+++ b/src/shared/crypto.c
@@ -258,23 +258,32 @@ static inline void swap128(const uint8_t src[16], uint8_t dst[16])
 
 bool bt_crypto_sign_att(struct bt_crypto *crypto, const uint8_t key[16],
 					const uint8_t *m, uint16_t m_len,
-					uint8_t signature[12])
+					int32_t sign_cnt, uint8_t signature[12])
 {
 	int fd;
 	int len;
 	uint8_t tmp[16], out[16];
+	uint16_t msg_len = m_len + sizeof(uint32_t);
+	uint8_t msg[msg_len];
 
-	if (!crypto)
-		return false;
+	memset(msg, 0, msg_len);
+	memcpy(msg, m, m_len);
+
+	/* Add sign_counter to the message */
+	if (sign_cnt >= 0)
+		put_le32(sign_cnt, msg + msg_len);
+	else
+		msg_len = m_len;
 
 	/* The most significant octet of key corresponds to key[0] */
 	swap128(key, tmp);
 
+	memcpy(signature, tmp + 4, 12);
 	fd = alg_new(crypto->cmac_aes, tmp, 16);
 	if (fd < 0)
 		return false;
 
-	len = send(fd, m, m_len, 0);
+	len = send(fd, msg, msg_len, 0);
 	if (len < 0)
 		return false;
 
@@ -283,6 +292,13 @@ bool bt_crypto_sign_att(struct bt_crypto *crypto, const uint8_t key[16],
 		return false;
 
 	/*
+	 *  If there is sign counter available it should be placed in the
+	 *  signature as specified in BT spec. 4.1 Vol[3], Part C,
+	 *  chapter 10.4.1
+	 */
+	if (sign_cnt >= 0)
+		put_le32(sign_cnt, out + 8);
+	/*
 	 * The most significant octet of hash corresponds to out[0]  - swap it.
 	 * Then truncate in most significant bit first order to a length of
 	 * 12 octets
diff --git a/src/shared/crypto.h b/src/shared/crypto.h
index 64faed2..1985424 100644
--- a/src/shared/crypto.h
+++ b/src/shared/crypto.h
@@ -48,4 +48,5 @@ bool bt_crypto_s1(struct bt_crypto *crypto, const uint8_t k[16],
 			uint8_t res[16]);
 bool bt_crypto_sign_att(struct bt_crypto *crypto, const uint8_t key[16],
 					const uint8_t *m, uint16_t m_len,
+					int32_t sign_cnt,
 					uint8_t signature[12]);
diff --git a/unit/test-crypto.c b/unit/test-crypto.c
index 8b44f4e..34828e0 100644
--- a/unit/test-crypto.c
+++ b/unit/test-crypto.c
@@ -139,7 +139,7 @@ static void test_sign(gconstpointer data)
 	const struct test_data *d = data;
 
 	memset(t, 0, 12);
-	if (!bt_crypto_sign_att(crypto, key, d->msg, d->msg_len, t))
+	if (!bt_crypto_sign_att(crypto, key, d->msg, d->msg_len, -1, t))
 		g_assert(true);
 
 	if (g_test_verbose()) {
-- 
1.8.4

[PATCH v2 02/12] attrib: Add helpers to enc and dec signed write command

[Lukasz Rymanowski]

---
 attrib/att.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 attrib/att.h | 11 +++++++++++
 2 files changed, 66 insertions(+)

diff --git a/attrib/att.c b/attrib/att.c
index 8e9c06d..e7d5682 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -561,6 +561,61 @@ uint16_t dec_write_cmd(const uint8_t *pdu, size_t len, uint16_t *handle,
 	return len;
 }
 
+uint16_t enc_signed_write_cmd(uint16_t handle,
+					const uint8_t *value, size_t vlen,
+					const uint8_t signature[12],
+					uint8_t *pdu, size_t len)
+{
+	const uint16_t hdr_len = sizeof(pdu[0]) + sizeof(handle);
+	const uint16_t min_len =  hdr_len + ATT_SIGNATURE_LEN;
+
+	if (pdu == NULL)
+		return 0;
+
+	if (vlen > len - min_len)
+		vlen = len - min_len;
+
+	pdu[0] = ATT_OP_SIGNED_WRITE_CMD;
+	put_le16(handle, &pdu[1]);
+
+	if (vlen > 0)
+		memcpy(&pdu[hdr_len], value, vlen);
+
+	memcpy(&pdu[hdr_len + vlen], signature, ATT_SIGNATURE_LEN);
+
+	return min_len + vlen;
+}
+
+uint16_t dec_signed_write_cmd(const uint8_t *pdu, size_t len,
+						uint16_t *handle,
+						uint8_t *value, size_t *vlen,
+						uint8_t signature[12])
+{
+	const uint16_t hdr_len = sizeof(pdu[0]) + sizeof(*handle);
+	const uint16_t min_len =  hdr_len + ATT_SIGNATURE_LEN;
+
+
+	if (pdu == NULL)
+		return 0;
+
+	if (value == NULL || vlen == NULL || handle == NULL)
+		return 0;
+
+	if (len < min_len)
+		return 0;
+
+	if (pdu[0] != ATT_OP_SIGNED_WRITE_CMD)
+		return 0;
+
+	*vlen = len - min_len;
+	*handle = get_le16(&pdu[1]);
+	memcpy(value, pdu + hdr_len, *vlen);
+
+	memcpy(signature, pdu + hdr_len + *vlen, ATT_SIGNATURE_LEN);
+
+	return len;
+}
+
 uint16_t enc_write_req(uint16_t handle, const uint8_t *value, size_t vlen,
 						uint8_t *pdu, size_t len)
 {
diff --git a/attrib/att.h b/attrib/att.h
index c612d80..068ddf8 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -22,6 +22,9 @@
  *
  */
 
+/* Len of signature in write signed packet */
+#define ATT_SIGNATURE_LEN		12
+
 /* Attribute Protocol Opcodes */
 #define ATT_OP_ERROR			0x01
 #define ATT_OP_MTU_REQ			0x02
@@ -129,6 +132,14 @@ uint16_t enc_write_cmd(uint16_t handle, const uint8_t *value, size_t vlen,
 						uint8_t *pdu, size_t len);
 uint16_t dec_write_cmd(const uint8_t *pdu, size_t len, uint16_t *handle,
 						uint8_t *value, size_t *vlen);
+uint16_t enc_signed_write_cmd(uint16_t handle,
+					const uint8_t *value, size_t vlen,
+					const uint8_t signature[12],
+					uint8_t *pdu, size_t len);
+uint16_t dec_signed_write_cmd(const uint8_t *pdu, size_t len,
+						uint16_t *handle,
+						uint8_t *value, size_t *vlen,
+						uint8_t signature[12]);
 struct att_data_list *dec_read_by_type_resp(const uint8_t *pdu, size_t len);
 uint16_t enc_write_req(uint16_t handle, const uint8_t *value, size_t vlen,
 						uint8_t *pdu, size_t len);
-- 
1.8.4

[PATCH v2 03/12] attrib/gatt: Add wrapper to send signed write command

[Lukasz Rymanowski]

---
 attrib/gatt.c | 16 ++++++++++++++++
 attrib/gatt.h |  5 +++++
 2 files changed, 21 insertions(+)

diff --git a/attrib/gatt.c b/attrib/gatt.c
index 73eaf7a..ce08003 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -1065,6 +1065,22 @@ guint gatt_write_cmd(GAttrib *attrib, uint16_t handle, const uint8_t *value,
 	return g_attrib_send(attrib, 0, buf, plen, NULL, user_data, notify);
 }
 
+guint gatt_signed_write_cmd(GAttrib *attrib, uint16_t handle,
+						const uint8_t *value, int vlen,
+						const uint8_t signature[12],
+						GDestroyNotify notify,
+						gpointer user_data)
+{
+	uint8_t *buf;
+	size_t buflen;
+	guint16 plen;
+
+	buf = g_attrib_get_buffer(attrib, &buflen);
+	plen = enc_signed_write_cmd(handle, value, vlen, signature, buf,
+									buflen);
+	return g_attrib_send(attrib, 0, buf, plen, NULL, user_data, notify);
+}
+
 static sdp_data_t *proto_seq_find(sdp_list_t *proto_list)
 {
 	sdp_list_t *list;
diff --git a/attrib/gatt.h b/attrib/gatt.h
index 7d055f0..2d869e3 100644
--- a/attrib/gatt.h
+++ b/attrib/gatt.h
@@ -105,6 +105,11 @@ guint gatt_execute_write(GAttrib *attrib, uint8_t flags,
 guint gatt_write_cmd(GAttrib *attrib, uint16_t handle, const uint8_t *value,
 			int vlen, GDestroyNotify notify, gpointer user_data);
 
+guint gatt_signed_write_cmd(GAttrib *attrib, uint16_t handle,
+						const uint8_t *value, int vlen,
+						const uint8_t signature[12],
+						GDestroyNotify notify,
+						gpointer user_data);
 guint gatt_read_char_by_uuid(GAttrib *attrib, uint16_t start, uint16_t end,
 				bt_uuid_t *uuid, GAttribResultFunc func,
 				gpointer user_data);
-- 
1.8.4

[PATCH v2 04/12] android/bluetooth: Expose API to get CSRK for device

[Lukasz Rymanowski]

GATT will take CSRK key and sign counter each time it needs it to sign
att package or to verify att package
---
 android/bluetooth.c | 31 +++++++++++++++++++++++++++++++
 android/bluetooth.h |  8 ++++++++
 2 files changed, 39 insertions(+)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 379d8ea..2ef7846 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -116,6 +116,14 @@ struct device {
 
 	bool found; /* if device is found in current discovery session */
 	unsigned int confirm_id; /* mgtm command id if command pending */
+
+	bool valid_remote_csrk;
+	uint8_t remote_csrk[16];
+	uint32_t remote_sign_cnt;
+
+	bool valid_local_csrk;
+	uint8_t local_csrk[16];
+	uint32_t local_sign_cnt;
 };
 
 struct browse_req {
@@ -3199,6 +3207,29 @@ bool bt_read_device_rssi(const bdaddr_t *addr, bt_read_device_rssi_done cb,
 	return true;
 }
 
+bool bt_get_csrk(const bdaddr_t *addr, enum bt_csrk_type type, uint8_t key[16],
+							uint32_t *sign_cnt)
+{
+	struct device *dev;
+	bool local = (type == LOCAL_CSRK);
+
+	dev = find_device(addr);
+	if (!dev)
+		return false;
+
+	if (local && dev->valid_local_csrk) {
+		memcpy(key, dev->local_csrk, 16);
+		*sign_cnt = dev->local_sign_cnt;
+	} else if (!local && dev->valid_remote_csrk) {
+		memcpy(key, dev->remote_csrk, 16);
+		*sign_cnt = dev->remote_sign_cnt;
+	} else {
+		return false;
+	}
+
+	return true;
+}
+
 static uint8_t set_adapter_scan_mode(const void *buf, uint16_t len)
 {
 	const uint8_t *mode = buf;
diff --git a/android/bluetooth.h b/android/bluetooth.h
index 6a3e766..35a5b52 100644
--- a/android/bluetooth.h
+++ b/android/bluetooth.h
@@ -21,6 +21,11 @@
  *
  */
 
+enum bt_csrk_type {
+	LOCAL_CSRK,
+	REMOTE_CSRK,
+};
+
 typedef void (*bt_bluetooth_ready)(int err, const bdaddr_t *addr);
 bool bt_bluetooth_start(int index, bool mgmt_dbg, bt_bluetooth_ready cb);
 
@@ -55,3 +60,6 @@ typedef void (*bt_read_device_rssi_done)(uint8_t status, const bdaddr_t *addr,
 						int8_t rssi, void *user_data);
 bool bt_read_device_rssi(const bdaddr_t *addr, bt_read_device_rssi_done cb,
 							void *user_data);
+
+bool bt_get_csrk(const bdaddr_t *addr, enum bt_csrk_type type,
+					uint8_t key[16], uint32_t *sign_cnt);
-- 
1.8.4

[PATCH v2 05/12] android/bluetooth: Expose API to update sign counter

[Lukasz Rymanowski]

This is needed so GATT can notify GAP that write sign has been done and
sign counter shall be increased as per spec.
---
 android/bluetooth.c | 14 ++++++++++++++
 android/bluetooth.h |  2 ++
 2 files changed, 16 insertions(+)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 2ef7846..8df4d91 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -3230,6 +3230,20 @@ bool bt_get_csrk(const bdaddr_t *addr, enum bt_csrk_type type, uint8_t key[16],
 	return true;
 }
 
+void bt_update_sign_counter(const bdaddr_t *addr, enum bt_csrk_type type)
+{
+	struct device *dev;
+
+	dev = find_device(addr);
+	if (!dev)
+		return;
+
+	if (type == LOCAL_CSRK)
+		dev->local_sign_cnt++;
+	else
+		dev->remote_sign_cnt++;
+}
+
 static uint8_t set_adapter_scan_mode(const void *buf, uint16_t len)
 {
 	const uint8_t *mode = buf;
diff --git a/android/bluetooth.h b/android/bluetooth.h
index 35a5b52..c4fb375 100644
--- a/android/bluetooth.h
+++ b/android/bluetooth.h
@@ -63,3 +63,5 @@ bool bt_read_device_rssi(const bdaddr_t *addr, bt_read_device_rssi_done cb,
 
 bool bt_get_csrk(const bdaddr_t *addr, enum bt_csrk_type type,
 					uint8_t key[16], uint32_t *sign_cnt);
+
+void bt_update_sign_counter(const bdaddr_t *addr, enum bt_csrk_type type);
-- 
1.8.4

[PATCH v2 06/12] android/bluetooth: Add support to read CSRK from the kernel

[Lukasz Rymanowski]

---
 android/bluetooth.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 8df4d91..c599e4a 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -1804,6 +1804,39 @@ static void new_long_term_key_event(uint16_t index, uint16_t length,
 	/* TODO browse services here? */
 }
 
+static void new_csrk_callback(uint16_t index, uint16_t length,
+					const void *param, void *user_data)
+{
+	const struct mgmt_ev_new_csrk *ev = param;
+	struct device *dev;
+	char dst[18];
+
+	if (length < sizeof(*ev)) {
+		error("Too small csrk event (%u bytes)", length);
+		return;
+	}
+
+	ba2str(&ev->key.addr.bdaddr, dst);
+	dev = find_device(&ev->key.addr.bdaddr);
+	if (!dev)
+		return;
+
+	if (ev->key.master == 0x01) {
+		memcpy(dev->remote_csrk, ev->key.val, 16);
+		dev->remote_sign_cnt = 0;
+		dev->valid_remote_csrk = true;
+	} else if (ev->key.master == 0x00) {
+		memcpy(dev->local_csrk, ev->key.val, 16);
+		dev->local_sign_cnt = 0;
+		dev->valid_local_csrk = true;
+	} else {
+		error("Unknown key type 02%02x", ev->key.master);
+		return;
+	}
+
+	/*TODO: Store it. Remember about Sign Counter*/
+}
+
 static void register_mgmt_handlers(void)
 {
 	mgmt_register(mgmt_if, MGMT_EV_NEW_SETTINGS, adapter.index,
@@ -1853,6 +1886,10 @@ static void register_mgmt_handlers(void)
 
 	mgmt_register(mgmt_if, MGMT_EV_NEW_LONG_TERM_KEY, adapter.index,
 					new_long_term_key_event, NULL, NULL);
+
+	mgmt_register(mgmt_if, MGMT_EV_NEW_CSRK, adapter.index,
+						new_csrk_callback, NULL, NULL);
+
 }
 
 static void load_link_keys_complete(uint8_t status, uint16_t length,
-- 
1.8.4

[PATCH v2 07/12] android/bluetooth: Store CSRK

[Lukasz Rymanowski]

CSRK is generated while paring and should be used for sign att packets
when LE devices uses write signed command.
---
 android/bluetooth.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index c599e4a..fc481f9 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -1804,6 +1804,47 @@ static void new_long_term_key_event(uint16_t index, uint16_t length,
 	/* TODO browse services here? */
 }
 
+static void store_csrk(struct device *dev)
+{
+	GKeyFile *key_file;
+	char key_str[33];
+	char addr[18];
+	int i;
+	gsize length = 0;
+	char *data;
+
+	ba2str(&dev->bdaddr, addr);
+
+	key_file = g_key_file_new();
+	if (!g_key_file_load_from_file(key_file, DEVICES_FILE, 0, NULL)) {
+		g_key_file_free(key_file);
+		return;
+	}
+
+	if (dev->valid_local_csrk) {
+
+		for (i = 0; i < 16; i++)
+			sprintf(key_str + (i * 2), "%2.2X",
+							dev->local_csrk[i]);
+
+		g_key_file_set_string(key_file, addr, "LocalCSRK", key_str);
+	}
+
+	if (dev->valid_remote_csrk) {
+		for (i = 0; i < 16; i++)
+			sprintf(key_str + (i * 2), "%2.2X",
+							dev->remote_csrk[i]);
+
+		g_key_file_set_string(key_file, addr, "RemoteCSRK", key_str);
+	}
+
+	data = g_key_file_to_data(key_file, &length, NULL);
+	g_file_set_contents(DEVICES_FILE, data, length, NULL);
+	g_free(data);
+
+	g_key_file_free(key_file);
+}
+
 static void new_csrk_callback(uint16_t index, uint16_t length,
 					const void *param, void *user_data)
 {
@@ -1834,7 +1875,10 @@ static void new_csrk_callback(uint16_t index, uint16_t length,
 		return;
 	}
 
-	/*TODO: Store it. Remember about Sign Counter*/
+	if (ev->store_hint)
+		store_csrk(dev);
+
+	/*TODO: Store sign counter */
 }
 
 static void register_mgmt_handlers(void)
-- 
1.8.4

[PATCH v2 08/12] android/bluetooth: Read CSRK from the storage on startup

[Lukasz Rymanowski]

---
 android/bluetooth.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index fc481f9..83aae9f 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -2343,6 +2343,28 @@ static struct device *create_device_from_info(GKeyFile *key_file,
 		dev->le_bonded = true;
 	}
 
+	str = g_key_file_get_string(key_file, peer, "LocalCSRK", NULL);
+	if (str) {
+		int i;
+
+		dev->valid_local_csrk = true;
+		for (i = 0; i < 16; i++)
+			sscanf(str + (i * 2), "%02hhX", &dev->local_csrk[i]);
+
+		g_free(str);
+	}
+
+	str = g_key_file_get_string(key_file, peer, "RemoteCSRK", NULL);
+	if (str) {
+		int i;
+
+		dev->valid_remote_csrk = true;
+		for (i = 0; i < 16; i++)
+			sscanf(str + (i * 2), "%02hhX", &dev->remote_csrk[i]);
+
+		g_free(str);
+	}
+
 	str = g_key_file_get_string(key_file, peer, "Name", NULL);
 	if (str) {
 		g_free(dev->name);
-- 
1.8.4

[PATCH v2 09/12] android/bluetooth: Store sign counter needed for aes-cmac sign

[Lukasz Rymanowski]

If CSRK is valid between sessions we should remember sign counter.
Therefor store it.
---
 android/bluetooth.c | 41 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 83aae9f..082441f 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -1877,8 +1877,6 @@ static void new_csrk_callback(uint16_t index, uint16_t length,
 
 	if (ev->store_hint)
 		store_csrk(dev);
-
-	/*TODO: Store sign counter */
 }
 
 static void register_mgmt_handlers(void)
@@ -2352,6 +2350,9 @@ static struct device *create_device_from_info(GKeyFile *key_file,
 			sscanf(str + (i * 2), "%02hhX", &dev->local_csrk[i]);
 
 		g_free(str);
+
+		dev->local_sign_cnt = g_key_file_get_integer(key_file, peer,
+						"LocalCSRKSignCounter", NULL);
 	}
 
 	str = g_key_file_get_string(key_file, peer, "RemoteCSRK", NULL);
@@ -2363,6 +2364,9 @@ static struct device *create_device_from_info(GKeyFile *key_file,
 			sscanf(str + (i * 2), "%02hhX", &dev->remote_csrk[i]);
 
 		g_free(str);
+
+		dev->remote_sign_cnt = g_key_file_get_integer(key_file, peer,
+						"RemoteCSRKSignCounter", NULL);
 	}
 
 	str = g_key_file_get_string(key_file, peer, "Name", NULL);
@@ -3333,6 +3337,37 @@ bool bt_get_csrk(const bdaddr_t *addr, enum bt_csrk_type type, uint8_t key[16],
 	return true;
 }
 
+static void store_sign_counter(struct device *dev, enum bt_csrk_type type)
+{
+	const char *sign_cnt_s;
+	uint32_t sign_cnt;
+	GKeyFile *key_file;
+	bool local = (type == LOCAL_CSRK);
+
+	gsize length = 0;
+	char addr[18];
+	char *data;
+
+	key_file = g_key_file_new();
+	if (!g_key_file_load_from_file(key_file, DEVICES_FILE, 0, NULL)) {
+		g_key_file_free(key_file);
+		return;
+	}
+
+	ba2str(&dev->bdaddr, addr);
+
+	sign_cnt_s = local ? "LocalCSRKSignCounter" : "RemoteCSRKSignCounter";
+	sign_cnt = local ? dev->local_sign_cnt : dev->remote_sign_cnt;
+
+	g_key_file_set_integer(key_file, addr, sign_cnt_s, sign_cnt);
+
+	data = g_key_file_to_data(key_file, &length, NULL);
+	g_file_set_contents(DEVICES_FILE, data, length, NULL);
+	g_free(data);
+
+	g_key_file_free(key_file);
+}
+
 void bt_update_sign_counter(const bdaddr_t *addr, enum bt_csrk_type type)
 {
 	struct device *dev;
@@ -3345,6 +3380,8 @@ void bt_update_sign_counter(const bdaddr_t *addr, enum bt_csrk_type type)
 		dev->local_sign_cnt++;
 	else
 		dev->remote_sign_cnt++;
+
+	store_sign_counter(dev, type);
 }
 
 static uint8_t set_adapter_scan_mode(const void *buf, uint16_t len)
-- 
1.8.4

[PATCH v2 10/12] android/gatt: Add crypto needed for sign write

[Lukasz Rymanowski]

---
 android/Android.mk  |  1 +
 android/Makefile.am |  1 +
 android/gatt.c      | 11 ++++++++++-
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/android/Android.mk b/android/Android.mk
index 4235a7c..4548ed7 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -53,6 +53,7 @@ LOCAL_SRC_FILES := \
 	bluez/src/shared/hfp.c \
 	bluez/src/shared/gatt-db.c \
 	bluez/src/shared/io-glib.c \
+	bluez/src/shared/crypto.c \
 	bluez/src/sdpd-database.c \
 	bluez/src/sdpd-service.c \
 	bluez/src/sdpd-request.c \
diff --git a/android/Makefile.am b/android/Makefile.am
index e663790..1bde83d 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -31,6 +31,7 @@ android_bluetoothd_SOURCES = android/main.c \
 				src/shared/ringbuf.h src/shared/ringbuf.c \
 				src/shared/hfp.h src/shared/hfp.c \
 				src/shared/gatt-db.h src/shared/gatt-db.c \
+				src/shared/crypto.h src/shared/crypto.c \
 				android/bluetooth.h android/bluetooth.c \
 				android/hidhost.h android/hidhost.c \
 				android/ipc-common.h \
diff --git a/android/gatt.c b/android/gatt.c
index 89da60d..90f52f4 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -44,6 +44,7 @@
 #include "src/shared/util.h"
 #include "src/shared/queue.h"
 #include "src/shared/gatt-db.h"
+#include "src/shared/crypto.h"
 #include "attrib/gattrib.h"
 #include "attrib/att.h"
 #include "attrib/gatt.h"
@@ -180,6 +181,8 @@ static struct gatt_db *gatt_db = NULL;
 
 static GIOChannel *listening_io = NULL;
 
+static struct bt_crypto *crypto = NULL;
+
 static void bt_le_discovery_stop_cb(void);
 
 static bool is_bluetooth_uuid(const uint8_t *uuid)
@@ -5144,9 +5147,10 @@ bool bt_gatt_register(struct ipc *ipc, const bdaddr_t *addr)
 	app_connections = queue_new();
 	listen_apps = queue_new();
 	gatt_db = gatt_db_new();
+	crypto = bt_crypto_new();
 
 	if (!gatt_devices || !gatt_apps || !listen_apps ||
-						!app_connections || !gatt_db) {
+				!app_connections || !gatt_db || !crypto) {
 		error("gatt: Failed to allocate memory for queues");
 
 		queue_destroy(gatt_apps, NULL);
@@ -5167,6 +5171,8 @@ bool bt_gatt_register(struct ipc *ipc, const bdaddr_t *addr)
 		g_io_channel_unref(listening_io);
 		listening_io = NULL;
 
+		bt_crypto_unref(crypto);
+
 		return false;
 	}
 
@@ -5208,4 +5214,7 @@ void bt_gatt_unregister(void)
 
 	g_io_channel_unref(listening_io);
 	listening_io = NULL;
+
+	bt_crypto_unref(crypto);
+	crypto = NULL;
 }
-- 
1.8.4

[PATCH v2 11/12] android/gatt: Add support for signed write command

[Lukasz Rymanowski]

---
 android/gatt.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/android/gatt.c b/android/gatt.c
index 90f52f4..e36042a 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -2528,6 +2528,38 @@ static void write_char_cb(guint8 status, const guint8 *pdu, guint16 len,
 	free(data);
 }
 
+static bool signed_write_cmd(struct gatt_device *dev, uint16_t handle,
+					const uint8_t *value, uint16_t vlen)
+{
+	uint8_t s[ATT_SIGNATURE_LEN];
+	uint8_t csrk[16];
+	uint32_t sign_cnt;
+
+	memset(csrk, 0, 16);
+
+	if (!bt_get_csrk(&dev->bdaddr, LOCAL_CSRK, csrk, &sign_cnt)) {
+		error("gatt: Could not get csrk key");
+		return false;
+	}
+
+	memset(s, 0, ATT_SIGNATURE_LEN);
+
+	if (!bt_crypto_sign_att(crypto, csrk, value, vlen, sign_cnt, s)) {
+		error("gatt: Could not sign att data");
+		return false;
+	}
+
+	if (!gatt_signed_write_cmd(dev->attrib, handle, value, vlen, s, NULL,
+									NULL)) {
+		error("gatt: Could write signed cmd");
+		return false;
+	}
+
+	bt_update_sign_counter(&dev->bdaddr, LOCAL_CSRK);
+
+	return true;
+}
+
 static void handle_client_write_characteristic(const void *buf, uint16_t len)
 {
 	const struct hal_cmd_gatt_client_write_characteristic *cmd = buf;
@@ -2592,6 +2624,10 @@ static void handle_client_write_characteristic(const void *buf, uint16_t len)
 							cmd->value, cmd->len,
 							write_char_cb, cb_data);
 		break;
+	case GATT_WRITE_TYPE_SIGNED:
+		res = signed_write_cmd(conn->device, ch->ch.value_handle,
+					cmd->value, cmd->len);
+		break;
 	default:
 		error("gatt: Write type %d unsupported", cmd->write_type);
 		status = HAL_STATUS_UNSUPPORTED;
-- 
1.8.4

[PATCH v2 12/12] android/gatt: Add handling signed write from remote device

[Lukasz Rymanowski]

---
 android/gatt.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/android/gatt.c b/android/gatt.c
index e36042a..2a0e7bc 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4705,6 +4705,44 @@ static void write_cmd_request(const uint8_t *cmd, uint16_t cmd_len,
 	gatt_db_write(gatt_db, handle, 0, value, vlen, cmd[0], &dev->bdaddr);
 }
 
+static void write_signed_cmd_request(const uint8_t *cmd, uint16_t cmd_len,
+						struct gatt_device *dev)
+{
+	uint8_t value[ATT_DEFAULT_LE_MTU];
+	uint8_t s[ATT_SIGNATURE_LEN];
+	uint16_t handle;
+	uint16_t len;
+	size_t vlen;
+	uint8_t csrk[16];
+	uint32_t sign_cnt;
+
+	if (!bt_get_csrk(&dev->bdaddr, REMOTE_CSRK, csrk, &sign_cnt)) {
+		error("gatt: No valid csrk from remote device");
+		return;
+	}
+
+	len = dec_signed_write_cmd(cmd, cmd_len, &handle, value, &vlen, s);
+	if (len) {
+		uint8_t t[ATT_SIGNATURE_LEN];
+
+		/* Generate signature and verify it */
+		if (!bt_crypto_sign_att(crypto, csrk, value, vlen, sign_cnt,
+									t)) {
+			error("gatt: Error when generating att signature");
+			return;
+		}
+
+		if (memcmp(t, s, ATT_SIGNATURE_LEN)) {
+			error("gatt: signature does not match");
+			return;
+		}
+		/* Signature OK, proceed with write */
+		bt_update_sign_counter(&dev->bdaddr, REMOTE_CSRK);
+		gatt_db_write(gatt_db, handle, 0, value, vlen, cmd[0],
+								&dev->bdaddr);
+	}
+}
+
 static uint8_t write_req_request(const uint8_t *cmd, uint16_t cmd_len,
 						struct gatt_device *dev)
 {
@@ -4791,6 +4829,10 @@ static void att_handler(const uint8_t *ipdu, uint16_t len, gpointer user_data)
 		write_cmd_request(ipdu, len, dev);
 		/* No response on write cmd */
 		return;
+	case ATT_OP_SIGNED_WRITE_CMD:
+		write_signed_cmd_request(ipdu, len, dev);
+		/* No response on write signed cmd */
+		return;
 	case ATT_OP_PREP_WRITE_REQ:
 		status = write_prep_request(ipdu, len, dev);
 		if (!status)
-- 
1.8.4

Re: [PATCH v2 01/12] shared/crypto: Extend bt_crypto_sign_att with sign counter

[Johan Hedberg]

Hi Lukasz,

On Thu, May 22, 2014, Lukasz Rymanowski wrote:
> Note: For testing purpose it is possible to provide sign counter
> less then 0.
<snip>
>  bool bt_crypto_sign_att(struct bt_crypto *crypto, const uint8_t key[16],
>  					const uint8_t *m, uint16_t m_len,
> -					uint8_t signature[12])
> +					int32_t sign_cnt, uint8_t signature[12])
<snip>
> +	/* Add sign_counter to the message */
> +	if (sign_cnt >= 0)
> +		put_le32(sign_cnt, msg + msg_len);
> +	else
> +		msg_len = m_len;
<snip>
>  	/*
> +	 *  If there is sign counter available it should be placed in the
> +	 *  signature as specified in BT spec. 4.1 Vol[3], Part C,
> +	 *  chapter 10.4.1
> +	 */
> +	if (sign_cnt >= 0)
> +		put_le32(sign_cnt, out + 8);

Could you elaborate a bit on what exactly this "testing purpose" is and
why it really needs to be part of the API? I don't see anywhere in the
spec where it'd give us a choice of not having a counter available (I
might have missed it though).

Even if this is part of the API you're now restricting the range of
possible counter values to half of what the specification would allow,
i.e. the type should be int64_t with an added check that you don't allow
values beyond UINT32_MAX (however I'm unconvinced that this API should
have a signed parameter to begin with).

Johan

Re: [PATCH v2 01/12] shared/crypto: Extend bt_crypto_sign_att with sign counter

[Lukasz Rymanowski]

Hi Johan,

On 22 May 2014 12:28, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Lukasz,
>
> On Thu, May 22, 2014, Lukasz Rymanowski wrote:
>> Note: For testing purpose it is possible to provide sign counter
>> less then 0.
> <snip>
>>  bool bt_crypto_sign_att(struct bt_crypto *crypto, const uint8_t key[16],
>>                                       const uint8_t *m, uint16_t m_len,
>> -                                     uint8_t signature[12])
>> +                                     int32_t sign_cnt, uint8_t signature[12])
> <snip>
>> +     /* Add sign_counter to the message */
>> +     if (sign_cnt >= 0)
>> +             put_le32(sign_cnt, msg + msg_len);
>> +     else
>> +             msg_len = m_len;
> <snip>
>>       /*
>> +      *  If there is sign counter available it should be placed in the
>> +      *  signature as specified in BT spec. 4.1 Vol[3], Part C,
>> +      *  chapter 10.4.1
>> +      */
>> +     if (sign_cnt >= 0)
>> +             put_le32(sign_cnt, out + 8);
>
> Could you elaborate a bit on what exactly this "testing purpose" is and
> why it really needs to be part of the API? I don't see anywhere in the
> spec where it'd give us a choice of not having a counter available (I
> might have missed it though).
>

I'm using test vectors from the spec in the test-crypto and those does
not use sign counter.
Maybe solution is that I generate own test vectors for those unit tests?

> Even if this is part of the API you're now restricting the range of
> possible counter values to half of what the specification would allow,
> i.e. the type should be int64_t with an added check that you don't allow
> values beyond UINT32_MAX (however I'm unconvinced that this API should
> have a signed parameter to begin with).
>
> Johan

BR
Lukasz