All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH BlueZ v3 1/4] doc/gatt-api: Add authorization options for attributes
@ 2018-05-25  7:38 Grzegorz Kolodziejczyk
  2018-05-25  7:38 ` [PATCH BlueZ v3 2/4] shared/gatt-server: Request authorization for prepare writes Grzegorz Kolodziejczyk
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Grzegorz Kolodziejczyk @ 2018-05-25  7:38 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 | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
index 0f1cc9029..914fc7031 100644
--- a/doc/gatt-api.txt
+++ b/doc/gatt-api.txt
@@ -85,6 +85,9 @@ Methods		array{byte} ReadValue(dict options)
 			Possible options: "offset": Start offset
 					  "device": Device path (Server only)
 					  "link": Link type (Server only)
+					  "prep-authorize": boolean Is prepare
+							    authorization
+							    request
 
 			Possible Errors: org.bluez.Error.Failed
 					 org.bluez.Error.InProgress
@@ -251,6 +254,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 +293,9 @@ Methods		array{byte} ReadValue(dict flags)
 			Possible options: "offset": Start offset
 					  "device": Device path (Server only)
 					  "link": Link type (Server only)
+					  "prep-authorize": boolean Is prepare
+							    authorization
+							    request
 
 			Possible Errors: org.bluez.Error.Failed
 					 org.bluez.Error.InProgress
@@ -322,6 +334,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.
+
 GATT Profile hierarchy
 =====================
 
-- 
2.13.6


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH BlueZ v3 2/4] shared/gatt-server: Request authorization for prepare writes.
  2018-05-25  7:38 [PATCH BlueZ v3 1/4] doc/gatt-api: Add authorization options for attributes Grzegorz Kolodziejczyk
@ 2018-05-25  7:38 ` Grzegorz Kolodziejczyk
  2018-05-25  7:38 ` [PATCH BlueZ v3 3/4] client: Add authorized property handling to characteristic attribute Grzegorz Kolodziejczyk
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Grzegorz Kolodziejczyk @ 2018-05-25  7:38 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      | 93 ++++++++++++++++++++++++++++++++++++++++--------
 src/shared/gatt-server.c | 64 ++++++++++++++++++++++++++++-----
 2 files changed, 134 insertions(+), 23 deletions(-)

diff --git a/src/gatt-database.c b/src/gatt-database.c
index 0ac5b75b0..f46d96271 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -133,6 +133,8 @@ struct external_chrc {
 	struct queue *pending_reads;
 	struct queue *pending_writes;
 	unsigned int ntfy_cnt;
+	bool prep_authorized;
+	bool req_prep_authorization;
 };
 
 struct external_desc {
@@ -144,6 +146,8 @@ struct external_desc {
 	bool handled;
 	struct queue *pending_reads;
 	struct queue *pending_writes;
+	bool prep_authorized;
+	bool req_prep_authorization;
 };
 
 struct pending_op {
@@ -154,6 +158,8 @@ struct pending_op {
 	struct gatt_db_attribute *attrib;
 	struct queue *owner_queue;
 	struct iovec data;
+	bool is_characteristic;
+	bool prep_authorize;
 };
 
 struct notify {
@@ -1937,6 +1943,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_authorize)
+		dict_append_entry(iter, "prep-authorize", DBUS_TYPE_BOOLEAN,
+							&op->prep_authorize);
 }
 
 static void read_setup_cb(DBusMessageIter *iter, void *user_data)
@@ -2008,6 +2017,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 +2038,16 @@ static void write_reply_cb(DBusMessage *message, void *user_data)
 		goto done;
 	}
 
+	if (op->prep_authorize) {
+		if (op->is_characteristic) {
+			chrc = gatt_db_attribute_get_user_data(op->attrib);
+			chrc->prep_authorized = true;
+		} else {
+			desc = gatt_db_attribute_get_user_data(op->attrib);
+			desc->prep_authorized = true;
+		}
+	}
+
 	dbus_message_iter_init(message, &iter);
 	if (dbus_message_iter_has_next(&iter)) {
 		/*
@@ -2045,9 +2066,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_authorize)
 {
 	struct pending_op *op;
 
@@ -2062,6 +2084,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_authorize = prep_authorize;
 	queue_push_tail(owner_queue, op);
 
 	return op;
@@ -2073,12 +2097,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_authorize)
 {
 	struct pending_op *op;
 
 	op = pending_write_new(device, owner_queue, attrib, id, value, len,
-							offset, link_type);
+					offset, link_type, is_characteristic,
+					prep_authorize);
 
 	if (g_dbus_proxy_method_call(proxy, "WriteValue", write_setup_cb,
 					owner_queue ? write_reply_cb : NULL,
@@ -2191,7 +2218,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 +2256,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,
@@ -2532,8 +2559,24 @@ static void desc_write_cb(struct gatt_db_attribute *attrib,
 		goto fail;
 	}
 
+	if (opcode == BT_ATT_OP_PREP_WRITE_REQ) {
+		if (!desc->prep_authorized && desc->req_prep_authorization)
+			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->prep_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:
@@ -2544,12 +2587,16 @@ static bool database_add_desc(struct external_service *service,
 						struct external_desc *desc)
 {
 	bt_uuid_t uuid;
+	DBusMessageIter iter;
 
 	if (!parse_uuid(desc->proxy, &uuid)) {
 		error("Failed to read \"UUID\" property of descriptor");
 		return false;
 	}
 
+	desc->req_prep_authorization = g_dbus_proxy_get_property(desc->proxy,
+							"Authorize", &iter);
+
 	desc->attrib = gatt_db_service_add_descriptor(service->attrib, &uuid,
 							desc->perm,
 							desc_read_cb,
@@ -2614,6 +2661,25 @@ 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->prep_authorized && chrc->req_prep_authorization)
+			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->prep_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 +2696,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:
@@ -2679,6 +2740,7 @@ static bool database_add_chrc(struct external_service *service,
 {
 	bt_uuid_t uuid;
 	const struct queue_entry *entry;
+	DBusMessageIter iter;
 
 	if (!parse_uuid(chrc->proxy, &uuid)) {
 		error("Failed to read \"UUID\" property of characteristic");
@@ -2690,6 +2752,9 @@ static bool database_add_chrc(struct external_service *service,
 		return false;
 	}
 
+	chrc->req_prep_authorization = g_dbus_proxy_get_property(chrc->proxy,
+							"Authorize", &iter);
+
 	chrc->attrib = gatt_db_service_add_characteristic(service->attrib,
 						&uuid, chrc->perm,
 						chrc->props, chrc_read_cb,
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] 6+ messages in thread

* [PATCH BlueZ v3 3/4] client: Add authorized property handling to characteristic attribute
  2018-05-25  7:38 [PATCH BlueZ v3 1/4] doc/gatt-api: Add authorization options for attributes Grzegorz Kolodziejczyk
  2018-05-25  7:38 ` [PATCH BlueZ v3 2/4] shared/gatt-server: Request authorization for prepare writes Grzegorz Kolodziejczyk
