2021-08-03 11:46:19

by Yun-hao Chung

[permalink] [raw]
Subject: [Bluez PATCH v9 00/13] Admin policy series

From: Yun-Hao Chung <[email protected]>


Hi manintainers,

This series is to
1. Implement a few methods in core so that a plugin can have control of
allowing / disallowing certain service connections.
2. Implement the AdminPolicy plugin. The plugin provides interfaces
AdminPolicySet and AdminPolicyStatus. For each policy, users should
set the value thorugh AdminPolicySet and query the current setting
through AdminPolicyStatus. We separeted these two interfaces so that
developers can assign different groups of users to these interfaces.
Currently the only policy is ServiceAllowList, which make bluez only
allow a list of service by specified their UUIDs, but the plugin is
also expected to provide more controls over other bluez behaviors.
Since the second part is a plugin, it might not be necessary to land in
upstream tree.

Thanks.

Changes in v9:
- Fix gitlint error in patch 'core: add device callbacks to adapter
driver'

Changes in v8:
- Remove changes in profiles/health/

Changes in v7:
- Fix compiler errors in profiles/hdp.c

Changes in v6:
- include <errno.h> instead of <error.h> in plugins/admin.c

Changes in v5:
- Fix compiler errors in plugins/admin.c

Changes in v4:
- Update commit message (admin_policy -> admin)
- remove old plugins/admin_policy.c

Changes in v3:
- Rename plugins/admin_policy.c -> plugins/admin.c
- Use device_added callback in btd_adapter_driver instead of listen for
dbus
- Add authorization method in profiles/health/mcap.c and block incoming
connections in adapter authorization function.

Changes in v2:
- Move bt_uuid_hash and bt_uuid_equal functions to adapter.c.
- Modify the criteria to say a device is `Affected` from any-of-uuid
to any-of-auto-connect-profile.
- Remove the code to remove/reprobe disallowed/allowed profiles,
instead, check if the service is allowed in bt_io_accept connect_cb.
- Fix a typo in emit_property_change in
plugin/admin_policy.c:set_service_allowlist
- Instead of using device_state_cb, utilize D-BUS client to watch device
added/removed.
- Add a document in doc/

Yun-Hao Chung (13):
core: add is_allowed property in btd_service
core: add device callbacks to adapter driver
core: add adapter and device allowed_uuid functions
core: block not allowed UUID connect in auth
plugins: new plugin
plugins/admin: add admin_policy adapter driver
plugins/admin: add ServiceAllowList method
plugins/admin: add ServiceAllowList property
plugins/admin: add device callbacks
plugins/admin: add AffectedByPolicy property
plugins/admin: persist policy settings
doc: add description of admin policy
doc: add admin policy file storage description

Makefile.plugins | 5 +
bootstrap-configure | 1 +
configure.ac | 4 +
doc/admin-policy-api.txt | 65 +++++
doc/settings-storage.txt | 20 ++
plugins/admin.c | 590 +++++++++++++++++++++++++++++++++++++++
src/adapter.c | 169 ++++++++++-
src/adapter.h | 22 +-
src/device.c | 65 ++++-
src/device.h | 2 +
src/profile.c | 11 +
src/service.c | 33 +++
src/service.h | 2 +
13 files changed, 980 insertions(+), 9 deletions(-)
create mode 100644 doc/admin-policy-api.txt
create mode 100644 plugins/admin.c

--
2.32.0.554.ge1b32706d8-goog



2021-08-03 11:46:19

by Yun-hao Chung

[permalink] [raw]
Subject: [Bluez PATCH v9 02/13] core: add device callbacks to adapter driver

From: Yun-Hao Chung <[email protected]>

This adds the following callbacks to btd_adapter_driver.

device_added: called when a device is added to the adapter
device_removed: called when a device is removed from the adapter
device_resolved: called when all services of the device have been
resolved.
---

Changes in v9:
- Fix gitlint error

Changes in v8:
- Add device_resolved.
- Remove space before function pointer arguments.

src/adapter.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++----
src/adapter.h | 14 +++++++---
src/device.c | 2 ++
3 files changed, 82 insertions(+), 8 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 663b778e4a5d..5a20f4c6239e 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1216,6 +1216,9 @@ void adapter_service_remove(struct btd_adapter *adapter, uint32_t handle)
remove_record_from_server(rec->handle);
}

+static void adapter_add_device(struct btd_adapter *adapter,
+ struct btd_device *device);
+
static struct btd_device *adapter_create_device(struct btd_adapter *adapter,
const bdaddr_t *bdaddr,
uint8_t bdaddr_type)
@@ -1226,8 +1229,7 @@ static struct btd_device *adapter_create_device(struct btd_adapter *adapter,
if (!device)
return NULL;

- adapter->devices = g_slist_append(adapter->devices, device);
-
+ adapter_add_device(adapter, device);
return device;
}

@@ -1254,6 +1256,9 @@ static void service_auth_cancel(struct service_auth *auth)
g_free(auth);
}

