2014-09-25 06:35:27

by Gu, Chao Jie

[permalink] [raw]
Subject: [PATCH 0/4] support signed write commmand

This patch set adds signed write command outgoing.

Gu Chaojie (4):
shared/att.c: Add signed command outgoing part
tools/btgatt-client: Add signed write CSRK option
shared/gatt-client: Add CSRK part to support signed write
shared/att: provide CSRK related function to upperlayer

src/shared/att-types.h | 3 ++
src/shared/att.c | 101 ++++++++++++++++++++++++++++++++++++++++++++--
src/shared/att.h | 13 ++++++
src/shared/gatt-client.c | 28 ++++++++++---
src/shared/gatt-client.h | 5 +++
tools/btgatt-client.c | 35 +++++++++++++++-
6 files changed, 175 insertions(+), 10 deletions(-)

--
1.7.10.4



2014-09-25 08:34:28

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 3/4] shared/gatt-client: Add CSRK part to support signed write

Hi Chaojie,

> This patch Add set CSRK API to btgatt-client tool and add verfiy CSRK
> valid before bt_att_send signed write commmand to remote side.
>
> ---
> src/shared/gatt-client.c | 21 +++++++++++++++++++--
> src/shared/gatt-client.h | 5 +++++
> 2 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> index b1619c6..93ef6d0 100644
> --- a/src/shared/gatt-client.c
> +++ b/src/shared/gatt-client.c
> @@ -983,9 +983,15 @@ bool bt_gatt_client_write_without_response(struct bt_gatt_client *client,
> put_le16(value_handle, pdu);
> memcpy(pdu + 2, value, length);
>
> - if (signed_write)
> - return bt_att_send(client->att, BT_ATT_OP_SIGNED_WRITE_CMD,
> + if (signed_write) {
> + if (bt_att_csrk_is_valid(client->att, LOCAL_CSRK))
> + return bt_att_send(client->att,
> + BT_ATT_OP_SIGNED_WRITE_CMD,
> pdu, sizeof(pdu), NULL, NULL, NULL);
> + else
> + return 0;
> +
> + }
>
> return bt_att_send(client->att, BT_ATT_OP_WRITE_CMD, pdu, sizeof(pdu),
> NULL, NULL, NULL);
> @@ -1357,3 +1363,14 @@ bool bt_gatt_client_write_long_value(struct bt_gatt_client *client,
>
> return true;
> }
> +
> +bool bt_gatt_client_set_csrk(struct bt_gatt_client *client,
> + enum bt_csrk_type type,
> + bool valid_csrk,
> + uint8_t key[16])
> +{
> + if (!client)
> + return false;
> +
> + return bt_att_set_csrk(client->att, type, valid_csrk, key);
> +}
> diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
> index 8b0d334..01b01e9 100644
> --- a/src/shared/gatt-client.h
> +++ b/src/shared/gatt-client.h
> @@ -123,3 +123,8 @@ bool bt_gatt_client_write_long_value(struct bt_gatt_client *client,
> bt_gatt_client_write_long_callback_t callback,
> void *user_data,
> bt_gatt_client_destroy_func_t destroy);
> +
> +bool bt_gatt_client_set_csrk(struct bt_gatt_client *client,
> + enum bt_csrk_type type,
> + bool valid_csrk,
> + uint8_t key[16]);

I prefer if we just have to separate function here:

bt_gatt_client_set_local_csrk()
bt_gatt_client_set_peer_csrk()

Introducing a type enum does not make a call to this API clearer. Naming the function here is a lot more helpful.

Regards

Marcel


2014-09-25 06:35:31

by Gu, Chao Jie

[permalink] [raw]
Subject: [PATCH 4/4] shared/att: provide CSRK related function to upperlayer

This patch add three CSRK related function to upper layer:
Set CSRK into struct bt_att,
Get CSRK from struct bt_att,
Decide CSRK is valid or not.

---
src/shared/att.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
src/shared/att.h | 13 ++++++++++++
2 files changed, 74 insertions(+)

diff --git a/src/shared/att.c b/src/shared/att.c
index f3f531a..b31eb9e 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -84,6 +84,10 @@ struct bt_att {
bool valid_local_csrk;
uint8_t local_csrk[16];
uint32_t local_sign_cnt;
+
+ bool valid_remote_csrk;
+ uint8_t remote_csrk[16];
+ uint32_t remote_sign_cnt;
};

enum att_op_type {
@@ -727,6 +731,8 @@ struct bt_att *bt_att_new(int fd)

att->fd = fd;

+ att->local_sign_cnt = 0;
+ att->remote_sign_cnt = 0;
att->mtu = BT_ATT_DEFAULT_LE_MTU;
att->buf = malloc(att->mtu);
if (!att->buf)
@@ -1150,3 +1156,58 @@ bool bt_att_unregister_all(struct bt_att *att)

return true;
}
+
+bool bt_att_set_csrk(struct bt_att *att, enum bt_csrk_type type,
+ bool valid_csrk, uint8_t key[16])
+{
+
+ bool local = (type == LOCAL_CSRK);
+
+ if (!att)
+ return false;
+
+ if (local) {
+ att->valid_local_csrk = valid_csrk;
+ memcpy(att->local_csrk, key, 16);
+ } else {
+ att->valid_remote_csrk = valid_csrk;
+ memcpy(att->remote_csrk, key, 16);
+ }
+
+ return true;
+}
+
+bool bt_att_get_csrk(struct bt_att *att, enum bt_csrk_type type,
+ uint8_t key[16], uint32_t *sign_cnt)
+{
+ bool local = (type == LOCAL_CSRK);
+
+ if (!att)
+ return false;
+
+ if (local && att->valid_local_csrk) {
+ memcpy(key, att->local_csrk, 16);
+ *sign_cnt = att->local_sign_cnt;
+ } else if (!local && att->valid_remote_csrk) {
+ memcpy(key, att->remote_csrk, 16);
+ *sign_cnt = att->remote_sign_cnt;
+ } else {
+ return false;
+ }
+
+ return true;
+}
+
+bool bt_att_csrk_is_valid(struct bt_att *att, enum bt_csrk_type type)
+{
+ bool local = (type == LOCAL_CSRK);
+
+ if (!att)
+ return false;
+
+ if (local)
+ return att->valid_local_csrk;
+ else
+ return att->valid_remote_csrk;
+
+}
diff --git a/src/shared/att.h b/src/shared/att.h
index 1063021..9a54f64 100644
--- a/src/shared/att.h
+++ b/src/shared/att.h
@@ -26,6 +26,11 @@

#include "src/shared/att-types.h"

+enum bt_csrk_type {
+ LOCAL_CSRK,
+ REMOTE_CSRK,
+};
+
struct bt_att;

struct bt_att *bt_att_new(int fd);
@@ -76,3 +81,11 @@ unsigned int bt_att_register_disconnect(struct bt_att *att,
bool bt_att_unregister_disconnect(struct bt_att *att, unsigned int id);

bool bt_att_unregister_all(struct bt_att *att);
+
+bool bt_att_set_csrk(struct bt_att *att, enum bt_csrk_type type,
+ bool valid_csrk, uint8_t key[16]);
+
+bool bt_att_get_csrk(struct bt_att *att, enum bt_csrk_type type,
+ uint8_t key[16], uint32_t *sign_cnt);
+
+bool bt_att_csrk_is_valid(struct bt_att *att, enum bt_csrk_type type);
--
1.7.10.4


2014-09-25 06:35:28

by Gu, Chao Jie

[permalink] [raw]
Subject: [PATCH 1/4] shared/att.c: Add signed command outgoing part

This patch focus on handling the singed write condition in ATT stack.
extended bt_att struct to store CSRK related information and struct
att_send_op to replace elment mtu by struct bt_att for signature PDU
need.

---
src/shared/att-types.h | 3 +++
src/shared/att.c | 40 +++++++++++++++++++++++++++++++++++++---
src/shared/gatt-client.c | 11 ++++++-----
3 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/src/shared/att-types.h b/src/shared/att-types.h
index b85c969..da8b6ae 100644
--- a/src/shared/att-types.h
+++ b/src/shared/att-types.h
@@ -25,6 +25,9 @@

#define BT_ATT_DEFAULT_LE_MTU 23

+/* Length of signature in write signed packet */
+#define BT_ATT_SIGNATURE_LEN 12
+
/* ATT protocol opcodes */
#define BT_ATT_OP_ERROR_RSP 0x01
#define BT_ATT_OP_MTU_REQ 0x02
diff --git a/src/shared/att.c b/src/shared/att.c
index a5cab45..f3f531a 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -36,6 +36,7 @@
#include "lib/uuid.h"
#include "src/shared/att.h"
#include "src/shared/att-types.h"
+#include "src/shared/crypto.h"

#define ATT_MIN_PDU_LEN 1 /* At least 1 byte for the opcode. */
#define ATT_OP_CMD_MASK 0x40
@@ -77,6 +78,12 @@ struct bt_att {
bt_att_debug_func_t debug_callback;
bt_att_destroy_func_t debug_destroy;
void *debug_data;
+
+ struct bt_crypto *crypto;
+
+ bool valid_local_csrk;
+ uint8_t local_csrk[16];
+ uint32_t local_sign_cnt;
};

enum att_op_type {
@@ -176,6 +183,8 @@ struct att_send_op {
bt_att_response_func_t callback;
bt_att_destroy_func_t destroy;
void *user_data;
+
+ struct bt_att *att;
};

static void destroy_att_send_op(void *data)
@@ -277,6 +286,10 @@ static bool encode_pdu(struct att_send_op *op, const void *pdu,
uint16_t length, uint16_t mtu)
{
uint16_t pdu_len = 1;
+ struct bt_att *att = op->att;
+
+ if (op->opcode & ATT_OP_SIGNED_MASK)
+ pdu_len += BT_ATT_SIGNATURE_LEN;

if (length && pdu)
pdu_len += length;
@@ -293,17 +306,32 @@ static bool encode_pdu(struct att_send_op *op, const void *pdu,
if (pdu_len > 1)
memcpy(op->pdu + 1, pdu, length);

+ if (op->opcode & ATT_OP_SIGNED_MASK) {
+ if (bt_crypto_sign_att(att->crypto,
+ att->local_csrk,
+ op->pdu,
+ 1 + length,
+ att->local_sign_cnt,
+ &((uint8_t *) op->pdu)[1 + length]))
+ att->local_sign_cnt++;
+ else
+ return false;
+ }
+
return true;
}

-static struct att_send_op *create_att_send_op(uint8_t opcode, const void *pdu,
- uint16_t length, uint16_t mtu,
+static struct att_send_op *create_att_send_op(uint8_t opcode,
+ const void *pdu,
+ uint16_t length,
+ struct bt_att *att,
bt_att_response_func_t callback,
void *user_data,
bt_att_destroy_func_t destroy)
{
struct att_send_op *op;
enum att_op_type op_type;
+ uint16_t mtu = att->mtu;

if (length && !pdu)
return NULL;
@@ -334,6 +362,7 @@ static struct att_send_op *create_att_send_op(uint8_t opcode, const void *pdu,
op->callback = callback;
op->destroy = destroy;
op->user_data = user_data;
+ op->att = att;

if (!encode_pdu(op, pdu, length, mtu)) {
free(op);
@@ -707,6 +736,10 @@ struct bt_att *bt_att_new(int fd)
if (!att->io)
goto fail;

+ att->crypto = bt_crypto_new();
+ if (!att->crypto)
+ goto fail;
+
att->req_queue = queue_new();
if (!att->req_queue)
goto fail;
@@ -741,6 +774,7 @@ fail:
queue_destroy(att->write_queue, NULL);
queue_destroy(att->notify_list, NULL);
queue_destroy(att->disconn_list, NULL);
+ bt_crypto_unref(att->crypto);
io_destroy(att->io);
free(att->buf);
free(att);
@@ -931,7 +965,7 @@ unsigned int bt_att_send(struct bt_att *att, uint8_t opcode,
if (!att || !att->io)
return 0;

- op = create_att_send_op(opcode, pdu, length, att->mtu, callback,
+ op = create_att_send_op(opcode, pdu, length, att, callback,
user_data, destroy);
if (!op)
return 0;
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 1a157ec..b1619c6 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -973,19 +973,20 @@ bool bt_gatt_client_read_long_value(struct bt_gatt_client *client,
bool bt_gatt_client_write_without_response(struct bt_gatt_client *client,
uint16_t value_handle,
bool signed_write,
- uint8_t *value, uint16_t length) {
+ uint8_t *value, uint16_t length)
+{
uint8_t pdu[2 + length];

if (!client)
return 0;

- /* TODO: Support this once bt_att_send supports signed writes. */
- if (signed_write)
- return 0;
-
put_le16(value_handle, pdu);
memcpy(pdu + 2, value, length);

+ if (signed_write)
+ return bt_att_send(client->att, BT_ATT_OP_SIGNED_WRITE_CMD,
+ pdu, sizeof(pdu), NULL, NULL, NULL);
+
return bt_att_send(client->att, BT_ATT_OP_WRITE_CMD, pdu, sizeof(pdu),
NULL, NULL, NULL);
}
--
1.7.10.4


2014-09-25 06:35:29

by Gu, Chao Jie

[permalink] [raw]
Subject: [PATCH 2/4] tools/btgatt-client: Add signed write CSRK option

This patch Add CSRK option to btgatt-client tool, use -c <key> to set
CSRK for signed write command need. This key from pairing to store 32
byte key string under group "LocalSignatureKey" in the info file.

---
tools/btgatt-client.c | 35 +++++++++++++++++++++++++++++++++--
1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index d1395b2..dfc15a3 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -447,11 +447,13 @@ static void write_value_usage(void)
printf("Usage: write-value [options] <value_handle> <value>\n"
"Options:\n"
"\t-w, --without-response\tWrite without response\n"
+ "\t-c, --csrk <key>\tCSRK for signed write command\n"
"e.g.:\n"
"\twrite-value 0x0001 00 01 00\n");
}

static struct option write_value_options[] = {
+ { "csrk", 1, 0, 'c' },
{ "without-response", 0, 0, 'w' },
{ }
};
@@ -465,6 +467,24 @@ static void write_cb(bool success, uint8_t att_ecode, void *user_data)
}
}

+static bool convert_csrk_key(char *optarg, uint8_t csrk[16])
+{
+ int i;
+ char value[2];
+
+ if (strlen(optarg) != 32) {
+ printf("csrk length is invalid\n");
+ return false;
+ } else {
+ for (i = 0; i < 16; i++) {
+ strncpy(value, optarg + (i * 2), 2);
+ csrk[i] = strtol(value, NULL, 16);
+ }
+ }
+
+ return true;
+}
+
static void cmd_write_value(struct client *cli, char *cmd_str)
{
int opt, i;
@@ -476,6 +496,11 @@ static void cmd_write_value(struct client *cli, char *cmd_str)
int length;
uint8_t *value = NULL;
bool without_response = false;
+ bool signed_write = false;
+ bool valid_csrk = false;
+ uint8_t csrk[16];
+
+ memset(csrk, 0, 16);

if (!bt_gatt_client_is_ready(cli->gatt)) {
printf("GATT client not initialized\n");
@@ -490,12 +515,18 @@ static void cmd_write_value(struct client *cli, char *cmd_str)

optind = 0;
argv[0] = "write-value";
- while ((opt = getopt_long(argc, argv, "+w", write_value_options,
+ while ((opt = getopt_long(argc, argv, "+wc:", write_value_options,
NULL)) != -1) {
switch (opt) {
case 'w':
without_response = true;
break;
+ case 'c':
+ signed_write = true;
+ valid_csrk = convert_csrk_key(optarg, csrk);
+ bt_gatt_client_set_csrk(cli->gatt, LOCAL_CSRK,
+ valid_csrk, csrk);
+ break;
default:
write_value_usage();
return;
@@ -551,7 +582,7 @@ static void cmd_write_value(struct client *cli, char *cmd_str)

if (without_response) {
if (!bt_gatt_client_write_without_response(cli->gatt, handle,
- false, value, length)) {
+ signed_write, value, length)) {
printf("Failed to initiate write without response "
"procedure\n");
return;
--
1.7.10.4


2014-09-25 06:35:30

by Gu, Chao Jie

[permalink] [raw]
Subject: [PATCH 3/4] shared/gatt-client: Add CSRK part to support signed write

This patch Add set CSRK API to btgatt-client tool and add verfiy CSRK
valid before bt_att_send signed write commmand to remote side.

---
src/shared/gatt-client.c | 21 +++++++++++++++++++--
src/shared/gatt-client.h | 5 +++++
2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index b1619c6..93ef6d0 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -983,9 +983,15 @@ bool bt_gatt_client_write_without_response(struct bt_gatt_client *client,
put_le16(value_handle, pdu);
memcpy(pdu + 2, value, length);

- if (signed_write)
- return bt_att_send(client->att, BT_ATT_OP_SIGNED_WRITE_CMD,
+ if (signed_write) {
+ if (bt_att_csrk_is_valid(client->att, LOCAL_CSRK))
+ return bt_att_send(client->att,
+ BT_ATT_OP_SIGNED_WRITE_CMD,
pdu, sizeof(pdu), NULL, NULL, NULL);
+ else
+ return 0;
+
+ }

return bt_att_send(client->att, BT_ATT_OP_WRITE_CMD, pdu, sizeof(pdu),
NULL, NULL, NULL);
@@ -1357,3 +1363,14 @@ bool bt_gatt_client_write_long_value(struct bt_gatt_client *client,

return true;
}
+
+bool bt_gatt_client_set_csrk(struct bt_gatt_client *client,
+ enum bt_csrk_type type,
+ bool valid_csrk,
+ uint8_t key[16])
+{
+ if (!client)
+ return false;
+
+ return bt_att_set_csrk(client->att, type, valid_csrk, key);
+}
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index 8b0d334..01b01e9 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -123,3 +123,8 @@ bool bt_gatt_client_write_long_value(struct bt_gatt_client *client,
bt_gatt_client_write_long_callback_t callback,
void *user_data,
bt_gatt_client_destroy_func_t destroy);
+
+bool bt_gatt_client_set_csrk(struct bt_gatt_client *client,
+ enum bt_csrk_type type,
+ bool valid_csrk,
+ uint8_t key[16]);
--
1.7.10.4