Hi Luiz,
On Fri, Feb 20, 2015 at 2:40 PM, Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
> Hi Lukasz,
>
> On Fri, Feb 20, 2015 at 3:25 PM, Luiz Augusto von Dentz
> <luiz.dentz@xxxxxxxxx> wrote:
>> Hi Lukasz,
>>
>> On Fri, Feb 20, 2015 at 2:04 PM, Lukasz Rymanowski
>> <lukasz.rymanowski@xxxxxxxxx> wrote:
>>> Hi Luiz,
>>>
>>> On Fri, Feb 20, 2015 at 11:38 AM, Luiz Augusto von Dentz
>>> <luiz.dentz@xxxxxxxxx> wrote:
>>>> From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
>>>>
>>>> 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@xxxxxxxxxxxxxxx
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>> --
>> Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
