[PATCHv2 1/6] shared/gatt: Define atribute permissions as uint32_t

[PATCHv2 1/6] shared/gatt: Define atribute permissions as uint32_t

[Marcin Kraglak]

---
 src/shared/gatt-db.c | 6 +++---
 src/shared/gatt-db.h | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index 36316af..90c09bb 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -48,7 +48,7 @@ struct gatt_db {
 struct gatt_db_attribute {
 	uint16_t handle;
 	bt_uuid_t uuid;
-	uint8_t permissions;
+	uint32_t permissions;
 	uint16_t value_len;
 	uint8_t *value;
 
@@ -269,7 +269,7 @@ static void set_attribute_data(struct gatt_db_attribute *attribute,
 
 uint16_t gatt_db_add_characteristic(struct gatt_db *db, uint16_t handle,
 						const bt_uuid_t *uuid,
-						uint8_t permissions,
+						uint32_t permissions,
 						uint8_t properties,
 						gatt_db_read_t read_func,
 						gatt_db_write_t write_func,
@@ -317,7 +317,7 @@ uint16_t gatt_db_add_characteristic(struct gatt_db *db, uint16_t handle,
 
 uint16_t gatt_db_add_char_descriptor(struct gatt_db *db, uint16_t handle,
 						const bt_uuid_t *uuid,
-						uint8_t permissions,
+						uint32_t permissions,
 						gatt_db_read_t read_func,
 						gatt_db_write_t write_func,
 						void *user_data)
diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
index 3d46730..350e34e 100644
--- a/src/shared/gatt-db.h
+++ b/src/shared/gatt-db.h
@@ -41,7 +41,7 @@ typedef void (*gatt_db_write_t) (uint16_t handle, uint16_t offset,
 
 uint16_t gatt_db_add_characteristic(struct gatt_db *db, uint16_t handle,
 						const bt_uuid_t *uuid,
-						uint8_t permissions,
+						uint32_t permissions,
 						uint8_t properties,
 						gatt_db_read_t read_func,
 						gatt_db_write_t write_func,
@@ -49,7 +49,7 @@ uint16_t gatt_db_add_characteristic(struct gatt_db *db, uint16_t handle,
 
 uint16_t gatt_db_add_char_descriptor(struct gatt_db *db, uint16_t handle,
 						const bt_uuid_t *uuid,
-						uint8_t permissions,
+						uint32_t permissions,
 						gatt_db_read_t read_func,
 						gatt_db_write_t write_func,
 						void *user_data);
-- 
1.9.0

[PATCHv2 2/6] shared/gatt: Add helper for getting attribute's permissions

[Marcin Kraglak]

It will return attribute's permissions or 0 if attribute was not found.
---
 src/shared/gatt-db.c | 25 +++++++++++++++++++++++++
 src/shared/gatt-db.h |  2 ++
 2 files changed, 27 insertions(+)

diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index 90c09bb..d6f3143 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -733,3 +733,28 @@ uint16_t gatt_db_get_end_handle(struct gatt_db *db, uint16_t handle)
 
 	return service->attributes[0]->handle + service->num_handles - 1;
 }
+
+uint32_t gatt_db_get_attribute_permissions(struct gatt_db *db, uint16_t handle)
+{
+	struct gatt_db_attribute *attribute;
+	struct gatt_db_service *service;
+	uint16_t service_handle;
+
+	service = queue_find(db->services, find_service_for_handle,
+							INT_TO_PTR(handle));
+	if (!service)
+		return 0;
+
+	service_handle = service->attributes[0]->handle;
+
+	/*
+	 * We can safely get attribute from attributes array with offset,
+	 * because find_service_for_handle() check if given handle is
+	 * in service range.
+	 */
+	attribute = service->attributes[handle - service_handle];
+	if (!attribute)
+		return 0;
+
+	return attribute->permissions;
+}
diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
index 350e34e..f2f2f4d 100644
--- a/src/shared/gatt-db.h
+++ b/src/shared/gatt-db.h
@@ -91,3 +91,5 @@ const bt_uuid_t *gatt_db_get_attribute_type(struct gatt_db *db,
 							uint16_t handle);
 
 uint16_t gatt_db_get_end_handle(struct gatt_db *db, uint16_t handle);
+
+uint32_t gatt_db_get_attribute_permissions(struct gatt_db *db, uint16_t handle);
-- 
1.9.0

[PATCHv2 3/6] android/hal-gatt: Define attribute permissions values

[Marcin Kraglak]

These values are defined in Android and should be used on daemon
side.
---
 android/hal-msg.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/android/hal-msg.h b/android/hal-msg.h
index 09bd9a0..b68a4c7 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -1590,3 +1590,12 @@ struct hal_ev_gatt_server_rsp_confirmation {
 	int32_t status;
 	int32_t handle;
 } __attribute__((packed));
+
+#define HAL_GATT_PERMISSION_READ			0x0001
+#define HAL_GATT_PERMISSION_READ_ENCRYPTED		0x0002
+#define HAL_GATT_PERMISSION_READ_ENCRYPTED_MITM		0x0004
+#define HAL_GATT_PERMISSION_WRITE			0x0010
+#define HAL_GATT_PERMISSION_WRITE_ENCRYPTED		0x0020
+#define HAL_GATT_PERMISSION_WRITE_ENCRYPTED_MITM	0x0040
+#define HAL_GATT_PERMISSION_WRITE_SIGNED		0x0080
+#define HAL_GATT_PERMISSION_WRITE_SIGNED_MITM		0x0100
-- 
1.9.0

[PATCHv2 4/6] android/gatt: Store proper attributes permissions in database

[Marcin Kraglak]

Convert Android HAL permissions to GATT server permissions and store
them in database.
---
 android/gatt.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 51 insertions(+), 4 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 70cd5fe..bfcbb62 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -57,6 +57,17 @@
 
 #define BASE_UUID16_OFFSET     12
 
+#define GATT_PERM_READ			0x00000001
+#define GATT_PERM_READ_ENCRYPTED	0x00000002
+#define GATT_PERM_READ_MITM		0x00000004
+#define GATT_PERM_READ_AUTHORIZATION	0x00000008
+#define GATT_PERM_WRITE			0x00000100
+#define GATT_PERM_WRITE_ENCRYPTED	0x00000200
+#define GATT_PERM_WRITE_MITM		0x00000400
+#define GATT_PERM_WRITE_AUTHORIZATION	0x00000800
+#define GATT_PERM_WRITE_SIGNED		0x00010000
+#define GATT_PERM_WRITE_SIGNED_MITM	0x00020000
+
 static const uint8_t BLUETOOTH_UUID[] = {
 	0xfb, 0x34, 0x9b, 0x5f, 0x80, 0x00, 0x00, 0x80,
 	0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
@@ -4013,6 +4024,40 @@ failed:
 								bdaddr);
 }
 
+static uint32_t android_to_gatt_permissions(int32_t hal_permissions)
+{
+	uint32_t permissions = 0;
+
+	if (hal_permissions & HAL_GATT_PERMISSION_READ)
+		permissions |= GATT_PERM_READ;
+
+	if (hal_permissions & HAL_GATT_PERMISSION_READ_ENCRYPTED)
+		permissions |= GATT_PERM_READ_ENCRYPTED | GATT_PERM_READ;
+
+	if (hal_permissions & HAL_GATT_PERMISSION_READ_ENCRYPTED_MITM)
+		permissions |= GATT_PERM_READ_MITM | GATT_PERM_READ_ENCRYPTED |
+								GATT_PERM_READ;
+
+	if (hal_permissions & HAL_GATT_PERMISSION_WRITE)
+		permissions |= GATT_PERM_WRITE;
+
+	if (hal_permissions & HAL_GATT_PERMISSION_WRITE_ENCRYPTED)
+		permissions |= GATT_PERM_WRITE_ENCRYPTED | GATT_PERM_WRITE;
+
+	if (hal_permissions & HAL_GATT_PERMISSION_WRITE_ENCRYPTED_MITM)
+		permissions |= GATT_PERM_WRITE_MITM |
+				GATT_PERM_WRITE_ENCRYPTED | GATT_PERM_WRITE;
+
+	if (hal_permissions & HAL_GATT_PERMISSION_WRITE_SIGNED)
+		permissions |= GATT_PERM_WRITE_SIGNED;
+
+	if (hal_permissions & HAL_GATT_PERMISSION_WRITE_SIGNED_MITM)
+		permissions |= GATT_PERM_WRITE_SIGNED_MITM |
+							GATT_PERM_WRITE_SIGNED;
+
+	return permissions;
+}
+
 static void handle_server_add_characteristic(const void *buf, uint16_t len)
 {
 	const struct hal_cmd_gatt_server_add_characteristic *cmd = buf;
@@ -4020,6 +4065,7 @@ static void handle_server_add_characteristic(const void *buf, uint16_t len)
 	struct gatt_app *server;
 	bt_uuid_t uuid;
 	uint8_t status;
+	uint32_t permissions;
 	int32_t app_id = cmd->server_if;
 
 	DBG("");
@@ -4033,11 +4079,11 @@ static void handle_server_add_characteristic(const void *buf, uint16_t len)
 	}
 
 	android2uuid(cmd->uuid, &uuid);
+	permissions = android_to_gatt_permissions(cmd->permissions);
 
-	/*FIXME: Handle properties. Register callback if needed. */
 	ev.char_handle = gatt_db_add_characteristic(gatt_db,
 							cmd->service_handle,
-							&uuid, cmd->permissions,
+							&uuid, permissions,
 							cmd->properties,
 							read_cb, write_cb,
 							INT_TO_PTR(app_id));
@@ -4067,6 +4113,7 @@ static void handle_server_add_descriptor(const void *buf, uint16_t len)
 	struct gatt_app *server;
 	bt_uuid_t uuid;
 	uint8_t status;
+	uint32_t permissions;
 	int32_t app_id = cmd->server_if;
 
 	DBG("");
@@ -4080,11 +4127,11 @@ static void handle_server_add_descriptor(const void *buf, uint16_t len)
 	}
 
 	android2uuid(cmd->uuid, &uuid);
+	permissions = android_to_gatt_permissions(cmd->permissions);
 
-	/*FIXME: Handle properties. Register callback if needed. */
 	ev.descr_handle = gatt_db_add_char_descriptor(gatt_db,
 							cmd->service_handle,
-							&uuid, cmd->permissions,
+							&uuid, permissions,
 							read_cb, write_cb,
 							INT_TO_PTR(app_id));
 	if (!ev.descr_handle)
-- 
1.9.0

[PATCHv2 5/6] android/gatt: Add proper permissions to built-in services

[Marcin Kraglak]

Set permissions to built in services.
---
 android/gatt.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index bfcbb62..cde8816 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -5078,7 +5078,7 @@ static void register_gap_service(void)
 	bt_uuid16_create(&uuid, GATT_CHARAC_DEVICE_NAME);
 	gap_srvc_data.dev_name =
 			gatt_db_add_characteristic(gatt_db, gap_srvc_data.srvc,
-							&uuid, 0,
+							&uuid, GATT_PERM_READ,
 							GATT_CHR_PROP_READ,
 							gap_read_cb, NULL,
 							NULL);
@@ -5087,7 +5087,7 @@ static void register_gap_service(void)
 	bt_uuid16_create(&uuid, GATT_CHARAC_APPEARANCE);
 	gap_srvc_data.appear =
 			gatt_db_add_characteristic(gatt_db, gap_srvc_data.srvc,
-							&uuid, 0,
+							&uuid, GATT_PERM_READ,
 							GATT_CHR_PROP_READ,
 							gap_read_cb, NULL,
 							NULL);
@@ -5096,7 +5096,7 @@ static void register_gap_service(void)
 	bt_uuid16_create(&uuid, GATT_CHARAC_PERIPHERAL_PRIV_FLAG);
 	gap_srvc_data.priv =
 			gatt_db_add_characteristic(gatt_db, gap_srvc_data.srvc,
-							&uuid, 0,
+							&uuid, GATT_PERM_READ,
 							GATT_CHR_PROP_READ,
 							gap_read_cb, NULL,
 							NULL);
@@ -5169,43 +5169,43 @@ static void register_device_info_service(void)
 
 	/* User data are not const hence (void *) cast is used */
 	bt_uuid16_create(&uuid, GATT_CHARAC_SYSTEM_ID);
-	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
+	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, GATT_PERM_READ,
 					GATT_CHR_PROP_READ,
 					device_info_read_cb, NULL,
 					(void *) device_info.system_id);
 
 	bt_uuid16_create(&uuid, GATT_CHARAC_MODEL_NUMBER_STRING);
-	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
+	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, GATT_PERM_READ,
 					GATT_CHR_PROP_READ,
 					device_info_read_cb, NULL,
 					(void *) device_info.model_number);
 
 	bt_uuid16_create(&uuid, GATT_CHARAC_SERIAL_NUMBER_STRING);
-	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
+	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, GATT_PERM_READ,
 					GATT_CHR_PROP_READ,
 					device_info_read_cb, NULL,
 					(void *) device_info.serial_number);
 
 	bt_uuid16_create(&uuid, GATT_CHARAC_FIRMWARE_REVISION_STRING);
-	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
+	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, GATT_PERM_READ,
 					GATT_CHR_PROP_READ,
 					device_info_read_cb, NULL,
 					(void *) device_info.firmware_rev);
 
 	bt_uuid16_create(&uuid, GATT_CHARAC_HARDWARE_REVISION_STRING);
-	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
+	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, GATT_PERM_READ,
 					GATT_CHR_PROP_READ,
 					device_info_read_cb, NULL,
 					(void *) device_info.hardware_rev);
 
 	bt_uuid16_create(&uuid, GATT_CHARAC_SOFTWARE_REVISION_STRING);
