[PATCH BlueZ 1/4] doc/gatt-api: Add encryption flags

[PATCH BlueZ 1/4] doc/gatt-api: Add encryption flags

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This add encryption flags which can be used when registering a service to
require encryption when accessing a characteristic.
---
 doc/gatt-api.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
index 088d285..8db35f2 100644
--- a/doc/gatt-api.txt
+++ b/doc/gatt-api.txt
@@ -148,6 +148,10 @@ Properties	string UUID [read-only]
 				"authenticated-signed-writes"
 				"reliable-write"
 				"writable-auxiliaries"
+				"encrypt-read"
+				"encrypt-write"
+				"encrypt-authenticated-read"
+				"encrypt-authenticated-write"
 
 		array{object} Descriptors [read-only]
 
-- 
2.1.0

[PATCH BlueZ 2/4] core/gatt: Add support for encryption flags

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds support to encryption related flags as defined in the
documentation.
---
 src/gatt-database.c    | 28 ++++++++++++++++++++++++++--
 src/shared/att-types.h | 30 ++++++++++++++++++++++--------
 2 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/src/gatt-database.c b/src/gatt-database.c
index 2261398..aaa8ad1 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -1241,6 +1241,14 @@ static bool parse_flags(GDBusProxy *proxy, uint8_t *props, uint8_t *ext_props)
 			*ext_props |= BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE;
 		else if (!strcmp("writable-auxiliaries", flag))
 			*ext_props |= BT_GATT_CHRC_EXT_PROP_WRITABLE_AUX;
+		else if (!strcmp("encrypt-read", flag))
+			*ext_props |= BT_GATT_CHRC_EXT_PROP_ENC_READ;
+		else if (!strcmp("encrypt-write", flag))
+			*ext_props |= BT_GATT_CHRC_EXT_PROP_ENC_WRITE;
+		else if (!strcmp("encrypt-authenticated-read", flag))
+			*ext_props |= BT_GATT_CHRC_EXT_PROP_AUTH_READ;
+		else if (!strcmp("encrypt-authenticated-write", flag))
+			*ext_props |= BT_GATT_CHRC_EXT_PROP_AUTH_WRITE;
 		else {
 			error("Invalid characteristic flag: %s", flag);
 			return false;
@@ -1668,12 +1676,28 @@ static uint32_t permissions_from_props(uint8_t props, uint8_t ext_props)
 
 	if (props & BT_GATT_CHRC_PROP_WRITE ||
 			props & BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP ||
-			ext_props & BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE)
+			ext_props & BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE ||
+			ext_props & BT_GATT_CHRC_EXT_PROP_ENC_WRITE ||
+			ext_props & BT_GATT_CHRC_EXT_PROP_AUTH_WRITE)
 		perm |= BT_ATT_PERM_WRITE;
 
-	if (props & BT_GATT_CHRC_PROP_READ)
+	if (props & BT_GATT_CHRC_PROP_READ ||
+			ext_props & BT_GATT_CHRC_EXT_PROP_ENC_READ ||
+			ext_props & BT_GATT_CHRC_EXT_PROP_AUTH_READ)
 		perm |= BT_ATT_PERM_READ;
 
+	if (ext_props & BT_GATT_CHRC_EXT_PROP_ENC_READ)
+		perm |= BT_ATT_PERM_READ_ENCRYPT;
+
+	if (ext_props & BT_GATT_CHRC_EXT_PROP_ENC_WRITE)
+		perm |= BT_ATT_PERM_WRITE_ENCRYPT;
+
+	if (ext_props & BT_GATT_CHRC_EXT_PROP_AUTH_READ)
+		perm |= BT_ATT_PERM_READ_AUTHEN;
+
+	if (ext_props & BT_GATT_CHRC_EXT_PROP_AUTH_WRITE)
+		perm |= BT_ATT_PERM_WRITE_AUTHEN;
+
 	return perm;
 }
 
