* [PATCH BlueZ v2 2/5] client: Define maximum attribute value length as initial value
2018-05-16 13:52 [PATCH BlueZ v2 1/5] client: Add missing duplicated string free Grzegorz Kolodziejczyk
@ 2018-05-16 13:52 ` Grzegorz Kolodziejczyk
2018-05-16 13:52 ` [PATCH BlueZ v2 3/5] doc/gatt-api: Add authorization options for attributes Grzegorz Kolodziejczyk
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Grzegorz Kolodziejczyk @ 2018-05-16 13:52 UTC (permalink / raw)
To: linux-bluetooth
Initial registered attribute value is set as maximal attribute length.
---
client/gatt.c | 54 +++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 45 insertions(+), 9 deletions(-)
diff --git a/client/gatt.c b/client/gatt.c
index c8267a75f..1b4e713ef 100644
--- a/client/gatt.c
+++ b/client/gatt.c
@@ -54,12 +54,15 @@
#define COLORED_CHG COLOR_YELLOW "CHG" COLOR_OFF
#define COLORED_DEL COLOR_RED "DEL" COLOR_OFF
+#define MAX_ATTR_VAL_LEN 512
+
struct desc {
struct chrc *chrc;
char *path;
char *uuid;
char **flags;
int value_len;
+ unsigned int max_val_len;
uint8_t *value;
};
@@ -71,6 +74,7 @@ struct chrc {
bool notifying;
GList *descs;
int value_len;
+ unsigned int max_val_len;
uint8_t *value;
uint16_t mtu;
struct io *write_io;
@@ -612,7 +616,7 @@ static void write_attribute(GDBusProxy *proxy, char *val_str, uint16_t offset)
{
struct iovec iov;
struct write_attribute_data wd;
- uint8_t value[512];
+ uint8_t value[MAX_ATTR_VAL_LEN];
char *entry;
unsigned int i;
@@ -689,7 +693,7 @@ void gatt_write_attribute(GDBusProxy *proxy, int argc, char *argv[])
static bool pipe_read(struct io *io, void *user_data)
{
struct chrc *chrc = user_data;
- uint8_t buf[512];
+ uint8_t buf[MAX_ATTR_VAL_LEN];
int fd = io_get_fd(io);
ssize_t bytes_read;
@@ -1612,7 +1616,8 @@ static DBusMessage *chrc_read_value(DBusConnection *conn, DBusMessage *msg,
return read_value(msg, &chrc->value[offset], chrc->value_len - offset);
}
-static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len)
+static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len,
+ int max_len)
{
DBusMessageIter array;
uint16_t offset = 0;
@@ -1629,6 +1634,9 @@ static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len)
if (parse_options(iter, &offset, NULL, NULL, NULL))
return -EINVAL;
+ if ((offset + read_len) > max_len)
+ return -EOVERFLOW;
+
if ((offset + read_len) > *len) {
*len = offset + read_len;
*value = g_realloc(*value, *len);
@@ -1646,6 +1654,7 @@ static void authorize_write_response(const char *input, void *user_data)
DBusMessageIter iter;
DBusMessage *reply;
char *err;
+ int errsv;
dbus_message_iter_init(pending_message, &iter);
@@ -1657,10 +1666,16 @@ static void authorize_write_response(const char *input, void *user_data)
chrc->authorized = true;
- if (parse_value_arg(&iter, &chrc->value, &chrc->value_len)) {
+ errsv = parse_value_arg(&iter, &chrc->value, &chrc->value_len,
+ chrc->max_val_len);
+ if (errsv == -EINVAL) {
err = "org.bluez.Error.InvalidArguments";
goto error;
+ } else if (errsv == -EOVERFLOW) {
+ err = "org.bluez.Error.InvalidValueLength";
+
+ goto error;
}
bt_shell_printf("[" COLORED_CHG "] Attribute %s written" , chrc->path);
@@ -1686,6 +1701,7 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
struct chrc *chrc = user_data;
DBusMessageIter iter;
char *str;
+ int errsv;
dbus_message_iter_init(msg, &iter);
@@ -1708,10 +1724,15 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
return NULL;
}
- if (parse_value_arg(&iter, &chrc->value, &chrc->value_len))
+ errsv = parse_value_arg(&iter, &chrc->value, &chrc->value_len,
+ chrc->max_val_len);
+ if (errsv == -EINVAL) {
return g_dbus_create_error(msg,
- "org.bluez.Error.InvalidArguments",
- NULL);
+ "org.bluez.Error.InvalidArguments", NULL);
+ } else if (errsv == -EOVERFLOW) {
+ return g_dbus_create_error(msg,
+ "org.bluez.Error.InvalidValueLength", NULL);
+ }
bt_shell_printf("[" COLORED_CHG "] Attribute %s written" , chrc->path);
@@ -1885,7 +1906,7 @@ static const GDBusMethodTable chrc_methods[] = {
static uint8_t *str2bytearray(char *arg, int *val_len)
{
- uint8_t value[512];
+ uint8_t value[MAX_ATTR_VAL_LEN];
char *entry;
unsigned int i;
@@ -1922,6 +1943,13 @@ static void chrc_set_value(const char *input, void *user_data)
g_free(chrc->value);
chrc->value = str2bytearray((char *) input, &chrc->value_len);
+
+ if (!chrc->value) {
+ print_chrc(chrc, COLORED_DEL);
+ chrc_unregister(chrc);
+ }
+
+ chrc->max_val_len = chrc->value_len;
}
void gatt_register_chrc(DBusConnection *conn, GDBusProxy *proxy,
@@ -2039,7 +2067,8 @@ static DBusMessage *desc_write_value(DBusConnection *conn, DBusMessage *msg,
dbus_message_iter_init(msg, &iter);
- if (parse_value_arg(&iter, &desc->value, &desc->value_len))
+ if (parse_value_arg(&iter, &desc->value, &desc->value_len,
+ desc->max_val_len))
return g_dbus_create_error(msg,
"org.bluez.Error.InvalidArguments",
NULL);
@@ -2143,6 +2172,13 @@ static void desc_set_value(const char *input, void *user_data)
g_free(desc->value);
desc->value = str2bytearray((char *) input, &desc->value_len);
+
+ if (!desc->value) {
+ print_desc(desc, COLORED_DEL);
+ desc_unregister(desc);
+ }
+
+ desc->max_val_len = desc->value_len;
}
void gatt_register_desc(DBusConnection *conn, GDBusProxy *proxy,
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH BlueZ v2 3/5] doc/gatt-api: Add authorization options for attributes
2018-05-16 13:52 [PATCH BlueZ v2 1/5] client: Add missing duplicated string free Grzegorz Kolodziejczyk
2018-05-16 13:52 ` [PATCH BlueZ v2 2/5] client: Define maximum attribute value length as initial value Grzegorz Kolodziejczyk
@ 2018-05-16 13:52 ` Grzegorz Kolodziejczyk
2018-05-16 13:52 ` [PATCH BlueZ v2 4/5] shared/gatt-server: Request authorization for prepare writes Grzegorz Kolodziejczyk
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Grzegorz Kolodziejczyk @ 2018-05-16 13:52 UTC (permalink / raw)
To: linux-bluetooth
This patch adds authorization property for attributes and prepare write
request for authorization option for write request. This is require to
handle correctly prepare writes, which may response with insufficient
authorization error.
---
doc/gatt-api.txt | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
index 0f1cc9029..fc61838df 100644
--- a/doc/gatt-api.txt
+++ b/doc/gatt-api.txt
@@ -85,6 +85,8 @@ Methods array{byte} ReadValue(dict options)
Possible options: "offset": Start offset
"device": Device path (Server only)
"link": Link type (Server only)
+ "authorize": boolean Is prepare write
+ authorization request
Possible Errors: org.bluez.Error.Failed
org.bluez.Error.InProgress
@@ -251,6 +253,12 @@ Properties string UUID [read-only]
"secure-read" (Server only)
"secure-write" (Server only)
+ boolean Authorize [read-only, optional] (Server only)
+
+ True, if authorization is required for attribute
+ operations. By default this valuie is set to false for
+ attribute.
+
Characteristic Descriptors hierarchy
====================================
@@ -284,6 +292,8 @@ Methods array{byte} ReadValue(dict flags)
Possible options: "offset": Start offset
"device": Device path (Server only)
"link": Link type (Server only)
+ "authorize": boolean Is authorization
+ request
Possible Errors: org.bluez.Error.Failed
org.bluez.Error.InProgress
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH BlueZ v2 4/5] shared/gatt-server: Request authorization for prepare writes.
2018-05-16 13:52 [PATCH BlueZ v2 1/5] client: Add missing duplicated string free Grzegorz Kolodziejczyk
2018-05-16 13:52 ` [PATCH BlueZ v2 2/5] client: Define maximum attribute value length as initial value Grzegorz Kolodziejczyk
2018-05-16 13:52 ` [PATCH BlueZ v2 3/5] doc/gatt-api: Add authorization options for attributes Grzegorz Kolodziejczyk
@ 2018-05-16 13:52 ` Grzegorz Kolodziejczyk
2018-05-21 9:51 ` Luiz Augusto von Dentz
2018-05-16 13:52 ` [PATCH BlueZ v2 5/5] client: Add authorized property handling to characteristic attribute Grzegorz Kolodziejczyk
2018-05-21 8:38 ` [PATCH BlueZ v2 1/5] client: Add missing duplicated string free Luiz Augusto von Dentz
4 siblings, 1 reply; 10+ messages in thread
From: Grzegorz Kolodziejczyk @ 2018-05-16 13:52 UTC (permalink / raw)
To: linux-bluetooth
This patch adds gatt-server possibility to request authorization from
application if needed and previously wasn't authorized. Authorization is
requested by sending message with set prepare write authorization reqest
to client.
---
src/gatt-database.c | 86 ++++++++++++++++++++++++++++++++++++++++--------
src/shared/gatt-server.c | 64 ++++++++++++++++++++++++++++++-----
2 files changed, 127 insertions(+), 23 deletions(-)
diff --git a/src/gatt-database.c b/src/gatt-database.c
index 0ac5b75b0..3326b40db 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -133,6 +133,7 @@ struct external_chrc {
struct queue *pending_reads;
struct queue *pending_writes;
unsigned int ntfy_cnt;
+ bool authorized;
};
struct external_desc {
@@ -144,6 +145,7 @@ struct external_desc {
bool handled;
struct queue *pending_reads;
struct queue *pending_writes;
+ bool authorized;
};
struct pending_op {
@@ -154,6 +156,8 @@ struct pending_op {
struct gatt_db_attribute *attrib;
struct queue *owner_queue;
struct iovec data;
+ bool is_characteristic;
+ bool authorize;
};
struct notify {
@@ -1937,6 +1941,9 @@ static void append_options(DBusMessageIter *iter, void *user_data)
&op->offset);
if (link)
dict_append_entry(iter, "link", DBUS_TYPE_STRING, &link);
+ if (op->authorize)
+ dict_append_entry(iter, "authorize", DBUS_TYPE_BOOLEAN,
+ &op->authorize);
}
static void read_setup_cb(DBusMessageIter *iter, void *user_data)
@@ -2008,6 +2015,8 @@ static void write_setup_cb(DBusMessageIter *iter, void *user_data)
static void write_reply_cb(DBusMessage *message, void *user_data)
{
struct pending_op *op = user_data;
+ struct external_chrc *chrc;
+ struct external_desc *desc;
DBusError err;
DBusMessageIter iter;
uint8_t ecode = 0;
@@ -2027,6 +2036,16 @@ static void write_reply_cb(DBusMessage *message, void *user_data)
goto done;
}
+ if (op->authorize) {
+ if (op->is_characteristic) {
+ chrc = gatt_db_attribute_get_user_data(op->attrib);
+ chrc->authorized = true;
+ } else {
+ desc = gatt_db_attribute_get_user_data(op->attrib);
+ desc->authorized = true;
+ }
+ }
+
dbus_message_iter_init(message, &iter);
if (dbus_message_iter_has_next(&iter)) {
/*
@@ -2045,9 +2064,10 @@ static struct pending_op *pending_write_new(struct btd_device *device,
struct queue *owner_queue,
struct gatt_db_attribute *attrib,
unsigned int id,
- const uint8_t *value,
- size_t len,
- uint16_t offset, uint8_t link_type)
+ const uint8_t *value, size_t len,
+ uint16_t offset, uint8_t link_type,
+ bool is_characteristic,
+ bool authorize)
{
struct pending_op *op;
@@ -2062,6 +2082,8 @@ static struct pending_op *pending_write_new(struct btd_device *device,
op->id = id;
op->offset = offset;
op->link_type = link_type;
+ op->is_characteristic = is_characteristic;
+ op->authorize = authorize;
queue_push_tail(owner_queue, op);
return op;
@@ -2073,12 +2095,15 @@ static struct pending_op *send_write(struct btd_device *device,
struct queue *owner_queue,
unsigned int id,
const uint8_t *value, size_t len,
- uint16_t offset, uint8_t link_type)
+ uint16_t offset, uint8_t link_type,
+ bool is_characteristic,
+ bool authorize)
{
struct pending_op *op;
op = pending_write_new(device, owner_queue, attrib, id, value, len,
- offset, link_type);
+ offset, link_type, is_characteristic,
+ authorize);
if (g_dbus_proxy_method_call(proxy, "WriteValue", write_setup_cb,
owner_queue ? write_reply_cb : NULL,
@@ -2191,7 +2216,7 @@ static void acquire_write_reply(DBusMessage *message, void *user_data)
retry:
send_write(op->device, op->attrib, chrc->proxy, NULL, op->id,
op->data.iov_base, op->data.iov_len, 0,
- op->link_type);
+ op->link_type, false, false);
}
static void acquire_write_setup(DBusMessageIter *iter, void *user_data)
@@ -2229,7 +2254,7 @@ static struct pending_op *acquire_write(struct external_chrc *chrc,
struct pending_op *op;
op = pending_write_new(device, NULL, attrib, id, value, len, 0,
- link_type);
+ link_type, false, false);
if (g_dbus_proxy_method_call(chrc->proxy, "AcquireWrite",
acquire_write_setup,
@@ -2520,6 +2545,7 @@ static void desc_write_cb(struct gatt_db_attribute *attrib,
{
struct external_desc *desc = user_data;
struct btd_device *device;
+ DBusMessageIter iter;
if (desc->attrib != attrib) {
error("Read callback called with incorrect attribute");
@@ -2532,8 +2558,25 @@ static void desc_write_cb(struct gatt_db_attribute *attrib,
goto fail;
}
+ if (opcode == BT_ATT_OP_PREP_WRITE_REQ) {
+ if (!desc->authorized && g_dbus_proxy_get_property(desc->proxy,
+ "Authorize", &iter))
+ send_write(device, attrib, desc->proxy,
+ desc->pending_writes, id, value, len,
+ offset, bt_att_get_link_type(att),
+ false, true);
+ else
+ gatt_db_attribute_write_result(attrib, id, 0);
+
+ return;
+ }
+
+ if (opcode == BT_ATT_OP_EXEC_WRITE_REQ)
+ desc->authorized = false;
+
if (send_write(device, attrib, desc->proxy, desc->pending_writes, id,
- value, len, offset, bt_att_get_link_type(att)))
+ value, len, offset, bt_att_get_link_type(att), false,
+ false))
return;
fail:
@@ -2614,6 +2657,26 @@ static void chrc_write_cb(struct gatt_db_attribute *attrib,
goto fail;
}
+ if (!(chrc->props & BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP))
+ queue = chrc->pending_writes;
+ else
+ queue = NULL;
+
+ if (opcode == BT_ATT_OP_PREP_WRITE_REQ) {
+ if (!chrc->authorized && g_dbus_proxy_get_property(chrc->proxy,
+ "Authorize", &iter))
+ send_write(device, attrib, chrc->proxy, queue,
+ id, value, len, offset,
+ bt_att_get_link_type(att), true, true);
+ else
+ gatt_db_attribute_write_result(attrib, id, 0);
+
+ return;
+ }
+
+ if (id == BT_ATT_OP_EXEC_WRITE_REQ)
+ chrc->authorized = false;
+
if (chrc->write_io) {
if (pipe_io_send(chrc->write_io, value, len) < 0) {
error("Unable to write: %s", strerror(errno));
@@ -2630,13 +2693,8 @@ static void chrc_write_cb(struct gatt_db_attribute *attrib,
return;
}
- if (!(chrc->props & BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP))
- queue = chrc->pending_writes;
- else
- queue = NULL;
-
if (send_write(device, attrib, chrc->proxy, queue, id, value, len,
- offset, bt_att_get_link_type(att)))
+ offset, bt_att_get_link_type(att), false, false))
return;
fail:
diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index 4b554f665..cdade76f8 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -1208,6 +1208,45 @@ static bool store_prep_data(struct bt_gatt_server *server,
return prep_data_new(server, handle, offset, length, value);
}
+struct prep_write_complete_data {
+ void *pdu;
+ uint16_t length;
+ struct bt_gatt_server *server;
+};
+
+static void prep_write_complete_cb(struct gatt_db_attribute *attr, int err,
+ void *user_data)
+{
+ struct prep_write_complete_data *pwcd = user_data;
+ uint16_t handle = 0;
+ uint16_t offset;
+
+ handle = get_le16(pwcd->pdu);
+
+ if (err) {
+ bt_att_send_error_rsp(pwcd->server->att,
+ BT_ATT_OP_PREP_WRITE_REQ, handle, err);
+ free(pwcd->pdu);
+ free(pwcd);
+
+ return;
+ }
+
+ offset = get_le16(pwcd->pdu + 2);
+
+ if (!store_prep_data(pwcd->server, handle, offset, pwcd->length - 4,
+ &((uint8_t *) pwcd->pdu)[4]))
+ bt_att_send_error_rsp(pwcd->server->att,
+ BT_ATT_OP_PREP_WRITE_RSP, handle,
+ BT_ATT_ERROR_INSUFFICIENT_RESOURCES);
+
+ bt_att_send(pwcd->server->att, BT_ATT_OP_PREP_WRITE_RSP, pwcd->pdu,
+ pwcd->length, NULL, NULL, NULL);
+
+ free(pwcd->pdu);
+ free(pwcd);
+}
+
static void prep_write_cb(uint8_t opcode, const void *pdu,
uint16_t length, void *user_data)
{
@@ -1215,7 +1254,8 @@ static void prep_write_cb(uint8_t opcode, const void *pdu,
uint16_t handle = 0;
uint16_t offset;
struct gatt_db_attribute *attr;
- uint8_t ecode;
+ struct prep_write_complete_data *pwcd;
+ uint8_t ecode, status;
if (length < 4) {
ecode = BT_ATT_ERROR_INVALID_PDU;
@@ -1245,15 +1285,21 @@ static void prep_write_cb(uint8_t opcode, const void *pdu,
if (ecode)
goto error;
- if (!store_prep_data(server, handle, offset, length - 4,
- &((uint8_t *) pdu)[4])) {
- ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
- goto error;
- }
+ pwcd = new0(struct prep_write_complete_data, 1);
+ pwcd->pdu = malloc(length);
+ memcpy(pwcd->pdu, pdu, length);
+ pwcd->length = length;
+ pwcd->server = server;
- bt_att_send(server->att, BT_ATT_OP_PREP_WRITE_RSP, pdu, length, NULL,
- NULL, NULL);
- return;
+ status = gatt_db_attribute_write(attr, offset, NULL, 0,
+ BT_ATT_OP_PREP_WRITE_REQ,
+ server->att,
+ prep_write_complete_cb, pwcd);
+
+ if (status)
+ return;
+
+ ecode = BT_ATT_ERROR_UNLIKELY;
error:
bt_att_send_error_rsp(server->att, opcode, handle, ecode);
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH BlueZ v2 4/5] shared/gatt-server: Request authorization for prepare writes.
2018-05-16 13:52 ` [PATCH BlueZ v2 4/5] shared/gatt-server: Request authorization for prepare writes Grzegorz Kolodziejczyk
@ 2018-05-21 9:51 ` Luiz Augusto von Dentz
2018-05-21 9:58 ` Grzegorz Kołodziejczyk
0 siblings, 1 reply; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2018-05-21 9:51 UTC (permalink / raw)
To: Grzegorz Kolodziejczyk; +Cc: linux-bluetooth@vger.kernel.org
Hi Grzegorz,
On Wed, May 16, 2018 at 4:52 PM, Grzegorz Kolodziejczyk
<grzegorz.kolodziejczyk@codecoup.pl> wrote:
> This patch adds gatt-server possibility to request authorization from
> application if needed and previously wasn't authorized. Authorization is
> requested by sending message with set prepare write authorization reqest
> to client.
> ---
> src/gatt-database.c | 86 ++++++++++++++++++++++++++++++++++++++++--------
> src/shared/gatt-server.c | 64 ++++++++++++++++++++++++++++++-----
> 2 files changed, 127 insertions(+), 23 deletions(-)
>
> diff --git a/src/gatt-database.c b/src/gatt-database.c
> index 0ac5b75b0..3326b40db 100644
> --- a/src/gatt-database.c
> +++ b/src/gatt-database.c
> @@ -133,6 +133,7 @@ struct external_chrc {
> struct queue *pending_reads;
> struct queue *pending_writes;
> unsigned int ntfy_cnt;
> + bool authorized;
> };
>
> struct external_desc {
> @@ -144,6 +145,7 @@ struct external_desc {
> bool handled;
> struct queue *pending_reads;
> struct queue *pending_writes;
> + bool authorized;
> };
>
> struct pending_op {
> @@ -154,6 +156,8 @@ struct pending_op {
> struct gatt_db_attribute *attrib;
> struct queue *owner_queue;
> struct iovec data;
> + bool is_characteristic;
> + bool authorize;
> };
>
> struct notify {
> @@ -1937,6 +1941,9 @@ static void append_options(DBusMessageIter *iter, void *user_data)
> &op->offset);
> if (link)
> dict_append_entry(iter, "link", DBUS_TYPE_STRING, &link);
> + if (op->authorize)
> + dict_append_entry(iter, "authorize", DBUS_TYPE_BOOLEAN,
> + &op->authorize);
> }
>
> static void read_setup_cb(DBusMessageIter *iter, void *user_data)
> @@ -2008,6 +2015,8 @@ static void write_setup_cb(DBusMessageIter *iter, void *user_data)
> static void write_reply_cb(DBusMessage *message, void *user_data)
> {
> struct pending_op *op = user_data;
> + struct external_chrc *chrc;
> + struct external_desc *desc;
> DBusError err;
> DBusMessageIter iter;
> uint8_t ecode = 0;
> @@ -2027,6 +2036,16 @@ static void write_reply_cb(DBusMessage *message, void *user_data)
> goto done;
> }
>
> + if (op->authorize) {
> + if (op->is_characteristic) {
> + chrc = gatt_db_attribute_get_user_data(op->attrib);
> + chrc->authorized = true;
> + } else {
> + desc = gatt_db_attribute_get_user_data(op->attrib);
> + desc->authorized = true;
> + }
> + }
> +
> dbus_message_iter_init(message, &iter);
> if (dbus_message_iter_has_next(&iter)) {
> /*
> @@ -2045,9 +2064,10 @@ static struct pending_op *pending_write_new(struct btd_device *device,
> struct queue *owner_queue,
> struct gatt_db_attribute *attrib,
> unsigned int id,
> - const uint8_t *value,
> - size_t len,
> - uint16_t offset, uint8_t link_type)
> + const uint8_t *value, size_t len,
> + uint16_t offset, uint8_t link_type,
> + bool is_characteristic,
> + bool authorize)
> {
> struct pending_op *op;
>
> @@ -2062,6 +2082,8 @@ static struct pending_op *pending_write_new(struct btd_device *device,
> op->id = id;
> op->offset = offset;
> op->link_type = link_type;
> + op->is_characteristic = is_characteristic;
> + op->authorize = authorize;
> queue_push_tail(owner_queue, op);
>
> return op;
> @@ -2073,12 +2095,15 @@ static struct pending_op *send_write(struct btd_device *device,
> struct queue *owner_queue,
> unsigned int id,
> const uint8_t *value, size_t len,
> - uint16_t offset, uint8_t link_type)
> + uint16_t offset, uint8_t link_type,
> + bool is_characteristic,
> + bool authorize)
> {
> struct pending_op *op;
>
> op = pending_write_new(device, owner_queue, attrib, id, value, len,
> - offset, link_type);
> + offset, link_type, is_characteristic,
> + authorize);
>
> if (g_dbus_proxy_method_call(proxy, "WriteValue", write_setup_cb,
> owner_queue ? write_reply_cb : NULL,
> @@ -2191,7 +2216,7 @@ static void acquire_write_reply(DBusMessage *message, void *user_data)
> retry:
> send_write(op->device, op->attrib, chrc->proxy, NULL, op->id,
> op->data.iov_base, op->data.iov_len, 0,
> - op->link_type);
> + op->link_type, false, false);
> }
>
> static void acquire_write_setup(DBusMessageIter *iter, void *user_data)
> @@ -2229,7 +2254,7 @@ static struct pending_op *acquire_write(struct external_chrc *chrc,
> struct pending_op *op;
>
> op = pending_write_new(device, NULL, attrib, id, value, len, 0,
> - link_type);
> + link_type, false, false);
>
> if (g_dbus_proxy_method_call(chrc->proxy, "AcquireWrite",
> acquire_write_setup,
> @@ -2520,6 +2545,7 @@ static void desc_write_cb(struct gatt_db_attribute *attrib,
> {
> struct external_desc *desc = user_data;
> struct btd_device *device;
> + DBusMessageIter iter;
>
> if (desc->attrib != attrib) {
> error("Read callback called with incorrect attribute");
> @@ -2532,8 +2558,25 @@ static void desc_write_cb(struct gatt_db_attribute *attrib,
> goto fail;
> }
>
> + if (opcode == BT_ATT_OP_PREP_WRITE_REQ) {
> + if (!desc->authorized && g_dbus_proxy_get_property(desc->proxy,
> + "Authorize", &iter))
> + send_write(device, attrib, desc->proxy,
> + desc->pending_writes, id, value, len,
> + offset, bt_att_get_link_type(att),
> + false, true);
> + else
> + gatt_db_attribute_write_result(attrib, id, 0);
> +
> + return;
> + }
> +
> + if (opcode == BT_ATT_OP_EXEC_WRITE_REQ)
> + desc->authorized = false;
> +
> if (send_write(device, attrib, desc->proxy, desc->pending_writes, id,
> - value, len, offset, bt_att_get_link_type(att)))
> + value, len, offset, bt_att_get_link_type(att), false,
> + false))
> return;
>
> fail:
> @@ -2614,6 +2657,26 @@ static void chrc_write_cb(struct gatt_db_attribute *attrib,
> goto fail;
> }
>
> + if (!(chrc->props & BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP))
> + queue = chrc->pending_writes;
> + else
> + queue = NULL;
> +
> + if (opcode == BT_ATT_OP_PREP_WRITE_REQ) {
> + if (!chrc->authorized && g_dbus_proxy_get_property(chrc->proxy,
> + "Authorize", &iter))
> + send_write(device, attrib, chrc->proxy, queue,
> + id, value, len, offset,
> + bt_att_get_link_type(att), true, true);
> + else
> + gatt_db_attribute_write_result(attrib, id, 0);
> +
> + return;
> + }
> +
> + if (id == BT_ATT_OP_EXEC_WRITE_REQ)
> + chrc->authorized = false;
> +
I guess you meant opcode == BT_ATT_OP_EXEC_WRITE_REQ.
> if (chrc->write_io) {
> if (pipe_io_send(chrc->write_io, value, len) < 0) {
> error("Unable to write: %s", strerror(errno));
> @@ -2630,13 +2693,8 @@ static void chrc_write_cb(struct gatt_db_attribute *attrib,
> return;
> }
>
> - if (!(chrc->props & BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP))
> - queue = chrc->pending_writes;
> - else
> - queue = NULL;
> -
> if (send_write(device, attrib, chrc->proxy, queue, id, value, len,
> - offset, bt_att_get_link_type(att)))
> + offset, bt_att_get_link_type(att), false, false))
> return;
>
> fail:
> diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
> index 4b554f665..cdade76f8 100644
> --- a/src/shared/gatt-server.c
> +++ b/src/shared/gatt-server.c
> @@ -1208,6 +1208,45 @@ static bool store_prep_data(struct bt_gatt_server *server,
> return prep_data_new(server, handle, offset, length, value);
> }
>
> +struct prep_write_complete_data {
> + void *pdu;
> + uint16_t length;
> + struct bt_gatt_server *server;
> +};
> +
> +static void prep_write_complete_cb(struct gatt_db_attribute *attr, int err,
> + void *user_data)
> +{
> + struct prep_write_complete_data *pwcd = user_data;
> + uint16_t handle = 0;
> + uint16_t offset;
> +
> + handle = get_le16(pwcd->pdu);
> +
> + if (err) {
> + bt_att_send_error_rsp(pwcd->server->att,
> + BT_ATT_OP_PREP_WRITE_REQ, handle, err);
> + free(pwcd->pdu);
> + free(pwcd);
> +
> + return;
> + }
> +
> + offset = get_le16(pwcd->pdu + 2);
> +
> + if (!store_prep_data(pwcd->server, handle, offset, pwcd->length - 4,
> + &((uint8_t *) pwcd->pdu)[4]))
> + bt_att_send_error_rsp(pwcd->server->att,
> + BT_ATT_OP_PREP_WRITE_RSP, handle,
> + BT_ATT_ERROR_INSUFFICIENT_RESOURCES);
> +
> + bt_att_send(pwcd->server->att, BT_ATT_OP_PREP_WRITE_RSP, pwcd->pdu,
> + pwcd->length, NULL, NULL, NULL);
> +
> + free(pwcd->pdu);
> + free(pwcd);
> +}
> +
> static void prep_write_cb(uint8_t opcode, const void *pdu,
> uint16_t length, void *user_data)
> {
> @@ -1215,7 +1254,8 @@ static void prep_write_cb(uint8_t opcode, const void *pdu,
> uint16_t handle = 0;
> uint16_t offset;
> struct gatt_db_attribute *attr;
> - uint8_t ecode;
> + struct prep_write_complete_data *pwcd;
> + uint8_t ecode, status;
>
> if (length < 4) {
> ecode = BT_ATT_ERROR_INVALID_PDU;
> @@ -1245,15 +1285,21 @@ static void prep_write_cb(uint8_t opcode, const void *pdu,
> if (ecode)
> goto error;
>
> - if (!store_prep_data(server, handle, offset, length - 4,
> - &((uint8_t *) pdu)[4])) {
> - ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
> - goto error;
> - }
> + pwcd = new0(struct prep_write_complete_data, 1);
> + pwcd->pdu = malloc(length);
> + memcpy(pwcd->pdu, pdu, length);
> + pwcd->length = length;
> + pwcd->server = server;
>
> - bt_att_send(server->att, BT_ATT_OP_PREP_WRITE_RSP, pdu, length, NULL,
> - NULL, NULL);
> - return;
> + status = gatt_db_attribute_write(attr, offset, NULL, 0,
> + BT_ATT_OP_PREP_WRITE_REQ,
> + server->att,
> + prep_write_complete_cb, pwcd);
> +
> + if (status)
> + return;
> +
> + ecode = BT_ATT_ERROR_UNLIKELY;
>
> error:
> bt_att_send_error_rsp(server->att, opcode, handle, ecode);
> --
> 2.13.6
>
> --
> 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] 10+ messages in thread* Re: [PATCH BlueZ v2 4/5] shared/gatt-server: Request authorization for prepare writes.
2018-05-21 9:51 ` Luiz Augusto von Dentz
@ 2018-05-21 9:58 ` Grzegorz Kołodziejczyk
0 siblings, 0 replies; 10+ messages in thread
From: Grzegorz Kołodziejczyk @ 2018-05-21 9:58 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
pon., 21 maj 2018 o 11:51 Luiz Augusto von Dentz <luiz.dentz@gmail.com>
napisa=C5=82(a):
> Hi Grzegorz,
> On Wed, May 16, 2018 at 4:52 PM, Grzegorz Kolodziejczyk
> <grzegorz.kolodziejczyk@codecoup.pl> wrote:
> > This patch adds gatt-server possibility to request authorization from
> > application if needed and previously wasn't authorized. Authorization i=
s
> > requested by sending message with set prepare write authorization reqes=
t
> > to client.
> > ---
> > src/gatt-database.c | 86
++++++++++++++++++++++++++++++++++++++++--------
> > src/shared/gatt-server.c | 64 ++++++++++++++++++++++++++++++-----
> > 2 files changed, 127 insertions(+), 23 deletions(-)
> >
> > diff --git a/src/gatt-database.c b/src/gatt-database.c
> > index 0ac5b75b0..3326b40db 100644
> > --- a/src/gatt-database.c
> > +++ b/src/gatt-database.c
> > @@ -133,6 +133,7 @@ struct external_chrc {
> > struct queue *pending_reads;
> > struct queue *pending_writes;
> > unsigned int ntfy_cnt;
> > + bool authorized;
> > };
> >
> > struct external_desc {
> > @@ -144,6 +145,7 @@ struct external_desc {
> > bool handled;
> > struct queue *pending_reads;
> > struct queue *pending_writes;
> > + bool authorized;
> > };
> >
> > struct pending_op {
> > @@ -154,6 +156,8 @@ struct pending_op {
> > struct gatt_db_attribute *attrib;
> > struct queue *owner_queue;
> > struct iovec data;
> > + bool is_characteristic;
> > + bool authorize;
> > };
> >
> > struct notify {
> > @@ -1937,6 +1941,9 @@ static void append_options(DBusMessageIter *iter,
void *user_data)
> > &op->offset);
> > if (link)
> > dict_append_entry(iter, "link", DBUS_TYPE_STRING,
&link);
> > + if (op->authorize)
> > + dict_append_entry(iter, "authorize", DBUS_TYPE_BOOLEAN,
> > + &op->authorize)=
;
> > }
> >
> > static void read_setup_cb(DBusMessageIter *iter, void *user_data)
> > @@ -2008,6 +2015,8 @@ static void write_setup_cb(DBusMessageIter *iter,
void *user_data)
> > static void write_reply_cb(DBusMessage *message, void *user_data)
> > {
> > struct pending_op *op =3D user_data;
> > + struct external_chrc *chrc;
> > + struct external_desc *desc;
> > DBusError err;
> > DBusMessageIter iter;
> > uint8_t ecode =3D 0;
> > @@ -2027,6 +2036,16 @@ static void write_reply_cb(DBusMessage *message,
void *user_data)
> > goto done;
> > }
> >
> > + if (op->authorize) {
> > + if (op->is_characteristic) {
> > + chrc =3D
gatt_db_attribute_get_user_data(op->attrib);
> > + chrc->authorized =3D true;
> > + } else {
> > + desc =3D
gatt_db_attribute_get_user_data(op->attrib);
> > + desc->authorized =3D true;
> > + }
> > + }
> > +
> > dbus_message_iter_init(message, &iter);
> > if (dbus_message_iter_has_next(&iter)) {
> > /*
> > @@ -2045,9 +2064,10 @@ static struct pending_op
*pending_write_new(struct btd_device *device,
> > struct queue *owner_queue,
> > struct gatt_db_attribute
*attrib,
> > unsigned int id,
> > - const uint8_t *value,
> > - size_t len,
> > - uint16_t offset, uint8_t
link_type)
> > + const uint8_t *value, size_t
len,
> > + uint16_t offset, uint8_t
link_type,
> > + bool is_characteristic,
> > + bool authorize)
> > {
> > struct pending_op *op;
> >
> > @@ -2062,6 +2082,8 @@ static struct pending_op
*pending_write_new(struct btd_device *device,
> > op->id =3D id;
> > op->offset =3D offset;
> > op->link_type =3D link_type;
> > + op->is_characteristic =3D is_characteristic;
> > + op->authorize =3D authorize;
> > queue_push_tail(owner_queue, op);
> >
> > return op;
> > @@ -2073,12 +2095,15 @@ static struct pending_op *send_write(struct
btd_device *device,
> > struct queue *owner_queue,
> > unsigned int id,
> > const uint8_t *value, size_t
len,
> > - uint16_t offset, uint8_t
link_type)
> > + uint16_t offset, uint8_t
link_type,
> > + bool is_characteristic,
> > + bool authorize)
> > {
> > struct pending_op *op;
> >
> > op =3D pending_write_new(device, owner_queue, attrib, id, value=
,
len,
> > - offset,
link_type);
> > + offset, link_type,
is_characteristic,
> > + authorize);
> >
> > if (g_dbus_proxy_method_call(proxy, "WriteValue",
write_setup_cb,
> > owner_queue ? write_reply_cb :
NULL,
> > @@ -2191,7 +2216,7 @@ static void acquire_write_reply(DBusMessage
*message, void *user_data)
> > retry:
> > send_write(op->device, op->attrib, chrc->proxy, NULL, op->id,
> > op->data.iov_base, op->data.iov_len, 0,
> > - op->link_type);
> > + op->link_type, false, false);
> > }
> >
> > static void acquire_write_setup(DBusMessageIter *iter, void *user_data=
)
> > @@ -2229,7 +2254,7 @@ static struct pending_op *acquire_write(struct
external_chrc *chrc,
> > struct pending_op *op;
> >
> > op =3D pending_write_new(device, NULL, attrib, id, value, len, =
0,
> > - link_type);
> > + link_type, false,
false);
> >
> > if (g_dbus_proxy_method_call(chrc->proxy, "AcquireWrite",
> > acquire_write_setup,
> > @@ -2520,6 +2545,7 @@ static void desc_write_cb(struct
gatt_db_attribute *attrib,
> > {
> > struct external_desc *desc =3D user_data;
> > struct btd_device *device;
> > + DBusMessageIter iter;
> >
> > if (desc->attrib !=3D attrib) {
> > error("Read callback called with incorrect attribute");
> > @@ -2532,8 +2558,25 @@ static void desc_write_cb(struct
gatt_db_attribute *attrib,
> > goto fail;
> > }
> >
> > + if (opcode =3D=3D BT_ATT_OP_PREP_WRITE_REQ) {
> > + if (!desc->authorized &&
g_dbus_proxy_get_property(desc->proxy,
> > + "Authorize",
&iter))
> > + send_write(device, attrib, desc->proxy,
> > + desc->pending_writes, id,
value, len,
> > + offset,
bt_att_get_link_type(att),
> > + false, true);
> > + else
> > + gatt_db_attribute_write_result(attrib, id, 0);
> > +
> > + return;
> > + }
> > +
> > + if (opcode =3D=3D BT_ATT_OP_EXEC_WRITE_REQ)
> > + desc->authorized =3D false;
> > +
> > if (send_write(device, attrib, desc->proxy,
desc->pending_writes, id,
> > - value, len, offset,
bt_att_get_link_type(att)))
> > + value, len, offset, bt_att_get_link_type(att),
false,
> > + false))
> > return;
> >
> > fail:
> > @@ -2614,6 +2657,26 @@ static void chrc_write_cb(struct
gatt_db_attribute *attrib,
> > goto fail;
> > }
> >
> > + if (!(chrc->props & BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP))
> > + queue =3D chrc->pending_writes;
> > + else
> > + queue =3D NULL;
> > +
> > + if (opcode =3D=3D BT_ATT_OP_PREP_WRITE_REQ) {
> > + if (!chrc->authorized &&
g_dbus_proxy_get_property(chrc->proxy,
> > + "Authorize",
&iter))
> > + send_write(device, attrib, chrc->proxy, queue,
> > + id, value, len, offset,
> > + bt_att_get_link_type(att),
true, true);
> > + else
> > + gatt_db_attribute_write_result(attrib, id, 0);
> > +
> > + return;
> > + }
> > +
> > + if (id =3D=3D BT_ATT_OP_EXEC_WRITE_REQ)
> > + chrc->authorized =3D false;
> > +
> I guess you meant opcode =3D=3D BT_ATT_OP_EXEC_WRITE_REQ.
Right - good catch.
> > if (chrc->write_io) {
> > if (pipe_io_send(chrc->write_io, value, len) < 0) {
> > error("Unable to write: %s", strerror(errno));
> > @@ -2630,13 +2693,8 @@ static void chrc_write_cb(struct
gatt_db_attribute *attrib,
> > return;
> > }
> >
> > - if (!(chrc->props & BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP))
> > - queue =3D chrc->pending_writes;
> > - else
> > - queue =3D NULL;
> > -
> > if (send_write(device, attrib, chrc->proxy, queue, id, value,
len,
> > - offset,
bt_att_get_link_type(att)))
> > + offset, bt_att_get_link_type(att), false,
false))
> > return;
> >
> > fail:
> > diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
> > index 4b554f665..cdade76f8 100644
> > --- a/src/shared/gatt-server.c
> > +++ b/src/shared/gatt-server.c
> > @@ -1208,6 +1208,45 @@ static bool store_prep_data(struct
bt_gatt_server *server,
> > return prep_data_new(server, handle, offset, length, value);
> > }
> >
> > +struct prep_write_complete_data {
> > + void *pdu;
> > + uint16_t length;
> > + struct bt_gatt_server *server;
> > +};
> > +
> > +static void prep_write_complete_cb(struct gatt_db_attribute *attr, int
err,
> > + void
*user_data)
> > +{
> > + struct prep_write_complete_data *pwcd =3D user_data;
> > + uint16_t handle =3D 0;
> > + uint16_t offset;
> > +
> > + handle =3D get_le16(pwcd->pdu);
> > +
> > + if (err) {
> > + bt_att_send_error_rsp(pwcd->server->att,
> > + BT_ATT_OP_PREP_WRITE_REQ,
handle, err);
> > + free(pwcd->pdu);
> > + free(pwcd);
> > +
> > + return;
> > + }
> > +
> > + offset =3D get_le16(pwcd->pdu + 2);
> > +
> > + if (!store_prep_data(pwcd->server, handle, offset, pwcd->length
- 4,
> > + &((uint8_t *)
pwcd->pdu)[4]))
> > + bt_att_send_error_rsp(pwcd->server->att,
> > + BT_ATT_OP_PREP_WRITE_RSP,
handle,
> > +
BT_ATT_ERROR_INSUFFICIENT_RESOURCES);
> > +
> > + bt_att_send(pwcd->server->att, BT_ATT_OP_PREP_WRITE_RSP,
pwcd->pdu,
> > + pwcd->length, NULL,
NULL, NULL);
> > +
> > + free(pwcd->pdu);
> > + free(pwcd);
> > +}
> > +
> > static void prep_write_cb(uint8_t opcode, const void *pdu,
> > uint16_t length, void
*user_data)
> > {
> > @@ -1215,7 +1254,8 @@ static void prep_write_cb(uint8_t opcode, const
void *pdu,
> > uint16_t handle =3D 0;
> > uint16_t offset;
> > struct gatt_db_attribute *attr;
> > - uint8_t ecode;
> > + struct prep_write_complete_data *pwcd;
> > + uint8_t ecode, status;
> >
> > if (length < 4) {
> > ecode =3D BT_ATT_ERROR_INVALID_PDU;
> > @@ -1245,15 +1285,21 @@ static void prep_write_cb(uint8_t opcode, const
void *pdu,
> > if (ecode)
> > goto error;
> >
> > - if (!store_prep_data(server, handle, offset, length - 4,
> > - &((uint8_t *) pdu)[4]))
{
> > - ecode =3D BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
> > - goto error;
> > - }
> > + pwcd =3D new0(struct prep_write_complete_data, 1);
> > + pwcd->pdu =3D malloc(length);
> > + memcpy(pwcd->pdu, pdu, length);
> > + pwcd->length =3D length;
> > + pwcd->server =3D server;
> >
> > - bt_att_send(server->att, BT_ATT_OP_PREP_WRITE_RSP, pdu, length,
NULL,
> > - NULL,
NULL);
> > - return;
> > + status =3D gatt_db_attribute_write(attr, offset, NULL, 0,
> > +
BT_ATT_OP_PREP_WRITE_REQ,
> > + server->att,
> > + prep_write_complete_cb,
pwcd);
> > +
> > + if (status)
> > + return;
> > +
> > + ecode =3D BT_ATT_ERROR_UNLIKELY;
> >
> > error:
> > bt_att_send_error_rsp(server->att, opcode, handle, ecode);
> > --
> > 2.13.6
> >
> > --
> > 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
Regards,
Grzegorz
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH BlueZ v2 5/5] client: Add authorized property handling to characteristic attribute
2018-05-16 13:52 [PATCH BlueZ v2 1/5] client: Add missing duplicated string free Grzegorz Kolodziejczyk
` (2 preceding siblings ...)
2018-05-16 13:52 ` [PATCH BlueZ v2 4/5] shared/gatt-server: Request authorization for prepare writes Grzegorz Kolodziejczyk
@ 2018-05-16 13:52 ` Grzegorz Kolodziejczyk
2018-05-21 9:58 ` Luiz Augusto von Dentz
2018-05-21 8:38 ` [PATCH BlueZ v2 1/5] client: Add missing duplicated string free Luiz Augusto von Dentz
4 siblings, 1 reply; 10+ messages in thread
From: Grzegorz Kolodziejczyk @ 2018-05-16 13:52 UTC (permalink / raw)
To: linux-bluetooth
This patch adds handling of authorized property to bluetoothctl.
---
client/gatt.c | 162 ++++++++++++++++++++++++++++++++++++++++------------------
1 file changed, 112 insertions(+), 50 deletions(-)
diff --git a/client/gatt.c b/client/gatt.c
index 1b4e713ef..6a9ba5687 100644
--- a/client/gatt.c
+++ b/client/gatt.c
@@ -80,7 +80,8 @@ struct chrc {
struct io *write_io;
struct io *notify_io;
bool authorization_req;
- bool authorized;
+ bool authorized_read;
+ bool authorized_write;
};
struct service {
@@ -1432,6 +1433,30 @@ static gboolean chrc_notify_acquired_exists(const GDBusPropertyTable *property,
return FALSE;
}
+static gboolean chrc_get_authorize(const GDBusPropertyTable *property,
+ DBusMessageIter *iter, void *data)
+{
+ struct chrc *chrc = data;
+ dbus_bool_t value;
+
+ value = chrc->authorization_req ? TRUE : FALSE;
+
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &value);
+
+ return TRUE;
+}
+
+static gboolean chrc_get_authorize_exists(const GDBusPropertyTable *property,
+ void *data)
+{
+ struct chrc *chrc = data;
+
+ if (chrc->authorization_req)
+ return TRUE;
+
+ return FALSE;
+}
+
static const GDBusPropertyTable chrc_properties[] = {
{ "UUID", "s", chrc_get_uuid, NULL, NULL },
{ "Service", "o", chrc_get_service, NULL, NULL },
@@ -1442,6 +1467,8 @@ static const GDBusPropertyTable chrc_properties[] = {
chrc_write_acquired_exists },
{ "NotifyAcquired", "b", chrc_get_notify_acquired, NULL,
chrc_notify_acquired_exists },
+ { "Authorize", "b", chrc_get_authorize, NULL,
+ chrc_get_authorize_exists },
{ }
};
@@ -1460,7 +1487,8 @@ static const char *path_to_address(const char *path)
}
static int parse_options(DBusMessageIter *iter, uint16_t *offset, uint16_t *mtu,
- char **device, char **link)
+ char **device, char **link,
+ bool *authorize)
{
DBusMessageIter dict;
@@ -1501,6 +1529,12 @@ static int parse_options(DBusMessageIter *iter, uint16_t *offset, uint16_t *mtu,
return -EINVAL;
if (link)
dbus_message_iter_get_basic(&value, link);
+ } else if (strcasecmp(key, "authorize") == 0) {
+ if (var != DBUS_TYPE_BOOLEAN)
+ return -EINVAL;
+ if (authorize)
+ dbus_message_iter_get_basic(&value,
+ authorize);
}
dbus_message_iter_next(&dict);
@@ -1554,7 +1588,7 @@ static void authorize_read_response(const char *input, void *user_data)
reply = read_value(pending_message, &chrc->value[aad->offset],
chrc->value_len - aad->offset);
- chrc->authorized = true;
+ chrc->authorized_read = true;
g_dbus_send_message(aad->conn, reply);
@@ -1578,18 +1612,15 @@ static DBusMessage *chrc_read_value(DBusConnection *conn, DBusMessage *msg,
dbus_message_iter_init(msg, &iter);
- if (parse_options(&iter, &offset, NULL, &device, &link))
+ if (parse_options(&iter, &offset, NULL, &device, &link, NULL))
return g_dbus_create_error(msg,
"org.bluez.Error.InvalidArguments",
NULL);
bt_shell_printf("ReadValue: %s offset %u link %s\n",
- path_to_address(device), offset, link);
+ path_to_address(device), offset, link);
- if (chrc->authorization_req && offset == 0)
- chrc->authorized = false;
-
- if (chrc->authorization_req && !chrc->authorized) {
+ if (chrc->authorization_req && !chrc->authorized_read) {
struct authorize_attribute_data *aad;
aad = g_new0(struct authorize_attribute_data, 1);
@@ -1616,33 +1647,31 @@ static DBusMessage *chrc_read_value(DBusConnection *conn, DBusMessage *msg,
return read_value(msg, &chrc->value[offset], chrc->value_len - offset);
}
-static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len,
- int max_len)
+static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len)
{
DBusMessageIter array;
- uint16_t offset = 0;
- uint8_t *read_value;
- int read_len;
if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_ARRAY)
return -EINVAL;
dbus_message_iter_recurse(iter, &array);
- dbus_message_iter_get_fixed_array(&array, &read_value, &read_len);
+ dbus_message_iter_get_fixed_array(&array, value, len);
- dbus_message_iter_next(iter);
- if (parse_options(iter, &offset, NULL, NULL, NULL))
- return -EINVAL;
+ return 0;
+}
- if ((offset + read_len) > max_len)
+static int write_value(int *dst_len, uint8_t **dst_value, uint8_t *src_val,
+ int src_len, uint16_t offset, uint16_t max_len)
+{
+ if ((offset + src_len) > max_len)
return -EOVERFLOW;
- if ((offset + read_len) > *len) {
- *len = offset + read_len;
- *value = g_realloc(*value, *len);
+ if ((offset + src_len) != *dst_len) {
+ *dst_len = offset + src_len;
+ *dst_value = g_realloc(*dst_value, *dst_len);
}
- memcpy(*value + offset, read_value, read_len);
+ memcpy(*dst_value + offset, src_val, src_len);
return 0;
}
@@ -1651,12 +1680,26 @@ static void authorize_write_response(const char *input, void *user_data)
{
struct authorize_attribute_data *aad = user_data;
struct chrc *chrc = aad->attribute;
+ bool authorize = false;
DBusMessageIter iter;
DBusMessage *reply;
+ int value_len;
+ uint8_t *value;
char *err;
- int errsv;
dbus_message_iter_init(pending_message, &iter);
+ if (parse_value_arg(&iter, &value, &value_len)) {
+ err = "org.bluez.Error.InvalidArguments";
+
+ goto error;
+ }
+
+ dbus_message_iter_next(&iter);
+ if (parse_options(&iter, NULL, NULL, NULL, NULL, &authorize)) {
+ err = "org.bluez.Error.InvalidArguments";
+
+ goto error;
+ }
if (!strcmp(input, "no")) {
err = "org.bluez.Error.NotAuthorized";
@@ -1664,15 +1707,19 @@ static void authorize_write_response(const char *input, void *user_data)
goto error;
}
- chrc->authorized = true;
+ chrc->authorized_write = true;
- errsv = parse_value_arg(&iter, &chrc->value, &chrc->value_len,
- chrc->max_val_len);
- if (errsv == -EINVAL) {
- err = "org.bluez.Error.InvalidArguments";
+ /* Authorization check of prepare writes */
+ if (authorize) {
+ reply = g_dbus_create_reply(pending_message, DBUS_TYPE_INVALID);
+ g_dbus_send_message(aad->conn, reply);
+ g_free(aad);
- goto error;
- } else if (errsv == -EOVERFLOW) {
+ return;
+ }
+
+ if (write_value(&chrc->value_len, &chrc->value, value, value_len,
+ aad->offset, chrc->max_val_len)) {
err = "org.bluez.Error.InvalidValueLength";
goto error;
@@ -1699,18 +1746,31 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
void *user_data)
{
struct chrc *chrc = user_data;
+ uint16_t offset = 0;
+ bool authorize = false;
DBusMessageIter iter;
+ int value_len;
+ uint8_t *value;
char *str;
- int errsv;
dbus_message_iter_init(msg, &iter);
- if (chrc->authorization_req && !chrc->authorized) {
+ if (parse_value_arg(&iter, &value, &value_len))
+ return g_dbus_create_error(msg,
+ "org.bluez.Error.InvalidArguments", NULL);
+
+ dbus_message_iter_next(&iter);
+ if (parse_options(&iter, &offset, NULL, NULL, NULL, &authorize))
+ return g_dbus_create_error(msg,
+ "org.bluez.Error.InvalidArguments", NULL);
+
+ if (chrc->authorization_req && !chrc->authorized_write) {
struct authorize_attribute_data *aad;
aad = g_new0(struct authorize_attribute_data, 1);
aad->conn = conn;
aad->attribute = chrc;
+ aad->offset = offset;
str = g_strdup_printf("Authorize attribute(%s) write (yes/no):",
chrc->path);
@@ -1724,15 +1784,14 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
return NULL;
}
- errsv = parse_value_arg(&iter, &chrc->value, &chrc->value_len,
- chrc->max_val_len);
- if (errsv == -EINVAL) {
- return g_dbus_create_error(msg,
- "org.bluez.Error.InvalidArguments", NULL);
- } else if (errsv == -EOVERFLOW) {
+ /* Authorization check of prepare writes */
+ if (authorize)
+ return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+
+ if (write_value(&chrc->value_len, &chrc->value, value, value_len,
+ offset, chrc->max_val_len))
return g_dbus_create_error(msg,
"org.bluez.Error.InvalidValueLength", NULL);
- }
bt_shell_printf("[" COLORED_CHG "] Attribute %s written" , chrc->path);
@@ -1794,7 +1853,7 @@ static DBusMessage *chrc_acquire_write(DBusConnection *conn, DBusMessage *msg,
"org.bluez.Error.NotPermitted",
NULL);
- if (parse_options(&iter, NULL, &chrc->mtu, &device, &link))
+ if (parse_options(&iter, NULL, &chrc->mtu, &device, &link, NULL))
return g_dbus_create_error(msg,
"org.bluez.Error.InvalidArguments",
NULL);
@@ -1826,7 +1885,7 @@ static DBusMessage *chrc_acquire_notify(DBusConnection *conn, DBusMessage *msg,
"org.bluez.Error.NotPermitted",
NULL);
- if (parse_options(&iter, NULL, &chrc->mtu, &device, &link))
+ if (parse_options(&iter, NULL, &chrc->mtu, &device, &link, NULL))
return g_dbus_create_error(msg,
"org.bluez.Error.InvalidArguments",
NULL);
@@ -2042,7 +2101,7 @@ static DBusMessage *desc_read_value(DBusConnection *conn, DBusMessage *msg,
dbus_message_iter_init(msg, &iter);
- if (parse_options(&iter, &offset, NULL, &device, &link))
+ if (parse_options(&iter, &offset, NULL, &device, &link, NULL))
return g_dbus_create_error(msg,
"org.bluez.Error.InvalidArguments",
NULL);
@@ -2064,21 +2123,24 @@ static DBusMessage *desc_write_value(DBusConnection *conn, DBusMessage *msg,
DBusMessageIter iter;
uint16_t offset = 0;
char *device = NULL, *link = NULL;
+ int value_len;
+ uint8_t *value;
dbus_message_iter_init(msg, &iter);
- if (parse_value_arg(&iter, &desc->value, &desc->value_len,
- desc->max_val_len))
+ if (parse_value_arg(&iter, &value, &value_len))
return g_dbus_create_error(msg,
- "org.bluez.Error.InvalidArguments",
- NULL);
+ "org.bluez.Error.InvalidArguments", NULL);
dbus_message_iter_next(&iter);
+ if (parse_options(&iter, &offset, NULL, &device, &link, NULL))
+ return g_dbus_create_error(msg,
+ "org.bluez.Error.InvalidArguments", NULL);
- if (parse_options(&iter, &offset, NULL, &device, &link))
+ if (write_value(&desc->value_len, &desc->value, value,
+ value_len, offset, desc->max_val_len))
return g_dbus_create_error(msg,
- "org.bluez.Error.InvalidArguments",
- NULL);
+ "org.bluez.Error.InvalidValueLength", NULL);
bt_shell_printf("WriteValue: %s offset %u link %s\n",
path_to_address(device), offset, link);
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH BlueZ v2 5/5] client: Add authorized property handling to characteristic attribute
2018-05-16 13:52 ` [PATCH BlueZ v2 5/5] client: Add authorized property handling to characteristic attribute Grzegorz Kolodziejczyk
@ 2018-05-21 9:58 ` Luiz Augusto von Dentz
2018-05-22 7:59 ` Grzegorz Kołodziejczyk
0 siblings, 1 reply; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2018-05-21 9:58 UTC (permalink / raw)
To: Grzegorz Kolodziejczyk; +Cc: linux-bluetooth@vger.kernel.org
Hi Grzegorz,
On Wed, May 16, 2018 at 4:52 PM, Grzegorz Kolodziejczyk
<grzegorz.kolodziejczyk@codecoup.pl> wrote:
> This patch adds handling of authorized property to bluetoothctl.
> ---
> client/gatt.c | 162 ++++++++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 112 insertions(+), 50 deletions(-)
>
> diff --git a/client/gatt.c b/client/gatt.c
> index 1b4e713ef..6a9ba5687 100644
> --- a/client/gatt.c
> +++ b/client/gatt.c
> @@ -80,7 +80,8 @@ struct chrc {
> struct io *write_io;
> struct io *notify_io;
> bool authorization_req;
> - bool authorized;
> + bool authorized_read;
Im confused why we need to track the authorization in the client? I
thought the idea was when registering the user would be asked if
authorization is required, or perhaps we set it along with flags, then
if there is any read/write requires the user would have to authorize
it. In addition to this we may actually use the trusted property to
skip authorizations if the device is trusted.
> + bool authorized_write;
> };
>
> struct service {
> @@ -1432,6 +1433,30 @@ static gboolean chrc_notify_acquired_exists(const GDBusPropertyTable *property,
> return FALSE;
> }
>
> +static gboolean chrc_get_authorize(const GDBusPropertyTable *property,
> + DBusMessageIter *iter, void *data)
> +{
> + struct chrc *chrc = data;
> + dbus_bool_t value;
> +
> + value = chrc->authorization_req ? TRUE : FALSE;
> +
> + dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &value);
> +
> + return TRUE;
> +}
> +
> +static gboolean chrc_get_authorize_exists(const GDBusPropertyTable *property,
> + void *data)
> +{
> + struct chrc *chrc = data;
> +
> + if (chrc->authorization_req)
> + return TRUE;
> +
> + return FALSE;
> +}
> +
> static const GDBusPropertyTable chrc_properties[] = {
> { "UUID", "s", chrc_get_uuid, NULL, NULL },
> { "Service", "o", chrc_get_service, NULL, NULL },
> @@ -1442,6 +1467,8 @@ static const GDBusPropertyTable chrc_properties[] = {
> chrc_write_acquired_exists },
> { "NotifyAcquired", "b", chrc_get_notify_acquired, NULL,
> chrc_notify_acquired_exists },
> + { "Authorize", "b", chrc_get_authorize, NULL,
> + chrc_get_authorize_exists },
> { }
> };
>
> @@ -1460,7 +1487,8 @@ static const char *path_to_address(const char *path)
> }
>
> static int parse_options(DBusMessageIter *iter, uint16_t *offset, uint16_t *mtu,
> - char **device, char **link)
> + char **device, char **link,
> + bool *authorize)
> {
> DBusMessageIter dict;
>
> @@ -1501,6 +1529,12 @@ static int parse_options(DBusMessageIter *iter, uint16_t *offset, uint16_t *mtu,
> return -EINVAL;
> if (link)
> dbus_message_iter_get_basic(&value, link);
> + } else if (strcasecmp(key, "authorize") == 0) {
> + if (var != DBUS_TYPE_BOOLEAN)
> + return -EINVAL;
> + if (authorize)
> + dbus_message_iter_get_basic(&value,
> + authorize);
> }
>
> dbus_message_iter_next(&dict);
> @@ -1554,7 +1588,7 @@ static void authorize_read_response(const char *input, void *user_data)
> reply = read_value(pending_message, &chrc->value[aad->offset],
> chrc->value_len - aad->offset);
>
> - chrc->authorized = true;
> + chrc->authorized_read = true;
>
> g_dbus_send_message(aad->conn, reply);
>
> @@ -1578,18 +1612,15 @@ static DBusMessage *chrc_read_value(DBusConnection *conn, DBusMessage *msg,
>
> dbus_message_iter_init(msg, &iter);
>
> - if (parse_options(&iter, &offset, NULL, &device, &link))
> + if (parse_options(&iter, &offset, NULL, &device, &link, NULL))
> return g_dbus_create_error(msg,
> "org.bluez.Error.InvalidArguments",
> NULL);
>
> bt_shell_printf("ReadValue: %s offset %u link %s\n",
> - path_to_address(device), offset, link);
> + path_to_address(device), offset, link);
>
> - if (chrc->authorization_req && offset == 0)
> - chrc->authorized = false;
> -
> - if (chrc->authorization_req && !chrc->authorized) {
> + if (chrc->authorization_req && !chrc->authorized_read) {
> struct authorize_attribute_data *aad;
>
> aad = g_new0(struct authorize_attribute_data, 1);
> @@ -1616,33 +1647,31 @@ static DBusMessage *chrc_read_value(DBusConnection *conn, DBusMessage *msg,
> return read_value(msg, &chrc->value[offset], chrc->value_len - offset);
> }
>
> -static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len,
> - int max_len)
> +static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len)
> {
> DBusMessageIter array;
> - uint16_t offset = 0;
> - uint8_t *read_value;
> - int read_len;
>
> if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_ARRAY)
> return -EINVAL;
>
> dbus_message_iter_recurse(iter, &array);
> - dbus_message_iter_get_fixed_array(&array, &read_value, &read_len);
> + dbus_message_iter_get_fixed_array(&array, value, len);
>
> - dbus_message_iter_next(iter);
> - if (parse_options(iter, &offset, NULL, NULL, NULL))
> - return -EINVAL;
> + return 0;
> +}
>
> - if ((offset + read_len) > max_len)
> +static int write_value(int *dst_len, uint8_t **dst_value, uint8_t *src_val,
> + int src_len, uint16_t offset, uint16_t max_len)
> +{
> + if ((offset + src_len) > max_len)
> return -EOVERFLOW;
>
> - if ((offset + read_len) > *len) {
> - *len = offset + read_len;
> - *value = g_realloc(*value, *len);
> + if ((offset + src_len) != *dst_len) {
> + *dst_len = offset + src_len;
> + *dst_value = g_realloc(*dst_value, *dst_len);
> }
>
> - memcpy(*value + offset, read_value, read_len);
> + memcpy(*dst_value + offset, src_val, src_len);
>
> return 0;
> }
> @@ -1651,12 +1680,26 @@ static void authorize_write_response(const char *input, void *user_data)
> {
> struct authorize_attribute_data *aad = user_data;
> struct chrc *chrc = aad->attribute;
> + bool authorize = false;
> DBusMessageIter iter;
> DBusMessage *reply;
> + int value_len;
> + uint8_t *value;
> char *err;
> - int errsv;
>
> dbus_message_iter_init(pending_message, &iter);
> + if (parse_value_arg(&iter, &value, &value_len)) {
> + err = "org.bluez.Error.InvalidArguments";
> +
> + goto error;
> + }
> +
> + dbus_message_iter_next(&iter);
> + if (parse_options(&iter, NULL, NULL, NULL, NULL, &authorize)) {
> + err = "org.bluez.Error.InvalidArguments";
> +
> + goto error;
> + }
>
> if (!strcmp(input, "no")) {
> err = "org.bluez.Error.NotAuthorized";
> @@ -1664,15 +1707,19 @@ static void authorize_write_response(const char *input, void *user_data)
> goto error;
> }
>
> - chrc->authorized = true;
> + chrc->authorized_write = true;
>
> - errsv = parse_value_arg(&iter, &chrc->value, &chrc->value_len,
> - chrc->max_val_len);
> - if (errsv == -EINVAL) {
> - err = "org.bluez.Error.InvalidArguments";
> + /* Authorization check of prepare writes */
> + if (authorize) {
> + reply = g_dbus_create_reply(pending_message, DBUS_TYPE_INVALID);
> + g_dbus_send_message(aad->conn, reply);
> + g_free(aad);
>
> - goto error;
> - } else if (errsv == -EOVERFLOW) {
> + return;
> + }
> +
> + if (write_value(&chrc->value_len, &chrc->value, value, value_len,
> + aad->offset, chrc->max_val_len)) {
> err = "org.bluez.Error.InvalidValueLength";
>
> goto error;
> @@ -1699,18 +1746,31 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
> void *user_data)
> {
> struct chrc *chrc = user_data;
> + uint16_t offset = 0;
> + bool authorize = false;
> DBusMessageIter iter;
> + int value_len;
> + uint8_t *value;
> char *str;
> - int errsv;
>
> dbus_message_iter_init(msg, &iter);
>
> - if (chrc->authorization_req && !chrc->authorized) {
> + if (parse_value_arg(&iter, &value, &value_len))
> + return g_dbus_create_error(msg,
> + "org.bluez.Error.InvalidArguments", NULL);
> +
> + dbus_message_iter_next(&iter);
> + if (parse_options(&iter, &offset, NULL, NULL, NULL, &authorize))
> + return g_dbus_create_error(msg,
> + "org.bluez.Error.InvalidArguments", NULL);
> +
> + if (chrc->authorization_req && !chrc->authorized_write) {
> struct authorize_attribute_data *aad;
>
> aad = g_new0(struct authorize_attribute_data, 1);
> aad->conn = conn;
> aad->attribute = chrc;
> + aad->offset = offset;
>
> str = g_strdup_printf("Authorize attribute(%s) write (yes/no):",
> chrc->path);
> @@ -1724,15 +1784,14 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
> return NULL;
> }
>
> - errsv = parse_value_arg(&iter, &chrc->value, &chrc->value_len,
> - chrc->max_val_len);
> - if (errsv == -EINVAL) {
> - return g_dbus_create_error(msg,
> - "org.bluez.Error.InvalidArguments", NULL);
> - } else if (errsv == -EOVERFLOW) {
> + /* Authorization check of prepare writes */
> + if (authorize)
> + return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
> +
> + if (write_value(&chrc->value_len, &chrc->value, value, value_len,
> + offset, chrc->max_val_len))
> return g_dbus_create_error(msg,
> "org.bluez.Error.InvalidValueLength", NULL);
> - }
>
> bt_shell_printf("[" COLORED_CHG "] Attribute %s written" , chrc->path);
>
> @@ -1794,7 +1853,7 @@ static DBusMessage *chrc_acquire_write(DBusConnection *conn, DBusMessage *msg,
> "org.bluez.Error.NotPermitted",
> NULL);
>
> - if (parse_options(&iter, NULL, &chrc->mtu, &device, &link))
> + if (parse_options(&iter, NULL, &chrc->mtu, &device, &link, NULL))
> return g_dbus_create_error(msg,
> "org.bluez.Error.InvalidArguments",
> NULL);
> @@ -1826,7 +1885,7 @@ static DBusMessage *chrc_acquire_notify(DBusConnection *conn, DBusMessage *msg,
> "org.bluez.Error.NotPermitted",
> NULL);
>
> - if (parse_options(&iter, NULL, &chrc->mtu, &device, &link))
> + if (parse_options(&iter, NULL, &chrc->mtu, &device, &link, NULL))
> return g_dbus_create_error(msg,
> "org.bluez.Error.InvalidArguments",
> NULL);
> @@ -2042,7 +2101,7 @@ static DBusMessage *desc_read_value(DBusConnection *conn, DBusMessage *msg,
>
> dbus_message_iter_init(msg, &iter);
>
> - if (parse_options(&iter, &offset, NULL, &device, &link))
> + if (parse_options(&iter, &offset, NULL, &device, &link, NULL))
> return g_dbus_create_error(msg,
> "org.bluez.Error.InvalidArguments",
> NULL);
> @@ -2064,21 +2123,24 @@ static DBusMessage *desc_write_value(DBusConnection *conn, DBusMessage *msg,
> DBusMessageIter iter;
> uint16_t offset = 0;
> char *device = NULL, *link = NULL;
> + int value_len;
> + uint8_t *value;
>
> dbus_message_iter_init(msg, &iter);
>
> - if (parse_value_arg(&iter, &desc->value, &desc->value_len,
> - desc->max_val_len))
> + if (parse_value_arg(&iter, &value, &value_len))
> return g_dbus_create_error(msg,
> - "org.bluez.Error.InvalidArguments",
> - NULL);
> + "org.bluez.Error.InvalidArguments", NULL);
>
> dbus_message_iter_next(&iter);
> + if (parse_options(&iter, &offset, NULL, &device, &link, NULL))
> + return g_dbus_create_error(msg,
> + "org.bluez.Error.InvalidArguments", NULL);
>
> - if (parse_options(&iter, &offset, NULL, &device, &link))
> + if (write_value(&desc->value_len, &desc->value, value,
> + value_len, offset, desc->max_val_len))
> return g_dbus_create_error(msg,
> - "org.bluez.Error.InvalidArguments",
> - NULL);
> + "org.bluez.Error.InvalidValueLength", NULL);
>
> bt_shell_printf("WriteValue: %s offset %u link %s\n",
> path_to_address(device), offset, link);
> --
> 2.13.6
>
> --
> 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] 10+ messages in thread* Re: [PATCH BlueZ v2 5/5] client: Add authorized property handling to characteristic attribute
2018-05-21 9:58 ` Luiz Augusto von Dentz
@ 2018-05-22 7:59 ` Grzegorz Kołodziejczyk
0 siblings, 0 replies; 10+ messages in thread
From: Grzegorz Kołodziejczyk @ 2018-05-22 7:59 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
pon., 21 maj 2018 o 11:58 Luiz Augusto von Dentz <luiz.dentz@gmail.com>
napisa=C5=82(a):
> Hi Grzegorz,
> On Wed, May 16, 2018 at 4:52 PM, Grzegorz Kolodziejczyk
> <grzegorz.kolodziejczyk@codecoup.pl> wrote:
> > This patch adds handling of authorized property to bluetoothctl.
> > ---
> > client/gatt.c | 162
++++++++++++++++++++++++++++++++++++++++------------------
> > 1 file changed, 112 insertions(+), 50 deletions(-)
> >
> > diff --git a/client/gatt.c b/client/gatt.c
> > index 1b4e713ef..6a9ba5687 100644
> > --- a/client/gatt.c
> > +++ b/client/gatt.c
> > @@ -80,7 +80,8 @@ struct chrc {
> > struct io *write_io;
> > struct io *notify_io;
> > bool authorization_req;
> > - bool authorized;
> > + bool authorized_read;
> Im confused why we need to track the authorization in the client? I
> thought the idea was when registering the user would be asked if
> authorization is required, or perhaps we set it along with flags, then
> if there is any read/write requires the user would have to authorize
> it. In addition to this we may actually use the trusted property to
> skip authorizations if the device is trusted.
Actually this patch set only extends authorization procedure for prepare
writes. Previous way of tracking authorization on client side behaves the
same. This is tracked to avoid auhorization requests for execute writes,
long reads (more than single iteration) and generally it's simplified to
one authorization per all write/read actions since this is only test app.
> > + bool authorized_write;
> > };
> >
> > struct service {
> > @@ -1432,6 +1433,30 @@ static gboolean
chrc_notify_acquired_exists(const GDBusPropertyTable *property,
> > return FALSE;
> > }
> >
> > +static gboolean chrc_get_authorize(const GDBusPropertyTable *property,
> > + DBusMessageIter *iter, void
*data)
> > +{
> > + struct chrc *chrc =3D data;
> > + dbus_bool_t value;
> > +
> > + value =3D chrc->authorization_req ? TRUE : FALSE;
> > +
> > + dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &value)=
;
> > +
> > + return TRUE;
> > +}
> > +
> > +static gboolean chrc_get_authorize_exists(const GDBusPropertyTable
*property,
> > + void
*data)
> > +{
> > + struct chrc *chrc =3D data;
> > +
> > + if (chrc->authorization_req)
> > + return TRUE;
> > +
> > + return FALSE;
> > +}
> > +
> > static const GDBusPropertyTable chrc_properties[] =3D {
> > { "UUID", "s", chrc_get_uuid, NULL, NULL },
> > { "Service", "o", chrc_get_service, NULL, NULL },
> > @@ -1442,6 +1467,8 @@ static const GDBusPropertyTable chrc_properties[]
=3D {
> > chrc_write_acquired_exists },
> > { "NotifyAcquired", "b", chrc_get_notify_acquired, NULL,
> > chrc_notify_acquired_exists },
> > + { "Authorize", "b", chrc_get_authorize, NULL,
> > +
chrc_get_authorize_exists },
> > { }
> > };
> >
> > @@ -1460,7 +1487,8 @@ static const char *path_to_address(const char
*path)
> > }
> >
> > static int parse_options(DBusMessageIter *iter, uint16_t *offset,
uint16_t *mtu,
> > - char **device, char
**link)
> > + char **device, char
**link,
> > + bool *authorize)
> > {
> > DBusMessageIter dict;
> >
> > @@ -1501,6 +1529,12 @@ static int parse_options(DBusMessageIter *iter,
uint16_t *offset, uint16_t *mtu,
> > return -EINVAL;
> > if (link)
> > dbus_message_iter_get_basic(&value,
link);
> > + } else if (strcasecmp(key, "authorize") =3D=3D 0) {
> > + if (var !=3D DBUS_TYPE_BOOLEAN)
> > + return -EINVAL;
> > + if (authorize)
> > + dbus_message_iter_get_basic(&value,
> > +
authorize);
> > }
> >
> > dbus_message_iter_next(&dict);
> > @@ -1554,7 +1588,7 @@ static void authorize_read_response(const char
*input, void *user_data)
> > reply =3D read_value(pending_message, &chrc->value[aad->offset]=
,
> > chrc->value_len -
aad->offset);
> >
> > - chrc->authorized =3D true;
> > + chrc->authorized_read =3D true;
> >
> > g_dbus_send_message(aad->conn, reply);
> >
> > @@ -1578,18 +1612,15 @@ static DBusMessage
*chrc_read_value(DBusConnection *conn, DBusMessage *msg,
> >
> > dbus_message_iter_init(msg, &iter);
> >
> > - if (parse_options(&iter, &offset, NULL, &device, &link))
> > + if (parse_options(&iter, &offset, NULL, &device, &link, NULL))
> > return g_dbus_create_error(msg,
> >
"org.bluez.Error.InvalidArguments",
> > NULL);
> >
> > bt_shell_printf("ReadValue: %s offset %u link %s\n",
> > - path_to_address(device), offset, link);
> > + path_to_address(device),
offset, link);
> >
> > - if (chrc->authorization_req && offset =3D=3D 0)
> > - chrc->authorized =3D false;
> > -
> > - if (chrc->authorization_req && !chrc->authorized) {
> > + if (chrc->authorization_req && !chrc->authorized_read) {
> > struct authorize_attribute_data *aad;
> >
> > aad =3D g_new0(struct authorize_attribute_data, 1);
> > @@ -1616,33 +1647,31 @@ static DBusMessage
*chrc_read_value(DBusConnection *conn, DBusMessage *msg,
> > return read_value(msg, &chrc->value[offset], chrc->value_len -
offset);
> > }
> >
> > -static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int
*len,
> > - int
max_len)
> > +static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int
*len)
> > {
> > DBusMessageIter array;
> > - uint16_t offset =3D 0;
> > - uint8_t *read_value;
> > - int read_len;
> >
> > if (dbus_message_iter_get_arg_type(iter) !=3D DBUS_TYPE_ARRAY)
> > return -EINVAL;
> >
> > dbus_message_iter_recurse(iter, &array);
> > - dbus_message_iter_get_fixed_array(&array, &read_value,
&read_len);
> > + dbus_message_iter_get_fixed_array(&array, value, len);
> >
> > - dbus_message_iter_next(iter);
> > - if (parse_options(iter, &offset, NULL, NULL, NULL))
> > - return -EINVAL;
> > + return 0;
> > +}
> >
> > - if ((offset + read_len) > max_len)
> > +static int write_value(int *dst_len, uint8_t **dst_value, uint8_t
*src_val,
> > + int src_len, uint16_t offset, uint16_t
max_len)
> > +{
> > + if ((offset + src_len) > max_len)
> > return -EOVERFLOW;
> >
> > - if ((offset + read_len) > *len) {
> > - *len =3D offset + read_len;
> > - *value =3D g_realloc(*value, *len);
> > + if ((offset + src_len) !=3D *dst_len) {
> > + *dst_len =3D offset + src_len;
> > + *dst_value =3D g_realloc(*dst_value, *dst_len);
> > }
> >
> > - memcpy(*value + offset, read_value, read_len);
> > + memcpy(*dst_value + offset, src_val, src_len);
> >
> > return 0;
> > }
> > @@ -1651,12 +1680,26 @@ static void authorize_write_response(const char
*input, void *user_data)
> > {
> > struct authorize_attribute_data *aad =3D user_data;
> > struct chrc *chrc =3D aad->attribute;
> > + bool authorize =3D false;
> > DBusMessageIter iter;
> > DBusMessage *reply;
> > + int value_len;
> > + uint8_t *value;
> > char *err;
> > - int errsv;
> >
> > dbus_message_iter_init(pending_message, &iter);
> > + if (parse_value_arg(&iter, &value, &value_len)) {
> > + err =3D "org.bluez.Error.InvalidArguments";
> > +
> > + goto error;
> > + }
> > +
> > + dbus_message_iter_next(&iter);
> > + if (parse_options(&iter, NULL, NULL, NULL, NULL, &authorize)) {
> > + err =3D "org.bluez.Error.InvalidArguments";
> > +
> > + goto error;
> > + }
> >
> > if (!strcmp(input, "no")) {
> > err =3D "org.bluez.Error.NotAuthorized";
> > @@ -1664,15 +1707,19 @@ static void authorize_write_response(const char
*input, void *user_data)
> > goto error;
> > }
> >
> > - chrc->authorized =3D true;
> > + chrc->authorized_write =3D true;
> >
> > - errsv =3D parse_value_arg(&iter, &chrc->value, &chrc->value_len=
,
> > -
chrc->max_val_len);
> > - if (errsv =3D=3D -EINVAL) {
> > - err =3D "org.bluez.Error.InvalidArguments";
> > + /* Authorization check of prepare writes */
> > + if (authorize) {
> > + reply =3D g_dbus_create_reply(pending_message,
DBUS_TYPE_INVALID);
> > + g_dbus_send_message(aad->conn, reply);
> > + g_free(aad);
> >
> > - goto error;
> > - } else if (errsv =3D=3D -EOVERFLOW) {
> > + return;
> > + }
> > +
> > + if (write_value(&chrc->value_len, &chrc->value, value,
value_len,
> > + aad->offset,
chrc->max_val_len)) {
> > err =3D "org.bluez.Error.InvalidValueLength";
> >
> > goto error;
> > @@ -1699,18 +1746,31 @@ static DBusMessage
*chrc_write_value(DBusConnection *conn, DBusMessage *msg,
> > void *user_data=
)
> > {
> > struct chrc *chrc =3D user_data;
> > + uint16_t offset =3D 0;
> > + bool authorize =3D false;
> > DBusMessageIter iter;
> > + int value_len;
> > + uint8_t *value;
> > char *str;
> > - int errsv;
> >
> > dbus_message_iter_init(msg, &iter);
> >
> > - if (chrc->authorization_req && !chrc->authorized) {
> > + if (parse_value_arg(&iter, &value, &value_len))
> > + return g_dbus_create_error(msg,
> > + "org.bluez.Error.InvalidArguments",
NULL);
> > +
> > + dbus_message_iter_next(&iter);
> > + if (parse_options(&iter, &offset, NULL, NULL, NULL, &authorize)=
)
> > + return g_dbus_create_error(msg,
> > + "org.bluez.Error.InvalidArguments",
NULL);
> > +
> > + if (chrc->authorization_req && !chrc->authorized_write) {
> > struct authorize_attribute_data *aad;
> >
> > aad =3D g_new0(struct authorize_attribute_data, 1);
> > aad->conn =3D conn;
> > aad->attribute =3D chrc;
> > + aad->offset =3D offset;
> >
> > str =3D g_strdup_printf("Authorize attribute(%s) write
(yes/no):",
> >
chrc->path);
> > @@ -1724,15 +1784,14 @@ static DBusMessage
*chrc_write_value(DBusConnection *conn, DBusMessage *msg,
> > return NULL;
> > }
> >
> > - errsv =3D parse_value_arg(&iter, &chrc->value, &chrc->value_len=
,
> > -
chrc->max_val_len);
> > - if (errsv =3D=3D -EINVAL) {
> > - return g_dbus_create_error(msg,
> > - "org.bluez.Error.InvalidArguments",
NULL);
> > - } else if (errsv =3D=3D -EOVERFLOW) {
> > + /* Authorization check of prepare writes */
> > + if (authorize)
> > + return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
> > +
> > + if (write_value(&chrc->value_len, &chrc->value, value,
value_len,
> > + offset,
chrc->max_val_len))
> > return g_dbus_create_error(msg,
> > "org.bluez.Error.InvalidValueLength",
NULL);
> > - }
> >
> > bt_shell_printf("[" COLORED_CHG "] Attribute %s written" ,
chrc->path);
> >
> > @@ -1794,7 +1853,7 @@ static DBusMessage
*chrc_acquire_write(DBusConnection *conn, DBusMessage *msg,
> > "org.bluez.Error.NotPermitted",
> > NULL);
> >
> > - if (parse_options(&iter, NULL, &chrc->mtu, &device, &link))
> > + if (parse_options(&iter, NULL, &chrc->mtu, &device, &link,
NULL))
> > return g_dbus_create_error(msg,
> >
"org.bluez.Error.InvalidArguments",
> > NULL);
> > @@ -1826,7 +1885,7 @@ static DBusMessage
*chrc_acquire_notify(DBusConnection *conn, DBusMessage *msg,
> > "org.bluez.Error.NotPermitted",
> > NULL);
> >
> > - if (parse_options(&iter, NULL, &chrc->mtu, &device, &link))
> > + if (parse_options(&iter, NULL, &chrc->mtu, &device, &link,
NULL))
> > return g_dbus_create_error(msg,
> >
"org.bluez.Error.InvalidArguments",
> > NULL);
> > @@ -2042,7 +2101,7 @@ static DBusMessage
*desc_read_value(DBusConnection *conn, DBusMessage *msg,
> >
> > dbus_message_iter_init(msg, &iter);
> >
> > - if (parse_options(&iter, &offset, NULL, &device, &link))
> > + if (parse_options(&iter, &offset, NULL, &device, &link, NULL))
> > return g_dbus_create_error(msg,
> >
"org.bluez.Error.InvalidArguments",
> > NULL);
> > @@ -2064,21 +2123,24 @@ static DBusMessage
*desc_write_value(DBusConnection *conn, DBusMessage *msg,
> > DBusMessageIter iter;
> > uint16_t offset =3D 0;
> > char *device =3D NULL, *link =3D NULL;
> > + int value_len;
> > + uint8_t *value;
> >
> > dbus_message_iter_init(msg, &iter);
> >
> > - if (parse_value_arg(&iter, &desc->value, &desc->value_len,
> > -
desc->max_val_len))
> > + if (parse_value_arg(&iter, &value, &value_len))
> > return g_dbus_create_error(msg,
> > -
"org.bluez.Error.InvalidArguments",
> > - NULL);
> > + "org.bluez.Error.InvalidArguments",
NULL);
> >
> > dbus_message_iter_next(&iter);
> > + if (parse_options(&iter, &offset, NULL, &device, &link, NULL))
> > + return g_dbus_create_error(msg,
> > + "org.bluez.Error.InvalidArguments",
NULL);
> >
> > - if (parse_options(&iter, &offset, NULL, &device, &link))
> > + if (write_value(&desc->value_len, &desc->value, value,
> > + value_len, offset,
desc->max_val_len))
> > return g_dbus_create_error(msg,
> > -
"org.bluez.Error.InvalidArguments",
> > - NULL);
> > + "org.bluez.Error.InvalidValueLength",
NULL);
> >
> > bt_shell_printf("WriteValue: %s offset %u link %s\n",
> > path_to_address(device), offset, link);
> > --
> > 2.13.6
> >
> > --
> > 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
Grzegorz Ko=C5=82odziejczyk
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH BlueZ v2 1/5] client: Add missing duplicated string free
2018-05-16 13:52 [PATCH BlueZ v2 1/5] client: Add missing duplicated string free Grzegorz Kolodziejczyk
` (3 preceding siblings ...)
2018-05-16 13:52 ` [PATCH BlueZ v2 5/5] client: Add authorized property handling to characteristic attribute Grzegorz Kolodziejczyk
@ 2018-05-21 8:38 ` Luiz Augusto von Dentz
4 siblings, 0 replies; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2018-05-21 8:38 UTC (permalink / raw)
To: Grzegorz Kolodziejczyk; +Cc: linux-bluetooth@vger.kernel.org
Hi Grzegorz,
On Wed, May 16, 2018 at 4:52 PM, Grzegorz Kolodziejczyk
<grzegorz.kolodziejczyk@codecoup.pl> wrote:
> This patch free duplicated strings in read, write attribute callbacks.
> ---
> client/gatt.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/client/gatt.c b/client/gatt.c
> index d59d1ba1e..c8267a75f 100644
> --- a/client/gatt.c
> +++ b/client/gatt.c
> @@ -1598,6 +1598,7 @@ static DBusMessage *chrc_read_value(DBusConnection *conn, DBusMessage *msg,
>
> bt_shell_prompt_input("gatt", str, authorize_read_response,
> aad);
> + g_free(str);
>
> pending_message = dbus_message_ref(msg);
>
> @@ -1700,6 +1701,7 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
>
> bt_shell_prompt_input("gatt", str, authorize_write_response,
> aad);
> + g_free(str);
>
> pending_message = dbus_message_ref(msg);
>
> --
> 2.13.6
Patches 1 and 2 applied, thanks.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 10+ messages in thread