@ 2018-05-25  7:38 ` Grzegorz Kolodziejczyk
  2018-05-25  7:38 ` [PATCH BlueZ v3 4/4] client: Don't require authorization for trusted devices Grzegorz Kolodziejczyk
  2018-05-25  7:52 ` [PATCH BlueZ v3 1/4] doc/gatt-api: Add authorization options for attributes Luiz Augusto von Dentz
  3 siblings, 0 replies; 6+ messages in thread
From: Grzegorz Kolodziejczyk @ 2018-05-25  7:38 UTC (permalink / raw)
  To: linux-bluetooth

This patch adds handling of characteristic prepare write authorized
property to bluetoothctl.
---
 client/gatt.c | 158 +++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 107 insertions(+), 51 deletions(-)

diff --git a/client/gatt.c b/client/gatt.c
index d37a6f55c..13c5d8e49 100644
--- a/client/gatt.c
+++ b/client/gatt.c
@@ -80,7 +80,6 @@ struct chrc {
 	struct io *write_io;
 	struct io *notify_io;
 	bool authorization_req;
-	bool authorized;
 };
 
 struct service {
@@ -1581,6 +1580,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 },
@@ -1591,6 +1614,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 },
 	{ }
 };
 
@@ -1609,7 +1634,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_authorize)
 {
 	DBusMessageIter dict;
 
@@ -1650,6 +1676,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-authorize") == 0) {
+			if (var != DBUS_TYPE_BOOLEAN)
+				return -EINVAL;
+			if (prep_authorize)
+				dbus_message_iter_get_basic(&value,
+								prep_authorize);
 		}
 
 		dbus_message_iter_next(&dict);
@@ -1703,8 +1735,6 @@ 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;
-
 	g_dbus_send_message(aad->conn, reply);
 
 	g_free(aad);
@@ -1727,18 +1757,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) {
 		struct authorize_attribute_data *aad;
 
 		aad = g_new0(struct authorize_attribute_data, 1);
@@ -1765,33 +1792,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;
 }
@@ -1800,12 +1825,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_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, &prep_authorize)) {
+		err = "org.bluez.Error.InvalidArguments";
+
+		goto error;
+	}
 
 	if (!strcmp(input, "no")) {
 		err = "org.bluez.Error.NotAuthorized";
@@ -1813,15 +1852,17 @@ static void authorize_write_response(const char *input, void *user_data)
 		goto error;
 	}
 
-	chrc->authorized = true;
+	/* Authorization check of prepare writes */
+	if (prep_authorize) {
+		reply = g_dbus_create_reply(pending_message, DBUS_TYPE_INVALID);
+		g_dbus_send_message(aad->conn, reply);
+		g_free(aad);
 
-	errsv = parse_value_arg(&iter, &chrc->value, &chrc->value_len,
-							chrc->max_val_len);
-	if (errsv == -EINVAL) {
-		err = "org.bluez.Error.InvalidArguments";
+		return;
+	}
 
-		goto error;
-	} else if (errsv == -EOVERFLOW) {
+	if (write_value(&chrc->value_len, &chrc->value, value, value_len,
+					aad->offset, chrc->max_val_len)) {
 		err = "org.bluez.Error.InvalidValueLength";
 
 		goto error;
@@ -1848,18 +1889,31 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
 							void *user_data)
 {
 	struct chrc *chrc = user_data;
+	uint16_t offset = 0;
+	bool prep_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, &prep_authorize))
+		return g_dbus_create_error(msg,
+				"org.bluez.Error.InvalidArguments", NULL);
+
+	if (chrc->authorization_req) {
 		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);
@@ -1873,15 +1927,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_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);
 
@@ -1943,7 +1996,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);
@@ -1975,7 +2028,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);
@@ -2191,7 +2244,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);
@@ -2213,21 +2266,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] 6+ messages in thread

* [PATCH BlueZ v3 4/4] client: Don't require authorization for trusted devices
  2018-05-25  7:38 [PATCH BlueZ v3 1/4] doc/gatt-api: Add authorization options for attributes Grzegorz Kolodziejczyk
  2018-05-25  7:38 ` [PATCH BlueZ v3 2/4] shared/gatt-server: Request authorization for prepare writes Grzegorz Kolodziejczyk
  2018-05-25  7:38 ` [PATCH BlueZ v3 3/4] client: Add authorized property handling to characteristic attribute Grzegorz Kolodziejczyk
@ 2018-05-25  7:38 ` Grzegorz Kolodziejczyk
  2018-05-25  7:52 ` [PATCH BlueZ v3 1/4] doc/gatt-api: Add authorization options for attributes Luiz Augusto von Dentz
  3 siblings, 0 replies; 6+ messages in thread
From: Grzegorz Kolodziejczyk @ 2018-05-25  7:38 UTC (permalink / raw)
  To: linux-bluetooth

This patch adds possibility to ommit authorization request from trusted
devices.
---
 client/gatt.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/client/gatt.c b/client/gatt.c
index 13c5d8e49..629912675 100644
--- a/client/gatt.c
+++ b/client/gatt.c
@@ -1746,6 +1746,20 @@ error:
 	g_free(aad);
 }
 
+static bool is_device_trusted(const char *path)
+{
+	GDBusProxy *proxy;
+	DBusMessageIter iter;
+	bool trusted;
+
+	proxy = bt_shell_get_env(path);
+
+	if (g_dbus_proxy_get_property(proxy, "Trusted", &iter))
+		dbus_message_iter_get_basic(&iter, &trusted);
+
+	return trusted;
+}
+
 static DBusMessage *chrc_read_value(DBusConnection *conn, DBusMessage *msg,
 							void *user_data)
 {
@@ -1765,7 +1779,7 @@ static DBusMessage *chrc_read_value(DBusConnection *conn, DBusMessage *msg,
 	bt_shell_printf("ReadValue: %s offset %u link %s\n",
 					path_to_address(device), offset, link);
 
-	if (chrc->authorization_req) {
+	if (!is_device_trusted(device) && chrc->authorization_req) {
 		struct authorize_attribute_data *aad;
 
 		aad = g_new0(struct authorize_attribute_data, 1);
@@ -1891,6 +1905,7 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
 	struct chrc *chrc = user_data;
 	uint16_t offset = 0;
 	bool prep_authorize = false;
+	char *device = NULL;
 	DBusMessageIter iter;
 	int value_len;
 	uint8_t *value;
@@ -1903,11 +1918,11 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
 				"org.bluez.Error.InvalidArguments", NULL);
 
 	dbus_message_iter_next(&iter);
-	if (parse_options(&iter, &offset, NULL, NULL, NULL, &prep_authorize))
+	if (parse_options(&iter, &offset, NULL, &device, NULL, &prep_authorize))
 		return g_dbus_create_error(msg,
 				"org.bluez.Error.InvalidArguments", NULL);
 
-	if (chrc->authorization_req) {
+	if (!is_device_trusted(device) && chrc->authorization_req) {
 		struct authorize_attribute_data *aad;
 
 		aad = g_new0(struct authorize_attribute_data, 1);
-- 
2.13.6


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH BlueZ v3 1/4] doc/gatt-api: Add authorization options for attributes
  2018-05-25  7:38 [PATCH BlueZ v3 1/4] doc/gatt-api: Add authorization options for attributes Grzegorz Kolodziejczyk
                   ` (2 preceding siblings ...)
  2018-05-25  7:38 ` [PATCH BlueZ v3 4/4] client: Don't require authorization for trusted devices Grzegorz Kolodziejczyk
@ 2018-05-25  7:52 ` Luiz Augusto von Dentz
  2018-05-25  8:06   ` Grzegorz Kołodziejczyk
  3 siblings, 1 reply; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2018-05-25  7:52 UTC (permalink / raw)
  To: Grzegorz Kolodziejczyk; +Cc: linux-bluetooth

Hi Grzegorz,

On Fri, May 25, 2018 at 10:38 AM, 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 | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
> index 0f1cc9029..914fc7031 100644
> --- a/doc/gatt-api.txt
> +++ b/doc/gatt-api.txt
> @@ -85,6 +85,9 @@ Methods               array{byte} ReadValue(dict options)
>                         Possible options: "offset": Start offset
>                                           "device": Device path (Server only)
>                                           "link": Link type (Server only)
> +                                         "prep-authorize": boolean Is prepare
> +                                                           authorization
> +                                                           request
>
>                         Possible Errors: org.bluez.Error.Failed
>                                          org.bluez.Error.InProgress
> @@ -251,6 +254,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.

One of the things we discussed yesterday was to move to Flags as
authorization, that way it is not even possible to set it to false.

>  Characteristic Descriptors hierarchy
>  ====================================
>
> @@ -284,6 +293,9 @@ Methods             array{byte} ReadValue(dict flags)
>                         Possible options: "offset": Start offset
>                                           "device": Device path (Server only)
>                                           "link": Link type (Server only)
> +                                         "prep-authorize": boolean Is prepare
> +                                                           authorization
> +                                                           request
>
>                         Possible Errors: org.bluez.Error.Failed
>                                          org.bluez.Error.InProgress
> @@ -322,6 +334,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.
> +
>  GATT Profile hierarchy
>  =====================
>
> --
> 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] 6+ messages in thread

* Re: [PATCH BlueZ v3 1/4] doc/gatt-api: Add authorization options for attributes
  2018-05-25  7:52 ` [PATCH BlueZ v3 1/4] doc/gatt-api: Add authorization options for attributes Luiz Augusto von Dentz