-	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
+	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, GATT_PERM_READ,
 					GATT_CHR_PROP_READ,
 					device_info_read_cb, NULL,
 					(void *) device_info.software_rev);
 
 	bt_uuid16_create(&uuid, GATT_CHARAC_MANUFACTURER_NAME_STRING);
-	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
+	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, GATT_PERM_READ,
 					GATT_CHR_PROP_READ,
 					device_info_read_cb, NULL,
 					(void *) device_info.manufacturer_name);
@@ -5250,13 +5250,14 @@ static void register_gatt_service(void)
 	srvc_handle = gatt_db_add_service(gatt_db, &uuid, true, 4);
 
 	bt_uuid16_create(&uuid, GATT_CHARAC_SERVICE_CHANGED);
-	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
+	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, GATT_PERM_READ,
 					GATT_CHR_PROP_INDICATE, NULL, NULL,
 					NULL);
 
 	bt_uuid16_create(&uuid, GATT_CLIENT_CHARAC_CFG_UUID);
-	gatt_db_add_char_descriptor(gatt_db, srvc_handle, &uuid, 0, NULL,
-					gatt_srvc_change_register_cb, NULL);
+	gatt_db_add_char_descriptor(gatt_db, srvc_handle, &uuid, GATT_PERM_READ,
+					NULL, gatt_srvc_change_register_cb,
+					NULL);
 
 	gatt_db_service_set_active(gatt_db, srvc_handle, true);
 }