diff --git a/src/shared/att-types.h b/src/shared/att-types.h
index 10a42f2..ce531d1 100644
--- a/src/shared/att-types.h
+++ b/src/shared/att-types.h
@@ -106,12 +106,18 @@ struct bt_att_pdu_error_rsp {
  * "Access", "Encryption", "Authentication", and "Authorization". A bitmask of
  * permissions is a byte that encodes a combination of these.
  */
-#define BT_ATT_PERM_READ	0x01
-#define BT_ATT_PERM_WRITE	0x02
-#define BT_ATT_PERM_ENCRYPT	0x04
-#define BT_ATT_PERM_AUTHEN	0x08
-#define BT_ATT_PERM_AUTHOR	0x10
-#define BT_ATT_PERM_NONE	0x20
+#define BT_ATT_PERM_READ		0x01
+#define BT_ATT_PERM_WRITE		0x02
+#define BT_ATT_PERM_READ_ENCRYPT	0x04
+#define BT_ATT_PERM_WRITE_ENCRYPT	0x08
+#define BT_ATT_PERM_ENCRYPT		BT_ATT_PERM_READ_ENCRYPT | \
+					BT_ATT_PERM_WRITE_ENCRYPT
+#define BT_ATT_PERM_READ_AUTHEN		0x10
+#define BT_ATT_PERM_WRITE_AUTHEN	0x20
+#define BT_ATT_PERM_AUTHEN		BT_ATT_PERM_READ_AUTHEN | \
+					BT_ATT_PERM_WRITE_AUTHEN
+#define BT_ATT_PERM_AUTHOR		0x40
+#define BT_ATT_PERM_NONE		0x80
 
 /* GATT Characteristic Properties Bitfield values */
 #define BT_GATT_CHRC_PROP_BROADCAST			0x01
@@ -124,5 +130,13 @@ struct bt_att_pdu_error_rsp {
 #define BT_GATT_CHRC_PROP_EXT_PROP			0x80
 
 /* GATT Characteristic Extended Properties Bitfield values */
-#define BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE	0x01
-#define BT_GATT_CHRC_EXT_PROP_WRITABLE_AUX	0x02
+#define BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE		0x01
+#define BT_GATT_CHRC_EXT_PROP_WRITABLE_AUX		0x02
+#define BT_GATT_CHRC_EXT_PROP_ENC_READ			0x04
+#define BT_GATT_CHRC_EXT_PROP_ENC_WRITE			0x08
+#define BT_GATT_CHRC_EXT_PROP_ENC	BT_GATT_CHRC_EXT_PROP_ENC_READ | \
+					BT_GATT_CHRC_EXT_PROP_ENC_WRITE
+#define BT_GATT_CHRC_EXT_PROP_AUTH_READ			0x10
+#define BT_GATT_CHRC_EXT_PROP_AUTH_WRITE		0x20
+#define BT_GATT_CHRC_EXT_PROP_AUTH	BT_GATT_CHRC_EXT_PROP_AUTH_READ | \
+					BT_GATT_CHRC_EXT_PROP_AUTH_WRITE
-- 
2.1.0

[PATCH BlueZ 3/4] shared/att: Add own security definitions

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This defines security level at ATT level so it is not necessary to use
transport specific defines.
---
 src/shared/att-types.h | 5 +++++
 src/shared/att.c       | 2 +-
 unit/test-gatt.c       | 2 +-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/shared/att-types.h b/src/shared/att-types.h
index ce531d1..ea4c009 100644
--- a/src/shared/att-types.h
+++ b/src/shared/att-types.h
@@ -27,6 +27,11 @@
 #define __packed __attribute__((packed))
 #endif
 
+#define BT_ATT_SECURITY_NONE	0
+#define BT_ATT_SECURITY_LOW	1
+#define BT_ATT_SECURITY_MEDIUM	2
+#define BT_ATT_SECURITY_HIGH	3
+
 #define BT_ATT_DEFAULT_LE_MTU	23
 #define BT_ATT_MAX_LE_MTU	517
 #define BT_ATT_MAX_VALUE_LEN	512
diff --git a/src/shared/att.c b/src/shared/att.c
index 3b37cdf..f24da18 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -1364,7 +1364,7 @@ bool bt_att_set_sec_level(struct bt_att *att, int level)
 {
 	struct bt_security sec;
 
-	if (!att || level < BT_SECURITY_LOW || level > BT_SECURITY_HIGH)
+	if (!att || level < BT_ATT_SECURITY_LOW || level > BT_ATT_SECURITY_HIGH)
 		return false;
 
 	if (!att->io_on_l2cap) {
diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index 415680b..caaacbd 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -1002,7 +1002,7 @@ static void test_signed_write_seclevel(struct context *context)
 	g_assert(bt_att_set_local_key(context->att, key, local_counter,
 								context));
 
-	g_assert(bt_att_set_sec_level(context->att, BT_SECURITY_MEDIUM));
+	g_assert(bt_att_set_sec_level(context->att, BT_ATT_SECURITY_MEDIUM));
 
 	g_assert(bt_gatt_client_write_without_response(context->client,
 							step->handle,
-- 
2.1.0

[PATCH BlueZ 4/4] shared/gatt-server: Check attribute permissions

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

---
 src/shared/att-types.h   | 16 ++++-----
 src/shared/gatt-server.c | 93 ++++++++++++++++++++++++++----------------------
 2 files changed, 58 insertions(+), 51 deletions(-)

diff --git a/src/shared/att-types.h b/src/shared/att-types.h
index ea4c009..ee20992 100644
--- a/src/shared/att-types.h
+++ b/src/shared/att-types.h
@@ -115,12 +115,12 @@ struct bt_att_pdu_error_rsp {
 #define BT_ATT_PERM_WRITE		0x02
 #define BT_ATT_PERM_READ_ENCRYPT	0x04
 #define BT_ATT_PERM_WRITE_ENCRYPT	0x08
-#define BT_ATT_PERM_ENCRYPT		BT_ATT_PERM_READ_ENCRYPT | \
-					BT_ATT_PERM_WRITE_ENCRYPT
+#define BT_ATT_PERM_ENCRYPT		(BT_ATT_PERM_READ_ENCRYPT | \
+					BT_ATT_PERM_WRITE_ENCRYPT)
 #define BT_ATT_PERM_READ_AUTHEN		0x10
 #define BT_ATT_PERM_WRITE_AUTHEN	0x20
-#define BT_ATT_PERM_AUTHEN		BT_ATT_PERM_READ_AUTHEN | \
-					BT_ATT_PERM_WRITE_AUTHEN
+#define BT_ATT_PERM_AUTHEN		(BT_ATT_PERM_READ_AUTHEN | \
+					BT_ATT_PERM_WRITE_AUTHEN)
 #define BT_ATT_PERM_AUTHOR		0x40
 #define BT_ATT_PERM_NONE		0x80
 
@@ -139,9 +139,9 @@ struct bt_att_pdu_error_rsp {
 #define BT_GATT_CHRC_EXT_PROP_WRITABLE_AUX		0x02
 #define BT_GATT_CHRC_EXT_PROP_ENC_READ			0x04
 #define BT_GATT_CHRC_EXT_PROP_ENC_WRITE			0x08
-#define BT_GATT_CHRC_EXT_PROP_ENC	BT_GATT_CHRC_EXT_PROP_ENC_READ | \
-					BT_GATT_CHRC_EXT_PROP_ENC_WRITE
+#define BT_GATT_CHRC_EXT_PROP_ENC	(BT_GATT_CHRC_EXT_PROP_ENC_READ | \
+					BT_GATT_CHRC_EXT_PROP_ENC_WRITE)
 #define BT_GATT_CHRC_EXT_PROP_AUTH_READ			0x10
 #define BT_GATT_CHRC_EXT_PROP_AUTH_WRITE		0x20
-#define BT_GATT_CHRC_EXT_PROP_AUTH	BT_GATT_CHRC_EXT_PROP_AUTH_READ | \
-					BT_GATT_CHRC_EXT_PROP_AUTH_WRITE
+#define BT_GATT_CHRC_EXT_PROP_AUTH	(BT_GATT_CHRC_EXT_PROP_AUTH_READ | \
+					BT_GATT_CHRC_EXT_PROP_AUTH_WRITE)
diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index b5f55ad..ae77dcc 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -377,12 +377,39 @@ done:
 	process_read_by_type(op);
 }
 
+static uint8_t check_permissions(struct bt_gatt_server *server,
+				struct gatt_db_attribute *attr, uint32_t mask)
+{
+	uint32_t perm;
+	int security;
+
+	perm = gatt_db_attribute_get_permissions(attr);
+
+	if (perm && mask & BT_ATT_PERM_READ && !(perm & BT_ATT_PERM_READ))
+		return BT_ATT_ERROR_READ_NOT_PERMITTED;
+
+	if (perm && mask & BT_ATT_PERM_WRITE && !(perm & BT_ATT_PERM_WRITE))
+		return BT_ATT_ERROR_WRITE_NOT_PERMITTED;
+
+	perm &= mask;
+	if (!perm)
+		return 0;
+
+	security = bt_att_get_sec_level(server->att);
+	if (perm & BT_ATT_PERM_AUTHEN && security < BT_ATT_SECURITY_HIGH)
+		return BT_ATT_ERROR_AUTHENTICATION;
+
+	if (perm & BT_ATT_PERM_ENCRYPT && security < BT_ATT_SECURITY_MEDIUM)
+		return BT_ATT_ERROR_INSUFFICIENT_ENCRYPTION;
+
+	return 0;
+}
+
 static void process_read_by_type(struct async_read_op *op)
 {
 	struct bt_gatt_server *server = op->server;
 	uint8_t ecode;
 	struct gatt_db_attribute *attr;
-	uint32_t perm;
 
 	attr = queue_pop_head(op->db_data);
 
@@ -395,18 +422,11 @@ static void process_read_by_type(struct async_read_op *op)
 		return;
 	}
 
-	perm = gatt_db_attribute_get_permissions(attr);
-
-	/*
-	 * Check for the READ access permission. Encryption,
-	 * authentication, and authorization permissions need to be
-	 * checked by the read handler, since bt_att is agnostic to
-	 * connection type and doesn't have security information on it.
-	 */
-	if (perm && !(perm & BT_ATT_PERM_READ)) {
-		ecode = BT_ATT_ERROR_READ_NOT_PERMITTED;
+	ecode = check_permissions(server, attr, BT_ATT_PERM_READ |
+						BT_ATT_PERM_READ_AUTHEN |
+						BT_ATT_PERM_READ_ENCRYPT);
+	if (ecode)
 		goto error;
-	}
 
 	if (gatt_db_attribute_read(attr, 0, op->opcode, server->att,
 					read_by_type_read_complete_cb, op))
@@ -752,7 +772,6 @@ static void write_cb(uint8_t opcode, const void *pdu,
 	uint16_t handle = 0;
 	struct async_write_op *op = NULL;
 	uint8_t ecode;
-	uint32_t perm;
 
 	if (length < 2) {
 		ecode = BT_ATT_ERROR_INVALID_PDU;
@@ -771,12 +790,11 @@ static void write_cb(uint8_t opcode, const void *pdu,
 				(opcode == BT_ATT_OP_WRITE_REQ) ? "Req" : "Cmd",
 				handle);
 
-	perm = gatt_db_attribute_get_permissions(attr);
-
-	if (!(perm & BT_ATT_PERM_WRITE)) {
-		ecode = BT_ATT_ERROR_WRITE_NOT_PERMITTED;
+	ecode = check_permissions(server, attr, BT_ATT_PERM_WRITE |
+						BT_ATT_PERM_WRITE_AUTHEN |
+						BT_ATT_PERM_WRITE_ENCRYPT);
+	if (ecode)
 		goto error;
-	}
 
 	if (server->pending_write_op) {
 		ecode = BT_ATT_ERROR_UNLIKELY;
@@ -871,7 +889,6 @@ static void handle_read_req(struct bt_gatt_server *server, uint8_t opcode,
 {
 	struct gatt_db_attribute *attr;
 	uint8_t ecode;
-	uint32_t perm;
 	struct async_read_op *op = NULL;
 
 	attr = gatt_db_get_attribute(server->db, handle);
@@ -885,12 +902,11 @@ static void handle_read_req(struct bt_gatt_server *server, uint8_t opcode,
 			opcode == BT_ATT_OP_READ_BLOB_REQ ? "Blob " : "",
 			handle);
 
-	perm = gatt_db_attribute_get_permissions(attr);
-
-	if (perm && !(perm & BT_ATT_PERM_READ)) {
-		ecode = BT_ATT_ERROR_READ_NOT_PERMITTED;
+	ecode = check_permissions(server, attr, BT_ATT_PERM_READ |
+						BT_ATT_PERM_READ_AUTHEN |
+						BT_ATT_PERM_READ_ENCRYPT);
+	if (ecode)
 		goto error;
-	}
 
 	if (server->pending_read_op) {
 		ecode = BT_ATT_ERROR_UNLIKELY;
@@ -980,8 +996,8 @@ static void read_multiple_complete_cb(struct gatt_db_attribute *attr, int err,
 {
 	struct read_multiple_resp_data *data = user_data;
 	struct gatt_db_attribute *next_attr;
-	uint32_t perm;
 	uint16_t handle = gatt_db_attribute_get_handle(attr);
+	uint8_t ecode;
 
 	if (err != 0) {
 		bt_att_send_error_rsp(data->server->att,
@@ -990,12 +1006,12 @@ static void read_multiple_complete_cb(struct gatt_db_attribute *attr, int err,
 		return;
 	}
 
-	perm = gatt_db_attribute_get_permissions(attr);
-
-	if (perm && !(perm & BT_ATT_PERM_READ)) {
+	ecode = check_permissions(data->server, attr, BT_ATT_PERM_READ |
+						BT_ATT_PERM_READ_AUTHEN |
+						BT_ATT_PERM_READ_ENCRYPT);
+	if (ecode) {
 		bt_att_send_error_rsp(data->server->att,
-					BT_ATT_OP_READ_MULT_REQ, handle,
-					BT_ATT_ERROR_READ_NOT_PERMITTED);
+					BT_ATT_OP_READ_MULT_REQ, handle, ecode);
 		read_multiple_resp_data_free(data);
 		return;
 	}
@@ -1107,7 +1123,6 @@ static void prep_write_cb(uint8_t opcode, const void *pdu,
 	uint16_t offset;
 	struct gatt_db_attribute *attr;
 	uint8_t ecode;
-	uint32_t perm;
 
 	if (length < 4) {
 		ecode = BT_ATT_ERROR_INVALID_PDU;
@@ -1131,19 +1146,11 @@ static void prep_write_cb(uint8_t opcode, const void *pdu,
 	util_debug(server->debug_callback, server->debug_data,
 				"Prep Write Req - handle: 0x%04x", handle);
 
-	perm = gatt_db_attribute_get_permissions(attr);
-
-	/*
-	 * TODO: The "Prepare Write" request requires security permission checks
-	 * to be performed before the write is executed. I.e., we can't leave
-	 * the permission check to the upper layer since we can't call
-	 * gatt_db_write until the entire queue is atomically processed during
-	 * an "Execute Write" request. Figure out how to make this check here.
-	 */
-	if (!(perm & BT_ATT_PERM_WRITE)) {
-		ecode = BT_ATT_ERROR_WRITE_NOT_PERMITTED;
+	ecode = check_permissions(server, attr, BT_ATT_PERM_WRITE |
+						BT_ATT_PERM_WRITE_AUTHEN |
+						BT_ATT_PERM_WRITE_ENCRYPT);
+	if (ecode)
 		goto error;
-	}
 
 	prep_data = new0(struct prep_write_data, 1);
 	if (!prep_data) {
-- 
2.1.0

Re: [PATCH BlueZ 1/4] doc/gatt-api: Add encryption flags

[Arman Uguray]

Hi Luiz,

> On Thu, Apr 23, 2015 at 7:20 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This add encryption flags which can be used when registering a service to
> require encryption when accessing a characteristic.
> ---
>  doc/gatt-api.txt | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
> index 088d285..8db35f2 100644
> --- a/doc/gatt-api.txt
> +++ b/doc/gatt-api.txt
> @@ -148,6 +148,10 @@ Properties string UUID [read-only]
>                                 "authenticated-signed-writes"
>                                 "reliable-write"
>                                 "writable-auxiliaries"
> +                               "encrypt-read"
> +                               "encrypt-write"
> +                               "encrypt-authenticated-read"
> +                               "encrypt-authenticated-write"

So I guess you decided not to go with a "Permissions" property? I
guess that's fine but we need to handle this for descriptors as well.
So probably add a "Flags" property to GattDescriptor1 also. There
we'll want read, write, encrypt-*, and encrypt-authenticated-*.

>
>                 array{object} Descriptors [read-only]
>
> --
> 2.1.0
>
> --
> 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

Can you also update test/example-gatt-server with new characteristics
and descriptors that demonstrate the permissions feature? You can add
them as new vendor-specific test characteristics (there's a
vendor-specific test service in there) and/or modify one of the other
two example services (HR and Battery). Let's get that into this patch
set so we know we have some way to test it.

Thanks,
Arman

Re: [PATCH BlueZ 1/4] doc/gatt-api: Add encryption flags

[Luiz Augusto von Dentz]

Hi Arman,

On Thu, Apr 23, 2015 at 10:14 PM, Arman Uguray <armansito@chromium.org> wrote:
> Hi Luiz,
>
>> On Thu, Apr 23, 2015 at 7:20 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>
>> This add encryption flags which can be used when registering a service to
>> require encryption when accessing a characteristic.
>> ---
>>  doc/gatt-api.txt | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
>> index 088d285..8db35f2 100644
>> --- a/doc/gatt-api.txt
>> +++ b/doc/gatt-api.txt
>> @@ -148,6 +148,10 @@ Properties string UUID [read-only]
>>                                 "authenticated-signed-writes"
>>                                 "reliable-write"
>>                                 "writable-auxiliaries"
>> +                               "encrypt-read"
>> +                               "encrypt-write"
>> +                               "encrypt-authenticated-read"
>> +                               "encrypt-authenticated-write"
>
> So I guess you decided not to go with a "Permissions" property? I
> guess that's fine but we need to handle this for descriptors as well.
> So probably add a "Flags" property to GattDescriptor1 also. There
> we'll want read, write, encrypt-*, and encrypt-authenticated-*.

Sure, will do that in the next set. What is more important now is
getting feedback if the API is good enough, I have been debating in
using secure instead of encrypt, what do you think?

>>
>>                 array{object} Descriptors [read-only]
>>
>> --
>> 2.1.0
>>
>> --
>> 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
>
> Can you also update test/example-gatt-server with new characteristics
> and descriptors that demonstrate the permissions feature? You can add
> them as new vendor-specific test characteristics (there's a
> vendor-specific test service in there) and/or modify one of the other
> two example services (HR and Battery). Let's get that into this patch
> set so we know we have some way to test it.

Yep, I was thinking about that just now, will add this in the next version.


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 1/4] doc/gatt-api: Add encryption flags

[Arman Uguray]

Hi Luiz,

> On Thu, Apr 23, 2015 at 12:21 PM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Thu, Apr 23, 2015 at 10:14 PM, Arman Uguray <armansito@chromium.org> wrote:
>> Hi Luiz,
>>
>>> On Thu, Apr 23, 2015 at 7:20 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>>
>>> This add encryption flags which can be used when registering a service to
>>> require encryption when accessing a characteristic.
>>> ---
>>>  doc/gatt-api.txt | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
>>> index 088d285..8db35f2 100644
>>> --- a/doc/gatt-api.txt
>>> +++ b/doc/gatt-api.txt
>>> @@ -148,6 +148,10 @@ Properties string UUID [read-only]
>>>                                 "authenticated-signed-writes"
>>>                                 "reliable-write"
>>>                                 "writable-auxiliaries"
>>> +                               "encrypt-read"
>>> +                               "encrypt-write"
>>> +                               "encrypt-authenticated-read"
>>> +                               "encrypt-authenticated-write"
>>
>> So I guess you decided not to go with a "Permissions" property? I
>> guess that's fine but we need to handle this for descriptors as well.
>> So probably add a "Flags" property to GattDescriptor1 also. There
>> we'll want read, write, encrypt-*, and encrypt-authenticated-*.
>
> Sure, will do that in the next set. What is more important now is
> getting feedback if the API is good enough, I have been debating in
> using secure instead of encrypt, what do you think?
>

I'm fine with encrypt, I think staying closer to the GATT spec lingo
will be less confusing for developers. "secure" is somewhat unclear
whereas encrypt is very explicit and it has a matching ATT protocol
error that a developer can immediately associate this permission with.

Looking through some of the other platform APIs, I see these:

iOS:

typedef enum {
    CBAttributePermissionsReadable = 0x01,
    CBAttributePermissionsWriteable = 0x02,
    CBAttributePermissionsReadEncryptionRequired = 0x04,
    CBAttributePermissionsWriteEncryptionRequired = 0x08,
} CBAttributePermissions;

and Android has READ_ENCRYPTED, READ_ENCRYPTED_MITM, etc as you
already know. So "encrypt" should sound familiar to developers.

>>>                 array{object} Descriptors [read-only]
>>>
>>> --
>>> 2.1.0
>>>
>>> --
>>> 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
>>
>> Can you also update test/example-gatt-server with new characteristics
>> and descriptors that demonstrate the permissions feature? You can add
>> them as new vendor-specific test characteristics (there's a
>> vendor-specific test service in there) and/or modify one of the other
>> two example services (HR and Battery). Let's get that into this patch
>> set so we know we have some way to test it.
>
> Yep, I was thinking about that just now, will add this in the next version.
>

Cool, thanks.

>
> --
> Luiz Augusto von Dentz

-Arman