[PATCH BlueZ 1/5] client: Add missing duplicated string free

[PATCH BlueZ 1/5] client: Add missing duplicated string free

[Grzegorz Kolodziejczyk]

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

[PATCH BlueZ 2/5] client: Define maximum attribute value length as initial value

[Grzegorz Kolodziejczyk]

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

[PATCH BlueZ 3/5] doc/gatt-api: Add authorization options for attributes

[Grzegorz Kolodziejczyk]

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 | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
index 0f1cc9029..7d2517ccc 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)
+					  "prep-authz-req": boolean Is prepare write
+							    authorization request
 
 			Possible Errors: org.bluez.Error.Failed
 					 org.bluez.Error.InProgress
@@ -251,6 +253,11 @@ 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.
+
 Characteristic Descriptors hierarchy
 ====================================
 
@@ -284,6 +291,8 @@ Methods		array{byte} ReadValue(dict flags)
 			Possible options: "offset": Start offset
 					  "device": Device path (Server only)
 					  "link": Link type (Server only)
+					  "prep-authz-req": boolean Is prepare write
+							    authorization request
 
 			Possible Errors: org.bluez.Error.Failed
 					 org.bluez.Error.InProgress
-- 
2.13.6

[PATCH BlueZ 4/5] shared/gatt-server: Request authorization for prepare writes.

[Grzegorz Kolodziejczyk]

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..17ad570a2 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 prep_authz_req;
 };
 
 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->prep_authz_req)
+		dict_append_entry(iter, "prep-write-req", DBUS_TYPE_BOOLEAN,
+							&op->prep_authz_req);
 }
 
 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->prep_authz_req) {
+		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 prep_authz_req)
 {
 	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->prep_authz_req = prep_authz_req;
 	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 prep_authz_req)
 {
 	struct pending_op *op;
 
 	op = pending_write_new(device, owner_queue, attrib, id, value, len,
-							offset, link_type);
+					offset, link_type, is_characteristic,
+					prep_authz_req);
 
 	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

[PATCH BlueZ 5/5] client: Add authorized property handling to characteristic attribute

[Grzegorz Kolodziejczyk]

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..cfbd0e3c1 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 *prep_authz_req)
 {
 	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, "prep_authz_req") == 0) {
+			if (var != DBUS_TYPE_BOOLEAN)
+				return -EINVAL;
+			if (prep_authz_req)
+				dbus_message_iter_get_basic(&value,
+								prep_authz_req);
 		}
 
 		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 prep_authz_req = 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, &prep_authz_req)) {
+		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 (prep_authz_req) {
+		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 prep_authz_req = 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, &prep_authz_req))
+		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 (prep_authz_req)
+		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

Re: [PATCH BlueZ 3/5] doc/gatt-api: Add authorization options for attributes

[Luiz Augusto von Dentz]

Hi Grzegorz,
On Wed, May 9, 2018 at 2:08 PM Grzegorz Kolodziejczyk <
grzegorz.kolodziejczyk@codecoup.pl> wrote:

> 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 | 9 +++++++++
>   1 file changed, 9 insertions(+)

> diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
> index 0f1cc9029..7d2517ccc 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)
> +                                         "prep-authz-req": boolean Is
prepare write
> +                                                           authorization
request

I'd keep it simple and just call it authorize.

>                          Possible Errors: org.bluez.Error.Failed
>                                           org.bluez.Error.InProgress
> @@ -251,6 +253,11 @@ 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.

We should probably mention the default is false if the property is not
available.

>   Characteristic Descriptors hierarchy
>   ====================================

> @@ -284,6 +291,8 @@ Methods             array{byte} ReadValue(dict flags)
>                          Possible options: "offset": Start offset
>                                            "device": Device path (Server
only)
>                                            "link": Link type (Server only)
> +                                         "prep-authz-req": boolean Is
prepare write
> +                                                           authorization
request

Do we need and authorize flag for read? I thought this was only for prepare
write.

>                          Possible Errors: org.bluez.Error.Failed
>                                           org.bluez.Error.InProgress
> --
> 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

Re: [PATCH BlueZ 3/5] doc/gatt-api: Add authorization options for attributes

[Grzegorz Kołodziejczyk]

Hi Luiz,
=C5=9Br., 16 maj 2018 o 09:16 Luiz Augusto von Dentz <luiz.dentz@gmail.com>
napisa=C5=82(a):

> Hi Grzegorz,
> On Wed, May 9, 2018 at 2:08 PM Grzegorz Kolodziejczyk <
> grzegorz.kolodziejczyk@codecoup.pl> wrote:

> > 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 | 9 +++++++++
> >   1 file changed, 9 insertions(+)

> > diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
> > index 0f1cc9029..7d2517ccc 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 (Serve=
r
> only)
> >                                            "link": Link type (Server
only)
> > +                                         "prep-authz-req": boolean Is
> prepare write
> > +
authorization
> request

> I'd keep it simple and just call it authorize.
Ok.

> >                          Possible Errors: org.bluez.Error.Failed
> >                                           org.bluez.Error.InProgress
> > @@ -251,6 +253,11 @@ 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 attribut=
e
> > +                       operations.

> We should probably mention the default is false if the property is not
> available.
Ok.

> >   Characteristic Descriptors hierarchy
> >   =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

> > @@ -284,6 +291,8 @@ Methods             array{byte} ReadValue(dict
flags)
> >                          Possible options: "offset": Start offset
> >                                            "device": Device path (Serve=
r
> only)
> >                                            "link": Link type (Server
only)
> > +                                         "prep-authz-req": boolean Is
> prepare write
> > +
authorization
> request

> Do we need and authorize flag for read? I thought this was only for
prepare
> write.
I guess this is a confusing patch presentatnion formating :) this option is
for WriteValue not for ReadValue in real, but somehow git format patch
treat "ReadValue" as Method... or something in this way.

> >                          Possible Errors: org.bluez.Error.Failed
> >                                           org.bluez.Error.InProgress
> > --
> > 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