Skip to contentSkip to conversation

shared/gatt-server: Add bt_gatt_server_set_permissions

PATCH Superseded Luiz Augusto von Dentz 2 participants 3 messages Apr 8 Bluetooth drivers

lore
Share & tools
lore

Luiz Augusto von Dentz

author maintainer
#0 lore raw

PATCH v2 1/2 shared/gatt-server: Add bt_gatt_server_set_permissions

From: Luiz Augusto von Dentz <[email protected]>

This adds bt_gatt_server_set_permissions which can be used to enabled/disable permission checking before operations which can be useful on testing only environment where encryption/pairing is not desirable/needed.

---
 src/shared/gatt-server.c | 13 +++++++++++++
 src/shared/gatt-server.h |  2 ++
 2 files changed, 15 insertions(+)
src/shared/gatt-server.c +13 -0
index 27ae7e79ed3d..6273899965c0 100644
@@ -93,6 +93,7 @@ struct bt_gatt_server {
 	struct bt_att *att;
 	int ref_count;
 	uint16_t mtu;
+	bool perms;
 
 	unsigned int mtu_id;
 	unsigned int read_by_grp_type_id;
@@ -414,6 +415,9 @@ static uint8_t check_permissions(struct bt_gatt_server *server,
 	uint32_t perm;
 	int security;
 
+	if (!server->perms)
+		return 0;
+
 	perm = gatt_db_attribute_get_permissions(attr);
 
 	if (perm && mask & BT_ATT_PERM_READ && !(perm & BT_ATT_PERM_READ))
@@ -1652,6 +1656,7 @@ struct bt_gatt_server *bt_gatt_server_new(struct gatt_db *db,
 	server->db = gatt_db_ref(db);
 	server->att = bt_att_ref(att);
 	server->mtu = MAX(mtu, BT_ATT_DEFAULT_LE_MTU);
+	server->perms = true;
 	server->max_prep_queue_len = DEFAULT_MAX_PREP_QUEUE_LEN;
 	server->prep_queue = queue_new();
 	server->min_enc_size = min_enc_size;
@@ -1680,6 +1685,14 @@ struct bt_att *bt_gatt_server_get_att(struct bt_gatt_server *server)
 	return server->att;
 }
 
+void bt_gatt_server_set_permissions(struct bt_gatt_server *server, bool value)
+{
+	if (!server)
+		return;
+
+	server->perms = value;
+}
+
 struct bt_gatt_server *bt_gatt_server_ref(struct bt_gatt_server *server)
 {
 	if (!server)
src/shared/gatt-server.h +2 -0
index de98a0d04747..ea49f2960d71 100644
@@ -17,6 +17,8 @@ struct bt_gatt_server *bt_gatt_server_new(struct gatt_db *db,
 					uint8_t min_enc_size);
 uint16_t bt_gatt_server_get_mtu(struct bt_gatt_server *server);
 struct bt_att *bt_gatt_server_get_att(struct bt_gatt_server *server);
+void bt_gatt_server_set_permissions(struct bt_gatt_server *server,
+					bool value);
 
 struct bt_gatt_server *bt_gatt_server_ref(struct bt_gatt_server *server);
 void bt_gatt_server_unref(struct bt_gatt_server *server);
Signature · 1 line
2.53.0
author maintainer
#1 lore raw

PATCH v2 2/2 main.conf: Add GATT.Security option

From: Luiz Augusto von Dentz <[email protected]>

This adds GATT.Security option which by default is set to auto to detect if encryption/authentication is required on demand, but if Testing is set enables the user to enter a desirable security level.

In case the security level is low it then proceeds to disable checking GATT attribute permissions for server operations.

---
 src/btd.h     |  1 +
 src/device.c  | 16 +++++++++++++---
 src/main.c    | 35 +++++++++++++++++++++++++++++++++++
 src/main.conf |  8 ++++++++
 4 files changed, 57 insertions(+), 3 deletions(-)
src/btd.h +1 -0
index 1b521706d05e..c84a600d109a 100644
@@ -158,6 +158,7 @@ struct btd_opts {
 	uint8_t		gatt_channels;
 	bool		gatt_client;
 	enum bt_gatt_export_t gatt_export;
+	bool		gatt_seclevel;
 	enum mps_mode_t	mps;
 
 	struct btd_avdtp_opts avdtp;
src/device.c +13 -3
index cfbde307bcc9..3f86e60bd666 100644
@@ -6300,7 +6300,7 @@ static void gatt_client_init(struct btd_device *device)
 	if (btd_opts.gatt_channels > 1)
 		features |= BT_GATT_CHRC_CLI_FEAT_EATT;
 
-	if (device->bonding) {
+	if (!btd_opts.gatt_seclevel && device->bonding) {
 		DBG("Elevating security level since bonding is in progress");
 		bt_att_set_security(device->att, BT_ATT_SECURITY_MEDIUM);
 	}
@@ -6371,6 +6371,9 @@ static void gatt_server_init(struct btd_device *device,
 	if (device->ltk)
 		bt_att_set_enc_key_size(device->att, device->ltk->enc_size);
 
+	if (btd_opts.gatt_seclevel == BT_ATT_SECURITY_LOW)
+		bt_gatt_server_set_permissions(device->server, false);
+
 	bt_gatt_server_set_debug(device->server, gatt_debug, NULL, NULL);
 
 	btd_gatt_database_server_connected(database, device->server);
@@ -6442,7 +6445,8 @@ bool device_attach_att(struct btd_device *dev, GIOChannel *io)
 		return false;
 	}
 
-	if (sec_level == BT_IO_SEC_LOW && dev->le_state.paired) {
+	if (!btd_opts.gatt_seclevel && sec_level == BT_IO_SEC_LOW &&
+					dev->le_state.paired) {
 		DBG("Elevating security level since LTK is available");
 
 		sec_level = BT_IO_SEC_MEDIUM;
@@ -6482,6 +6486,10 @@ bool device_attach_att(struct btd_device *dev, GIOChannel *io)
 		bt_att_set_remote_key(dev->att, dev->remote_csrk->key,
 							remote_counter, dev);
 
+	/* Force security level if it has been set */
+	if (btd_opts.gatt_seclevel)
+		bt_att_set_security(dev->att, btd_opts.gatt_seclevel);
+
 	database = btd_adapter_get_database(dev->adapter);
 
 	dst = device_get_address(dev);
@@ -6597,7 +6605,9 @@ int device_connect_le(struct btd_device *dev)
 	/* Set as initiator */
 	dev->le_state.initiator = true;
 
-	if (dev->le_state.paired)
+	if (btd_opts.gatt_seclevel)
+		sec_level = btd_opts.gatt_seclevel;
+	else if (dev->le_state.paired)
 		sec_level = BT_IO_SEC_MEDIUM;
 	else
 		sec_level = BT_IO_SEC_LOW;
src/main.c +35 -0
index 59df0ad4ca3a..818f7c06ef66 100644
@@ -152,6 +152,7 @@ static const char *gatt_options[] = {
 	"Channels",
 	"Client",
 	"ExportClaimedServices",
+	"Security",
 	NULL
 };
 
@@ -1112,6 +1113,38 @@ static void parse_gatt_export(GKeyFile *config)
 	g_free(str);
 }
 
+static uint8_t parse_gatt_seclevel_str(const char *str)
+{
+	if (!strcmp(str, "auto"))
+		return BT_ATT_SECURITY_AUTO;
+	else if (!strcmp(str, "low") || !strcmp(str, "1"))
+		return BT_ATT_SECURITY_LOW;
+	else if (!strcmp(str, "medium") || !strcmp(str, "2"))
+		return BT_ATT_SECURITY_MEDIUM;
+	else if (!strcmp(str, "high") || !strcmp(str, "3"))
+		return BT_ATT_SECURITY_HIGH;
+	else if (!strcmp(str, "fips") || !strcmp(str, "4"))
+		return BT_ATT_SECURITY_FIPS;
+
+	DBG("Invalid value for Security=%s", str);
+	return BT_ATT_SECURITY_AUTO;
+}
+
+static void parse_gatt_seclevel(GKeyFile *config)
+{
+	char *str = NULL;
+
+	if (!btd_opts.testing)
+		return;
+
+	parse_config_string(config, "GATT", "Security", &str);
+	if (!str)
+		return;
+
+	btd_opts.gatt_seclevel = parse_gatt_seclevel_str(str);
+	g_free(str);
+}
+
 static void parse_gatt(GKeyFile *config)
 {
 	parse_gatt_cache(config);
@@ -1122,6 +1155,7 @@ static void parse_gatt(GKeyFile *config)
 				1, 6);
 	parse_config_bool(config, "GATT", "Client", &btd_opts.gatt_client);
 	parse_gatt_export(config);
+	parse_gatt_seclevel(config);
 }
 
 static void parse_csis_sirk(GKeyFile *config)
@@ -1269,6 +1303,7 @@ static void init_defaults(void)
 	btd_opts.gatt_channels = 1;
 	btd_opts.gatt_client = true;
 	btd_opts.gatt_export = BT_GATT_EXPORT_READ_ONLY;
+	btd_opts.gatt_seclevel = BT_ATT_SECURITY_AUTO;
 
 	btd_opts.avdtp.session_mode = BT_IO_MODE_BASIC;
 	btd_opts.avdtp.stream_mode = BT_IO_MODE_BASIC;
src/main.conf +8 -0
index fd1ace651da7..52eb3854addc 100644
@@ -291,6 +291,14 @@ KernelExperimental = 6fbaf188-05e0-496a-9885-d6ddfdb4e03e
 # Default: read-only
 #ExportClaimedServices = read-only
 
+# Security level:
+# Sets security level of ATT channel, setting security anything other than
+# auto requires Testing to be set, setting to low disables GATT server
+# attribite permissions.
+# Possible values: auto, [low=1, medium=2, high=3, fips=4 (Testing = true)]
+# Default = auto
+# Security = auto
+
 [CSIS]
 # SIRK - Set Identification Resolution Key which is common for all the
 # sets. They SIRK key is used to identify its sets. This can be any
Signature · 1 line
2.53.0
#2 lore raw
In reply to Luiz Augusto von Dentz

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1078903

---Test result---

Test Summary:

CheckPatch                    PENDING   0.37 seconds
GitLint                       PENDING   0.36 seconds
BuildEll                      PASS      19.88 seconds
BluezMake                     PASS      620.11 seconds
MakeCheck                     PASS      18.19 seconds
MakeDistcheck                 PASS      239.32 seconds
CheckValgrind                 PASS      286.41 seconds
CheckSmatch                   WARNING   341.71 seconds
bluezmakeextell               PASS      177.17 seconds
IncrementalBuild              PENDING   0.34 seconds
ScanBuild                     PASS      990.84 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/shared/gatt-server.c:279:25: warning: Variable length array is used.src/shared/gatt-server.c:622:25: warning: Variable length array is used.src/shared/gatt-server.c:720:25: warning: Variable length array is used.src/shared/gatt-server.c:279:25: warning: Variable length array is used.src/shared/gatt-server.c:622:25: warning: Variable length array is used.src/shared/gatt-server.c:720:25: warning: Variable length array is used.src/shared/gatt-server.c:279:25: warning: Variable length array is used.src/shared/gatt-server.c:622:25: warning: Variable length array is used.src/shared/gatt-server.c:720:25: warning: Variable length array is used.
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:

https://github.com/bluez/bluez/pull/2025/checks

---
Regards,
Linux Bluetooth