@ 2018-05-25  8:06   ` Grzegorz Kołodziejczyk
  0 siblings, 0 replies; 6+ messages in thread
From: Grzegorz Kołodziejczyk @ 2018-05-25  8:06 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

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

> Hi Grzegorz,

> On Fri, May 25, 2018 at 10:38 AM, 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 | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
> > index 0f1cc9029..914fc7031 100644
> > --- a/doc/gatt-api.txt
> > +++ b/doc/gatt-api.txt
> > @@ -85,6 +85,9 @@ Methods               array{byte} ReadValue(dict
options)
> >                         Possible options: "offset": Start offset
> >                                           "device": Device path (Server
only)
> >                                           "link": Link type (Server
only)
> > +                                         "prep-authorize": boolean Is
prepare
> > +
authorization
> > +                                                           request
> >
> >                         Possible Errors: org.bluez.Error.Failed
> >                                          org.bluez.Error.InProgress
> > @@ -251,6 +254,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 attribut=
e
> > +                       operations. By default this valuie is set to
false for
> > +                       attribute.

> One of the things we discussed yesterday was to move to Flags as
> authorization, that way it is not even possible to set it to false.

Ok - now I got it, I've missunderstood it.
> >  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 +293,9 @@ Methods             array{byte} ReadValue(dict
flags)
> >                         Possible options: "offset": Start offset
> >                                           "device": Device path (Server
only)
> >                                           "link": Link type (Server
only)
> > +                                         "prep-authorize": boolean Is
prepare
> > +
authorization
> > +                                                           request
> >
> >                         Possible Errors: org.bluez.Error.Failed
> >                                          org.bluez.Error.InProgress
> > @@ -322,6 +334,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 attribut=
e
> > +                       operations. By default this valuie is set to
false for
> > +                       attribute.
> > +
> >  GATT Profile hierarchy
> >  =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> >
> > --
> > 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] 6+ messages in thread

end of thread, other threads:[~2018-05-25  8:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-25  7:38 [PATCH BlueZ v3 1/4] doc/gatt-api: Add authorization options for attributes Grzegorz Kolodziejczyk
2018-05-25  7:38 ` [PATCH BlueZ v3 2/4] shared/gatt-server: Request authorization for prepare writes Grzegorz Kolodziejczyk
2018-05-25  7:38 ` [PATCH BlueZ v3 3/4] client: Add authorized property handling to characteristic attribute Grzegorz Kolodziejczyk
2018-05-25  7:38 ` [PATCH BlueZ v3 4/4] client: Don't require authorization for trusted devices Grzegorz Kolodziejczyk
2018-05-25  7:52 ` [PATCH BlueZ v3 1/4] doc/gatt-api: Add authorization options for attributes Luiz Augusto von Dentz
2018-05-25  8:06   ` Grzegorz Kołodziejczyk

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.