* [RFC BlueZ 1/3] shared/att.c: Add signing key support
@ 2015-02-20 10:38 Luiz Augusto von Dentz
2015-02-20 10:38 ` [RFC BlueZ 2/3] shared/gatt-client: Add support for signed write Luiz Augusto von Dentz
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Luiz Augusto von Dentz @ 2015-02-20 10:38 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This adds support for setting local and remote signing keys along with
a callback for fetching/updating signing counter.
---
src/shared/att.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
src/shared/att.h | 6 ++++
2 files changed, 104 insertions(+), 4 deletions(-)
diff --git a/src/shared/att.c b/src/shared/att.c
index 152f49c..5363907 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -36,6 +36,7 @@
#include "lib/bluetooth.h"
#include "lib/uuid.h"
#include "src/shared/att.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
@@ -52,6 +53,9 @@
#define BT_ERROR_ALREADY_IN_PROGRESS 0xfe
#define BT_ERROR_OUT_OF_RANGE 0xff
+/* Length of signature in write signed packet */
+#define BT_ATT_SIGNATURE_LEN 12
+
struct att_send_op;
struct bt_att {
@@ -85,6 +89,17 @@ struct bt_att {
bt_att_debug_func_t debug_callback;
bt_att_destroy_func_t debug_destroy;
void *debug_data;
+
+ struct bt_crypto *crypto;
+
+ struct sign_write_info *local_info;
+ struct sign_write_info *remote_info;
+};
+
+struct sign_write_info {
+ uint8_t key[16];
+ bt_att_counter_func_t counter;
+ void *user_data;
};
enum att_op_type {
@@ -184,6 +199,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)
@@ -266,6 +283,12 @@ 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;
+ struct sign_write_info *info;
+ uint32_t sign_cnt;
+
+ if (op->opcode & ATT_OP_SIGNED_MASK)
+ pdu_len += BT_ATT_SIGNATURE_LEN;
if (length && pdu)
pdu_len += length;
@@ -282,17 +305,36 @@ static bool encode_pdu(struct att_send_op *op, const void *pdu,
if (pdu_len > 1)
memcpy(op->pdu + 1, pdu, length);
- return true;
+ if (!(op->opcode & ATT_OP_SIGNED_MASK))
+ return true;
+
+ info = att->local_info;
+ if (!info)
+ goto fail;
+
+ if (!info->counter(&sign_cnt, info->user_data))
+ goto fail;
+
+ if ((bt_crypto_sign_att(att->crypto, info->key, op->pdu, 1 + length,
+ sign_cnt, &((uint8_t *) op->pdu)[1 + length])))
+ return true;
+
+fail:
+ free(op->pdu);
+ return false;
}
-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;
@@ -323,6 +365,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);
@@ -810,6 +853,7 @@ static void bt_att_free(struct bt_att *att)
destroy_att_send_op(att->pending_ind);
io_destroy(att->io);
+ bt_crypto_unref(att->crypto);
queue_destroy(att->req_queue, NULL);
queue_destroy(att->ind_queue, NULL);
@@ -841,6 +885,8 @@ struct bt_att *bt_att_new(int fd)
att->fd = fd;
+ att->local_info = NULL;
+ att->remote_info = NULL;
att->mtu = BT_ATT_DEFAULT_LE_MTU;
att->buf = malloc(att->mtu);
if (!att->buf)
@@ -850,6 +896,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;
@@ -964,6 +1014,16 @@ bool bt_att_set_mtu(struct bt_att *att, uint16_t mtu)
if (!buf)
return false;
+ if (att->local_info) {
+ free(att->local_info);
+ att->local_info = NULL;
+ }
+
+ if (att->remote_info) {
+ free(att->remote_info);
+ att->remote_info = NULL;
+ }
+
free(att->buf);
att->mtu = mtu;
@@ -1047,7 +1107,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;
@@ -1307,3 +1367,37 @@ bool bt_att_set_sec_level(struct bt_att *att, int level)
return true;
}
+
+static bool sign_info_set_key(struct sign_write_info **info, uint8_t key[16],
+ bt_att_counter_func_t func, void *user_data)
+{
+ if (!(*info)) {
+ *info = new0(struct sign_write_info, 1);
+ if (!(*info))
+ return false;
+ }
+
+ (*info)->counter = func;
+ (*info)->user_data = user_data;
+ memcpy((*info)->key, key, 16);
+
+ return true;
+}
+
+bool bt_att_set_local_key(struct bt_att *att, uint8_t sign_key[16],
+ bt_att_counter_func_t func, void *user_data)
+{
+ if (!att)
+ return false;
+
+ return sign_info_set_key(&att->local_info, sign_key, func, user_data);
+}
+
+bool bt_att_set_remote_key(struct bt_att *att, uint8_t sign_key[16],
+ bt_att_counter_func_t func, void *user_data)
+{
+ if (!att)
+ return false;
+
+ return sign_info_set_key(&att->remote_info, sign_key, func, user_data);
+}
diff --git a/src/shared/att.h b/src/shared/att.h
index 5256ff9..a440aaf 100644
--- a/src/shared/att.h
+++ b/src/shared/att.h
@@ -46,6 +46,7 @@ typedef void (*bt_att_debug_func_t)(const char *str, void *user_data);
typedef void (*bt_att_timeout_func_t)(unsigned int id, uint8_t opcode,
void *user_data);
typedef void (*bt_att_disconnect_func_t)(int err, void *user_data);
+typedef bool (*bt_att_counter_func_t)(uint32_t *sign_cnt, void *user_data);
bool bt_att_set_debug(struct bt_att *att, bt_att_debug_func_t callback,
void *user_data, bt_att_destroy_func_t destroy);
@@ -84,3 +85,8 @@ bool bt_att_unregister_all(struct bt_att *att);
int bt_att_get_sec_level(struct bt_att *att);
bool bt_att_set_sec_level(struct bt_att *att, int level);
+
+bool bt_att_set_local_key(struct bt_att *att, uint8_t sign_key[16],
+ bt_att_counter_func_t func, void *user_data);
+bool bt_att_set_remote_key(struct bt_att *att, uint8_t sign_key[16],
+ bt_att_counter_func_t func, void *user_data);
--
2.1.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [RFC BlueZ 2/3] shared/gatt-client: Add support for signed write 2015-02-20 10:38 [RFC BlueZ 1/3] shared/att.c: Add signing key support Luiz Augusto von Dentz @ 2015-02-20 10:38 ` Luiz Augusto von Dentz 2015-02-20 10:38 ` [RFC BlueZ 3/3] tools/btgatt-client: Add signed write support Luiz Augusto von Dentz 2015-02-20 12:04 ` [RFC BlueZ 1/3] shared/att.c: Add signing key support Lukasz Rymanowski 2 siblings, 0 replies; 9+ messages in thread From: Luiz Augusto von Dentz @ 2015-02-20 10:38 UTC (permalink / raw) To: linux-bluetooth From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> --- src/shared/gatt-client.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c index ac2692f..a966dd0 100644 --- a/src/shared/gatt-client.c +++ b/src/shared/gatt-client.c @@ -2129,10 +2129,6 @@ unsigned int bt_gatt_client_write_without_response( if (!client) return 0; - /* TODO: Support this once bt_att_send supports signed writes. */ - if (signed_write) - return 0; - req = request_create(client); if (!req) return 0; @@ -2140,9 +2136,10 @@ unsigned int bt_gatt_client_write_without_response( put_le16(value_handle, pdu); memcpy(pdu + 2, value, length); - req->att_id = bt_att_send(client->att, BT_ATT_OP_WRITE_CMD, - pdu, sizeof(pdu), - NULL, NULL, NULL); + req->att_id = bt_att_send(client->att, + signed_write ? BT_ATT_OP_SIGNED_WRITE_CMD : + BT_ATT_OP_WRITE_CMD, pdu, sizeof(pdu), + NULL, NULL, NULL); if (!req->att_id) { request_unref(req); return 0; -- 2.1.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [RFC BlueZ 3/3] tools/btgatt-client: Add signed write support 2015-02-20 10:38 [RFC BlueZ 1/3] shared/att.c: Add signing key support Luiz Augusto von Dentz 2015-02-20 10:38 ` [RFC BlueZ 2/3] shared/gatt-client: Add support for signed write Luiz Augusto von Dentz @ 2015-02-20 10:38 ` Luiz Augusto von Dentz 2015-02-20 10:55 ` Johan Hedberg 2015-02-20 12:04 ` [RFC BlueZ 1/3] shared/att.c: Add signing key support Lukasz Rymanowski 2 siblings, 1 reply; 9+ messages in thread From: Luiz Augusto von Dentz @ 2015-02-20 10:38 UTC (permalink / raw) To: linux-bluetooth From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> --- tools/btgatt-client.c | 101 ++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 86 insertions(+), 15 deletions(-) diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c index e59d5db..af82991 100644 --- a/tools/btgatt-client.c +++ b/tools/btgatt-client.c @@ -64,6 +64,7 @@ static bool verbose = false; struct client { int fd; + struct bt_att *att; struct gatt_db *db; struct bt_gatt_client *gatt; }; @@ -169,7 +170,6 @@ static void service_removed_cb(struct gatt_db_attribute *attr, void *user_data) static struct client *client_create(int fd, uint16_t mtu) { struct client *cli; - struct bt_att *att; cli = new0(struct client, 1); if (!cli) { @@ -177,24 +177,25 @@ static struct client *client_create(int fd, uint16_t mtu) return NULL; } - att = bt_att_new(fd); - if (!att) { + cli->att = bt_att_new(fd); + if (!cli->att) { fprintf(stderr, "Failed to initialze ATT transport layer\n"); - bt_att_unref(att); + bt_att_unref(cli->att); free(cli); return NULL; } - if (!bt_att_set_close_on_unref(att, true)) { + if (!bt_att_set_close_on_unref(cli->att, true)) { fprintf(stderr, "Failed to set up ATT transport layer\n"); - bt_att_unref(att); + bt_att_unref(cli->att); free(cli); return NULL; } - if (!bt_att_register_disconnect(att, att_disconnect_cb, NULL, NULL)) { + if (!bt_att_register_disconnect(cli->att, att_disconnect_cb, NULL, + NULL)) { fprintf(stderr, "Failed to set ATT disconnect handler\n"); - bt_att_unref(att); + bt_att_unref(cli->att); free(cli); return NULL; } @@ -203,16 +204,16 @@ static struct client *client_create(int fd, uint16_t mtu) cli->db = gatt_db_new(); if (!cli->db) { fprintf(stderr, "Failed to create GATT database\n"); - bt_att_unref(att); + bt_att_unref(cli->att); free(cli); return NULL; } - cli->gatt = bt_gatt_client_new(cli->db, att, mtu); + cli->gatt = bt_gatt_client_new(cli->db, cli->att, mtu); if (!cli->gatt) { fprintf(stderr, "Failed to create GATT client\n"); gatt_db_unref(cli->db); - bt_att_unref(att); + bt_att_unref(cli->att); free(cli); return NULL; } @@ -221,7 +222,7 @@ static struct client *client_create(int fd, uint16_t mtu) NULL, NULL); if (verbose) { - bt_att_set_debug(att, att_debug_cb, "att: ", NULL); + bt_att_set_debug(cli->att, att_debug_cb, "att: ", NULL); bt_gatt_client_set_debug(cli->gatt, gatt_debug_cb, "gatt: ", NULL); } @@ -231,7 +232,6 @@ static struct client *client_create(int fd, uint16_t mtu) NULL); /* bt_gatt_client already holds a reference */ - bt_att_unref(att); gatt_db_unref(cli->db); return cli; @@ -240,6 +240,8 @@ static struct client *client_create(int fd, uint16_t mtu) static void client_destroy(struct client *cli) { bt_gatt_client_unref(cli->gatt); + bt_att_unref(cli->att); + free(cli); } static void print_uuid(const bt_uuid_t *uuid) @@ -625,12 +627,14 @@ 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-s, --signed-write\tsigned write command\n" "e.g.:\n" "\twrite-value 0x0001 00 01 00\n"); } static struct option write_value_options[] = { { "without-response", 0, 0, 'w' }, + { "signed-write", 0, 0, 's' }, { } }; @@ -655,6 +659,7 @@ 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; if (!bt_gatt_client_is_ready(cli->gatt)) { printf("GATT client not initialized\n"); @@ -669,12 +674,15 @@ 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, "+ws", write_value_options, NULL)) != -1) { switch (opt) { case 'w': without_response = true; break; + case 's': + signed_write = true; + break; default: write_value_usage(); return; @@ -728,7 +736,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"); goto done; @@ -1043,6 +1051,68 @@ static void cmd_get_sec_level(struct client *cli, char *cmd_str) printf("Security level: %u\n", level); } +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; + } + + for (i = 0; i < 16; i++) { + strncpy(value, optarg + (i * 2), 2); + csrk[i] = strtol(value, NULL, 16); + } + + return true; +} + +static void set_csrk_usage(void) +{ + printf("Usage: set-csrk [options]\nOptions:\n" + "\t -c, --csrk <csrk>\tCSRK\n" + "e.g.:\n" + "\tset-csrk -c D8515948451FEA320DC05A2E88308188\n"); +} + +static bool local_counter(uint32_t *sign_cnt, void *user_data) +{ + static uint32_t cnt = 0; + + *sign_cnt = cnt++; + + return true; +} + +static void cmd_set_csrk(struct client *cli, char *cmd_str) +{ + char *argv[3]; + int argc = 0; + uint8_t csrk[16]; + + memset(csrk, 0, 16); + + if (!parse_args(cmd_str, 2, argv, &argc)) { + set_csrk_usage(); + return; + } + + if (argc != 2) { + set_csrk_usage(); + return; + } + + if (!strcmp(argv[0], "-c") || !strcmp(argv[0], "--csrk")) { + if (convert_csrk_key(argv[1], csrk)) + bt_att_set_local_key(cli->att, csrk, local_counter, + cli); + + } else + set_csrk_usage(); +} + static void cmd_help(struct client *cli, char *cmd_str); typedef void (*command_func_t)(struct client *cli, char *cmd_str); @@ -1071,6 +1141,7 @@ static struct { "Set security level on le connection"}, { "get-sec-level", cmd_get_sec_level, "Get security level on le connection"}, + { "set-csrk", cmd_set_csrk, "\tSet CSRK for signed write command"}, { } }; -- 2.1.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFC BlueZ 3/3] tools/btgatt-client: Add signed write support 2015-02-20 10:38 ` [RFC BlueZ 3/3] tools/btgatt-client: Add signed write support Luiz Augusto von Dentz @ 2015-02-20 10:55 ` Johan Hedberg 2015-02-20 11:47 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 9+ messages in thread From: Johan Hedberg @ 2015-02-20 10:55 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth Hi Luiz, On Fri, Feb 20, 2015, Luiz Augusto von Dentz wrote: > +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; > + } > + > + for (i = 0; i < 16; i++) { > + strncpy(value, optarg + (i * 2), 2); > + csrk[i] = strtol(value, NULL, 16); It doesn't look like you've got enough space in 'value' for this. You'd need 2 + 1 to have it nul-terminater. However, I think you can completely get away with the need of this temporary variable by using sscanf(), i.e. something like: sscanf(optarg + (i * 2), "%2hhx", &csrk[i]); Johan ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC BlueZ 3/3] tools/btgatt-client: Add signed write support 2015-02-20 10:55 ` Johan Hedberg @ 2015-02-20 11:47 ` Luiz Augusto von Dentz 0 siblings, 0 replies; 9+ messages in thread From: Luiz Augusto von Dentz @ 2015-02-20 11:47 UTC (permalink / raw) To: Luiz Augusto von Dentz, linux-bluetooth@vger.kernel.org Hi Johan, On Fri, Feb 20, 2015 at 12:55 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote: > Hi Luiz, > > On Fri, Feb 20, 2015, Luiz Augusto von Dentz wrote: >> +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; >> + } >> + >> + for (i = 0; i < 16; i++) { >> + strncpy(value, optarg + (i * 2), 2); >> + csrk[i] = strtol(value, NULL, 16); > > It doesn't look like you've got enough space in 'value' for this. You'd > need 2 + 1 to have it nul-terminater. However, I think you can > completely get away with the need of this temporary variable by using > sscanf(), i.e. something like: > > sscanf(optarg + (i * 2), "%2hhx", &csrk[i]); Yep, this sounds much much better. -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC BlueZ 1/3] shared/att.c: Add signing key support 2015-02-20 10:38 [RFC BlueZ 1/3] shared/att.c: Add signing key support Luiz Augusto von Dentz 2015-02-20 10:38 ` [RFC BlueZ 2/3] shared/gatt-client: Add support for signed write Luiz Augusto von Dentz 2015-02-20 10:38 ` [RFC BlueZ 3/3] tools/btgatt-client: Add signed write support Luiz Augusto von Dentz @ 2015-02-20 12:04 ` Lukasz Rymanowski 2015-02-20 13:25 ` Luiz Augusto von Dentz 2 siblings, 1 reply; 9+ messages in thread From: Lukasz Rymanowski @ 2015-02-20 12:04 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org Hi Luiz, On Fri, Feb 20, 2015 at 11:38 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > This adds support for setting local and remote signing keys along with > a callback for fetching/updating signing counter. > --- > src/shared/att.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- > src/shared/att.h | 6 ++++ > 2 files changed, 104 insertions(+), 4 deletions(-) > > diff --git a/src/shared/att.c b/src/shared/att.c > index 152f49c..5363907 100644 > --- a/src/shared/att.c > +++ b/src/shared/att.c > @@ -36,6 +36,7 @@ > #include "lib/bluetooth.h" > #include "lib/uuid.h" > #include "src/shared/att.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 > @@ -52,6 +53,9 @@ > #define BT_ERROR_ALREADY_IN_PROGRESS 0xfe > #define BT_ERROR_OUT_OF_RANGE 0xff > > +/* Length of signature in write signed packet */ > +#define BT_ATT_SIGNATURE_LEN 12 > + > struct att_send_op; > > struct bt_att { > @@ -85,6 +89,17 @@ struct bt_att { > bt_att_debug_func_t debug_callback; > bt_att_destroy_func_t debug_destroy; > void *debug_data; > + > + struct bt_crypto *crypto; > + > + struct sign_write_info *local_info; > + struct sign_write_info *remote_info; > +}; > + > +struct sign_write_info { > + uint8_t key[16]; > + bt_att_counter_func_t counter; > + void *user_data; > }; > > enum att_op_type { > @@ -184,6 +199,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) > @@ -266,6 +283,12 @@ 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; > + struct sign_write_info *info; > + uint32_t sign_cnt; > + > + if (op->opcode & ATT_OP_SIGNED_MASK) > + pdu_len += BT_ATT_SIGNATURE_LEN; > > if (length && pdu) > pdu_len += length; > @@ -282,17 +305,36 @@ static bool encode_pdu(struct att_send_op *op, const void *pdu, > if (pdu_len > 1) > memcpy(op->pdu + 1, pdu, length); > > - return true; > + if (!(op->opcode & ATT_OP_SIGNED_MASK)) > + return true; > + > + info = att->local_info; > + if (!info) > + goto fail; > + > + if (!info->counter(&sign_cnt, info->user_data)) > + goto fail; > + > + if ((bt_crypto_sign_att(att->crypto, info->key, op->pdu, 1 + length, > + sign_cnt, &((uint8_t *) op->pdu)[1 + length]))) > + return true; > + > +fail: > + free(op->pdu); > + return false; > } > > -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; > @@ -323,6 +365,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); > @@ -810,6 +853,7 @@ static void bt_att_free(struct bt_att *att) > destroy_att_send_op(att->pending_ind); > > io_destroy(att->io); > + bt_crypto_unref(att->crypto); > > queue_destroy(att->req_queue, NULL); > queue_destroy(att->ind_queue, NULL); > @@ -841,6 +885,8 @@ struct bt_att *bt_att_new(int fd) > > att->fd = fd; > > + att->local_info = NULL; > + att->remote_info = NULL; > att->mtu = BT_ATT_DEFAULT_LE_MTU; > att->buf = malloc(att->mtu); > if (!att->buf) > @@ -850,6 +896,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; > @@ -964,6 +1014,16 @@ bool bt_att_set_mtu(struct bt_att *att, uint16_t mtu) > if (!buf) > return false; > > + if (att->local_info) { > + free(att->local_info); > + att->local_info = NULL; > + } > + > + if (att->remote_info) { > + free(att->remote_info); > + att->remote_info = NULL; > + } > + Why we want to do it here? I think we should add it bt_att_free() instead. > free(att->buf); > > att->mtu = mtu; > @@ -1047,7 +1107,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); This should be probably separate patch. \Lukasz > if (!op) > return 0; > @@ -1307,3 +1367,37 @@ bool bt_att_set_sec_level(struct bt_att *att, int level) > > return true; > } > + > +static bool sign_info_set_key(struct sign_write_info **info, uint8_t key[16], > + bt_att_counter_func_t func, void *user_data) > +{ > + if (!(*info)) { > + *info = new0(struct sign_write_info, 1); > + if (!(*info)) > + return false; > + } > + > + (*info)->counter = func; > + (*info)->user_data = user_data; > + memcpy((*info)->key, key, 16); > + > + return true; > +} > + > +bool bt_att_set_local_key(struct bt_att *att, uint8_t sign_key[16], > + bt_att_counter_func_t func, void *user_data) > +{ > + if (!att) > + return false; > + > + return sign_info_set_key(&att->local_info, sign_key, func, user_data); > +} > + > +bool bt_att_set_remote_key(struct bt_att *att, uint8_t sign_key[16], > + bt_att_counter_func_t func, void *user_data) > +{ > + if (!att) > + return false; > + > + return sign_info_set_key(&att->remote_info, sign_key, func, user_data); > +} > diff --git a/src/shared/att.h b/src/shared/att.h > index 5256ff9..a440aaf 100644 > --- a/src/shared/att.h > +++ b/src/shared/att.h > @@ -46,6 +46,7 @@ typedef void (*bt_att_debug_func_t)(const char *str, void *user_data); > typedef void (*bt_att_timeout_func_t)(unsigned int id, uint8_t opcode, > void *user_data); > typedef void (*bt_att_disconnect_func_t)(int err, void *user_data); > +typedef bool (*bt_att_counter_func_t)(uint32_t *sign_cnt, void *user_data); > > bool bt_att_set_debug(struct bt_att *att, bt_att_debug_func_t callback, > void *user_data, bt_att_destroy_func_t destroy); > @@ -84,3 +85,8 @@ bool bt_att_unregister_all(struct bt_att *att); > > int bt_att_get_sec_level(struct bt_att *att); > bool bt_att_set_sec_level(struct bt_att *att, int level); > + > +bool bt_att_set_local_key(struct bt_att *att, uint8_t sign_key[16], > + bt_att_counter_func_t func, void *user_data); > +bool bt_att_set_remote_key(struct bt_att *att, uint8_t sign_key[16], > + bt_att_counter_func_t func, void *user_data); > -- > 2.1.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC BlueZ 1/3] shared/att.c: Add signing key support 2015-02-20 12:04 ` [RFC BlueZ 1/3] shared/att.c: Add signing key support Lukasz Rymanowski @ 2015-02-20 13:25 ` Luiz Augusto von Dentz 2015-02-20 13:40 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 9+ messages in thread From: Luiz Augusto von Dentz @ 2015-02-20 13:25 UTC (permalink / raw) To: Lukasz Rymanowski; +Cc: linux-bluetooth@vger.kernel.org Hi Lukasz, On Fri, Feb 20, 2015 at 2:04 PM, Lukasz Rymanowski <lukasz.rymanowski@gmail.com> wrote: > Hi Luiz, > > On Fri, Feb 20, 2015 at 11:38 AM, Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: >> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> >> >> This adds support for setting local and remote signing keys along with >> a callback for fetching/updating signing counter. >> --- >> src/shared/att.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- >> src/shared/att.h | 6 ++++ >> 2 files changed, 104 insertions(+), 4 deletions(-) >> >> diff --git a/src/shared/att.c b/src/shared/att.c >> index 152f49c..5363907 100644 >> --- a/src/shared/att.c >> +++ b/src/shared/att.c >> @@ -36,6 +36,7 @@ >> #include "lib/bluetooth.h" >> #include "lib/uuid.h" >> #include "src/shared/att.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 >> @@ -52,6 +53,9 @@ >> #define BT_ERROR_ALREADY_IN_PROGRESS 0xfe >> #define BT_ERROR_OUT_OF_RANGE 0xff >> >> +/* Length of signature in write signed packet */ >> +#define BT_ATT_SIGNATURE_LEN 12 >> + >> struct att_send_op; >> >> struct bt_att { >> @@ -85,6 +89,17 @@ struct bt_att { >> bt_att_debug_func_t debug_callback; >> bt_att_destroy_func_t debug_destroy; >> void *debug_data; >> + >> + struct bt_crypto *crypto; >> + >> + struct sign_write_info *local_info; >> + struct sign_write_info *remote_info; >> +}; >> + >> +struct sign_write_info { >> + uint8_t key[16]; >> + bt_att_counter_func_t counter; >> + void *user_data; >> }; >> >> enum att_op_type { >> @@ -184,6 +199,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) >> @@ -266,6 +283,12 @@ 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; >> + struct sign_write_info *info; >> + uint32_t sign_cnt; >> + >> + if (op->opcode & ATT_OP_SIGNED_MASK) >> + pdu_len += BT_ATT_SIGNATURE_LEN; >> >> if (length && pdu) >> pdu_len += length; >> @@ -282,17 +305,36 @@ static bool encode_pdu(struct att_send_op *op, const void *pdu, >> if (pdu_len > 1) >> memcpy(op->pdu + 1, pdu, length); >> >> - return true; >> + if (!(op->opcode & ATT_OP_SIGNED_MASK)) >> + return true; >> + >> + info = att->local_info; >> + if (!info) >> + goto fail; >> + >> + if (!info->counter(&sign_cnt, info->user_data)) >> + goto fail; >> + >> + if ((bt_crypto_sign_att(att->crypto, info->key, op->pdu, 1 + length, >> + sign_cnt, &((uint8_t *) op->pdu)[1 + length]))) >> + return true; >> + >> +fail: >> + free(op->pdu); >> + return false; >> } >> >> -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; >> @@ -323,6 +365,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); >> @@ -810,6 +853,7 @@ static void bt_att_free(struct bt_att *att) >> destroy_att_send_op(att->pending_ind); >> >> io_destroy(att->io); >> + bt_crypto_unref(att->crypto); >> >> queue_destroy(att->req_queue, NULL); >> queue_destroy(att->ind_queue, NULL); >> @@ -841,6 +885,8 @@ struct bt_att *bt_att_new(int fd) >> >> att->fd = fd; >> >> + att->local_info = NULL; >> + att->remote_info = NULL; >> att->mtu = BT_ATT_DEFAULT_LE_MTU; >> att->buf = malloc(att->mtu); >> if (!att->buf) >> @@ -850,6 +896,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; >> @@ -964,6 +1014,16 @@ bool bt_att_set_mtu(struct bt_att *att, uint16_t mtu) >> if (!buf) >> return false; >> >> + if (att->local_info) { >> + free(att->local_info); >> + att->local_info = NULL; >> + } >> + >> + if (att->remote_info) { >> + free(att->remote_info); >> + att->remote_info = NULL; >> + } >> + > > Why we want to do it here? > > I think we should add it bt_att_free() instead. Indeed this is misplaced, I will move them to free. >> free(att->buf); >> >> att->mtu = mtu; >> @@ -1047,7 +1107,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); > > This should be probably separate patch. This looks like some artifact of initial patch this was based on, anyway I will fix it. > \Lukasz > >> if (!op) >> return 0; >> @@ -1307,3 +1367,37 @@ bool bt_att_set_sec_level(struct bt_att *att, int level) >> >> return true; >> } >> + >> +static bool sign_info_set_key(struct sign_write_info **info, uint8_t key[16], >> + bt_att_counter_func_t func, void *user_data) >> +{ >> + if (!(*info)) { >> + *info = new0(struct sign_write_info, 1); >> + if (!(*info)) >> + return false; >> + } >> + >> + (*info)->counter = func; >> + (*info)->user_data = user_data; >> + memcpy((*info)->key, key, 16); >> + >> + return true; >> +} >> + >> +bool bt_att_set_local_key(struct bt_att *att, uint8_t sign_key[16], >> + bt_att_counter_func_t func, void *user_data) >> +{ >> + if (!att) >> + return false; >> + >> + return sign_info_set_key(&att->local_info, sign_key, func, user_data); >> +} >> + >> +bool bt_att_set_remote_key(struct bt_att *att, uint8_t sign_key[16], >> + bt_att_counter_func_t func, void *user_data) >> +{ >> + if (!att) >> + return false; >> + >> + return sign_info_set_key(&att->remote_info, sign_key, func, user_data); >> +} >> diff --git a/src/shared/att.h b/src/shared/att.h >> index 5256ff9..a440aaf 100644 >> --- a/src/shared/att.h >> +++ b/src/shared/att.h >> @@ -46,6 +46,7 @@ typedef void (*bt_att_debug_func_t)(const char *str, void *user_data); >> typedef void (*bt_att_timeout_func_t)(unsigned int id, uint8_t opcode, >> void *user_data); >> typedef void (*bt_att_disconnect_func_t)(int err, void *user_data); >> +typedef bool (*bt_att_counter_func_t)(uint32_t *sign_cnt, void *user_data); >> >> bool bt_att_set_debug(struct bt_att *att, bt_att_debug_func_t callback, >> void *user_data, bt_att_destroy_func_t destroy); >> @@ -84,3 +85,8 @@ bool bt_att_unregister_all(struct bt_att *att); >> >> int bt_att_get_sec_level(struct bt_att *att); >> bool bt_att_set_sec_level(struct bt_att *att, int level); >> + >> +bool bt_att_set_local_key(struct bt_att *att, uint8_t sign_key[16], >> + bt_att_counter_func_t func, void *user_data); >> +bool bt_att_set_remote_key(struct bt_att *att, uint8_t sign_key[16], >> + bt_att_counter_func_t func, void *user_data); >> -- >> 2.1.0 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC BlueZ 1/3] shared/att.c: Add signing key support 2015-02-20 13:25 ` Luiz Augusto von Dentz @ 2015-02-20 13:40 ` Luiz Augusto von Dentz 2015-02-20 13:44 ` Lukasz Rymanowski 0 siblings, 1 reply; 9+ messages in thread From: Luiz Augusto von Dentz @ 2015-02-20 13:40 UTC (permalink / raw) To: Lukasz Rymanowski; +Cc: linux-bluetooth@vger.kernel.org Hi Lukasz, On Fri, Feb 20, 2015 at 3:25 PM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > Hi Lukasz, > > On Fri, Feb 20, 2015 at 2:04 PM, Lukasz Rymanowski > <lukasz.rymanowski@gmail.com> wrote: >> Hi Luiz, >> >> On Fri, Feb 20, 2015 at 11:38 AM, Luiz Augusto von Dentz >> <luiz.dentz@gmail.com> wrote: >>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> >>> >>> This adds support for setting local and remote signing keys along with >>> a callback for fetching/updating signing counter. >>> --- >>> src/shared/att.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- >>> src/shared/att.h | 6 ++++ >>> 2 files changed, 104 insertions(+), 4 deletions(-) >>> >>> diff --git a/src/shared/att.c b/src/shared/att.c >>> index 152f49c..5363907 100644 >>> --- a/src/shared/att.c >>> +++ b/src/shared/att.c >>> @@ -36,6 +36,7 @@ >>> #include "lib/bluetooth.h" >>> #include "lib/uuid.h" >>> #include "src/shared/att.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 >>> @@ -52,6 +53,9 @@ >>> #define BT_ERROR_ALREADY_IN_PROGRESS 0xfe >>> #define BT_ERROR_OUT_OF_RANGE 0xff >>> >>> +/* Length of signature in write signed packet */ >>> +#define BT_ATT_SIGNATURE_LEN 12 >>> + >>> struct att_send_op; >>> >>> struct bt_att { >>> @@ -85,6 +89,17 @@ struct bt_att { >>> bt_att_debug_func_t debug_callback; >>> bt_att_destroy_func_t debug_destroy; >>> void *debug_data; >>> + >>> + struct bt_crypto *crypto; >>> + >>> + struct sign_write_info *local_info; >>> + struct sign_write_info *remote_info; >>> +}; >>> + >>> +struct sign_write_info { >>> + uint8_t key[16]; >>> + bt_att_counter_func_t counter; >>> + void *user_data; >>> }; >>> >>> enum att_op_type { >>> @@ -184,6 +199,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) >>> @@ -266,6 +283,12 @@ 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; >>> + struct sign_write_info *info; >>> + uint32_t sign_cnt; >>> + >>> + if (op->opcode & ATT_OP_SIGNED_MASK) >>> + pdu_len += BT_ATT_SIGNATURE_LEN; >>> >>> if (length && pdu) >>> pdu_len += length; >>> @@ -282,17 +305,36 @@ static bool encode_pdu(struct att_send_op *op, const void *pdu, >>> if (pdu_len > 1) >>> memcpy(op->pdu + 1, pdu, length); >>> >>> - return true; >>> + if (!(op->opcode & ATT_OP_SIGNED_MASK)) >>> + return true; >>> + >>> + info = att->local_info; >>> + if (!info) >>> + goto fail; >>> + >>> + if (!info->counter(&sign_cnt, info->user_data)) >>> + goto fail; >>> + >>> + if ((bt_crypto_sign_att(att->crypto, info->key, op->pdu, 1 + length, >>> + sign_cnt, &((uint8_t *) op->pdu)[1 + length]))) >>> + return true; >>> + >>> +fail: >>> + free(op->pdu); >>> + return false; >>> } >>> >>> -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; >>> @@ -323,6 +365,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); >>> @@ -810,6 +853,7 @@ static void bt_att_free(struct bt_att *att) >>> destroy_att_send_op(att->pending_ind); >>> >>> io_destroy(att->io); >>> + bt_crypto_unref(att->crypto); >>> >>> queue_destroy(att->req_queue, NULL); >>> queue_destroy(att->ind_queue, NULL); >>> @@ -841,6 +885,8 @@ struct bt_att *bt_att_new(int fd) >>> >>> att->fd = fd; >>> >>> + att->local_info = NULL; >>> + att->remote_info = NULL; >>> att->mtu = BT_ATT_DEFAULT_LE_MTU; >>> att->buf = malloc(att->mtu); >>> if (!att->buf) >>> @@ -850,6 +896,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; >>> @@ -964,6 +1014,16 @@ bool bt_att_set_mtu(struct bt_att *att, uint16_t mtu) >>> if (!buf) >>> return false; >>> >>> + if (att->local_info) { >>> + free(att->local_info); >>> + att->local_info = NULL; >>> + } >>> + >>> + if (att->remote_info) { >>> + free(att->remote_info); >>> + att->remote_info = NULL; >>> + } >>> + >> >> Why we want to do it here? >> >> I think we should add it bt_att_free() instead. > > Indeed this is misplaced, I will move them to free. > >>> free(att->buf); >>> >>> att->mtu = mtu; >>> @@ -1047,7 +1107,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); >> >> This should be probably separate patch. > > This looks like some artifact of initial patch this was based on, > anyway I will fix it. It actually necessary since encode_pdu deal with signing which requires access to local_info, but I will cleanup this a little bit. >> \Lukasz >> >>> if (!op) >>> return 0; >>> @@ -1307,3 +1367,37 @@ bool bt_att_set_sec_level(struct bt_att *att, int level) >>> >>> return true; >>> } >>> + >>> +static bool sign_info_set_key(struct sign_write_info **info, uint8_t key[16], >>> + bt_att_counter_func_t func, void *user_data) >>> +{ >>> + if (!(*info)) { >>> + *info = new0(struct sign_write_info, 1); >>> + if (!(*info)) >>> + return false; >>> + } >>> + >>> + (*info)->counter = func; >>> + (*info)->user_data = user_data; >>> + memcpy((*info)->key, key, 16); >>> + >>> + return true; >>> +} >>> + >>> +bool bt_att_set_local_key(struct bt_att *att, uint8_t sign_key[16], >>> + bt_att_counter_func_t func, void *user_data) >>> +{ >>> + if (!att) >>> + return false; >>> + >>> + return sign_info_set_key(&att->local_info, sign_key, func, user_data); >>> +} >>> + >>> +bool bt_att_set_remote_key(struct bt_att *att, uint8_t sign_key[16], >>> + bt_att_counter_func_t func, void *user_data) >>> +{ >>> + if (!att) >>> + return false; >>> + >>> + return sign_info_set_key(&att->remote_info, sign_key, func, user_data); >>> +} >>> diff --git a/src/shared/att.h b/src/shared/att.h >>> index 5256ff9..a440aaf 100644 >>> --- a/src/shared/att.h >>> +++ b/src/shared/att.h >>> @@ -46,6 +46,7 @@ typedef void (*bt_att_debug_func_t)(const char *str, void *user_data); >>> typedef void (*bt_att_timeout_func_t)(unsigned int id, uint8_t opcode, >>> void *user_data); >>> typedef void (*bt_att_disconnect_func_t)(int err, void *user_data); >>> +typedef bool (*bt_att_counter_func_t)(uint32_t *sign_cnt, void *user_data); >>> >>> bool bt_att_set_debug(struct bt_att *att, bt_att_debug_func_t callback, >>> void *user_data, bt_att_destroy_func_t destroy); >>> @@ -84,3 +85,8 @@ bool bt_att_unregister_all(struct bt_att *att); >>> >>> int bt_att_get_sec_level(struct bt_att *att); >>> bool bt_att_set_sec_level(struct bt_att *att, int level); >>> + >>> +bool bt_att_set_local_key(struct bt_att *att, uint8_t sign_key[16], >>> + bt_att_counter_func_t func, void *user_data); >>> +bool bt_att_set_remote_key(struct bt_att *att, uint8_t sign_key[16], >>> + bt_att_counter_func_t func, void *user_data); >>> -- >>> 2.1.0 >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > Luiz Augusto von Dentz -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC BlueZ 1/3] shared/att.c: Add signing key support 2015-02-20 13:40 ` Luiz Augusto von Dentz @ 2015-02-20 13:44 ` Lukasz Rymanowski 0 siblings, 0 replies; 9+ messages in thread From: Lukasz Rymanowski @ 2015-02-20 13:44 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org Hi Luiz, On Fri, Feb 20, 2015 at 2:40 PM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > Hi Lukasz, > > On Fri, Feb 20, 2015 at 3:25 PM, Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: >> Hi Lukasz, >> >> On Fri, Feb 20, 2015 at 2:04 PM, Lukasz Rymanowski >> <lukasz.rymanowski@gmail.com> wrote: >>> Hi Luiz, >>> >>> On Fri, Feb 20, 2015 at 11:38 AM, Luiz Augusto von Dentz >>> <luiz.dentz@gmail.com> wrote: >>>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> >>>> >>>> This adds support for setting local and remote signing keys along with >>>> a callback for fetching/updating signing counter. >>>> --- >>>> src/shared/att.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- >>>> src/shared/att.h | 6 ++++ >>>> 2 files changed, 104 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/src/shared/att.c b/src/shared/att.c >>>> index 152f49c..5363907 100644 >>>> --- a/src/shared/att.c >>>> +++ b/src/shared/att.c >>>> @@ -36,6 +36,7 @@ >>>> #include "lib/bluetooth.h" >>>> #include "lib/uuid.h" >>>> #include "src/shared/att.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 >>>> @@ -52,6 +53,9 @@ >>>> #define BT_ERROR_ALREADY_IN_PROGRESS 0xfe >>>> #define BT_ERROR_OUT_OF_RANGE 0xff >>>> >>>> +/* Length of signature in write signed packet */ >>>> +#define BT_ATT_SIGNATURE_LEN 12 >>>> + >>>> struct att_send_op; >>>> >>>> struct bt_att { >>>> @@ -85,6 +89,17 @@ struct bt_att { >>>> bt_att_debug_func_t debug_callback; >>>> bt_att_destroy_func_t debug_destroy; >>>> void *debug_data; >>>> + >>>> + struct bt_crypto *crypto; >>>> + >>>> + struct sign_write_info *local_info; >>>> + struct sign_write_info *remote_info; >>>> +}; >>>> + >>>> +struct sign_write_info { >>>> + uint8_t key[16]; >>>> + bt_att_counter_func_t counter; >>>> + void *user_data; >>>> }; >>>> >>>> enum att_op_type { >>>> @@ -184,6 +199,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) >>>> @@ -266,6 +283,12 @@ 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; >>>> + struct sign_write_info *info; >>>> + uint32_t sign_cnt; >>>> + >>>> + if (op->opcode & ATT_OP_SIGNED_MASK) >>>> + pdu_len += BT_ATT_SIGNATURE_LEN; >>>> >>>> if (length && pdu) >>>> pdu_len += length; >>>> @@ -282,17 +305,36 @@ static bool encode_pdu(struct att_send_op *op, const void *pdu, >>>> if (pdu_len > 1) >>>> memcpy(op->pdu + 1, pdu, length); >>>> >>>> - return true; >>>> + if (!(op->opcode & ATT_OP_SIGNED_MASK)) >>>> + return true; >>>> + >>>> + info = att->local_info; >>>> + if (!info) >>>> + goto fail; >>>> + >>>> + if (!info->counter(&sign_cnt, info->user_data)) >>>> + goto fail; >>>> + >>>> + if ((bt_crypto_sign_att(att->crypto, info->key, op->pdu, 1 + length, >>>> + sign_cnt, &((uint8_t *) op->pdu)[1 + length]))) >>>> + return true; >>>> + >>>> +fail: >>>> + free(op->pdu); >>>> + return false; >>>> } >>>> >>>> -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; >>>> @@ -323,6 +365,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); >>>> @@ -810,6 +853,7 @@ static void bt_att_free(struct bt_att *att) >>>> destroy_att_send_op(att->pending_ind); >>>> >>>> io_destroy(att->io); >>>> + bt_crypto_unref(att->crypto); >>>> >>>> queue_destroy(att->req_queue, NULL); >>>> queue_destroy(att->ind_queue, NULL); >>>> @@ -841,6 +885,8 @@ struct bt_att *bt_att_new(int fd) >>>> >>>> att->fd = fd; >>>> >>>> + att->local_info = NULL; >>>> + att->remote_info = NULL; >>>> att->mtu = BT_ATT_DEFAULT_LE_MTU; >>>> att->buf = malloc(att->mtu); >>>> if (!att->buf) >>>> @@ -850,6 +896,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; >>>> @@ -964,6 +1014,16 @@ bool bt_att_set_mtu(struct bt_att *att, uint16_t mtu) >>>> if (!buf) >>>> return false; >>>> >>>> + if (att->local_info) { >>>> + free(att->local_info); >>>> + att->local_info = NULL; >>>> + } >>>> + >>>> + if (att->remote_info) { >>>> + free(att->remote_info); >>>> + att->remote_info = NULL; >>>> + } >>>> + >>> >>> Why we want to do it here? >>> >>> I think we should add it bt_att_free() instead. >> >> Indeed this is misplaced, I will move them to free. >> >>>> free(att->buf); >>>> >>>> att->mtu = mtu; >>>> @@ -1047,7 +1107,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); >>> >>> This should be probably separate patch. >> >> This looks like some artifact of initial patch this was based on, >> anyway I will fix it. > > It actually necessary since encode_pdu deal with signing which > requires access to local_info, but I will cleanup this a little bit. Yes I know, I just think that there should be separate patch which is changing parameters of create_att_send_op() > >>> \Lukasz >>> >>>> if (!op) >>>> return 0; >>>> @@ -1307,3 +1367,37 @@ bool bt_att_set_sec_level(struct bt_att *att, int level) >>>> >>>> return true; >>>> } >>>> + >>>> +static bool sign_info_set_key(struct sign_write_info **info, uint8_t key[16], >>>> + bt_att_counter_func_t func, void *user_data) >>>> +{ >>>> + if (!(*info)) { >>>> + *info = new0(struct sign_write_info, 1); >>>> + if (!(*info)) >>>> + return false; >>>> + } >>>> + >>>> + (*info)->counter = func; >>>> + (*info)->user_data = user_data; >>>> + memcpy((*info)->key, key, 16); >>>> + >>>> + return true; >>>> +} >>>> + >>>> +bool bt_att_set_local_key(struct bt_att *att, uint8_t sign_key[16], >>>> + bt_att_counter_func_t func, void *user_data) >>>> +{ >>>> + if (!att) >>>> + return false; >>>> + >>>> + return sign_info_set_key(&att->local_info, sign_key, func, user_data); >>>> +} >>>> + >>>> +bool bt_att_set_remote_key(struct bt_att *att, uint8_t sign_key[16], >>>> + bt_att_counter_func_t func, void *user_data) >>>> +{ >>>> + if (!att) >>>> + return false; >>>> + >>>> + return sign_info_set_key(&att->remote_info, sign_key, func, user_data); >>>> +} >>>> diff --git a/src/shared/att.h b/src/shared/att.h >>>> index 5256ff9..a440aaf 100644 >>>> --- a/src/shared/att.h >>>> +++ b/src/shared/att.h >>>> @@ -46,6 +46,7 @@ typedef void (*bt_att_debug_func_t)(const char *str, void *user_data); >>>> typedef void (*bt_att_timeout_func_t)(unsigned int id, uint8_t opcode, >>>> void *user_data); >>>> typedef void (*bt_att_disconnect_func_t)(int err, void *user_data); >>>> +typedef bool (*bt_att_counter_func_t)(uint32_t *sign_cnt, void *user_data); >>>> >>>> bool bt_att_set_debug(struct bt_att *att, bt_att_debug_func_t callback, >>>> void *user_data, bt_att_destroy_func_t destroy); >>>> @@ -84,3 +85,8 @@ bool bt_att_unregister_all(struct bt_att *att); >>>> >>>> int bt_att_get_sec_level(struct bt_att *att); >>>> bool bt_att_set_sec_level(struct bt_att *att, int level); >>>> + >>>> +bool bt_att_set_local_key(struct bt_att *att, uint8_t sign_key[16], >>>> + bt_att_counter_func_t func, void *user_data); >>>> +bool bt_att_set_remote_key(struct bt_att *att, uint8_t sign_key[16], >>>> + bt_att_counter_func_t func, void *user_data); >>>> -- >>>> 2.1.0 >>>> >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in >>>> the body of a message to majordomo@vger.kernel.org >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> >> >> -- >> Luiz Augusto von Dentz > > > > -- > Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-02-20 13:44 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-02-20 10:38 [RFC BlueZ 1/3] shared/att.c: Add signing key support Luiz Augusto von Dentz 2015-02-20 10:38 ` [RFC BlueZ 2/3] shared/gatt-client: Add support for signed write Luiz Augusto von Dentz 2015-02-20 10:38 ` [RFC BlueZ 3/3] tools/btgatt-client: Add signed write support Luiz Augusto von Dentz 2015-02-20 10:55 ` Johan Hedberg 2015-02-20 11:47 ` Luiz Augusto von Dentz 2015-02-20 12:04 ` [RFC BlueZ 1/3] shared/att.c: Add signing key support Lukasz Rymanowski 2015-02-20 13:25 ` Luiz Augusto von Dentz 2015-02-20 13:40 ` Luiz Augusto von Dentz 2015-02-20 13:44 ` Lukasz Rymanowski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.