On Fri, Jul 9, 2021 at 2:01 PM Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote: > > Hi Howard, > > On Wed, Jul 7, 2021 at 11:23 PM Howard Chung <howardchung@xxxxxxxxxx> wrote: > > > > From: Yun-Hao Chung <howardchung@xxxxxxxxxxxx> > > > > 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 <mcchou@xxxxxxxxxxxx> > > --- > > The following test steps were performed: > > 1. Set ServiceAllowList to > > ["0x1108","0x110A","0x110B","0x110C","0x110D","0x110E", > > "0x110F","0x1112","0x111E","0x111F","0x1203"] > > ( 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. > > All this explanation is great but it would really help if you have > added support for bluetoothctl to control this, we also need to Document it sounds good to me, but I notice that there is no document for any plugin yet. Where do you think we should put the document in? > document these interfaces in case someone else wants to use them (e.g: > other clients like bluetoothctl). For the bluetoothctl we could > perhaps an admin menu registered whenever the interfaces are available > and then a command allow [list/none/any] so the user can query when no > parameter is given or set a list of UUIDs. Btw, how is this supposed Adding commands in bluetoothctl sounds good to me as well. Can we implement this in a separate series? > to work with vendor UUIDs? I guess that would need to support UUID 128 > bit format in order to support that. Since we are using bt_string_to_uuid to parse the given string, internally it checks if it can be parsed as UUID128/UUID32/UUID16. > > > > > plugins/admin_policy.c | 123 ++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 122 insertions(+), 1 deletion(-) > > > > diff --git a/plugins/admin_policy.c b/plugins/admin_policy.c > > index 2ece871564e6..242b8d5dacb0 100644 > > --- a/plugins/admin_policy.c > > +++ b/plugins/admin_policy.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,17 +50,116 @@ 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) > > { > > if (policy_data) { > > @@ -64,8 +173,18 @@ 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"); > > + if (!g_dbus_register_interface(dbus_conn, adapter_get_path(adapter), > > + 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_get_path(adapter)); > > + return -EINVAL; > > + } > > > > + btd_info(policy_data->adapter_id, > > + "Admin Policy Set interface registered"); > > return 0; > > } > > > > @@ -79,6 +198,8 @@ static int admin_policy_init(void) > > { > > DBG(""); > > > > + dbus_conn = btd_get_dbus_connection(); > > + > > return btd_register_adapter_driver(&admin_policy_driver); > > } > > > > -- > > 2.32.0.93.g670b81a890-goog > > > > > -- > Luiz Augusto von Dentz