+static void adapter_remove_device(struct btd_adapter *adapter,
+ struct btd_device *device);
+
void btd_adapter_remove_device(struct btd_adapter *adapter,
struct btd_device *dev)
{
@@ -1261,7 +1266,7 @@ void btd_adapter_remove_device(struct btd_adapter *adapter,

adapter->connect_list = g_slist_remove(adapter->connect_list, dev);

- adapter->devices = g_slist_remove(adapter->devices, dev);
+ adapter_remove_device(adapter, dev);
btd_adv_monitor_device_remove(adapter->adv_monitor_manager, dev);

adapter->discovery_found = g_slist_remove(adapter->discovery_found,
@@ -4222,6 +4227,7 @@ static void probe_devices(void *user_data)
struct btd_device *device = user_data;

device_probe_profiles(device, btd_device_get_uuids(device));
+ device_resolved_drivers(device_get_adapter(device), device);
}

static bool load_bredr_defaults(struct btd_adapter *adapter,
@@ -4576,7 +4582,7 @@ static void load_devices(struct btd_adapter *adapter)
goto free;

btd_device_set_temporary(device, false);
- adapter->devices = g_slist_append(adapter->devices, device);
+ adapter_add_device(adapter, device);

/* TODO: register services from pre-loaded list of primaries */

@@ -4738,6 +4744,62 @@ void adapter_remove_profile(struct btd_adapter *adapter, gpointer p)
profile->adapter_remove(profile, adapter);
}

+static void device_added_drivers(struct btd_adapter *adapter,
+ struct btd_device *device)
+{
+ struct btd_adapter_driver *driver;
+ GSList *l;
+
+ for (l = adapter_drivers; l; l = l->next) {
+ driver = l->data;
+
+ if (driver->device_added)
+ driver->device_added(adapter, device);
+ }
+}
+
+static void device_removed_drivers(struct btd_adapter *adapter,
+ struct btd_device *device)
+{
+ struct btd_adapter_driver *driver;
+ GSList *l;
+
+ for (l = adapter_drivers; l; l = l->next) {
+ driver = l->data;
+
+ if (driver->device_removed)
+ driver->device_removed(adapter, device);
+ }
+}
+
+void device_resolved_drivers(struct btd_adapter *adapter,
+ struct btd_device *device)
+{
+ struct btd_adapter_driver *driver;
+ GSList *l;
+
+ for (l = adapter_drivers; l; l = l->next) {
+ driver = l->data;
+
+ if (driver->device_resolved)
+ driver->device_resolved(adapter, device);
+ }
+}
+
+static void adapter_add_device(struct btd_adapter *adapter,
+ struct btd_device *device)
+{
+ adapter->devices = g_slist_append(adapter->devices, device);
+ device_added_drivers(adapter, device);
+}
+
+static void adapter_remove_device(struct btd_adapter *adapter,
+ struct btd_device *device)
+{
+ adapter->devices = g_slist_remove(adapter->devices, device);
+ device_removed_drivers(adapter, device);
+}
+
static void adapter_add_connection(struct btd_adapter *adapter,
struct btd_device *device,
uint8_t bdaddr_type)
@@ -6355,8 +6417,10 @@ static void adapter_remove(struct btd_adapter *adapter)
g_slist_free(adapter->connect_list);
adapter->connect_list = NULL;

- for (l = adapter->devices; l; l = l->next)
+ for (l = adapter->devices; l; l = l->next) {
+ device_removed_drivers(adapter, l->data);
device_remove(l->data, FALSE);
+ }

g_slist_free(adapter->devices);
adapter->devices = NULL;
diff --git a/src/adapter.h b/src/adapter.h
index 60b5e3bcca34..3d69aeda14fb 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -105,11 +105,19 @@ void btd_adapter_set_class(struct btd_adapter *adapter, uint8_t major,

struct btd_adapter_driver {
const char *name;
- int (*probe) (struct btd_adapter *adapter);
- void (*remove) (struct btd_adapter *adapter);
- void (*resume) (struct btd_adapter *adapter);
+ int (*probe)(struct btd_adapter *adapter);
+ void (*remove)(struct btd_adapter *adapter);
+ void (*resume)(struct btd_adapter *adapter);
+ void (*device_added)(struct btd_adapter *adapter,
+ struct btd_device *device);
+ void (*device_removed)(struct btd_adapter *adapter,
+ struct btd_device *device);
+ void (*device_resolved)(struct btd_adapter *adapter,
+ struct btd_device *device);
};

+void device_resolved_drivers(struct btd_adapter *adapter,
+ struct btd_device *device);
typedef void (*service_auth_cb) (DBusError *derr, void *user_data);

void adapter_add_profile(struct btd_adapter *adapter, gpointer p);
diff --git a/src/device.c b/src/device.c
index b29aa195d19b..49dd57166532 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2633,6 +2633,8 @@ static void device_svc_resolved(struct btd_device *dev, uint8_t browse_type,
dev->svc_callbacks);
g_free(cb);
}
+
+ device_resolved_drivers(dev->adapter, dev);
}

static struct bonding_req *bonding_request_new(DBusMessage *msg,
--
2.32.0.554.ge1b32706d8-goog


2021-08-03 11:46:19

by Yun-hao Chung

[permalink] [raw]
Subject: [Bluez PATCH v9 09/13] plugins/admin: add device callbacks

From: Yun-Hao Chung <[email protected]>

This adds callbacks for device resolved and device removed. It is
necessary for implementation of "AffectedByPolicy" property since it
needs to register an interface for each device object and unregister it
once the device gets removed.
---
The following test steps were performed:
1. start discovery using UI
2. verify device_data were added by checking system log
3. stop discovery
4. verify device_data were removed after a few seconds by checking
system log

(no changes since v8)

Changes in v8:
- add device_data when we get called device_resolved instead of
device_added. Otherwise it is possible that a device service has not
yet been resolved so device_data->|affected| might not be correct.
Reviewed-by: Miao-chen Chou <[email protected]>

plugins/admin.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 78 insertions(+)

diff --git a/plugins/admin.c b/plugins/admin.c
index d89a77c8a123..0a0d8a39ed37 100644
--- a/plugins/admin.c
+++ b/plugins/admin.c
@@ -20,6 +20,7 @@

#include "src/adapter.h"
#include "src/dbus-common.h"
+#include "src/device.h"
#include "src/error.h"
#include "src/log.h"
#include "src/plugin.h"
@@ -29,7 +30,11 @@
#define ADMIN_POLICY_SET_INTERFACE "org.bluez.AdminPolicySet1"
#define ADMIN_POLICY_STATUS_INTERFACE "org.bluez.AdminPolicyStatus1"

+#define DBUS_BLUEZ_SERVICE "org.bluez"
+#define BTD_DEVICE_INTERFACE "org.bluez.Device1"
+
static DBusConnection *dbus_conn;
+static struct queue *devices; /* List of struct device_data objects */

/* |policy_data| has the same life cycle as btd_adapter */
static struct btd_admin_policy {
@@ -38,6 +43,11 @@ static struct btd_admin_policy {
struct queue *service_allowlist;
} *policy_data = NULL;

+struct device_data {
+ struct btd_device *device;
+ char *path;
+};
+
static struct btd_admin_policy *admin_policy_new(struct btd_adapter *adapter)
{
struct btd_admin_policy *admin_policy = NULL;
@@ -203,6 +213,37 @@ static const GDBusPropertyTable admin_policy_adapter_properties[] = {
{ }
};

+static bool device_data_match(const void *a, const void *b)
+{
+ const struct device_data *data = a;
+ const struct btd_device *dev = b;
+
+ if (!data) {
+ error("Unexpected NULL device_data");
+ return false;
+ }
+
+ return data->device == dev;
+}
+
+static void free_device_data(void *data)
+{
+ struct device_data *device_data = data;
+
+ g_free(device_data->path);
+ g_free(device_data);
+}
+
+static void remove_device_data(void *data)
+{
+ struct device_data *device_data = data;
+
+ DBG("device_data for %s removing", device_data->path);
+
+ queue_remove(devices, device_data);
+ free_device_data(device_data);
+}
+
static int admin_policy_adapter_probe(struct btd_adapter *adapter)
{
const char *adapter_path;
@@ -250,10 +291,45 @@ static int admin_policy_adapter_probe(struct btd_adapter *adapter)
return 0;
}

+static void admin_policy_device_added(struct btd_adapter *adapter,
+ struct btd_device *device)
+{
+ struct device_data *data;
+
+ if (queue_find(devices, device_data_match, device))
+ return;
+
+ data = g_new0(struct device_data, 1);
+ if (!data) {
+ btd_error(btd_adapter_get_index(adapter),
+ "Failed to allocate memory for device_data");
+ return;
+ }
+
+ data->device = device;
+ data->path = g_strdup(device_get_path(device));
+ queue_push_tail(devices, data);
+
+ DBG("device_data for %s added", data->path);
+}
+
+static void admin_policy_device_removed(struct btd_adapter *adapter,
+ struct btd_device *device)
+{
+ struct device_data *data;
+
+ data = queue_find(devices, device_data_match, device);
+
+ if (data)
+ remove_device_data(data);
+}
+
static struct btd_adapter_driver admin_policy_driver = {
.name = "admin_policy",
.probe = admin_policy_adapter_probe,
.resume = NULL,
+ .device_resolved = admin_policy_device_added,
+ .device_removed = admin_policy_device_removed
};

static int admin_init(void)
@@ -261,6 +337,7 @@ static int admin_init(void)
DBG("");

dbus_conn = btd_get_dbus_connection();
+ devices = queue_new();

return btd_register_adapter_driver(&admin_policy_driver);
}
@@ -270,6 +347,7 @@ static void admin_exit(void)
DBG("");

btd_unregister_adapter_driver(&admin_policy_driver);
+ queue_destroy(devices, free_device_data);

if (policy_data)
admin_policy_free(policy_data);
--
2.32.0.554.ge1b32706d8-goog


2021-08-03 11:46:19

by Yun-hao Chung

[permalink] [raw]
Subject: [Bluez PATCH v9 03/13] core: add adapter and device allowed_uuid functions

From: Yun-Hao Chung <[email protected]>

This implements functions in src/adapter.c and src/device.c for
plugins setting a list of allowed services.

Reviewed-by: Miao-chen Chou <[email protected]>
---

(no changes since v1)

src/adapter.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++
src/adapter.h | 8 +++++
src/device.c | 63 +++++++++++++++++++++++++++++++++++-
src/device.h | 2 ++
4 files changed, 162 insertions(+), 1 deletion(-)

diff --git a/src/adapter.c b/src/adapter.c
index 5a20f4c6239e..0ca4b4f6ff56 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -260,6 +260,8 @@ struct btd_adapter {

struct btd_battery_provider_manager *battery_provider_manager;

+ GHashTable *allowed_uuid_set; /* Set of allowed service UUIDs */
+
gboolean initialized;

GSList *pin_callbacks;
@@ -3494,6 +3496,93 @@ static DBusMessage *connect_device(DBusConnection *conn,
return NULL;
}

+static void update_device_allowed_services(void *data, void *user_data)
+{
+ struct btd_device *device = data;
+
+ btd_device_update_allowed_services(device);
+}
+
+static void add_uuid_to_uuid_set(void *data, void *user_data)
+{
+ bt_uuid_t *uuid = data;
+ GHashTable *uuid_set = user_data;
+
+ if (!uuid) {
+ error("Found NULL in UUID allowed list");
+ return;
+ }
+
+ g_hash_table_add(uuid_set, uuid);
+}
+
+static guint bt_uuid_hash(gconstpointer key)
+{
+ const bt_uuid_t *uuid = key;
+ bt_uuid_t uuid_128;
+ uint64_t *val;
+
+ if (!uuid)
+ return 0;
+
+ bt_uuid_to_uuid128(uuid, &uuid_128);
+ val = (uint64_t *)&uuid_128.value.u128;
+
+ return g_int64_hash(val) ^ g_int64_hash(val+1);
+}
+
+static gboolean bt_uuid_equal(gconstpointer v1, gconstpointer v2)
+{
+ const bt_uuid_t *uuid1 = v1;
+ const bt_uuid_t *uuid2 = v2;
+
+ if (!uuid1 || !uuid2)
+ return !uuid1 && !uuid2;
+
+ return bt_uuid_cmp(uuid1, uuid2) == 0;
+}
+
+bool btd_adapter_set_allowed_uuids(struct btd_adapter *adapter,
+ struct queue *uuids)
+{
+ if (!adapter)
+ return false;
+
+ if (adapter->allowed_uuid_set)
+ g_hash_table_destroy(adapter->allowed_uuid_set);
+
+ adapter->allowed_uuid_set = g_hash_table_new(bt_uuid_hash,
+ bt_uuid_equal);
+ if (!adapter->allowed_uuid_set) {
+ btd_error(adapter->dev_id,
+ "Failed to allocate allowed_uuid_set");
+ return false;
+ }
+
+ queue_foreach(uuids, add_uuid_to_uuid_set, adapter->allowed_uuid_set);
+ g_slist_foreach(adapter->devices, update_device_allowed_services, NULL);
+
+ return true;
+}
+
+bool btd_adapter_is_uuid_allowed(struct btd_adapter *adapter,
+ const char *uuid_str)
+{
+ bt_uuid_t uuid;
+
+ if (!adapter || !adapter->allowed_uuid_set)
+ return true;
+
+ if (bt_string_to_uuid(&uuid, uuid_str)) {
+ btd_error(adapter->dev_id,
+ "Failed to parse UUID string '%s'", uuid_str);
+ return false;
+ }
+
+ return !g_hash_table_size(adapter->allowed_uuid_set) ||
+ g_hash_table_contains(adapter->allowed_uuid_set, &uuid);
+}
+
static const GDBusMethodTable adapter_methods[] = {
{ GDBUS_ASYNC_METHOD("StartDiscovery", NULL, NULL, start_discovery) },
{ GDBUS_METHOD("SetDiscoveryFilter",
@@ -5466,6 +5555,7 @@ static void adapter_free(gpointer user_data)
g_free(adapter->stored_alias);
g_free(adapter->current_alias);
free(adapter->modalias);
+ g_hash_table_destroy(adapter->allowed_uuid_set);
g_free(adapter);
}

diff --git a/src/adapter.h b/src/adapter.h
index 3d69aeda14fb..35fa9fc5fd1f 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -25,6 +25,7 @@

struct btd_adapter;
struct btd_device;
+struct queue;

struct btd_adapter *btd_adapter_get_default(void);
bool btd_adapter_is_default(struct btd_adapter *adapter);
@@ -97,6 +98,8 @@ void adapter_service_remove(struct btd_adapter *adapter, uint32_t handle);

struct agent *adapter_get_agent(struct btd_adapter *adapter);

+bool btd_adapter_uuid_is_allowed(struct btd_adapter *adapter, const char *uuid);
+
struct btd_adapter *btd_adapter_ref(struct btd_adapter *adapter);
void btd_adapter_unref(struct btd_adapter *adapter);

@@ -248,3 +251,8 @@ enum kernel_features {
};

bool btd_has_kernel_features(uint32_t feature);
+
+bool btd_adapter_set_allowed_uuids(struct btd_adapter *adapter,
+ struct queue *uuids);
+bool btd_adapter_is_uuid_allowed(struct btd_adapter *adapter,
+ const char *uuid_str);
diff --git a/src/device.c b/src/device.c
index 49dd57166532..8071068123f2 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1929,6 +1929,56 @@ static int service_prio_cmp(gconstpointer a, gconstpointer b)
return p2->priority - p1->priority;
}

+bool btd_device_all_services_allowed(struct btd_device *dev)
+{
+ GSList *l;
+ struct btd_adapter *adapter = dev->adapter;
+ struct btd_service *service;
+ struct btd_profile *profile;
+
+ for (l = dev->services; l != NULL; l = g_slist_next(l)) {
+ service = l->data;
+ profile = btd_service_get_profile(service);
+
+ if (!profile || !profile->auto_connect)
+ continue;
+
+ if (!btd_adapter_is_uuid_allowed(adapter, profile->remote_uuid))
+ return false;
+ }
+
+ return true;
+}
+
+void btd_device_update_allowed_services(struct btd_device *dev)
+{
+ struct btd_adapter *adapter = dev->adapter;
+ struct btd_service *service;
+ struct btd_profile *profile;
+ GSList *l;
+ bool is_allowed;
+ char addr[18];
+
+ /* If service discovery is ongoing, let the service discovery complete
+ * callback call this function.
+ */
+ if (dev->browse) {
+ ba2str(&dev->bdaddr, addr);
+ DBG("service discovery of %s is ongoing. Skip updating allowed "
+ "services", addr);
+ return;
+ }
+
+ for (l = dev->services; l != NULL; l = g_slist_next(l)) {
+ service = l->data;
+ profile = btd_service_get_profile(service);
+
+ is_allowed = btd_adapter_is_uuid_allowed(adapter,
+ profile->remote_uuid);
+ btd_service_set_allowed(service, is_allowed);
+ }
+}
+
static GSList *create_pending_list(struct btd_device *dev, const char *uuid)
{
struct btd_service *service;
@@ -1937,9 +1987,14 @@ static GSList *create_pending_list(struct btd_device *dev, const char *uuid)

if (uuid) {
service = find_connectable_service(dev, uuid);
- if (service)
+
+ if (!service)
+ return dev->pending;
+
+ if (btd_service_is_allowed(service))
return g_slist_prepend(dev->pending, service);

+ info("service %s is blocked", uuid);
return dev->pending;
}

@@ -1950,6 +2005,11 @@ static GSList *create_pending_list(struct btd_device *dev, const char *uuid)
if (!p->auto_connect)
continue;

+ if (!btd_service_is_allowed(service)) {
+ info("service %s is blocked", p->remote_uuid);
+ continue;
+ }
+
if (g_slist_find(dev->pending, service))
continue;

@@ -2634,6 +2694,7 @@ static void device_svc_resolved(struct btd_device *dev, uint8_t browse_type,
g_free(cb);
}

+ btd_device_update_allowed_services(dev);
device_resolved_drivers(dev->adapter, dev);
}

diff --git a/src/device.h b/src/device.h
index 4ae9abe0dbb4..5f615cb4b6b2 100644
--- a/src/device.h
+++ b/src/device.h
@@ -175,5 +175,7 @@ uint32_t btd_device_get_current_flags(struct btd_device *dev);
void btd_device_flags_changed(struct btd_device *dev, uint32_t supported_flags,
uint32_t current_flags);

+bool btd_device_all_services_allowed(struct btd_device *dev);
+void btd_device_update_allowed_services(struct btd_device *dev);
void btd_device_init(void);
void btd_device_cleanup(void);
--
2.32.0.554.ge1b32706d8-goog


2021-08-03 11:46:19

by Yun-hao Chung

[permalink] [raw]
Subject: [Bluez PATCH v9 07/13] plugins/admin: add ServiceAllowList method

From: Yun-Hao Chung <[email protected]>

This adds code to register interface org.bluez.AdminPolicySet1.
The interface will provide methods to limit users to operate certain
functions of bluez, such as allow/disallow user to taggle adapter power,
or only allow users to connect services in the specified list, etc.

This patch also implements ServiceAllowlist in
org.bluez.AdminPolicySet1.

Reviewed-by: Miao-chen Chou <[email protected]>
---
The following test steps were performed:
1. Set ServiceAllowList to
["1108","110A","110B","110C","110D","110E",
"110F","1112","111E","111F","1203"]
( users are only allowed to connect headset )
2. Turn on paired WF1000XM3, and listen music on Youtube.
3. Turn on paired K830 (LE device), press any key on keyboard.
4. Turn on paired Samsung Bluetooth Keyboard EE-BT550 (BREDR device),
press any key on keyboard.
5. Set ServiceAllowList to
["1124","180A","180F","1812"]
( users are only allowed to connect HID devices )
6. Turn on paired WF1000XM3, and listen music on Youtube.
7. Turn on paired K830 (LE device), press any key on keyboard.
8. Turn on paired Samsung Bluetooth Keyboard EE-BT550 (BREDR device),
press any key on keyboard.
9. Set ServiceAllowList to []
( users are only allowed to connect any device. )
10. Turn on paired WF1000XM3, and listen music on Youtube.
11. Turn on paired K830 (LE device), press any key on keyboard.
12. Turn on paired Samsung Bluetooth Keyboard EE-BT550 (BREDR device),
press any key on keyboard.

Expected results:
Step 2,7,8,9,10,11 should success, and step 3,4,6 should fail.

(no changes since v1)

plugins/admin.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 126 insertions(+), 1 deletion(-)

diff --git a/plugins/admin.c b/plugins/admin.c
index 923e08cb836b..1fe2904d93d9 100644
--- a/plugins/admin.c
+++ b/plugins/admin.c
@@ -12,19 +12,29 @@
#include <config.h>
#endif

+#include <dbus/dbus.h>
+#include <gdbus/gdbus.h>
+
#include "lib/bluetooth.h"
+#include "lib/uuid.h"

#include "src/adapter.h"
+#include "src/dbus-common.h"
#include "src/error.h"
#include "src/log.h"
#include "src/plugin.h"

#include "src/shared/queue.h"

+#define ADMIN_POLICY_SET_INTERFACE "org.bluez.AdminPolicySet1"
+
+static DBusConnection *dbus_conn;
+
/* |policy_data| has the same life cycle as btd_adapter */
static struct btd_admin_policy {
struct btd_adapter *adapter;
uint16_t adapter_id;
+ struct queue *service_allowlist;
} *policy_data = NULL;

static struct btd_admin_policy *admin_policy_new(struct btd_adapter *adapter)
@@ -40,19 +50,120 @@ static struct btd_admin_policy *admin_policy_new(struct btd_adapter *adapter)

admin_policy->adapter = adapter;
admin_policy->adapter_id = btd_adapter_get_index(adapter);
+ admin_policy->service_allowlist = NULL;

return admin_policy;
}

+static void free_service_allowlist(struct queue *q)
+{
+ queue_destroy(q, g_free);
+}
+
static void admin_policy_free(void *data)
{
struct btd_admin_policy *admin_policy = data;

+ free_service_allowlist(admin_policy->service_allowlist);
g_free(admin_policy);
}

+static struct queue *parse_allow_service_list(struct btd_adapter *adapter,
+ DBusMessage *msg)
+{
+ DBusMessageIter iter, arr_iter;
+ struct queue *uuid_list = NULL;
+
+ dbus_message_iter_init(msg, &iter);
+ if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_ARRAY)
+ return NULL;
+
+ uuid_list = queue_new();
+ dbus_message_iter_recurse(&iter, &arr_iter);
+ do {
+ const int type = dbus_message_iter_get_arg_type(&arr_iter);
+ char *uuid_param;
+ bt_uuid_t *uuid;
+
+ if (type == DBUS_TYPE_INVALID)
+ break;
+
+ if (type != DBUS_TYPE_STRING)
+ goto failed;
+
+ dbus_message_iter_get_basic(&arr_iter, &uuid_param);
+
+ uuid = g_try_malloc(sizeof(*uuid));
+ if (!uuid)
+ goto failed;
+
+ if (bt_string_to_uuid(uuid, uuid_param)) {
+ g_free(uuid);
+ goto failed;
+ }
+
+ queue_push_head(uuid_list, uuid);
+
+ dbus_message_iter_next(&arr_iter);
+ } while (true);
+
+ return uuid_list;
+
+failed:
+ queue_destroy(uuid_list, g_free);
+ return NULL;
+}
+
+static bool service_allowlist_set(struct btd_admin_policy *admin_policy,
+ struct queue *uuid_list)
+{
+ struct btd_adapter *adapter = admin_policy->adapter;
+
+ if (!btd_adapter_set_allowed_uuids(adapter, uuid_list))
+ return false;
+
+ free_service_allowlist(admin_policy->service_allowlist);
+ admin_policy->service_allowlist = uuid_list;
+
+ return true;
+}
+
+static DBusMessage *set_service_allowlist(DBusConnection *conn,
+ DBusMessage *msg, void *user_data)
+{
+ struct btd_admin_policy *admin_policy = user_data;
+ struct btd_adapter *adapter = admin_policy->adapter;
+ struct queue *uuid_list = NULL;
+ const char *sender = dbus_message_get_sender(msg);
+
+ DBG("sender %s", sender);
+
+ /* Parse parameters */
+ uuid_list = parse_allow_service_list(adapter, msg);
+ if (!uuid_list) {
+ btd_error(admin_policy->adapter_id,
+ "Failed on parsing allowed service list");
+ return btd_error_invalid_args(msg);
+ }
+
+ if (!service_allowlist_set(admin_policy, uuid_list)) {
+ free_service_allowlist(uuid_list);
+ return btd_error_failed(msg, "service_allowlist_set failed");
+ }
+
+ return dbus_message_new_method_return(msg);
+}
+
+static const GDBusMethodTable admin_policy_adapter_methods[] = {
+ { GDBUS_METHOD("SetServiceAllowList", GDBUS_ARGS({ "UUIDs", "as" }),
+ NULL, set_service_allowlist) },
+ { }
+};
+
static int admin_policy_adapter_probe(struct btd_adapter *adapter)
{
+ const char *adapter_path;
+
if (policy_data) {
btd_warn(policy_data->adapter_id,
"Policy data already exists");
@@ -64,8 +175,20 @@ static int admin_policy_adapter_probe(struct btd_adapter *adapter)
if (!policy_data)
return -ENOMEM;

- btd_info(policy_data->adapter_id, "Admin Policy has been enabled");
+ adapter_path = adapter_get_path(adapter);

+ if (!g_dbus_register_interface(dbus_conn, adapter_path,
+ ADMIN_POLICY_SET_INTERFACE,
+ admin_policy_adapter_methods, NULL,
+ NULL, policy_data, admin_policy_free)) {
+ btd_error(policy_data->adapter_id,
+ "Admin Policy Set interface init failed on path %s",
+ adapter_path);
+ return -EINVAL;
+ }
+
+ btd_info(policy_data->adapter_id,
+ "Admin Policy Set interface registered");
return 0;
}

@@ -79,6 +202,8 @@ static int admin_init(void)
{
DBG("");

+ dbus_conn = btd_get_dbus_connection();
+
return btd_register_adapter_driver(&admin_policy_driver);
}

--
2.32.0.554.ge1b32706d8-goog


2021-08-03 11:46:20

by Yun-hao Chung

[permalink] [raw]
Subject: [Bluez PATCH v9 10/13] plugins/admin: add AffectedByPolicy property

From: Yun-Hao Chung <[email protected]>

This adds property to indicate if a device has any service that is being
blocked by admin policy.

Reviewed-by: Miao-chen Chou <[email protected]>
---
The following test steps were performed:
1. Set ServiceAllowList to []
2. Verify AffectedByPolicy of K830 is False
3. Set ServiceAllowList to
["1800"]
4. Verify AffectedByPolicy of K830 is False
5. Set ServiceAllowList to ["1800","1801","180A","180F","1812"]
6. Verify AffectedByPolicy of K830 is True

(no changes since v1)

plugins/admin.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 71 insertions(+), 2 deletions(-)

diff --git a/plugins/admin.c b/plugins/admin.c
index 0a0d8a39ed37..7936f8c11475 100644
--- a/plugins/admin.c
+++ b/plugins/admin.c
@@ -46,6 +46,7 @@ static struct btd_admin_policy {
struct device_data {
struct btd_device *device;
char *path;
+ bool affected;
};

static struct btd_admin_policy *admin_policy_new(struct btd_adapter *adapter)
@@ -139,6 +140,27 @@ static bool service_allowlist_set(struct btd_admin_policy *admin_policy,
return true;
}

+static void update_device_affected(void *data, void *user_data)
+{
+ struct device_data *dev_data = data;
+ bool affected;
+
+ if (!dev_data) {
+ error("Unexpected NULL device_data when updating device");
+ return;
+ }
+
+ affected = !btd_device_all_services_allowed(dev_data->device);
+
+ if (affected == dev_data->affected)
+ return;
+
+ dev_data->affected = affected;
+
+ g_dbus_emit_property_changed(dbus_conn, dev_data->path,
+ ADMIN_POLICY_STATUS_INTERFACE, "AffectedByPolicy");
+}
+
static DBusMessage *set_service_allowlist(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
@@ -167,6 +189,8 @@ static DBusMessage *set_service_allowlist(DBusConnection *conn,
ADMIN_POLICY_STATUS_INTERFACE,
"ServiceAllowList");

+ queue_foreach(devices, update_device_affected, NULL);
+
return dbus_message_new_method_return(msg);
}

@@ -226,6 +250,28 @@ static bool device_data_match(const void *a, const void *b)
return data->device == dev;
}

+static gboolean property_get_affected_by_policy(
+ const GDBusPropertyTable *property,
+ DBusMessageIter *iter, void *user_data)
+{
+ struct device_data *data = user_data;
+
+ if (!data) {
+ error("Unexpected error: device_data is NULL");
+ return FALSE;
+ }
+
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN,
+ &data->affected);
+
+ return TRUE;
+}
+
+static const GDBusPropertyTable admin_policy_device_properties[] = {
+ { "AffectedByPolicy", "b", property_get_affected_by_policy },
+ { }
+};
+
static void free_device_data(void *data)
{
struct device_data *device_data = data;
@@ -308,11 +354,33 @@ static void admin_policy_device_added(struct btd_adapter *adapter,

data->device = device;
data->path = g_strdup(device_get_path(device));
+ data->affected = !btd_device_all_services_allowed(data->device);
+
+ if (!g_dbus_register_interface(dbus_conn, data->path,
+ ADMIN_POLICY_STATUS_INTERFACE,
+ NULL, NULL,
+ admin_policy_device_properties,
+ data, remove_device_data)) {
+ btd_error(btd_adapter_get_index(adapter),
+ "Admin Policy Status interface init failed on path %s",
+ device_get_path(device));
+ free_device_data(data);
+ return;
+ }
+
queue_push_tail(devices, data);

DBG("device_data for %s added", data->path);
}

+static void unregister_device_data(void *data, void *user_data)
+{
+ struct device_data *dev_data = data;
+
+ g_dbus_unregister_interface(dbus_conn, dev_data->path,
+ ADMIN_POLICY_STATUS_INTERFACE);
+}
+
static void admin_policy_device_removed(struct btd_adapter *adapter,
struct btd_device *device)
{
@@ -321,7 +389,7 @@ static void admin_policy_device_removed(struct btd_adapter *adapter,
data = queue_find(devices, device_data_match, device);

if (data)
- remove_device_data(data);
+ unregister_device_data(data, NULL);
}

static struct btd_adapter_driver admin_policy_driver = {
@@ -347,7 +415,8 @@ static void admin_exit(void)
DBG("");

btd_unregister_adapter_driver(&admin_policy_driver);
- queue_destroy(devices, free_device_data);
+ queue_foreach(devices, unregister_device_data, NULL);
+ queue_destroy(devices, g_free);

if (policy_data)
admin_policy_free(policy_data);
--
2.32.0.554.ge1b32706d8-goog


2021-08-03 11:46:23

by Yun-hao Chung

[permalink] [raw]
Subject: [Bluez PATCH v9 13/13] doc: add admin policy file storage description

From: Yun-Hao Chung <[email protected]>

This adds storage description of admin policy file in
doc/settings-storage.txt
---

(no changes since v1)

doc/settings-storage.txt | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/doc/settings-storage.txt b/doc/settings-storage.txt
index d21150f09ecb..1d96cd66d94f 100644
--- a/doc/settings-storage.txt
+++ b/doc/settings-storage.txt
@@ -36,6 +36,7 @@ root, named based on the address, which contains:

- a settings file for the local adapter
- an attributes file containing attributes of supported LE services
+ - an admin policy file containing current values of admin policies
- a cache directory containing:
- one file per device, named by remote device address, which contains
device name
@@ -50,6 +51,7 @@ So the directory structure is:
/var/lib/bluetooth/<adapter address>/
./settings
./attributes
+ ./admin_policy_settings
./cache/
./<remote device address>
./<remote device address>
@@ -140,6 +142,24 @@ Sample:
Value=4578616D706C6520446576696365


+Admin Policy file format
+======================
+
+The admin policy file stores the current value of each admin policy.
+
+[General] group contains:
+
+ ServiceAllowlist List of List of service UUID allowed by
+ strings adapter in 128-bits format, separated
+ by ','. Default is empty.
+
+Sample:
+ [General]
+ ServiceAllowlist=
+
+
+
+
CCC file format
======================

--
2.32.0.554.ge1b32706d8-goog


2021-08-03 11:46:27

by Yun-hao Chung

[permalink] [raw]
Subject: [Bluez PATCH v9 06/13] plugins/admin: add admin_policy adapter driver

From: Yun-Hao Chung <[email protected]>

This adds code to register admin_policy driver to adapter when
admin plugin is enabled.

The following test steps were performed:
1. restart bluetoothd
2. check if "Admin Policy is enabled" in system log

Reviewed-by: Miao-chen Chou <[email protected]>
---

(no changes since v1)

plugins/admin.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 67 insertions(+)

diff --git a/plugins/admin.c b/plugins/admin.c
index 42866bcf7be2..923e08cb836b 100644
--- a/plugins/admin.c
+++ b/plugins/admin.c
@@ -12,17 +12,84 @@
#include <config.h>
#endif

+#include "lib/bluetooth.h"
+
+#include "src/adapter.h"
+#include "src/error.h"
#include "src/log.h"
#include "src/plugin.h"

+#include "src/shared/queue.h"
+
+/* |policy_data| has the same life cycle as btd_adapter */
+static struct btd_admin_policy {
+ struct btd_adapter *adapter;
+ uint16_t adapter_id;
+} *policy_data = NULL;
+
+static struct btd_admin_policy *admin_policy_new(struct btd_adapter *adapter)
+{
+ struct btd_admin_policy *admin_policy = NULL;
+
+ admin_policy = g_try_malloc(sizeof(*admin_policy));
+ if (!admin_policy) {
+ btd_error(btd_adapter_get_index(adapter),
+ "Failed to allocate memory for admin_policy");
+ return NULL;
+ }
+
+ admin_policy->adapter = adapter;
+ admin_policy->adapter_id = btd_adapter_get_index(adapter);
+
+ return admin_policy;
+}
+
+static void admin_policy_free(void *data)
+{
+ struct btd_admin_policy *admin_policy = data;
+
+ g_free(admin_policy);
+}
+
+static int admin_policy_adapter_probe(struct btd_adapter *adapter)
+{
+ if (policy_data) {
+ btd_warn(policy_data->adapter_id,
+ "Policy data already exists");
+ admin_policy_free(policy_data);
+ policy_data = NULL;
+ }
+
+ policy_data = admin_policy_new(adapter);
+ if (!policy_data)
+ return -ENOMEM;
+
+ btd_info(policy_data->adapter_id, "Admin Policy has been enabled");
+
+ return 0;
+}
+
+static struct btd_adapter_driver admin_policy_driver = {
+ .name = "admin_policy",
+ .probe = admin_policy_adapter_probe,
+ .resume = NULL,
+};
+
static int admin_init(void)
{
DBG("");
+
+ return btd_register_adapter_driver(&admin_policy_driver);
}

static void admin_exit(void)
{
DBG("");
+
+ btd_unregister_adapter_driver(&admin_policy_driver);
+
+ if (policy_data)
+ admin_policy_free(policy_data);
}

BLUETOOTH_PLUGIN_DEFINE(admin, VERSION,
--
2.32.0.554.ge1b32706d8-goog


2021-08-03 11:46:58

by Yun-hao Chung

[permalink] [raw]
Subject: [Bluez PATCH v9 08/13] plugins/admin: add ServiceAllowList property

From: Yun-Hao Chung <[email protected]>

This adds code to register interface org.bluez.AdminPolicyStatus.
The interface will provide read-only properties to indicate the current
settings of admin policies. We separate this from AdminPolicySet so that
normal clients can check current policy settings while only a few
clients can change policies.

This patch also adds readonly property ServiceAllowlist to
AdminPolicyStatus1, which indicates the current setting of service
allowlist.

Reviewed-by: Miao-chen Chou <[email protected]>
---
The following test steps were performed:
1. Set ServiceAllowList to ["1124","180A","180F","1812"]
2. Verify ServiceAllowList is ["1124","180A","180F","1812"] in UUID-128
form
3. Set ServiceAllowList to []
4. Verify ServiceAllowList is []

(no changes since v1)

plugins/admin.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)

diff --git a/plugins/admin.c b/plugins/admin.c
index 1fe2904d93d9..d89a77c8a123 100644
--- a/plugins/admin.c
+++ b/plugins/admin.c
@@ -27,6 +27,7 @@
#include "src/shared/queue.h"

#define ADMIN_POLICY_SET_INTERFACE "org.bluez.AdminPolicySet1"
+#define ADMIN_POLICY_STATUS_INTERFACE "org.bluez.AdminPolicyStatus1"

static DBusConnection *dbus_conn;

@@ -151,6 +152,11 @@ static DBusMessage *set_service_allowlist(DBusConnection *conn,
return btd_error_failed(msg, "service_allowlist_set failed");
}

+ g_dbus_emit_property_changed(dbus_conn,
+ adapter_get_path(policy_data->adapter),
+ ADMIN_POLICY_STATUS_INTERFACE,
+ "ServiceAllowList");
+
return dbus_message_new_method_return(msg);
}

@@ -160,6 +166,43 @@ static const GDBusMethodTable admin_policy_adapter_methods[] = {
{ }
};

+void append_service_uuid(void *data, void *user_data)
+{
+ bt_uuid_t *uuid = data;
+ DBusMessageIter *entry = user_data;
+ char uuid_str[MAX_LEN_UUID_STR];
+ const char *uuid_str_ptr = uuid_str;
+
+ if (!uuid) {
+ error("Unexpected NULL uuid data in service_allowlist");
+ return;
+ }
+
+ bt_uuid_to_string(uuid, uuid_str, MAX_LEN_UUID_STR);
+ dbus_message_iter_append_basic(entry, DBUS_TYPE_STRING, &uuid_str_ptr);
+}
+
+static gboolean property_get_service_allowlist(
+ const GDBusPropertyTable *property,
+ DBusMessageIter *iter, void *user_data)
+{
+ struct btd_admin_policy *admin_policy = user_data;
+ DBusMessageIter entry;
+
+ dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
+ DBUS_TYPE_STRING_AS_STRING, &entry);
+ queue_foreach(admin_policy->service_allowlist, append_service_uuid,
+ &entry);
+ dbus_message_iter_close_container(iter, &entry);
+
+ return TRUE;
+}
+
+static const GDBusPropertyTable admin_policy_adapter_properties[] = {
+ { "ServiceAllowList", "as", property_get_service_allowlist },
+ { }
+};
+
static int admin_policy_adapter_probe(struct btd_adapter *adapter)
{
const char *adapter_path;
@@ -189,6 +232,21 @@ static int admin_policy_adapter_probe(struct btd_adapter *adapter)

btd_info(policy_data->adapter_id,
"Admin Policy Set interface registered");
+
+ if (!g_dbus_register_interface(dbus_conn, adapter_path,
+ ADMIN_POLICY_STATUS_INTERFACE,
+ NULL, NULL,
+ admin_policy_adapter_properties,
+ policy_data, admin_policy_free)) {
+ btd_error(policy_data->adapter_id,
+ "Admin Policy Status interface init failed on path %s",
+ adapter_path);
+ return -EINVAL;
+ }
+
+ btd_info(policy_data->adapter_id,
+ "Admin Policy Status interface registered");
+
return 0;
}

--
2.32.0.554.ge1b32706d8-goog


2021-08-03 11:47:39

by Yun-hao Chung

[permalink] [raw]
Subject: [Bluez PATCH v9 12/13] doc: add description of admin policy

From: Yun-Hao Chung <[email protected]>

This adds admin-policy-api.txt.

Reviewed-by: Miao-chen Chou <[email protected]>
---

(no changes since v1)

doc/admin-policy-api.txt | 65 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 65 insertions(+)
create mode 100644 doc/admin-policy-api.txt

diff --git a/doc/admin-policy-api.txt b/doc/admin-policy-api.txt
new file mode 100644
index 000000000000..3f116901dbd7
--- /dev/null
+++ b/doc/admin-policy-api.txt
@@ -0,0 +1,65 @@
+BlueZ D-Bus Admin Policy API description
+***********************************
+
+This API provides methods to control the behavior of bluez as an administrator.
+
+Interface AdminPolicySet1 provides methods to set policies. Once the policy is
+set successfully, it will affect all clients and stay persistently even after
+restarting Bluetooth Daemon. The only way to clear it is to overwrite the
+policy with the same method.
+
+Interface AdminPolicyStatus1 provides readonly properties to indicate the
+current values of admin policy.
+
+
+Admin Policy Set hierarchy
+=================
+
+Service org.bluez
+Interface org.bluez.AdminPolicySet1
+Object path [variable prefix]/{hci0,hci1,...}
+
+Methods void SetServiceAllowList(array{string} UUIDs)
+
+ This method sets the service allowlist by specifying
+ service UUIDs.
+
+ When SetServiceAllowList is called, bluez will block
+ incoming and outgoing connections to the service not in
+ UUIDs for all of the clients.
+
+ Any subsequent calls to this method will supersede any
+ previously set allowlist values. Calling this method
+ with an empty array will allow any service UUIDs to be
+ used.
+
+ The default value is an empty array.
+
+ Possible errors: org.bluez.Error.InvalidArguments
+ org.bluez.Error.Failed
+
+
+Admin Policy Status hierarchy
+=================
+
+Service org.bluez
+Interface org.bluez.AdminPolicyStatus1
+Object path [variable prefix]/{hci0,hci1,...}
+
+Properties array{string} ServiceAllowList [readonly]
+
+ Current value of service allow list.
+
+
+
+Admin Policy Status hierarchy
+=================
+
+Service org.bluez
+Interface org.bluez.AdminPolicyStatus1
+Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
+
+Properties bool IsAffectedByPolicy [readonly]
+
+ Indicate if there is any auto-connect profile in this
+ device is not allowed by admin policy.
--
2.32.0.554.ge1b32706d8-goog


2021-08-03 11:47:53

by Yun-hao Chung

[permalink] [raw]
Subject: [Bluez PATCH v9 11/13] plugins/admin: persist policy settings

From: Yun-Hao Chung <[email protected]>

This adds code to store the ServiceAllowlist to file
/var/lib/bluetooth/{MAC_ADDR}/admin_policy
The stored settings will be loaded upon admin_policy initialized.

Reviewed-by: Miao-chen Chou <[email protected]>
---
The following test steps were performed:
1. Set ServiceAllowlist to ["1124","180A","180F","1812", "1801"]
2. restart bluetoothd
3. Verify ServiceAllowlist is ["1124","180A","180F","1812","1801"] in
UUID-128 form
4. Set ServiceAllowlist to []
5. restart bluetoothd
6. Verify ServiceAllowlist is []

(no changes since v8)

Changes in v8:
- Move store_policy_settings earlier to avoid forward declaration.

plugins/admin.c | 167 +++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 165 insertions(+), 2 deletions(-)

diff --git a/plugins/admin.c b/plugins/admin.c
index 7936f8c11475..428a5528cc88 100644
--- a/plugins/admin.c
+++ b/plugins/admin.c
@@ -14,6 +14,9 @@

#include <dbus/dbus.h>
#include <gdbus/gdbus.h>
+#include <sys/file.h>
+#include <sys/stat.h>
+#include <errno.h>

#include "lib/bluetooth.h"
#include "lib/uuid.h"
@@ -24,11 +27,13 @@
#include "src/error.h"
#include "src/log.h"
#include "src/plugin.h"
+#include "src/textfile.h"

#include "src/shared/queue.h"

#define ADMIN_POLICY_SET_INTERFACE "org.bluez.AdminPolicySet1"
#define ADMIN_POLICY_STATUS_INTERFACE "org.bluez.AdminPolicyStatus1"
+#define ADMIN_POLICY_STORAGE STORAGEDIR "/admin_policy_settings"

#define DBUS_BLUEZ_SERVICE "org.bluez"
#define BTD_DEVICE_INTERFACE "org.bluez.Device1"
@@ -161,6 +166,161 @@ static void update_device_affected(void *data, void *user_data)
ADMIN_POLICY_STATUS_INTERFACE, "AffectedByPolicy");
}

+static void free_uuid_strings(char **uuid_strs, gsize num)
+{
+ gsize i;
+
+ for (i = 0; i < num; i++)
+ g_free(uuid_strs[i]);
+ g_free(uuid_strs);
+}
+
+static char **new_uuid_strings(struct queue *allowlist, gsize *num)
+{
+ const struct queue_entry *entry = NULL;
+ bt_uuid_t *uuid = NULL;
+ char **uuid_strs = NULL;
+ gsize i = 0, allowlist_num;
+
+ /* Set num to a non-zero number so that whoever call this could know if
+ * this function success or not
+ */
+ *num = 1;
+
+ allowlist_num = queue_length(allowlist);
+ uuid_strs = g_try_malloc_n(allowlist_num, sizeof(char *));
+ if (!uuid_strs)
+ return NULL;
+
+ for (entry = queue_get_entries(allowlist); entry != NULL;
+ entry = entry->next) {
+ uuid = entry->data;
+ uuid_strs[i] = g_try_malloc0(MAX_LEN_UUID_STR * sizeof(char));
+
+ if (!uuid_strs[i])
+ goto failed;
+
+ bt_uuid_to_string(uuid, uuid_strs[i], MAX_LEN_UUID_STR);
+ i++;
+ }
+
+ *num = allowlist_num;
+ return uuid_strs;
+
+failed:
+ free_uuid_strings(uuid_strs, i);
+
+ return NULL;
+}
+
+static void store_policy_settings(struct btd_admin_policy *admin_policy)
+{
+ GKeyFile *key_file = NULL;
+ char *filename = ADMIN_POLICY_STORAGE;
+ char *key_file_data = NULL;
+ char **uuid_strs = NULL;
+ gsize length, num_uuids;
+
+ key_file = g_key_file_new();
+
+ uuid_strs = new_uuid_strings(admin_policy->service_allowlist,
+ &num_uuids);
+
+ if (!uuid_strs && num_uuids) {
+ btd_error(admin_policy->adapter_id,
+ "Failed to allocate uuid strings");
+ goto failed;
+ }
+
+ g_key_file_set_string_list(key_file, "General", "ServiceAllowlist",
+ (const gchar * const *)uuid_strs,
+ num_uuids);
+
+ if (create_file(ADMIN_POLICY_STORAGE, 0600) < 0) {
+ btd_error(admin_policy->adapter_id, "create %s failed, %s",
+ filename, strerror(errno));
+ goto failed;
+ }
+
+ key_file_data = g_key_file_to_data(key_file, &length, NULL);
+ g_file_set_contents(ADMIN_POLICY_STORAGE, key_file_data, length, NULL);
+
+ g_free(key_file_data);
+ free_uuid_strings(uuid_strs, num_uuids);
+
+failed:
+ g_key_file_free(key_file);
+}
+
+static void key_file_load_service_allowlist(GKeyFile *key_file,
+ struct btd_admin_policy *admin_policy)
+{
+ GError *gerr = NULL;
+ struct queue *uuid_list = NULL;
+ gchar **uuids = NULL;
+ gsize num, i;
+
+ uuids = g_key_file_get_string_list(key_file, "General",
+ "ServiceAllowlist", &num, &gerr);
+
+ if (gerr) {
+ btd_error(admin_policy->adapter_id,
+ "Failed to load ServiceAllowlist");
+ g_error_free(gerr);
+ return;
+ }
+
+ uuid_list = queue_new();
+ for (i = 0; i < num; i++) {
+ bt_uuid_t *uuid = g_try_malloc(sizeof(*uuid));
+
+ if (!uuid)
+ goto failed;
+
+ if (bt_string_to_uuid(uuid, *uuids)) {
+
+ btd_error(admin_policy->adapter_id,
+ "Failed to convert '%s' to uuid struct",
+ *uuids);
+
+ g_free(uuid);
+ goto failed;
+ }
+
+ queue_push_tail(uuid_list, uuid);
+ uuids++;
+ }
+
+ if (!service_allowlist_set(admin_policy, uuid_list))
+ goto failed;
+
+ return;
+failed:
+ free_service_allowlist(uuid_list);
+}
+
+static void load_policy_settings(struct btd_admin_policy *admin_policy)
+{
+ GKeyFile *key_file;
+ char *filename = ADMIN_POLICY_STORAGE;
+ struct stat st;
+
+ if (stat(filename, &st) < 0) {
+ btd_error(admin_policy->adapter_id,
+ "Failed to get file %s information",
+ filename);
+ return;
+ }
+
+ key_file = g_key_file_new();
+
+ g_key_file_load_from_file(key_file, filename, 0, NULL);
+
+ key_file_load_service_allowlist(key_file, admin_policy);
+
+ g_key_file_free(key_file);
+}
+
static DBusMessage *set_service_allowlist(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
@@ -179,7 +339,9 @@ static DBusMessage *set_service_allowlist(DBusConnection *conn,
return btd_error_invalid_args(msg);
}

- if (!service_allowlist_set(admin_policy, uuid_list)) {
+ if (service_allowlist_set(admin_policy, uuid_list)) {
+ store_policy_settings(admin_policy);
+ } else {
free_service_allowlist(uuid_list);
return btd_error_failed(msg, "service_allowlist_set failed");
}
@@ -200,7 +362,7 @@ static const GDBusMethodTable admin_policy_adapter_methods[] = {
{ }
};

-void append_service_uuid(void *data, void *user_data)
+static void append_service_uuid(void *data, void *user_data)
{
bt_uuid_t *uuid = data;
DBusMessageIter *entry = user_data;
@@ -305,6 +467,7 @@ static int admin_policy_adapter_probe(struct btd_adapter *adapter)
if (!policy_data)
return -ENOMEM;

+ load_policy_settings(policy_data);
adapter_path = adapter_get_path(adapter);

if (!g_dbus_register_interface(dbus_conn, adapter_path,
--
2.32.0.554.ge1b32706d8-goog


2021-08-04 22:55:25

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [Bluez PATCH v9 00/13] Admin policy series

Hi Howard,

On Tue, Aug 3, 2021 at 4:43 AM Howard Chung <[email protected]> wrote:
>
> From: Yun-Hao Chung <[email protected]>
>
>
> Hi manintainers,
>
> This series is to
> 1. Implement a few methods in core so that a plugin can have control of
> allowing / disallowing certain service connections.
> 2. Implement the AdminPolicy plugin. The plugin provides interfaces
> AdminPolicySet and AdminPolicyStatus. For each policy, users should
> set the value thorugh AdminPolicySet and query the current setting
> through AdminPolicyStatus. We separeted these two interfaces so that
> developers can assign different groups of users to these interfaces.
> Currently the only policy is ServiceAllowList, which make bluez only
> allow a list of service by specified their UUIDs, but the plugin is
> also expected to provide more controls over other bluez behaviors.
> Since the second part is a plugin, it might not be necessary to land in
> upstream tree.
>
> Thanks.
>
> Changes in v9:
> - Fix gitlint error in patch 'core: add device callbacks to adapter
> driver'
>
> Changes in v8:
> - Remove changes in profiles/health/
>
> Changes in v7:
> - Fix compiler errors in profiles/hdp.c
>
> Changes in v6:
> - include <errno.h> instead of <error.h> in plugins/admin.c
>
> Changes in v5:
> - Fix compiler errors in plugins/admin.c
>
> Changes in v4:
> - Update commit message (admin_policy -> admin)
> - remove old plugins/admin_policy.c
>
> Changes in v3:
> - Rename plugins/admin_policy.c -> plugins/admin.c
> - Use device_added callback in btd_adapter_driver instead of listen for
> dbus
> - Add authorization method in profiles/health/mcap.c and block incoming
> connections in adapter authorization function.
>
> Changes in v2:
> - Move bt_uuid_hash and bt_uuid_equal functions to adapter.c.
> - Modify the criteria to say a device is `Affected` from any-of-uuid
> to any-of-auto-connect-profile.
> - Remove the code to remove/reprobe disallowed/allowed profiles,
> instead, check if the service is allowed in bt_io_accept connect_cb.
> - Fix a typo in emit_property_change in
> plugin/admin_policy.c:set_service_allowlist
> - Instead of using device_state_cb, utilize D-BUS client to watch device
> added/removed.
> - Add a document in doc/
>
> Yun-Hao Chung (13):
> core: add is_allowed property in btd_service
> core: add device callbacks to adapter driver
> core: add adapter and device allowed_uuid functions
> core: block not allowed UUID connect in auth
> plugins: new plugin
> plugins/admin: add admin_policy adapter driver
> plugins/admin: add ServiceAllowList method
> plugins/admin: add ServiceAllowList property
> plugins/admin: add device callbacks
> plugins/admin: add AffectedByPolicy property
> plugins/admin: persist policy settings
> doc: add description of admin policy
> doc: add admin policy file storage description
>
> Makefile.plugins | 5 +
> bootstrap-configure | 1 +
> configure.ac | 4 +
> doc/admin-policy-api.txt | 65 +++++
> doc/settings-storage.txt | 20 ++
> plugins/admin.c | 590 +++++++++++++++++++++++++++++++++++++++
> src/adapter.c | 169 ++++++++++-
> src/adapter.h | 22 +-
> src/device.c | 65 ++++-
> src/device.h | 2 +
> src/profile.c | 11 +
> src/service.c | 33 +++
> src/service.h | 2 +
> 13 files changed, 980 insertions(+), 9 deletions(-)
> create mode 100644 doc/admin-policy-api.txt
> create mode 100644 plugins/admin.c
>
> --
> 2.32.0.554.ge1b32706d8-goog

Applied, thanks.

--
Luiz Augusto von Dentz