* [PATCH v3 0/4] support signed write command
@ 2014-09-26 7:48 Gu Chaojie
2014-09-26 7:48 ` [PATCH v3 1/4] shared/att.c: Add signed command outgoing and CSRK function Gu Chaojie
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Gu Chaojie @ 2014-09-26 7:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Gu Chaojie
This patch set submit Makefile.tools change to fix patch set v2 link issue.
And remove type enum in the CSRK related funtion make a call to these API
more clear.
Gu Chaojie (4):
shared/att.c: Add signed command outgoing and CSRK function
shared/gatt-client: Add CSRK part to support signed write
tools/btgatt-client: Add signed write CSRK option
Modify Makefile.tool to link
Makefile.tools | 3 +-
src/shared/att-types.h | 3 ++
src/shared/att.c | 120 ++++++++++++++++++++++++++++++++++++++++++++--
src/shared/att.h | 16 +++++++
src/shared/gatt-client.c | 27 +++++++++--
src/shared/gatt-client.h | 4 ++
tools/btgatt-client.c | 35 +++++++++++++-
7 files changed, 197 insertions(+), 11 deletions(-)
--
1.7.10.4
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v3 1/4] shared/att.c: Add signed command outgoing and CSRK function 2014-09-26 7:48 [PATCH v3 0/4] support signed write command Gu Chaojie @ 2014-09-26 7:48 ` Gu Chaojie 2014-09-26 22:30 ` Lukasz Rymanowski 2014-09-26 7:48 ` [PATCH v3 2/4] shared/gatt-client: Add CSRK part to support signed write Gu Chaojie ` (2 subsequent siblings) 3 siblings, 1 reply; 16+ messages in thread From: Gu Chaojie @ 2014-09-26 7:48 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gu Chaojie Compared with patch v2, remove type enum in all CSRK related funciton to make these API more clear. --- src/shared/att-types.h | 3 ++ src/shared/att.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++-- src/shared/att.h | 16 +++++++ 3 files changed, 136 insertions(+), 3 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..4562a8c 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,16 @@ 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; + + bool valid_remote_csrk; + uint8_t remote_csrk[16]; + uint32_t remote_sign_cnt; }; enum att_op_type { @@ -176,6 +187,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 +290,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 +310,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 +366,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); @@ -698,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) @@ -707,6 +742,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 +780,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 +971,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; @@ -1116,3 +1156,77 @@ bool bt_att_unregister_all(struct bt_att *att) return true; } + +bool bt_att_set_local_csrk(struct bt_att *att, bool valid_local_csrk, + uint8_t key[16]) +{ + if (!att) + return false; + + att->valid_local_csrk = valid_local_csrk; + memcpy(att->local_csrk, key, 16); + + return true; +} + +bool bt_att_set_remote_csrk(struct bt_att *att, bool valid_remote_csrk, + uint8_t key[16]) +{ + if (!att) + return false; + + att->valid_remote_csrk = valid_remote_csrk; + memcpy(att->remote_csrk, key, 16); + + return true; + +} + +bool bt_att_get_local_csrk(struct bt_att *att, uint8_t key[16], + uint32_t *local_sign_cnt) +{ + if (!att) + return false; + + if (att->valid_local_csrk) { + memcpy(key, att->local_csrk, 16); + *local_sign_cnt = att->local_sign_cnt; + } else { + return false; + } + + return true; +} + +bool bt_att_get_remote_csrk(struct bt_att *att, uint8_t key[16], + uint32_t *remote_sign_cnt) +{ + if (!att) + return false; + + if (att->valid_remote_csrk) { + memcpy(key, att->remote_csrk, 16); + *remote_sign_cnt = att->remote_sign_cnt; + } else { + return false; + } + + return true; +} + +bool bt_att_local_csrk_is_valid(struct bt_att *att) +{ + if (!att) + return false; + + return att->valid_local_csrk; +} + +bool bt_att_remote_csrk_is_valid(struct bt_att *att) +{ + if (!att) + return false; + + return att->valid_remote_csrk; + +} diff --git a/src/shared/att.h b/src/shared/att.h index 1063021..306d44c 100644 --- a/src/shared/att.h +++ b/src/shared/att.h @@ -76,3 +76,19 @@ 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_local_csrk(struct bt_att *att, bool valid_local_csrk, + uint8_t key[16]); + +bool bt_att_set_remote_csrk(struct bt_att *att, bool valid_remote_csrk, + uint8_t key[16]); + +bool bt_att_get_local_csrk(struct bt_att *att, uint8_t key[16], + uint32_t *local_sign_cnt); + +bool bt_att_get_remote_csrk(struct bt_att *att, uint8_t key[16], + uint32_t *remote_sign_cnt); + +bool bt_att_local_csrk_is_valid(struct bt_att *att); + +bool bt_att_remote_csrk_is_valid(struct bt_att *att); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/4] shared/att.c: Add signed command outgoing and CSRK function 2014-09-26 7:48 ` [PATCH v3 1/4] shared/att.c: Add signed command outgoing and CSRK function Gu Chaojie @ 2014-09-26 22:30 ` Lukasz Rymanowski 2014-09-27 9:26 ` Gu, Chao Jie 0 siblings, 1 reply; 16+ messages in thread From: Lukasz Rymanowski @ 2014-09-26 22:30 UTC (permalink / raw) To: Gu Chaojie; +Cc: linux-bluetooth Hi Chaojie, On Fri, Sep 26, 2014 at 9:48 AM, Gu Chaojie <chao.jie.gu@intel.com> wrote: > > Compared with patch v2, remove type enum in all CSRK related funciton to > make these API more clear. > > --- > src/shared/att-types.h | 3 ++ > src/shared/att.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++-- > src/shared/att.h | 16 +++++++ > 3 files changed, 136 insertions(+), 3 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..4562a8c 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,16 @@ 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; > + > + bool valid_remote_csrk; > + uint8_t remote_csrk[16]; > + uint32_t remote_sign_cnt; > }; > > enum att_op_type { > @@ -176,6 +187,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 +290,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 +310,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 +366,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); > @@ -698,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) > @@ -707,6 +742,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 +780,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 +971,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; > @@ -1116,3 +1156,77 @@ bool bt_att_unregister_all(struct bt_att *att) > > return true; > } > + > +bool bt_att_set_local_csrk(struct bt_att *att, bool valid_local_csrk, > + uint8_t key[16]) > +{ > + if (!att) > + return false; > + > + att->valid_local_csrk = valid_local_csrk; > + memcpy(att->local_csrk, key, 16); > + > + return true; > +} > + > +bool bt_att_set_remote_csrk(struct bt_att *att, bool valid_remote_csrk, > + uint8_t key[16]) > +{ > + if (!att) > + return false; > + > + att->valid_remote_csrk = valid_remote_csrk; > + memcpy(att->remote_csrk, key, 16); > + > + return true; > + > +} > + > +bool bt_att_get_local_csrk(struct bt_att *att, uint8_t key[16], > + uint32_t *local_sign_cnt) Why we need that get function ? If we assume that signing is done in bt_att then there is need only to feed bt_att with the CSRK. BTW. There should be a way to set sign_cnt as well as this shall be in same storage as CSRK I can guess that idea behind this bt_att_get_ is that client of bt_att should have possibility to update its sign_cnt after each transaction. It is in order to update storage. If so I would propose to have some callback registered in bt_att for this purpose. BR Lukasz > > +{ > + if (!att) > + return false; > + > + if (att->valid_local_csrk) { > + memcpy(key, att->local_csrk, 16); > + *local_sign_cnt = att->local_sign_cnt; > + } else { > + return false; > + } > + > + return true; > +} > + > +bool bt_att_get_remote_csrk(struct bt_att *att, uint8_t key[16], > + uint32_t *remote_sign_cnt) > +{ > + if (!att) > + return false; > + > + if (att->valid_remote_csrk) { > + memcpy(key, att->remote_csrk, 16); > + *remote_sign_cnt = att->remote_sign_cnt; > + } else { > + return false; > + } > + > + return true; > +} > + > +bool bt_att_local_csrk_is_valid(struct bt_att *att) > +{ > + if (!att) > + return false; > + > + return att->valid_local_csrk; > +} > + > +bool bt_att_remote_csrk_is_valid(struct bt_att *att) > +{ > + if (!att) > + return false; > + > + return att->valid_remote_csrk; > + > +} > diff --git a/src/shared/att.h b/src/shared/att.h > index 1063021..306d44c 100644 > --- a/src/shared/att.h > +++ b/src/shared/att.h > @@ -76,3 +76,19 @@ 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_local_csrk(struct bt_att *att, bool valid_local_csrk, > + uint8_t key[16]); > + > +bool bt_att_set_remote_csrk(struct bt_att *att, bool valid_remote_csrk, > + uint8_t key[16]); > + > +bool bt_att_get_local_csrk(struct bt_att *att, uint8_t key[16], > + uint32_t *local_sign_cnt); > + > +bool bt_att_get_remote_csrk(struct bt_att *att, uint8_t key[16], > + uint32_t *remote_sign_cnt); > + > +bool bt_att_local_csrk_is_valid(struct bt_att *att); > + > +bool bt_att_remote_csrk_is_valid(struct bt_att *att); > -- > 1.7.10.4 > > -- > 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] 16+ messages in thread
* RE: [PATCH v3 1/4] shared/att.c: Add signed command outgoing and CSRK function 2014-09-26 22:30 ` Lukasz Rymanowski @ 2014-09-27 9:26 ` Gu, Chao Jie 2014-09-29 8:08 ` Lukasz Rymanowski 0 siblings, 1 reply; 16+ messages in thread From: Gu, Chao Jie @ 2014-09-27 9:26 UTC (permalink / raw) To: Lukasz Rymanowski; +Cc: linux-bluetooth SGkgTHVrYXN6LA0KDQo+ID4gKw0KPiA+ICtib29sIGJ0X2F0dF9nZXRfbG9jYWxfY3NyayhzdHJ1 Y3QgYnRfYXR0ICphdHQsIHVpbnQ4X3Qga2V5WzE2XSwNCj4gPiArICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgdWludDMyX3QgKmxvY2FsX3NpZ25fY250KQ0KPiANCj4gDQo+ IFdoeSB3ZSBuZWVkIHRoYXQgZ2V0IGZ1bmN0aW9uID8gSWYgd2UgYXNzdW1lIHRoYXQgc2lnbmlu ZyBpcyBkb25lIGluIGJ0X2F0dCB0aGVuDQo+IHRoZXJlIGlzIG5lZWQgb25seSB0byBmZWVkIGJ0 X2F0dCB3aXRoIHRoZSBDU1JLLg0KPiAgQlRXLiBUaGVyZSBzaG91bGQgYmUgYSB3YXkgdG8gc2V0 IHNpZ25fY250IGFzIHdlbGwgYXMgdGhpcyBzaGFsbCBiZSBpbiBzYW1lDQo+IHN0b3JhZ2UgYXMg Q1NSSw0KPiANCj4gSSBjYW4gZ3Vlc3MgdGhhdCBpZGVhIGJlaGluZCB0aGlzIGJ0X2F0dF9nZXRf IGlzIHRoYXQgY2xpZW50IG9mIGJ0X2F0dCBzaG91bGQgaGF2ZQ0KPiBwb3NzaWJpbGl0eSB0byB1 cGRhdGUgaXRzIHNpZ25fY250IGFmdGVyIGVhY2ggdHJhbnNhY3Rpb24uDQo+IEl0IGlzIGluIG9y ZGVyIHRvIHVwZGF0ZSBzdG9yYWdlLiBJZiBzbyAgSSB3b3VsZCBwcm9wb3NlIHRvIGhhdmUgc29t ZSBjYWxsYmFjaw0KPiByZWdpc3RlcmVkIGluIGJ0X2F0dCBmb3IgdGhpcyBwdXJwb3NlLg0KPg0K QWNjb3JkaW5nIHRvIHlvdXIgcHJvcG9zYWwsIEkgYW0gbm90IGNsZWFyIHdoZXJlIGFuZCB3aGVu IHlvdSB3YW50IHRvIHJlZ2lzdGVyDQpjYWxsYmFjayB0byBnZXQgc2lnbl9jbnQsIGFuZCBJIHRo aW5rIHJlZ2lzdGVyIG5ldyBjYWxsYmFjayBmb3Igb25seSBvbmUgc2lnbmVkX2NudA0KUGFyYW1l dGVyLCBpdCBtYWtlIHNlbnNlID8NCg0KSW4gbXkgdGhvdWdodCAsIHRoaXMgbWV0aG9kIHVzZXIg YWxzbyBjYW4gZ2V0IHNpZ25kX2NudCBhZnRlciBzaWduZWRfd3JpdGVfY21kLg0KSW4gZW5jb2Rl X3BkdSBmdW5jdGlvbiwgYWZ0ZXIgd2UgZW5jb2RlIHBkdSB3aXRoIENTUkssIHRoZW4gaXQgd2ls bCB1cGRhdGUgc2lnbl9jbnQNCmluIHRoZSBidF9hdHQgc3RydWN0dXJlIGZvciBuZXh0IHNpZ25l ZCB3cml0ZSBjb21tYW5kLg0KDQpJZiBjbGllbnQgb2YgYnRfYXR0IG5lZWQgdG8gdXBkYXRlIGl0 cyBzaWduX2NudCBpbiB0aGUgc3RvcmFnZSwgd2hlbiBidF9hdHRfc2VuZCBmb3INClNJR05FRF9X UklURV9DT01NQU5EIHJldHVybiB0dXJlLCB1c2VyIGNhbiBjYWxsIGJ0X2F0dF9nZXRfbG9jYWxf Y3NyayB0byANCmdldCBuZXdlc3Qgc2lnbl9jbnQgYW5kIHN0b3JlIGl0Lg0KDQpCZXN0IFJlZ2Fy ZHMNCkNoYW9qaWUgR3UNCg== ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/4] shared/att.c: Add signed command outgoing and CSRK function 2014-09-27 9:26 ` Gu, Chao Jie @ 2014-09-29 8:08 ` Lukasz Rymanowski 2014-09-29 8:48 ` Gu, Chao Jie 0 siblings, 1 reply; 16+ messages in thread From: Lukasz Rymanowski @ 2014-09-29 8:08 UTC (permalink / raw) To: Gu, Chao Jie; +Cc: linux-bluetooth Hi Chaojie, On Sat, Sep 27, 2014 at 11:26 AM, Gu, Chao Jie <chao.jie.gu@intel.com> wrote: > Hi Lukasz, > >> > + >> > +bool bt_att_get_local_csrk(struct bt_att *att, uint8_t key[16], >> > + uint32_t *local_sign_cnt) >> >> >> Why we need that get function ? If we assume that signing is done in bt_att then >> there is need only to feed bt_att with the CSRK. >> BTW. There should be a way to set sign_cnt as well as this shall be in same >> storage as CSRK >> >> I can guess that idea behind this bt_att_get_ is that client of bt_att should have >> possibility to update its sign_cnt after each transaction. >> It is in order to update storage. If so I would propose to have some callback >> registered in bt_att for this purpose. >> > According to your proposal, I am not clear where and when you want to register > callback to get sign_cnt, At the same time and from the same place where CSRK is provided to bt_att. I guess that both values (key and counter) will be stored in same database. > and I think register new callback for only one signed_cnt > Parameter, it make sense ? Why not? :) > > In my thought , this method user also can get signd_cnt after signed_write_cmd. > In encode_pdu function, after we encode pdu with CSRK, then it will update sign_cnt > in the bt_att structure for next signed write command. > > If client of bt_att need to update its sign_cnt in the storage, when bt_att_send for > SIGNED_WRITE_COMMAND return ture, user can call bt_att_get_local_csrk to > get newest sign_cnt and store it. The thing is that it is not "if". We need to make sure that sign counter is updated in the database and I think that having callback is convenient for that. Of course you can have some helper which retrieves sign counter after each att transaction and then store it. We need to remember that local sign counter is valid as long as CSRK is, so in order to do any sign write after reconnect we need to have updated sing counter, otherwise remote will reject write command. BR Lukasz > > Best Regards > Chaojie Gu ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v3 1/4] shared/att.c: Add signed command outgoing and CSRK function 2014-09-29 8:08 ` Lukasz Rymanowski @ 2014-09-29 8:48 ` Gu, Chao Jie 0 siblings, 0 replies; 16+ messages in thread From: Gu, Chao Jie @ 2014-09-29 8:48 UTC (permalink / raw) To: Lukasz Rymanowski; +Cc: linux-bluetooth SGkgTHVrYXN6LA0KIA0KPiA+DQo+ID4gSW4gbXkgdGhvdWdodCAsIHRoaXMgbWV0aG9kIHVzZXIg YWxzbyBjYW4gZ2V0IHNpZ25kX2NudCBhZnRlciBzaWduZWRfd3JpdGVfY21kLg0KPiA+IEluIGVu Y29kZV9wZHUgZnVuY3Rpb24sIGFmdGVyIHdlIGVuY29kZSBwZHUgd2l0aCBDU1JLLCB0aGVuIGl0 IHdpbGwNCj4gPiB1cGRhdGUgc2lnbl9jbnQgaW4gdGhlIGJ0X2F0dCBzdHJ1Y3R1cmUgZm9yIG5l eHQgc2lnbmVkIHdyaXRlIGNvbW1hbmQuDQo+ID4NCj4gPiBJZiBjbGllbnQgb2YgYnRfYXR0IG5l ZWQgdG8gdXBkYXRlIGl0cyBzaWduX2NudCBpbiB0aGUgc3RvcmFnZSwgd2hlbg0KPiA+IGJ0X2F0 dF9zZW5kIGZvciBTSUdORURfV1JJVEVfQ09NTUFORCByZXR1cm4gdHVyZSwgdXNlciBjYW4gY2Fs bA0KPiA+IGJ0X2F0dF9nZXRfbG9jYWxfY3NyayB0byBnZXQgbmV3ZXN0IHNpZ25fY250IGFuZCBz dG9yZSBpdC4NCj4gDQo+IFRoZSB0aGluZyBpcyB0aGF0IGl0IGlzIG5vdCAiaWYiLiAgV2UgbmVl ZCB0byBtYWtlIHN1cmUgdGhhdCBzaWduIGNvdW50ZXIgaXMgdXBkYXRlZA0KPiBpbiB0aGUgZGF0 YWJhc2UgYW5kIEkgdGhpbmsgdGhhdCBoYXZpbmcgY2FsbGJhY2sgaXMgY29udmVuaWVudCBmb3Ig dGhhdC4gT2YgY291cnNlIHlvdQ0KPiBjYW4gaGF2ZSBzb21lIGhlbHBlciB3aGljaCByZXRyaWV2 ZXMgc2lnbiBjb3VudGVyIGFmdGVyIGVhY2ggYXR0IHRyYW5zYWN0aW9uIGFuZA0KPiB0aGVuIHN0 b3JlIGl0Lg0KPiANCj4gV2UgbmVlZCB0byByZW1lbWJlciAgdGhhdCBsb2NhbCBzaWduIGNvdW50 ZXIgaXMgdmFsaWQgYXMgbG9uZyBhcyBDU1JLIGlzLCBzbyBpbg0KPiBvcmRlciB0byBkbyBhbnkg c2lnbiB3cml0ZSBhZnRlciByZWNvbm5lY3Qgd2UgbmVlZCB0byBoYXZlIHVwZGF0ZWQgc2luZyBj b3VudGVyLA0KPiBvdGhlcndpc2UgcmVtb3RlIHdpbGwgcmVqZWN0IHdyaXRlIGNvbW1hbmQuDQo+ IA0KDQpXZSBjYW4gc2VlIGFuZHJvaWQvZ2F0dC5jICwgd2hlbiBnYXR0X3NpZ25lZF93cml0ZV9j bWQgZG9uZSwNCkNhbGwgYnRfdXBkYXRlX3NpZ25fY291bnQgdG8gdXBhdGUgYW5kIHN0b3JlIHRo ZSBzaWduZWRfY250Lg0KDQpUaGVyZSBpcyB0aGUgc2FtZSBjb25kaXRpb24gaGVyZS4gWW91IGNh biBjYWxsIGJ0X2F0dF9nZXRfbG9jYWxfY3NyayB0byB1cGRhdGUgYW5kIA0KU3RvcmUgaXQgYWZ0 ZXIgc2lnbmVkIHdyaXRlIGNvbW1hbmQsIGl0IGlzIG1vcmUgZWFzeSB0aGFuIHRvIHJlZ2lzdGVy IGNhbGxiYWNrIHRvIA0KbGlzdGVuIHRoZSBTaWduZWRfY250IGNoYW5nZS4gQW5kIEkgZG9uJ3Qg c2VlIGFueSBjYWxsYmFjayB0cmlnZ2VyIGNvbmRpdGlvbiBmb3IgdGhpcyANCnNpZ25lZF9jbnQg cGFyYW1ldGVyIGluIHRoZSBidF9hdHRfc2VuZCBmdW5jdGlvbi4NCiANCkJlc3QgUmVnYXJkcw0K Q2hhb2ppZSBHdQ0K ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 2/4] shared/gatt-client: Add CSRK part to support signed write 2014-09-26 7:48 [PATCH v3 0/4] support signed write command Gu Chaojie 2014-09-26 7:48 ` [PATCH v3 1/4] shared/att.c: Add signed command outgoing and CSRK function Gu Chaojie @ 2014-09-26 7:48 ` Gu Chaojie 2014-09-26 9:55 ` Marcel Holtmann 2014-09-26 22:35 ` Lukasz Rymanowski 2014-09-26 7:48 ` [PATCH v3 3/4] tools/btgatt-client: Add signed write CSRK option Gu Chaojie 2014-09-26 7:48 ` [PATCH v3 4/4] Modify Makefile.tool to link Gu Chaojie 3 siblings, 2 replies; 16+ messages in thread From: Gu Chaojie @ 2014-09-26 7:48 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gu Chaojie Compared with patch v2, remove type enum in CSRK funciton for btgatt-client tool to easy use this API. --- src/shared/gatt-client.c | 27 ++++++++++++++++++++++----- src/shared/gatt-client.h | 4 ++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c index 6dc8e95..aee3969 100644 --- a/src/shared/gatt-client.c +++ b/src/shared/gatt-client.c @@ -1753,19 +1753,26 @@ 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) { + if (bt_att_local_csrk_is_valid(client->att)) + 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); } @@ -2246,3 +2253,13 @@ bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client, client->need_notify_cleanup = true; return true; } + +bool bt_gatt_client_set_local_csrk(struct bt_gatt_client *client, + bool valid_local_csrk, + uint8_t key[16]) +{ + if (!client) + return false; + + return bt_att_set_local_csrk(client->att, valid_local_csrk, key); +} diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h index 6807f6b..0684f30 100644 --- a/src/shared/gatt-client.h +++ b/src/shared/gatt-client.h @@ -162,3 +162,7 @@ bool bt_gatt_client_register_notify(struct bt_gatt_client *client, bt_gatt_client_destroy_func_t destroy); bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client, unsigned int id); + +bool bt_gatt_client_set_local_csrk(struct bt_gatt_client *client, + bool valid_local_csrk, + uint8_t key[16]); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/4] shared/gatt-client: Add CSRK part to support signed write 2014-09-26 7:48 ` [PATCH v3 2/4] shared/gatt-client: Add CSRK part to support signed write Gu Chaojie @ 2014-09-26 9:55 ` Marcel Holtmann 2014-09-27 8:42 ` Gu, Chao Jie 2014-09-26 22:35 ` Lukasz Rymanowski 1 sibling, 1 reply; 16+ messages in thread From: Marcel Holtmann @ 2014-09-26 9:55 UTC (permalink / raw) To: Gu Chaojie; +Cc: linux-bluetooth Hi Chajie, > Compared with patch v2, remove type enum in CSRK funciton for btgatt-client > tool to easy use this API. > > --- > src/shared/gatt-client.c | 27 ++++++++++++++++++++++----- > src/shared/gatt-client.h | 4 ++++ > 2 files changed, 26 insertions(+), 5 deletions(-) > > diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c > index 6dc8e95..aee3969 100644 > --- a/src/shared/gatt-client.c > +++ b/src/shared/gatt-client.c > @@ -1753,19 +1753,26 @@ 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) { > + if (bt_att_local_csrk_is_valid(client->att)) > + 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); > } > @@ -2246,3 +2253,13 @@ bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client, > client->need_notify_cleanup = true; > return true; > } > + > +bool bt_gatt_client_set_local_csrk(struct bt_gatt_client *client, > + bool valid_local_csrk, > + uint8_t key[16]) > +{ > + if (!client) > + return false; > + > + return bt_att_set_local_csrk(client->att, valid_local_csrk, key); > +} > diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h > index 6807f6b..0684f30 100644 > --- a/src/shared/gatt-client.h > +++ b/src/shared/gatt-client.h > @@ -162,3 +162,7 @@ bool bt_gatt_client_register_notify(struct bt_gatt_client *client, > bt_gatt_client_destroy_func_t destroy); > bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client, > unsigned int id); > + > +bool bt_gatt_client_set_local_csrk(struct bt_gatt_client *client, > + bool valid_local_csrk, > + uint8_t key[16]); why do we have the parameter valid_local_csrk. I find that highly confusing. If you do not have a valid CSRK, then do not call this function. It should be that simple. Regards Marcel ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v3 2/4] shared/gatt-client: Add CSRK part to support signed write 2014-09-26 9:55 ` Marcel Holtmann @ 2014-09-27 8:42 ` Gu, Chao Jie 2014-09-27 8:51 ` Marcel Holtmann 0 siblings, 1 reply; 16+ messages in thread From: Gu, Chao Jie @ 2014-09-27 8:42 UTC (permalink / raw) To: Marcel Holtmann; +Cc: linux-bluetooth Hi Marcel, > > +bool bt_gatt_client_set_local_csrk(struct bt_gatt_client *client, > > + bool valid_local_csrk, > > + uint8_t key[16]); >=20 > why do we have the parameter valid_local_csrk. I find that highly confusi= ng. If you > do not have a valid CSRK, then do not call this function. It should be th= at simple. >=20 My initial purpose to set this parameter valid_local_csrk here, user can ca= ll one API to set flag valid_local_csrk ture/false and CSRK key in one time. According to your proposal, I think it really result in confusion for user. So for more clear to call API , I think we should split two API for upper l= ayer: 1. When we need to set CSRK , check the CSRK is valid , then call bt_gatt_c= lient_set_local_csrk Just to pass one parameter key to function and in the function to set fla= g valid_local_csrk is true. 2 When two device dispair each other, we should give user possibility to un= set flag and CSRK. So in this condition , we have to create bt_gatt_client_unset_local_csrk(= struct bt_gatt_client *client) to clear CSRK and set flag valid_local_csrk to false. Do you think this thought would be ok for you? Best Regards Chaojie Gu ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/4] shared/gatt-client: Add CSRK part to support signed write 2014-09-27 8:42 ` Gu, Chao Jie @ 2014-09-27 8:51 ` Marcel Holtmann 0 siblings, 0 replies; 16+ messages in thread From: Marcel Holtmann @ 2014-09-27 8:51 UTC (permalink / raw) To: Gu, Chao Jie; +Cc: linux-bluetooth Hi Chao Jie, >>> +bool bt_gatt_client_set_local_csrk(struct bt_gatt_client *client, >>> + bool valid_local_csrk, >>> + uint8_t key[16]); >> >> why do we have the parameter valid_local_csrk. I find that highly confusing. If you >> do not have a valid CSRK, then do not call this function. It should be that simple. >> > > My initial purpose to set this parameter valid_local_csrk here, user can call one API > to set flag valid_local_csrk ture/false and CSRK key in one time. > According to your proposal, I think it really result in confusion for user. > > So for more clear to call API , I think we should split two API for upper layer: > 1. When we need to set CSRK , check the CSRK is valid , then call bt_gatt_client_set_local_csrk > Just to pass one parameter key to function and in the function to set flag valid_local_csrk is true. > > 2 When two device dispair each other, we should give user possibility to unset flag and CSRK. > So in this condition , we have to create bt_gatt_client_unset_local_csrk(struct bt_gatt_client *client) > to clear CSRK and set flag valid_local_csrk to false. > > Do you think this thought would be ok for you? I am not really worried about these details anyway. If you remove the bond between two devices, then the connection will be terminated. Which means that next time around you get a new bt_att object. If you do not have any CSRK the next time, you just not set them in the first place. Regards Marcel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/4] shared/gatt-client: Add CSRK part to support signed write 2014-09-26 7:48 ` [PATCH v3 2/4] shared/gatt-client: Add CSRK part to support signed write Gu Chaojie 2014-09-26 9:55 ` Marcel Holtmann @ 2014-09-26 22:35 ` Lukasz Rymanowski 2014-09-27 9:36 ` Gu, Chao Jie 1 sibling, 1 reply; 16+ messages in thread From: Lukasz Rymanowski @ 2014-09-26 22:35 UTC (permalink / raw) To: Gu Chaojie; +Cc: linux-bluetooth Hi Chaojie, On Fri, Sep 26, 2014 at 9:48 AM, Gu Chaojie <chao.jie.gu@intel.com> wrote: > Compared with patch v2, remove type enum in CSRK funciton for btgatt-client > tool to easy use this API. > > --- > src/shared/gatt-client.c | 27 ++++++++++++++++++++++----- > src/shared/gatt-client.h | 4 ++++ > 2 files changed, 26 insertions(+), 5 deletions(-) > > diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c > index 6dc8e95..aee3969 100644 > --- a/src/shared/gatt-client.c > +++ b/src/shared/gatt-client.c > @@ -1753,19 +1753,26 @@ 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) { > + if (bt_att_local_csrk_is_valid(client->att)) In my opinion bt_att_local_csrk_is_valid API seems to be unnecessary as CSRK can be checked inside bt_att_send function directly. BR \Łukasz > + 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); > } > @@ -2246,3 +2253,13 @@ bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client, > client->need_notify_cleanup = true; > return true; > } > + > +bool bt_gatt_client_set_local_csrk(struct bt_gatt_client *client, > + bool valid_local_csrk, > + uint8_t key[16]) > +{ > + if (!client) > + return false; > + > + return bt_att_set_local_csrk(client->att, valid_local_csrk, key); > +} > diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h > index 6807f6b..0684f30 100644 > --- a/src/shared/gatt-client.h > +++ b/src/shared/gatt-client.h > @@ -162,3 +162,7 @@ bool bt_gatt_client_register_notify(struct bt_gatt_client *client, > bt_gatt_client_destroy_func_t destroy); > bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client, > unsigned int id); > + > +bool bt_gatt_client_set_local_csrk(struct bt_gatt_client *client, > + bool valid_local_csrk, > + uint8_t key[16]); > -- > 1.7.10.4 > > -- > 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] 16+ messages in thread
* RE: [PATCH v3 2/4] shared/gatt-client: Add CSRK part to support signed write 2014-09-26 22:35 ` Lukasz Rymanowski @ 2014-09-27 9:36 ` Gu, Chao Jie 2014-09-29 8:09 ` Lukasz Rymanowski 0 siblings, 1 reply; 16+ messages in thread From: Gu, Chao Jie @ 2014-09-27 9:36 UTC (permalink / raw) To: Lukasz Rymanowski; +Cc: linux-bluetooth SGkgTHVrYXN6LA0KPiA+IC0tLQ0KPiA+ICBzcmMvc2hhcmVkL2dhdHQtY2xpZW50LmMgfCAgIDI3 ICsrKysrKysrKysrKysrKysrKysrKystLS0tLQ0KPiA+ICBzcmMvc2hhcmVkL2dhdHQtY2xpZW50 LmggfCAgICA0ICsrKysNCj4gPiAgMiBmaWxlcyBjaGFuZ2VkLCAyNiBpbnNlcnRpb25zKCspLCA1 IGRlbGV0aW9ucygtKQ0KPiA+DQo+ID4gZGlmZiAtLWdpdCBhL3NyYy9zaGFyZWQvZ2F0dC1jbGll bnQuYyBiL3NyYy9zaGFyZWQvZ2F0dC1jbGllbnQuYyBpbmRleA0KPiA+IDZkYzhlOTUuLmFlZTM5 NjkgMTAwNjQ0DQo+ID4gLS0tIGEvc3JjL3NoYXJlZC9nYXR0LWNsaWVudC5jDQo+ID4gKysrIGIv c3JjL3NoYXJlZC9nYXR0LWNsaWVudC5jDQo+ID4gQEAgLTE3NTMsMTkgKzE3NTMsMjYgQEAgYm9v bCBidF9nYXR0X2NsaWVudF9yZWFkX2xvbmdfdmFsdWUoc3RydWN0DQo+ID4gYnRfZ2F0dF9jbGll bnQgKmNsaWVudCwgIGJvb2wgYnRfZ2F0dF9jbGllbnRfd3JpdGVfd2l0aG91dF9yZXNwb25zZShz dHJ1Y3QNCj4gYnRfZ2F0dF9jbGllbnQgKmNsaWVudCwNCj4gPiAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgdWludDE2X3QgdmFsdWVfaGFuZGxlLA0KPiA+ICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBib29sIHNpZ25lZF93cml0ZSwNCj4gPiAt ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgdWludDhfdCAqdmFsdWUsIHVp bnQxNl90IGxlbmd0aCkgew0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICB1aW50OF90ICp2YWx1ZSwgdWludDE2X3QNCj4gPiArbGVuZ3RoKSB7DQo+ID4gICAgICAg ICB1aW50OF90IHBkdVsyICsgbGVuZ3RoXTsNCj4gPg0KPiA+ICAgICAgICAgaWYgKCFjbGllbnQp DQo+ID4gICAgICAgICAgICAgICAgIHJldHVybiAwOw0KPiA+DQo+ID4gLSAgICAgICAvKiBUT0RP OiBTdXBwb3J0IHRoaXMgb25jZSBidF9hdHRfc2VuZCBzdXBwb3J0cyBzaWduZWQgd3JpdGVzLiAq Lw0KPiA+IC0gICAgICAgaWYgKHNpZ25lZF93cml0ZSkNCj4gPiAtICAgICAgICAgICAgICAgcmV0 dXJuIDA7DQo+ID4gLQ0KPiA+ICAgICAgICAgcHV0X2xlMTYodmFsdWVfaGFuZGxlLCBwZHUpOw0K PiA+ICAgICAgICAgbWVtY3B5KHBkdSArIDIsIHZhbHVlLCBsZW5ndGgpOw0KPiA+DQo+ID4gKyAg ICAgICBpZiAoc2lnbmVkX3dyaXRlKSB7DQo+ID4gKyAgICAgICAgICAgICAgIGlmIChidF9hdHRf bG9jYWxfY3Nya19pc192YWxpZChjbGllbnQtPmF0dCkpDQo+IA0KPiBJbiBteSBvcGluaW9uIGJ0 X2F0dF9sb2NhbF9jc3JrX2lzX3ZhbGlkIEFQSSBzZWVtcyB0byBiZSB1bm5lY2Vzc2FyeSBhcyBD U1JLIGNhbg0KPiBiZSBjaGVja2VkIGluc2lkZSBidF9hdHRfc2VuZCBmdW5jdGlvbiBkaXJlY3Rs eS4NCj4gDQoNCkkgYWdyZWUgd2l0aCB5b3UgdGhhdCBpdCBpcyBhbHNvIG9rIHRvIGJlIGNoZWNr ZWQgaW5zaWRlIGJ0X2F0dF9zZW5kIGZ1bmN0aW9uIGFuZCBoYXZlIHRob3VnaHQNCmFib3V0IHRo aXMgcHJvYmxlbSBiZWZvcmUuIA0KQW5kIEkgdGhpbmsgdGhlcmUgYXJlIHR3byBhZHZhbnRhZ2Vz IG9mIGNoZWNraW5nIHZhbGlkIG91dHNpZGUgYnRfYXR0X3NlbmQgZnVuY3Rpb24NCg0KMS4gcHJv dmlkZSBBUEkgdG8gdXNlciB0byBjaGVjayBDU1JLIHN0YXR1cyBpZiB0aGV5IHdhbnQuDQoyLiBp ZiB0aGUgQ1NSSyBpcyBpbnZhbGlkICwgd2UgY2FuIGp1c3QgcmV0dXJuIGZhbHNlIHF1aWNrbHkg LCB0aGVyZSBpcyBubyBuZWVkIHRvIGdvIG9uIGNhbGxpbmcNCmRvd24gYnRfYXR0X3NlbmQgdGhl biByZXR1cm4gZmFsc2UgYmFjay4gSXQgd2lsbCBiZSBtb3JlIGVmZmljaWVudCBJIHRoaW5rLg0K DQpXaGF0J3MgeW91ciBwb2ludCBhYm91dCB0aGlzID8NCg0KQmVzdCBSZWdhcmRzDQpDaGFvamll IEd1DQoNCg== ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/4] shared/gatt-client: Add CSRK part to support signed write 2014-09-27 9:36 ` Gu, Chao Jie @ 2014-09-29 8:09 ` Lukasz Rymanowski 2014-09-29 8:53 ` Gu, Chao Jie 0 siblings, 1 reply; 16+ messages in thread From: Lukasz Rymanowski @ 2014-09-29 8:09 UTC (permalink / raw) To: Gu, Chao Jie; +Cc: linux-bluetooth HI Chaojie, On Sat, Sep 27, 2014 at 11:36 AM, Gu, Chao Jie <chao.jie.gu@intel.com> wrote: > Hi Lukasz, >> > --- >> > src/shared/gatt-client.c | 27 ++++++++++++++++++++++----- >> > src/shared/gatt-client.h | 4 ++++ >> > 2 files changed, 26 insertions(+), 5 deletions(-) >> > >> > diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c index >> > 6dc8e95..aee3969 100644 >> > --- a/src/shared/gatt-client.c >> > +++ b/src/shared/gatt-client.c >> > @@ -1753,19 +1753,26 @@ 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) { >> > + if (bt_att_local_csrk_is_valid(client->att)) >> >> In my opinion bt_att_local_csrk_is_valid API seems to be unnecessary as CSRK can >> be checked inside bt_att_send function directly. >> > > I agree with you that it is also ok to be checked inside bt_att_send function and have thought > about this problem before. > And I think there are two advantages of checking valid outside bt_att_send function > > 1. provide API to user to check CSRK status if they want. Ok, the question is why user would like to do it? Maybe I'm missing something. > 2. if the CSRK is invalid , we can just return false quickly , there is no need to go on calling > down bt_att_send then return false back. It will be more efficient I think. If bt_att_send() will check the CSRK on the beginning then there is no difference. > > What's your point about this ? > > Best Regards > Chaojie Gu > ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v3 2/4] shared/gatt-client: Add CSRK part to support signed write 2014-09-29 8:09 ` Lukasz Rymanowski @ 2014-09-29 8:53 ` Gu, Chao Jie 0 siblings, 0 replies; 16+ messages in thread From: Gu, Chao Jie @ 2014-09-29 8:53 UTC (permalink / raw) To: Lukasz Rymanowski; +Cc: linux-bluetooth SGkgTHVrYXN6LA0KDQo+ID4+IEluIG15IG9waW5pb24gYnRfYXR0X2xvY2FsX2NzcmtfaXNfdmFs aWQgQVBJIHNlZW1zIHRvIGJlIHVubmVjZXNzYXJ5DQo+ID4+IGFzIENTUksgY2FuIGJlIGNoZWNr ZWQgaW5zaWRlIGJ0X2F0dF9zZW5kIGZ1bmN0aW9uIGRpcmVjdGx5Lg0KPiA+Pg0KPiA+DQo+ID4g SSBhZ3JlZSB3aXRoIHlvdSB0aGF0IGl0IGlzIGFsc28gb2sgdG8gYmUgY2hlY2tlZCBpbnNpZGUg YnRfYXR0X3NlbmQNCj4gPiBmdW5jdGlvbiBhbmQgaGF2ZSB0aG91Z2h0IGFib3V0IHRoaXMgcHJv YmxlbSBiZWZvcmUuDQo+ID4gQW5kIEkgdGhpbmsgdGhlcmUgYXJlIHR3byBhZHZhbnRhZ2VzIG9m IGNoZWNraW5nIHZhbGlkIG91dHNpZGUNCj4gPiBidF9hdHRfc2VuZCBmdW5jdGlvbg0KPiA+DQo+ ID4gMS4gcHJvdmlkZSBBUEkgdG8gdXNlciB0byBjaGVjayBDU1JLIHN0YXR1cyBpZiB0aGV5IHdh bnQuDQo+IA0KPiBPaywgdGhlIHF1ZXN0aW9uIGlzIHdoeSB1c2VyIHdvdWxkIGxpa2UgIHRvIGRv IGl0PyBNYXliZSBJJ20gbWlzc2luZyBzb21ldGhpbmcuDQo+IA0KPiA+IDIuIGlmIHRoZSBDU1JL IGlzIGludmFsaWQgLCB3ZSBjYW4ganVzdCByZXR1cm4gZmFsc2UgcXVpY2tseSAsIHRoZXJlDQo+ ID4gaXMgbm8gbmVlZCB0byBnbyBvbiBjYWxsaW5nIGRvd24gYnRfYXR0X3NlbmQgdGhlbiByZXR1 cm4gZmFsc2UgYmFjay4gSXQgd2lsbCBiZQ0KPiBtb3JlIGVmZmljaWVudCBJIHRoaW5rLg0KPiAN Cj4gSWYgIGJ0X2F0dF9zZW5kKCkgd2lsbCBjaGVjayB0aGUgQ1NSSyBvbiB0aGUgYmVnaW5uaW5n IHRoZW4gdGhlcmUgaXMgbm8gZGlmZmVyZW5jZS4NCj4gDQoNClllcyAsIHdlIGNhbiBjaGVjayB0 aGUgQ1NSSyBpbiB0aGUgYmVnaW5uaW5nIG9mIHRoZSBidF9hdHRfc2VuZCBmdW5jdGlvbi4gSG93 ZXZlciwNCllvdSBjYW4gc2VlIGluIHRoZSBidF9hdHRfc2VuZCBmdW5jdGlvbiB0aGVyZSBpcyBj bGVhciBhbmQgZ2VuZXJhbCBoYW5kbGUgY29tbWFuZA0KcHJvY2VzcyAsIEkgZG8gbm90IHRoaW5r IGlzIGEgZ29vZCBzdHlsZSBoZXJlIHRvIHB1dCBhIHNwZWNpZmljIHNpZ25lZF93cml0ZV9jbWQg c3R1ZmYgDQppbiB0aGUgYmVnaW5uaW5nIHRvIGNoZWNrIENTUksgdmFsaWQgb3Igbm90Lg0KDQpU aGFua3MNCkNoYW9qaWUgR3UNCg== ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 3/4] tools/btgatt-client: Add signed write CSRK option 2014-09-26 7:48 [PATCH v3 0/4] support signed write command Gu Chaojie 2014-09-26 7:48 ` [PATCH v3 1/4] shared/att.c: Add signed command outgoing and CSRK function Gu Chaojie 2014-09-26 7:48 ` [PATCH v3 2/4] shared/gatt-client: Add CSRK part to support signed write Gu Chaojie @ 2014-09-26 7:48 ` Gu Chaojie 2014-09-26 7:48 ` [PATCH v3 4/4] Modify Makefile.tool to link Gu Chaojie 3 siblings, 0 replies; 16+ messages in thread From: Gu Chaojie @ 2014-09-26 7:48 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gu Chaojie --- 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 d900e08..0b6d545 100644 --- a/tools/btgatt-client.c +++ b/tools/btgatt-client.c @@ -486,11 +486,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' }, { } }; @@ -504,6 +506,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; @@ -515,6 +535,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"); @@ -529,12 +554,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_local_csrk(cli->gatt, + valid_csrk, csrk); + break; default: write_value_usage(); return; @@ -588,7 +619,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; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 4/4] Modify Makefile.tool to link 2014-09-26 7:48 [PATCH v3 0/4] support signed write command Gu Chaojie ` (2 preceding siblings ...) 2014-09-26 7:48 ` [PATCH v3 3/4] tools/btgatt-client: Add signed write CSRK option Gu Chaojie @ 2014-09-26 7:48 ` Gu Chaojie 3 siblings, 0 replies; 16+ messages in thread From: Gu Chaojie @ 2014-09-26 7:48 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gu Chaojie Fix patch set v2 link issue when compiling. --- Makefile.tools | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile.tools b/Makefile.tools index 4807813..8823fc0 100644 --- a/Makefile.tools +++ b/Makefile.tools @@ -365,7 +365,8 @@ tools_btgatt_client_SOURCES = tools/btgatt-client.c src/uuid-helper.c \ src/shared/timeout.h src/shared/timeout-mainloop.c \ src/shared/att-types.h src/shared/att.h src/shared/att.c \ src/shared/gatt-helpers.h src/shared/gatt-helpers.c \ - src/shared/gatt-client.h src/shared/gatt-client.c + src/shared/gatt-client.h src/shared/gatt-client.c \ + src/shared/crypto.h src/shared/crypto.c tools_btgatt_client_LDADD = lib/libbluetooth-internal.la EXTRA_DIST += tools/bdaddr.1 -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2014-09-29 8:53 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-09-26 7:48 [PATCH v3 0/4] support signed write command Gu Chaojie 2014-09-26 7:48 ` [PATCH v3 1/4] shared/att.c: Add signed command outgoing and CSRK function Gu Chaojie 2014-09-26 22:30 ` Lukasz Rymanowski 2014-09-27 9:26 ` Gu, Chao Jie 2014-09-29 8:08 ` Lukasz Rymanowski 2014-09-29 8:48 ` Gu, Chao Jie 2014-09-26 7:48 ` [PATCH v3 2/4] shared/gatt-client: Add CSRK part to support signed write Gu Chaojie 2014-09-26 9:55 ` Marcel Holtmann 2014-09-27 8:42 ` Gu, Chao Jie 2014-09-27 8:51 ` Marcel Holtmann 2014-09-26 22:35 ` Lukasz Rymanowski 2014-09-27 9:36 ` Gu, Chao Jie 2014-09-29 8:09 ` Lukasz Rymanowski 2014-09-29 8:53 ` Gu, Chao Jie 2014-09-26 7:48 ` [PATCH v3 3/4] tools/btgatt-client: Add signed write CSRK option Gu Chaojie 2014-09-26 7:48 ` [PATCH v3 4/4] Modify Makefile.tool to link Gu Chaojie
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.