-- 
1.9.0

[PATCHv2 6/6] android/gatt: Check permissions before read execution

[Marcin Kraglak]

Check device security level and attribute permissions before read
execution.
---
 android/gatt.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/android/gatt.c b/android/gatt.c
index cde8816..2d0e06a 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -3817,13 +3817,101 @@ static bool match_dev_request_by_handle(const void *data, const void *user_data)
 	return handle_data->handle == handle;
 }
 
+static uint8_t check_device_permissions(struct gatt_device *device,
+					uint8_t opcode, uint32_t permissions)
+{
+	GIOChannel *io;
+	int sec_level;
+
+	io = g_attrib_get_channel(device->attrib);
+
+	if (!bt_io_get(io, NULL, BT_IO_OPT_SEC_LEVEL, &sec_level,
+							BT_IO_OPT_INVALID))
+		return ATT_ECODE_UNLIKELY;
+
+	switch (opcode) {
+	case ATT_OP_SIGNED_WRITE_CMD:
+		if (!(permissions & GATT_PERM_WRITE_SIGNED))
+				return ATT_ECODE_WRITE_NOT_PERM;
+
+		if ((permissions & GATT_PERM_WRITE_SIGNED_MITM) && sec_level <
+							BT_SECURITY_HIGH)
+			return ATT_ECODE_AUTHENTICATION;
+
+		break;
+	case ATT_OP_READ_BY_TYPE_REQ:
+	case ATT_OP_READ_REQ:
+	case ATT_OP_READ_BLOB_REQ:
+	case ATT_OP_READ_MULTI_REQ:
+	case ATT_OP_READ_BY_GROUP_REQ:
+		if (permissions && !(permissions & GATT_PERM_READ))
+			return ATT_ECODE_READ_NOT_PERM;
+
+		if ((permissions & GATT_PERM_READ_ENCRYPTED) && sec_level <
+							BT_SECURITY_MEDIUM)
+			return ATT_ECODE_INSUFF_ENC;
+
+		if ((permissions & GATT_PERM_READ_MITM) && sec_level <
+							BT_SECURITY_HIGH)
+			return ATT_ECODE_AUTHENTICATION;
+
+		if ((permissions & GATT_PERM_READ_AUTHORIZATION))
+			return ATT_ECODE_AUTHORIZATION;
+
+		break;
+	case ATT_OP_WRITE_REQ:
+	case ATT_OP_WRITE_CMD:
+	case ATT_OP_PREP_WRITE_REQ:
+	case ATT_OP_EXEC_WRITE_REQ:
+		if (!(permissions & GATT_PERM_WRITE))
+			return ATT_ECODE_WRITE_NOT_PERM;
+
+		if ((permissions & GATT_PERM_WRITE_ENCRYPTED) && sec_level <
+							BT_SECURITY_MEDIUM)
+			return ATT_ECODE_INSUFF_ENC;
+
+		if ((permissions & GATT_PERM_WRITE_MITM) &&
+						sec_level < BT_SECURITY_HIGH)
+			return ATT_ECODE_AUTHENTICATION;
+
+		if ((permissions & GATT_PERM_WRITE_AUTHORIZATION))
+			return ATT_ECODE_AUTHORIZATION;
+
+	break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
 static void read_requested_attributes(void *data, void *user_data)
 {
 	struct pending_request *resp_data = data;
 	struct request_processing_data *process_data = user_data;
+	uint32_t permissions;
 	uint8_t *value;
 	int value_len;
 
+	permissions = gatt_db_get_attribute_permissions(gatt_db,
+			resp_data->handle);
+
+	/*
+	 * Check if it is attribute we didn't declare permissions, like service
+	 * declaration or included service. Set permissions to read only
+	 */
+	if (permissions == 0)
+		permissions = GATT_PERM_READ;
+
+	resp_data->error = check_device_permissions(process_data->device,
+							process_data->opcode,
+							permissions);
+
+	if (resp_data->error) {
+		resp_data->state = REQUEST_DONE;
+		return;
+	}
+
 	if (!gatt_db_read(gatt_db, resp_data->handle,
 						resp_data->offset,
 						process_data->opcode,
-- 
1.9.0

Re: [PATCHv2 1/6] shared/gatt: Define atribute permissions as uint32_t

[Szymon Janc]

Hi Marcin,

On Monday 26 of May 2014 11:10:24 Marcin Kraglak wrote:
> ---
>  src/shared/gatt-db.c | 6 +++---
>  src/shared/gatt-db.h | 4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
> index 36316af..90c09bb 100644
> --- a/src/shared/gatt-db.c
> +++ b/src/shared/gatt-db.c
> @@ -48,7 +48,7 @@ struct gatt_db {
>  struct gatt_db_attribute {
>  	uint16_t handle;
>  	bt_uuid_t uuid;
> -	uint8_t permissions;
> +	uint32_t permissions;
>  	uint16_t value_len;
>  	uint8_t *value;
>  
> @@ -269,7 +269,7 @@ static void set_attribute_data(struct gatt_db_attribute *attribute,
>  
>  uint16_t gatt_db_add_characteristic(struct gatt_db *db, uint16_t handle,
>  						const bt_uuid_t *uuid,
> -						uint8_t permissions,
> +						uint32_t permissions,
>  						uint8_t properties,
>  						gatt_db_read_t read_func,
>  						gatt_db_write_t write_func,
> @@ -317,7 +317,7 @@ uint16_t gatt_db_add_characteristic(struct gatt_db *db, uint16_t handle,
>  
>  uint16_t gatt_db_add_char_descriptor(struct gatt_db *db, uint16_t handle,
>  						const bt_uuid_t *uuid,
> -						uint8_t permissions,
> +						uint32_t permissions,
>  						gatt_db_read_t read_func,
>  						gatt_db_write_t write_func,
>  						void *user_data)
> diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
> index 3d46730..350e34e 100644
> --- a/src/shared/gatt-db.h
> +++ b/src/shared/gatt-db.h
> @@ -41,7 +41,7 @@ typedef void (*gatt_db_write_t) (uint16_t handle, uint16_t offset,
>  
>  uint16_t gatt_db_add_characteristic(struct gatt_db *db, uint16_t handle,
>  						const bt_uuid_t *uuid,
> -						uint8_t permissions,
> +						uint32_t permissions,
>  						uint8_t properties,
>  						gatt_db_read_t read_func,
>  						gatt_db_write_t write_func,
> @@ -49,7 +49,7 @@ uint16_t gatt_db_add_characteristic(struct gatt_db *db, uint16_t handle,
>  
>  uint16_t gatt_db_add_char_descriptor(struct gatt_db *db, uint16_t handle,
>  						const bt_uuid_t *uuid,
> -						uint8_t permissions,
> +						uint32_t permissions,
>  						gatt_db_read_t read_func,
>  						gatt_db_write_t write_func,
>  						void *user_data);
> 

All patches applied (with some cleanups in the last one), thanks.

-- 
Best regards, 
Szymon Janc