[PATCH BlueZ 0/3] Per-device option to enable/disable internal profiles

[PATCH BlueZ 0/3] Per-device option to enable/disable internal profiles

[Sonny Sasaka]

This patch series adds a mechanism for clients to choose whether to
enable BlueZ internal profiles (e.g. A2DP, Battery) for specific
devices.

The motivation behind this feature is that some applications (e.g. Web
Bluetooth or Android apps) need to have control over all remove GATT
services, like Battery service. With "battery" plugin being enabled on
BlueZ, it becomes not possible for those apps to work properly because
BlueZ "hides" the Battery-related attributes from its GATT Client API.
Disabling the "battery" plugin won't solve the problem either, since we
do also need to enable the plugin so that we can use org.bluez.Battery1
API.

The solution that we propose is that clients can choose whether to
enable internal profiles for each device. Clients know when to enable
internal profiles (such as when a user chooses to pair/connect via a UI)
and when to disable internal profiles (such as when the connection is
initiated by a generic application).

Sonny Sasaka (3):
  doc: Add "AllowInternalProfiles" property to org.bluez.Device1
  device: Add "AllowInternalProfiles" property to org.bluez.Device1
  client: Add set-allow-internal-profiles command

 client/main.c      | 38 ++++++++++++++++++
 doc/device-api.txt | 13 +++++++
 src/device.c       | 96 ++++++++++++++++++++++++++++++++++++++++++++++
 src/hcid.h         |  2 +
 src/main.c         | 10 +++++
 src/main.conf      |  4 ++
 6 files changed, 163 insertions(+)

-- 
2.26.2

[PATCH BlueZ 1/3] doc: Add "AllowInternalProfiles" property to org.bluez.Device1

[Sonny Sasaka]

Reviewed-by: Alain Michaud <alainm@chromium.org>

---
 doc/device-api.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/doc/device-api.txt b/doc/device-api.txt
index 4e824d2de..5c2b2b26b 100644
--- a/doc/device-api.txt
+++ b/doc/device-api.txt
@@ -189,6 +189,19 @@ Properties	string Address [readonly]
 			drivers will also be removed and no new ones will
 			be probed as long as the device is blocked.
 
+		boolean AllowInternalProfiles [readwrite]
+
+			If set to true, connection to this device will activate
+			BlueZ internal profiles (e.g. A2DP, HOG, Battery).
+			If set to false, internal profiles will not be activated
+			allowing the client to handle profiles which would
+			otherwise be controlled by internal handlers.
+			The default is configurable in main.conf.
+
+			Write operation to this property can only be accepted
+			when the device is not connected, otherwise
+			org.bluez.Error.Failed is returned.
+
 		boolean WakeAllowed [readwrite]
 
 			If set to true this device will be allowed to wake the
-- 
2.26.2

[PATCH BlueZ 2/3] device: Add "AllowInternalProfiles" property to org.bluez.Device1

[Sonny Sasaka]

Some clients want to have full control over all profiles for specific
peer devices, for example to handle the GATT Battery profile. This patch
adds an option via a property in org.bluez.Device1 interface. Setting to
true will let BlueZ take control over internal profiles (such as A2DP,
HOG, Battery), and setting to false will prevent BlueZ from handling all
internal profiles, allowing the client to handle them.

Reviewed-by: Alain Michaud <alainm@chromium.org>

---
 src/device.c  | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++
 src/hcid.h    |  2 ++
 src/main.c    | 10 ++++++
 src/main.conf |  4 +++
 4 files changed, 112 insertions(+)

diff --git a/src/device.c b/src/device.c
index 0deee2707..bd8c80032 100644
--- a/src/device.c
+++ b/src/device.c
@@ -275,6 +275,7 @@ struct btd_device {
 	gboolean	auto_connect;
 	gboolean	disable_auto_connect;
 	gboolean	general_connect;
+	gboolean	allow_internal_profiles;
 
 	bool		legacy;
 	int8_t		rssi;
@@ -294,6 +295,10 @@ static const uint16_t uuid_list[] = {
 static int device_browse_gatt(struct btd_device *device, DBusMessage *msg);
 static int device_browse_sdp(struct btd_device *device, DBusMessage *msg);
 
+static void gatt_service_added(struct gatt_db_attribute *attr, void *user_data);
+static void gatt_service_removed(struct gatt_db_attribute *attr,
+					void *user_data);
+
 static struct bearer_state *get_state(struct btd_device *dev,
 							uint8_t bdaddr_type)
 {
@@ -436,6 +441,9 @@ static gboolean store_device_info_cb(gpointer user_data)
 	g_key_file_set_boolean(key_file, "General", "Blocked",
 							device->blocked);
 
+	g_key_file_set_boolean(key_file, "General", "AllowInternalProfiles",
+				device->allow_internal_profiles);
+
 	if (device->wake_override != WAKE_FLAG_DEFAULT) {
 		g_key_file_set_boolean(key_file, "General", "WakeAllowed",
 				       device->wake_override ==
@@ -1468,6 +1476,71 @@ static gboolean dev_property_wake_allowed_exist(
 	return device_get_wake_support(device);
 }
 
+static gboolean
+dev_property_get_allow_internal_profiles(const GDBusPropertyTable *property,
+						DBusMessageIter *iter,
+						void *data)
+{
+	struct btd_device *device = data;
+	dbus_bool_t allow_internal_profiles = device->allow_internal_profiles;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN,
+					&allow_internal_profiles);
+
+	return TRUE;
+}
+
+static void
+dev_property_set_allow_internal_profiles(const GDBusPropertyTable *property,
+						DBusMessageIter *value,
+						GDBusPendingPropertySet id,
+						void *data)
+{
+	struct btd_device *device = data;
+	dbus_bool_t b;
+
+	if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_BOOLEAN) {
+		g_dbus_pending_property_error(
+			id, ERROR_INTERFACE ".InvalidArguments",
+			"Invalid arguments in method call");
+		return;
+	}
+
+	if (device->le_state.connected || device->bredr_state.connected) {
+		g_dbus_pending_property_error(id, ERROR_INTERFACE ".Failed",
+					"Device is connected");
+		return;
+	}
+
+	dbus_message_iter_get_basic(value, &b);
+
+	device->allow_internal_profiles = b;
+
+	/* Remove GATT client cache */
+	gatt_db_unregister(device->db, device->db_id);
+	btd_gatt_client_destroy(device->client_dbus);
+	gatt_db_unref(device->db);
+
+	device->db = gatt_db_new();
+	device->client_dbus = btd_gatt_client_new(device);
+	device->db_id = gatt_db_register(device->db, gatt_service_added,
+			gatt_service_removed, device, NULL);
+
+	/* Re-probe all profiles */
+	while (device->services != NULL) {
+		struct btd_service *service = device->services->data;
+
+		device->services = g_slist_remove(device->services, service);
+		service_remove(service);
+	}
+
+	device_probe_profiles(device, device->uuids);
+
+	/* Update D-Bus property and reply client */
+	g_dbus_emit_property_changed(dbus_conn, device->path, DEVICE_INTERFACE,
+						"AllowInternalProfiles");
+	g_dbus_pending_property_success(id);
+}
 
 static gboolean disconnect_all(gpointer user_data)
 {
@@ -2944,6 +3017,9 @@ static const GDBusPropertyTable device_properties[] = {
 	{ "WakeAllowed", "b", dev_property_get_wake_allowed,
 				dev_property_set_wake_allowed,
 				dev_property_wake_allowed_exist },
+	{ "AllowInternalProfiles", "b",
+				dev_property_get_allow_internal_profiles,
+				dev_property_set_allow_internal_profiles },
 	{ }
 };
 
@@ -3196,6 +3272,7 @@ static void load_info(struct btd_device *device, const char *local,
 	char *str;
 	gboolean store_needed = FALSE;
 	gboolean blocked;
+	gboolean allow_internal_profiles;
 	gboolean wake_allowed;
 	char **uuids;
 	int source, vendor, product, version;
@@ -3283,6 +3360,20 @@ next:
 	if (blocked)
 		device_block(device, FALSE);
 
+	/* Load allow internal profiles */
+	allow_internal_profiles = g_key_file_get_boolean(
+		key_file, "General", "AllowInternalProfiles", &gerr);
+	if (!gerr) {
+		device->allow_internal_profiles = allow_internal_profiles;
+	} else {
+		/* Old config doesn't contain this item, so set it to true to
+		 * match the previous default behavior.
+		 */
+		device->allow_internal_profiles = true;
+		g_error_free(gerr);
+		gerr = NULL;
+	}
+
 	/* Load device profile list */
 	uuids = g_key_file_get_string_list(key_file, "General", "Services",
 						NULL, NULL);
@@ -3782,6 +3873,9 @@ static bool device_match_profile(struct btd_device *device,
 							bt_uuid_strcmp) == NULL)
 		return false;
 
+	if (!device->allow_internal_profiles && !profile->external)
+		return false;
+
 	return true;
 }
 
@@ -4055,6 +4149,8 @@ static struct btd_device *device_new(struct btd_adapter *adapter,
 
 	device->adapter = adapter;
 	device->temporary = true;
+	device->allow_internal_profiles =
+		main_opts.default_allow_internal_profiles;
 
 	device->db_id = gatt_db_register(device->db, gatt_service_added,
 					gatt_service_removed, device, NULL);
diff --git a/src/hcid.h b/src/hcid.h
index c21ac9980..82ea0cb14 100644
--- a/src/hcid.h
+++ b/src/hcid.h
@@ -114,6 +114,8 @@ struct main_opts {
 	uint8_t		key_size;
 
 	enum jw_repairing_t jw_repairing;
+
+	gboolean	default_allow_internal_profiles;
 };
 
 extern struct main_opts main_opts;
diff --git a/src/main.c b/src/main.c
index bacb44197..30218f8d0 100644
--- a/src/main.c
+++ b/src/main.c
@@ -89,6 +89,7 @@ static const char *supported_options[] = {
 	"FastConnectable",
 	"Privacy",
 	"JustWorksRepairing",
+	"DefaultAllowInternalProfiles",
 	NULL
 };
 
@@ -615,6 +616,13 @@ static void parse_config(GKeyFile *config)
 	else
 		main_opts.fast_conn = boolean;
 
+	boolean = g_key_file_get_boolean(config, "General",
+					"DefaultAllowInternalProfiles", &err);
+	if (err)
+		g_clear_error(&err);
+	else
+		main_opts.default_allow_internal_profiles = boolean;
+
 	str = g_key_file_get_string(config, "GATT", "Cache", &err);
 	if (err) {
 		DBG("%s", err->message);
@@ -691,6 +699,8 @@ static void init_defaults(void)
 	main_opts.gatt_cache = BT_GATT_CACHE_ALWAYS;
 	main_opts.gatt_mtu = BT_ATT_MAX_LE_MTU;
 	main_opts.gatt_channels = 3;
+
+	main_opts.default_allow_internal_profiles = true;
 }
 
 static void log_handler(const gchar *log_domain, GLogLevelFlags log_level,
diff --git a/src/main.conf b/src/main.conf
index 6a6f7d4b4..497394395 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -77,6 +77,10 @@
 # Defaults to "never"
 #JustWorksRepairing = never
 
+# The default value of "AllowInternalProfiles" property in org.bluez.Device1
+# interface. Defaults to 'true'.
+#DefaultAllowInternalProfiles = true
+
 [Controller]
 # The following values are used to load default adapter parameters.  BlueZ loads
 # the values into the kernel before the adapter is powered if the kernel
-- 
2.26.2

[PATCH BlueZ 3/3] client: Add set-allow-internal-profiles command

[Sonny Sasaka]

Reviewed-by: Alain Michaud <alainm@chromium.org>

---
 client/main.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/client/main.c b/client/main.c
index 9abada69f..c2b6e21e4 100644
--- a/client/main.c
+++ b/client/main.c
@@ -1678,6 +1678,7 @@ static void cmd_info(int argc, char *argv[])
 	print_property(proxy, "Connected");
 	print_property(proxy, "WakeAllowed");
 	print_property(proxy, "LegacyPairing");
+	print_property(proxy, "AllowInternalProfiles");
 	print_uuids(proxy);
 	print_property(proxy, "Modalias");
 	print_property(proxy, "ManufacturerData");
@@ -1838,6 +1839,39 @@ static void cmd_unblock(int argc, char *argv[])
 	return bt_shell_noninteractive_quit(EXIT_FAILURE);
 }
 
+static void cmd_set_allow_internal_profiles(int argc, char *argv[])
+{
+	GDBusProxy *proxy;
+	dbus_bool_t allow_internal_profiles;
+	char *str;
+
+	proxy = find_device(argc, argv);
+	if (!proxy)
+		return bt_shell_noninteractive_quit(EXIT_FAILURE);
+
+	if (strcmp(argv[2], "true") != 0 && strcmp(argv[2], "false") != 0) {
+		bt_shell_printf("Invalid argument: %s\n", argv[2]);
+		return bt_shell_noninteractive_quit(EXIT_FAILURE);
+	}
+
+	allow_internal_profiles = strcmp(argv[2], "true") == 0 ? true : false;
+
+	str = g_strdup_printf("%s allow internal profiles",
+				proxy_address(proxy));
+
+	if (g_dbus_proxy_set_property_basic(proxy, "AllowInternalProfiles",
+						DBUS_TYPE_BOOLEAN,
+						&allow_internal_profiles,
+						generic_callback,
+						str,
+						g_free) == TRUE)
+		return;
+
+	g_free(str);
+
+	return bt_shell_noninteractive_quit(EXIT_FAILURE);
+}
+
 static void remove_device_reply(DBusMessage *message, void *user_data)
 {
 	DBusError error;
@@ -2824,6 +2858,10 @@ static const struct bt_shell_menu main_menu = {
 								dev_generator },
 	{ "unblock",      "[dev]",    cmd_unblock, "Unblock device",
 								dev_generator },
+	{ "set-allow-internal-profiles", "<dev> <true/false>",
+					cmd_set_allow_internal_profiles,
+					"Set allow internal profiles",
+					dev_generator },
 	{ "remove",       "<dev>",    cmd_remove, "Remove device",
 							dev_generator },
 	{ "connect",      "<dev>",    cmd_connect, "Connect device",
-- 
2.26.2

Re: [PATCH BlueZ 0/3] Per-device option to enable/disable internal profiles

[Luiz Augusto von Dentz]

Hi Sonny,

On Mon, Jul 13, 2020 at 1:18 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
>
> This patch series adds a mechanism for clients to choose whether to
> enable BlueZ internal profiles (e.g. A2DP, Battery) for specific
> devices.
>
> The motivation behind this feature is that some applications (e.g. Web
> Bluetooth or Android apps) need to have control over all remove GATT
> services, like Battery service. With "battery" plugin being enabled on
> BlueZ, it becomes not possible for those apps to work properly because
> BlueZ "hides" the Battery-related attributes from its GATT Client API.
> Disabling the "battery" plugin won't solve the problem either, since we
> do also need to enable the plugin so that we can use org.bluez.Battery1
> API.
>
> The solution that we propose is that clients can choose whether to
> enable internal profiles for each device. Clients know when to enable
> internal profiles (such as when a user chooses to pair/connect via a UI)
> and when to disable internal profiles (such as when the connection is
> initiated by a generic application).

I wonder if it is not better to just have a flag indicating if the
profile shall claim exclusive access (such as GAP and GATT services),
so profiles that don't set that will have the services exposed so for
battery we can probably just have it exposed by default since it
doesn't appear to would be any conflicts on having it exposed.

> Sonny Sasaka (3):
>   doc: Add "AllowInternalProfiles" property to org.bluez.Device1
>   device: Add "AllowInternalProfiles" property to org.bluez.Device1
>   client: Add set-allow-internal-profiles command
>
>  client/main.c      | 38 ++++++++++++++++++
>  doc/device-api.txt | 13 +++++++
>  src/device.c       | 96 ++++++++++++++++++++++++++++++++++++++++++++++
>  src/hcid.h         |  2 +
>  src/main.c         | 10 +++++
>  src/main.conf      |  4 ++
>  6 files changed, 163 insertions(+)
>
> --
> 2.26.2
>


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 0/3] Per-device option to enable/disable internal profiles

[Sonny Sasaka]

Hi Luiz,

I considered having such an approach that gives exception to some
profile to not claim exclusive access. However, I think that this
approach has a drawback that it can only be guaranteed to work
correctly for profiles that contain only read-only attributes. Any
profile that contains writable attributes, naturally, cannot be
guaranteed to always work correctly (as is the case with the Battery
profile). Therefore, the usefulness of that feature will be very
limited.

I also considered the benefits of the AllowInternalProfiles approach:
* Applications can also have control over any profile, not just
Battery profile. For example, if in the future BlueZ has more internal
profiles, like (Blood Pressure Profile or any other profile that may
contain writable attributes), we can guarantee that applications can
still opt to have access to that profile, without relying on a profile
being "safe" to be shared by both BlueZ's internal and external
handlers.
* This has an added security benefit: applications which operate on a
specific GATT profile will not unintentionally activate internal
profiles such as HOG (which is able to hijack input control of the
host). This is the correct and expected behavior of Android apps that
connect over GATT and get access to a GATT profile.

Therefore I think that this approach, although more complex, has
longer lasting benefits. Let me know if you have any objection to
having such a feature.


On Mon, Jul 13, 2020 at 1:35 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Sonny,
>
> On Mon, Jul 13, 2020 at 1:18 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> >
> > This patch series adds a mechanism for clients to choose whether to
> > enable BlueZ internal profiles (e.g. A2DP, Battery) for specific
> > devices.
> >
> > The motivation behind this feature is that some applications (e.g. Web
> > Bluetooth or Android apps) need to have control over all remove GATT
> > services, like Battery service. With "battery" plugin being enabled on
> > BlueZ, it becomes not possible for those apps to work properly because
> > BlueZ "hides" the Battery-related attributes from its GATT Client API.
> > Disabling the "battery" plugin won't solve the problem either, since we
> > do also need to enable the plugin so that we can use org.bluez.Battery1
> > API.
> >
> > The solution that we propose is that clients can choose whether to
> > enable internal profiles for each device. Clients know when to enable
> > internal profiles (such as when a user chooses to pair/connect via a UI)
> > and when to disable internal profiles (such as when the connection is
> > initiated by a generic application).
>
> I wonder if it is not better to just have a flag indicating if the
> profile shall claim exclusive access (such as GAP and GATT services),
> so profiles that don't set that will have the services exposed so for
> battery we can probably just have it exposed by default since it
> doesn't appear to would be any conflicts on having it exposed.
>
> > Sonny Sasaka (3):
> >   doc: Add "AllowInternalProfiles" property to org.bluez.Device1
> >   device: Add "AllowInternalProfiles" property to org.bluez.Device1
> >   client: Add set-allow-internal-profiles command
> >
> >  client/main.c      | 38 ++++++++++++++++++
> >  doc/device-api.txt | 13 +++++++
> >  src/device.c       | 96 ++++++++++++++++++++++++++++++++++++++++++++++
> >  src/hcid.h         |  2 +
> >  src/main.c         | 10 +++++
> >  src/main.conf      |  4 ++
> >  6 files changed, 163 insertions(+)
> >
> > --
> > 2.26.2
> >
>
>
> --
> Luiz Augusto von Dentz

Re: [PATCH BlueZ 0/3] Per-device option to enable/disable internal profiles

[Luiz Augusto von Dentz]

Hi Sonny,

On Mon, Jul 13, 2020 at 3:04 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
>
> Hi Luiz,
>
> I considered having such an approach that gives exception to some
> profile to not claim exclusive access. However, I think that this
> approach has a drawback that it can only be guaranteed to work
> correctly for profiles that contain only read-only attributes. Any
> profile that contains writable attributes, naturally, cannot be
> guaranteed to always work correctly (as is the case with the Battery
> profile). Therefore, the usefulness of that feature will be very
> limited.
>
> I also considered the benefits of the AllowInternalProfiles approach:
> * Applications can also have control over any profile, not just
> Battery profile. For example, if in the future BlueZ has more internal
> profiles, like (Blood Pressure Profile or any other profile that may
> contain writable attributes), we can guarantee that applications can
> still opt to have access to that profile, without relying on a profile
> being "safe" to be shared by both BlueZ's internal and external
> handlers.
> * This has an added security benefit: applications which operate on a
> specific GATT profile will not unintentionally activate internal
> profiles such as HOG (which is able to hijack input control of the
> host). This is the correct and expected behavior of Android apps that
> connect over GATT and get access to a GATT profile.

Not sure I follow these arguments, it seems AllowInternalProfiles may
actually enable hijacking the profiles so I wonder if you got this
backwards as we can't let things like HoG be controlled by
applications directly it would be too easy to implement something like
a keylogger, or perhaps you are suggesting that there is another layer
for implementing the profiles? Note that it is intended that plugins
shall be used for profiles that need to be integrated system wide,
D-Bus interface shall be restricted to only application specific
profiles.

Note that we do allow external profiles to be registered with use of:

https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/profile-api.txt

And for GATT:

https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/gatt-api.txt#n366

We could perhaps make the assumption that once an application
registers itself as supporting a given profile we check if against a
blacklist of profiles that may have security implications, which
perhaps could be defined via main.conf or some other file, if that is
not the case the internal profile can be disabled and the D-Bus object
would be accessible over D-Bus. Also note that we do offer clients the
ability to have exclusive access with AcquireWrite and AcquireNotify.

> Therefore I think that this approach, although more complex, has
> longer lasting benefits. Let me know if you have any objection to
> having such a feature.
>
>
> On Mon, Jul 13, 2020 at 1:35 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Sonny,
> >
> > On Mon, Jul 13, 2020 at 1:18 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > >
> > > This patch series adds a mechanism for clients to choose whether to
> > > enable BlueZ internal profiles (e.g. A2DP, Battery) for specific
> > > devices.
> > >
> > > The motivation behind this feature is that some applications (e.g. Web
> > > Bluetooth or Android apps) need to have control over all remove GATT
> > > services, like Battery service. With "battery" plugin being enabled on
> > > BlueZ, it becomes not possible for those apps to work properly because
> > > BlueZ "hides" the Battery-related attributes from its GATT Client API.
> > > Disabling the "battery" plugin won't solve the problem either, since we
> > > do also need to enable the plugin so that we can use org.bluez.Battery1
> > > API.
> > >
> > > The solution that we propose is that clients can choose whether to
> > > enable internal profiles for each device. Clients know when to enable
> > > internal profiles (such as when a user chooses to pair/connect via a UI)
> > > and when to disable internal profiles (such as when the connection is
> > > initiated by a generic application).
> >
> > I wonder if it is not better to just have a flag indicating if the
> > profile shall claim exclusive access (such as GAP and GATT services),
> > so profiles that don't set that will have the services exposed so for
> > battery we can probably just have it exposed by default since it
> > doesn't appear to would be any conflicts on having it exposed.
> >
> > > Sonny Sasaka (3):
> > >   doc: Add "AllowInternalProfiles" property to org.bluez.Device1
> > >   device: Add "AllowInternalProfiles" property to org.bluez.Device1
> > >   client: Add set-allow-internal-profiles command
> > >
> > >  client/main.c      | 38 ++++++++++++++++++
> > >  doc/device-api.txt | 13 +++++++
> > >  src/device.c       | 96 ++++++++++++++++++++++++++++++++++++++++++++++
> > >  src/hcid.h         |  2 +
> > >  src/main.c         | 10 +++++
> > >  src/main.conf      |  4 ++
> > >  6 files changed, 163 insertions(+)
> > >
> > > --
> > > 2.26.2
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 0/3] Per-device option to enable/disable internal profiles

[Sonny Sasaka]

Hi Luiz,

On Mon, Jul 13, 2020 at 3:59 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Sonny,
>
> On Mon, Jul 13, 2020 at 3:04 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> >
> > Hi Luiz,
> >
> > I considered having such an approach that gives exception to some
> > profile to not claim exclusive access. However, I think that this
> > approach has a drawback that it can only be guaranteed to work
> > correctly for profiles that contain only read-only attributes. Any
> > profile that contains writable attributes, naturally, cannot be
> > guaranteed to always work correctly (as is the case with the Battery
> > profile). Therefore, the usefulness of that feature will be very
> > limited.
> >
> > I also considered the benefits of the AllowInternalProfiles approach:
> > * Applications can also have control over any profile, not just
> > Battery profile. For example, if in the future BlueZ has more internal
> > profiles, like (Blood Pressure Profile or any other profile that may
> > contain writable attributes), we can guarantee that applications can
> > still opt to have access to that profile, without relying on a profile
> > being "safe" to be shared by both BlueZ's internal and external
> > handlers.
> > * This has an added security benefit: applications which operate on a
> > specific GATT profile will not unintentionally activate internal
> > profiles such as HOG (which is able to hijack input control of the
> > host). This is the correct and expected behavior of Android apps that
> > connect over GATT and get access to a GATT profile.
>
> Not sure I follow these arguments, it seems AllowInternalProfiles may
> actually enable hijacking the profiles so I wonder if you got this
> backwards as we can't let things like HoG be controlled by
> applications directly it would be too easy to implement something like
> a keylogger, or perhaps you are suggesting that there is another layer
> for implementing the profiles? Note that it is intended that plugins
> shall be used for profiles that need to be integrated system wide,
> D-Bus interface shall be restricted to only application specific
> profiles.

I think you misunderstood my point about HOG hijacking. Consider the
following case:
1. A legit application (not System UI) on a host computer scans and
connects to a nearby peer. It makes a guess about the peer device
based on its advertising data. It intends to operate on a specific
GATT profile (not necessarily Battery).
2. The peer device turns out to be malicious. It runs HOG GATT server
and triggers the host's HOG profile to be active.
3. The malicious peer device's HOG GATT server can now maliciously
make mouse movements or enter keystrokes to the host.

In this case the user is considered being attacked, because he/she
doesn't consciously interact with the System UI to connect to a nearby
mouse/keyboard.
My example doesn't have to be HOG. It just happens to be a profile
which is attackable at the moment. My point is that, for applications
it's always safest to turn off all internal profiles to avoid such
incident. There is no use case where applications want to trigger
internal profiles.

Note 1: By "applications", I mean things like Android apps or
JavaScript apps which are not considered System's Bluetooth UI.

>
> Note that we do allow external profiles to be registered with use of:
>
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/profile-api.txt
>
> And for GATT:
>
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/gatt-api.txt#n366
>
> We could perhaps make the assumption that once an application
> registers itself as supporting a given profile we check if against a
> blacklist of profiles that may have security implications, which
> perhaps could be defined via main.conf or some other file, if that is
> not the case the internal profile can be disabled and the D-Bus object
> would be accessible over D-Bus. Also note that we do offer clients the
> ability to have exclusive access with AcquireWrite and AcquireNotify.
>
> > Therefore I think that this approach, although more complex, has
> > longer lasting benefits. Let me know if you have any objection to
> > having such a feature.
> >
> >
> > On Mon, Jul 13, 2020 at 1:35 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > Hi Sonny,
> > >
> > > On Mon, Jul 13, 2020 at 1:18 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > > >
> > > > This patch series adds a mechanism for clients to choose whether to
> > > > enable BlueZ internal profiles (e.g. A2DP, Battery) for specific
> > > > devices.
> > > >
> > > > The motivation behind this feature is that some applications (e.g. Web
> > > > Bluetooth or Android apps) need to have control over all remove GATT
> > > > services, like Battery service. With "battery" plugin being enabled on
> > > > BlueZ, it becomes not possible for those apps to work properly because
> > > > BlueZ "hides" the Battery-related attributes from its GATT Client API.
> > > > Disabling the "battery" plugin won't solve the problem either, since we
> > > > do also need to enable the plugin so that we can use org.bluez.Battery1
> > > > API.
> > > >
> > > > The solution that we propose is that clients can choose whether to
> > > > enable internal profiles for each device. Clients know when to enable
> > > > internal profiles (such as when a user chooses to pair/connect via a UI)
> > > > and when to disable internal profiles (such as when the connection is
> > > > initiated by a generic application).
> > >
> > > I wonder if it is not better to just have a flag indicating if the
> > > profile shall claim exclusive access (such as GAP and GATT services),
> > > so profiles that don't set that will have the services exposed so for
> > > battery we can probably just have it exposed by default since it
> > > doesn't appear to would be any conflicts on having it exposed.
> > >
> > > > Sonny Sasaka (3):
> > > >   doc: Add "AllowInternalProfiles" property to org.bluez.Device1
> > > >   device: Add "AllowInternalProfiles" property to org.bluez.Device1
> > > >   client: Add set-allow-internal-profiles command
> > > >
> > > >  client/main.c      | 38 ++++++++++++++++++
> > > >  doc/device-api.txt | 13 +++++++
> > > >  src/device.c       | 96 ++++++++++++++++++++++++++++++++++++++++++++++
> > > >  src/hcid.h         |  2 +
> > > >  src/main.c         | 10 +++++
> > > >  src/main.conf      |  4 ++
> > > >  6 files changed, 163 insertions(+)
> > > >
> > > > --
> > > > 2.26.2
> > > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz

Re: [PATCH BlueZ 0/3] Per-device option to enable/disable internal profiles

[Luiz Augusto von Dentz]

Hi Sonny,

On Mon, Jul 13, 2020 at 4:48 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
>
> Hi Luiz,
>
> On Mon, Jul 13, 2020 at 3:59 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Sonny,
> >
> > On Mon, Jul 13, 2020 at 3:04 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > >
> > > Hi Luiz,
> > >
> > > I considered having such an approach that gives exception to some
> > > profile to not claim exclusive access. However, I think that this
> > > approach has a drawback that it can only be guaranteed to work
> > > correctly for profiles that contain only read-only attributes. Any
> > > profile that contains writable attributes, naturally, cannot be
> > > guaranteed to always work correctly (as is the case with the Battery
> > > profile). Therefore, the usefulness of that feature will be very
> > > limited.
> > >
> > > I also considered the benefits of the AllowInternalProfiles approach:
> > > * Applications can also have control over any profile, not just
> > > Battery profile. For example, if in the future BlueZ has more internal
> > > profiles, like (Blood Pressure Profile or any other profile that may
> > > contain writable attributes), we can guarantee that applications can
> > > still opt to have access to that profile, without relying on a profile
> > > being "safe" to be shared by both BlueZ's internal and external
> > > handlers.
> > > * This has an added security benefit: applications which operate on a
> > > specific GATT profile will not unintentionally activate internal
> > > profiles such as HOG (which is able to hijack input control of the
> > > host). This is the correct and expected behavior of Android apps that
> > > connect over GATT and get access to a GATT profile.
> >
> > Not sure I follow these arguments, it seems AllowInternalProfiles may
> > actually enable hijacking the profiles so I wonder if you got this
> > backwards as we can't let things like HoG be controlled by
> > applications directly it would be too easy to implement something like
> > a keylogger, or perhaps you are suggesting that there is another layer
> > for implementing the profiles? Note that it is intended that plugins
> > shall be used for profiles that need to be integrated system wide,
> > D-Bus interface shall be restricted to only application specific
> > profiles.
>
> I think you misunderstood my point about HOG hijacking. Consider the
> following case:
> 1. A legit application (not System UI) on a host computer scans and
> connects to a nearby peer. It makes a guess about the peer device
> based on its advertising data. It intends to operate on a specific
> GATT profile (not necessarily Battery).
> 2. The peer device turns out to be malicious. It runs HOG GATT server
> and triggers the host's HOG profile to be active.
> 3. The malicious peer device's HOG GATT server can now maliciously
> make mouse movements or enter keystrokes to the host.

I'm not sure how you would like to prevent that, we could in theory
attempt to authorize every single profile before connecting, just like
it is done for legacy, but Im sure system would not be asking the user
what profiles to connect so they just end up trusting the device,
alternatively we could make ConnectProfile to work also for LE so you
can provide a UUID and nothing else would be exposed, but note that
this guess on the AD may actually be wrong and the device may end up
malfunctioning.

> In this case the user is considered being attacked, because he/she
> doesn't consciously interact with the System UI to connect to a nearby
> mouse/keyboard.
> My example doesn't have to be HOG. It just happens to be a profile
> which is attackable at the moment. My point is that, for applications
> it's always safest to turn off all internal profiles to avoid such
> incident. There is no use case where applications want to trigger
> internal profiles.
>
> Note 1: By "applications", I mean things like Android apps or
> JavaScript apps which are not considered System's Bluetooth UI.

Well that doesn't make my point moot, let's say we do enable
connecting by UUID and the application try to connect HoG, it could be
a malicious application trying to eavesdrop what the user is typing,
so this problem of malicious goes both ways Im afraid, besides the
performance penalty that one would have if we need to transport HID
over D-Bus.

More applications could be involved and then this all becomes a mess
if they have to fight over AllowInternalProfiles, so instead of using
a theoretical example we better find real apps and devices where
conflicts happens and work out case by case, adding ConnectProfile
should actually fix most of them if there is a single profile involved
by we could also thing about an alternative to connect multiples.
There is also the possibility of exposing the btd_service as objects,
I've actually had this implemented for the car industry, that way
AutoConnect property could actually be controlled on a per service
basis instead of having just one switch for everything.

> >
> > Note that we do allow external profiles to be registered with use of:
> >
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/profile-api.txt
> >
> > And for GATT:
> >
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/gatt-api.txt#n366
> >
> > We could perhaps make the assumption that once an application
> > registers itself as supporting a given profile we check if against a
> > blacklist of profiles that may have security implications, which
> > perhaps could be defined via main.conf or some other file, if that is
> > not the case the internal profile can be disabled and the D-Bus object
> > would be accessible over D-Bus. Also note that we do offer clients the
> > ability to have exclusive access with AcquireWrite and AcquireNotify.
> >
> > > Therefore I think that this approach, although more complex, has
> > > longer lasting benefits. Let me know if you have any objection to
> > > having such a feature.
> > >
> > >
> > > On Mon, Jul 13, 2020 at 1:35 PM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > >
> > > > Hi Sonny,
> > > >
> > > > On Mon, Jul 13, 2020 at 1:18 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > > > >
> > > > > This patch series adds a mechanism for clients to choose whether to
> > > > > enable BlueZ internal profiles (e.g. A2DP, Battery) for specific
> > > > > devices.
> > > > >
> > > > > The motivation behind this feature is that some applications (e.g. Web
> > > > > Bluetooth or Android apps) need to have control over all remove GATT
> > > > > services, like Battery service. With "battery" plugin being enabled on
> > > > > BlueZ, it becomes not possible for those apps to work properly because
> > > > > BlueZ "hides" the Battery-related attributes from its GATT Client API.
> > > > > Disabling the "battery" plugin won't solve the problem either, since we
> > > > > do also need to enable the plugin so that we can use org.bluez.Battery1
> > > > > API.
> > > > >
> > > > > The solution that we propose is that clients can choose whether to
> > > > > enable internal profiles for each device. Clients know when to enable
> > > > > internal profiles (such as when a user chooses to pair/connect via a UI)
> > > > > and when to disable internal profiles (such as when the connection is
> > > > > initiated by a generic application).
> > > >
> > > > I wonder if it is not better to just have a flag indicating if the
> > > > profile shall claim exclusive access (such as GAP and GATT services),
> > > > so profiles that don't set that will have the services exposed so for
> > > > battery we can probably just have it exposed by default since it
> > > > doesn't appear to would be any conflicts on having it exposed.
> > > >
> > > > > Sonny Sasaka (3):
> > > > >   doc: Add "AllowInternalProfiles" property to org.bluez.Device1
> > > > >   device: Add "AllowInternalProfiles" property to org.bluez.Device1
> > > > >   client: Add set-allow-internal-profiles command
> > > > >
> > > > >  client/main.c      | 38 ++++++++++++++++++
> > > > >  doc/device-api.txt | 13 +++++++
> > > > >  src/device.c       | 96 ++++++++++++++++++++++++++++++++++++++++++++++
> > > > >  src/hcid.h         |  2 +
> > > > >  src/main.c         | 10 +++++
> > > > >  src/main.conf      |  4 ++
> > > > >  6 files changed, 163 insertions(+)
> > > > >
> > > > > --
> > > > > 2.26.2
> > > > >
> > > >
> > > >
> > > > --
> > > > Luiz Augusto von Dentz
> >
> >
> >
> > --
> > Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 0/3] Per-device option to enable/disable internal profiles

[Sonny Sasaka]

Hi Luiz,

On Mon, Jul 13, 2020 at 10:28 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Sonny,
>
> On Mon, Jul 13, 2020 at 4:48 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> >
> > Hi Luiz,
> >
> > On Mon, Jul 13, 2020 at 3:59 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > Hi Sonny,
> > >
> > > On Mon, Jul 13, 2020 at 3:04 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > > >
> > > > Hi Luiz,
> > > >
> > > > I considered having such an approach that gives exception to some
> > > > profile to not claim exclusive access. However, I think that this
> > > > approach has a drawback that it can only be guaranteed to work
> > > > correctly for profiles that contain only read-only attributes. Any
> > > > profile that contains writable attributes, naturally, cannot be
> > > > guaranteed to always work correctly (as is the case with the Battery
> > > > profile). Therefore, the usefulness of that feature will be very
> > > > limited.
> > > >
> > > > I also considered the benefits of the AllowInternalProfiles approach:
> > > > * Applications can also have control over any profile, not just
> > > > Battery profile. For example, if in the future BlueZ has more internal
> > > > profiles, like (Blood Pressure Profile or any other profile that may
> > > > contain writable attributes), we can guarantee that applications can
> > > > still opt to have access to that profile, without relying on a profile
> > > > being "safe" to be shared by both BlueZ's internal and external
> > > > handlers.
> > > > * This has an added security benefit: applications which operate on a
> > > > specific GATT profile will not unintentionally activate internal
> > > > profiles such as HOG (which is able to hijack input control of the
> > > > host). This is the correct and expected behavior of Android apps that
> > > > connect over GATT and get access to a GATT profile.
> > >
> > > Not sure I follow these arguments, it seems AllowInternalProfiles may
> > > actually enable hijacking the profiles so I wonder if you got this
> > > backwards as we can't let things like HoG be controlled by
> > > applications directly it would be too easy to implement something like
> > > a keylogger, or perhaps you are suggesting that there is another layer
> > > for implementing the profiles? Note that it is intended that plugins
> > > shall be used for profiles that need to be integrated system wide,
> > > D-Bus interface shall be restricted to only application specific
> > > profiles.
> >
> > I think you misunderstood my point about HOG hijacking. Consider the
> > following case:
> > 1. A legit application (not System UI) on a host computer scans and
> > connects to a nearby peer. It makes a guess about the peer device
> > based on its advertising data. It intends to operate on a specific
> > GATT profile (not necessarily Battery).
> > 2. The peer device turns out to be malicious. It runs HOG GATT server
> > and triggers the host's HOG profile to be active.
> > 3. The malicious peer device's HOG GATT server can now maliciously
> > make mouse movements or enter keystrokes to the host.
>
> I'm not sure how you would like to prevent that, we could in theory
> attempt to authorize every single profile before connecting, just like
> it is done for legacy, but Im sure system would not be asking the user
> what profiles to connect so they just end up trusting the device,
> alternatively we could make ConnectProfile to work also for LE so you
> can provide a UUID and nothing else would be exposed, but note that
> this guess on the AD may actually be wrong and the device may end up
> malfunctioning.
>
> > In this case the user is considered being attacked, because he/she
> > doesn't consciously interact with the System UI to connect to a nearby
> > mouse/keyboard.
> > My example doesn't have to be HOG. It just happens to be a profile
> > which is attackable at the moment. My point is that, for applications
> > it's always safest to turn off all internal profiles to avoid such
> > incident. There is no use case where applications want to trigger
> > internal profiles.
> >
> > Note 1: By "applications", I mean things like Android apps or
> > JavaScript apps which are not considered System's Bluetooth UI.
>
> Well that doesn't make my point moot, let's say we do enable
> connecting by UUID and the application try to connect HoG, it could be
> a malicious application trying to eavesdrop what the user is typing,
> so this problem of malicious goes both ways Im afraid, besides the
> performance penalty that one would have if we need to transport HID
> over D-Bus.
If an application handles HOG, there will be nothing to eavesdrop
because that application shouldn't have an access to UHID in the first
place. If that malicious application had UHID access, that is already
a problem to begin with regardless of whether there is Bluetooth or
not. The security of this is handled above the Bluetooth layer. The
operating system that uses this feature is responsible for higher
layer security. For operating systems that don't need it I am okay
with adding an option to disable this feature altogether. But I can
see that there are systems that need it and I am not convinced that a
general purpose Bluetooth stack should not support it.

>
> More applications could be involved and then this all becomes a mess
> if they have to fight over AllowInternalProfiles, so instead of using
> a theoretical example we better find real apps and devices where
> conflicts happens and work out case by case, adding ConnectProfile
> should actually fix most of them if there is a single profile involved
> by we could also thing about an alternative to connect multiples.
> There is also the possibility of exposing the btd_service as objects,
> I've actually had this implemented for the car industry, that way
> AutoConnect property could actually be controlled on a per service
> basis instead of having just one switch for everything.
To be clear, applications do not have direct access to
AllowInternalProfiles. The higher layer operating system controls it.
This is just the same case as the org.bluez.Adapter1.Powered property
and many other examples where applications are not expected to have
direct control of. Therefore there should be no problem of many things
fighting over it if used correctly, just like many other properties.
Again, I am okay with adding an option to disable this for operating
systems that do not want it.

Note: I have been using the term "operating system" to refer to high
level components rather than the kernel.


>
> > >
> > > Note that we do allow external profiles to be registered with use of:
> > >
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/profile-api.txt
> > >
> > > And for GATT:
> > >
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/gatt-api.txt#n366
> > >
> > > We could perhaps make the assumption that once an application
> > > registers itself as supporting a given profile we check if against a
> > > blacklist of profiles that may have security implications, which
> > > perhaps could be defined via main.conf or some other file, if that is
> > > not the case the internal profile can be disabled and the D-Bus object
> > > would be accessible over D-Bus. Also note that we do offer clients the
> > > ability to have exclusive access with AcquireWrite and AcquireNotify.
> > >
> > > > Therefore I think that this approach, although more complex, has
> > > > longer lasting benefits. Let me know if you have any objection to
> > > > having such a feature.
> > > >
> > > >
> > > > On Mon, Jul 13, 2020 at 1:35 PM Luiz Augusto von Dentz
> > > > <luiz.dentz@gmail.com> wrote:
> > > > >
> > > > > Hi Sonny,
> > > > >
> > > > > On Mon, Jul 13, 2020 at 1:18 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > > > > >
> > > > > > This patch series adds a mechanism for clients to choose whether to
> > > > > > enable BlueZ internal profiles (e.g. A2DP, Battery) for specific
> > > > > > devices.
> > > > > >
> > > > > > The motivation behind this feature is that some applications (e.g. Web
> > > > > > Bluetooth or Android apps) need to have control over all remove GATT
> > > > > > services, like Battery service. With "battery" plugin being enabled on
> > > > > > BlueZ, it becomes not possible for those apps to work properly because
> > > > > > BlueZ "hides" the Battery-related attributes from its GATT Client API.
> > > > > > Disabling the "battery" plugin won't solve the problem either, since we
> > > > > > do also need to enable the plugin so that we can use org.bluez.Battery1
> > > > > > API.
> > > > > >
> > > > > > The solution that we propose is that clients can choose whether to
> > > > > > enable internal profiles for each device. Clients know when to enable
> > > > > > internal profiles (such as when a user chooses to pair/connect via a UI)
> > > > > > and when to disable internal profiles (such as when the connection is
> > > > > > initiated by a generic application).
> > > > >
> > > > > I wonder if it is not better to just have a flag indicating if the
> > > > > profile shall claim exclusive access (such as GAP and GATT services),
> > > > > so profiles that don't set that will have the services exposed so for
> > > > > battery we can probably just have it exposed by default since it
> > > > > doesn't appear to would be any conflicts on having it exposed.
> > > > >
> > > > > > Sonny Sasaka (3):
> > > > > >   doc: Add "AllowInternalProfiles" property to org.bluez.Device1
> > > > > >   device: Add "AllowInternalProfiles" property to org.bluez.Device1
> > > > > >   client: Add set-allow-internal-profiles command
> > > > > >
> > > > > >  client/main.c      | 38 ++++++++++++++++++
> > > > > >  doc/device-api.txt | 13 +++++++
> > > > > >  src/device.c       | 96 ++++++++++++++++++++++++++++++++++++++++++++++
> > > > > >  src/hcid.h         |  2 +
> > > > > >  src/main.c         | 10 +++++
> > > > > >  src/main.conf      |  4 ++
> > > > > >  6 files changed, 163 insertions(+)
> > > > > >
> > > > > > --
> > > > > > 2.26.2
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Luiz Augusto von Dentz
> > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz

Re: [PATCH BlueZ 0/3] Per-device option to enable/disable internal profiles

[Luiz Augusto von Dentz]

Hi Sonny,

On Tue, Jul 14, 2020 at 1:55 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
>
> Hi Luiz,
>
> On Mon, Jul 13, 2020 at 10:28 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Sonny,
> >
> > On Mon, Jul 13, 2020 at 4:48 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > >
> > > Hi Luiz,
> > >
> > > On Mon, Jul 13, 2020 at 3:59 PM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > >
> > > > Hi Sonny,
> > > >
> > > > On Mon, Jul 13, 2020 at 3:04 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > > > >
> > > > > Hi Luiz,
> > > > >
> > > > > I considered having such an approach that gives exception to some
> > > > > profile to not claim exclusive access. However, I think that this
> > > > > approach has a drawback that it can only be guaranteed to work
> > > > > correctly for profiles that contain only read-only attributes. Any
> > > > > profile that contains writable attributes, naturally, cannot be
> > > > > guaranteed to always work correctly (as is the case with the Battery
> > > > > profile). Therefore, the usefulness of that feature will be very
> > > > > limited.
> > > > >
> > > > > I also considered the benefits of the AllowInternalProfiles approach:
> > > > > * Applications can also have control over any profile, not just
> > > > > Battery profile. For example, if in the future BlueZ has more internal
> > > > > profiles, like (Blood Pressure Profile or any other profile that may
> > > > > contain writable attributes), we can guarantee that applications can
> > > > > still opt to have access to that profile, without relying on a profile
> > > > > being "safe" to be shared by both BlueZ's internal and external
> > > > > handlers.
> > > > > * This has an added security benefit: applications which operate on a
> > > > > specific GATT profile will not unintentionally activate internal
> > > > > profiles such as HOG (which is able to hijack input control of the
> > > > > host). This is the correct and expected behavior of Android apps that
> > > > > connect over GATT and get access to a GATT profile.
> > > >
> > > > Not sure I follow these arguments, it seems AllowInternalProfiles may
> > > > actually enable hijacking the profiles so I wonder if you got this
> > > > backwards as we can't let things like HoG be controlled by
> > > > applications directly it would be too easy to implement something like
> > > > a keylogger, or perhaps you are suggesting that there is another layer
> > > > for implementing the profiles? Note that it is intended that plugins
> > > > shall be used for profiles that need to be integrated system wide,
> > > > D-Bus interface shall be restricted to only application specific
> > > > profiles.
> > >
> > > I think you misunderstood my point about HOG hijacking. Consider the
> > > following case:
> > > 1. A legit application (not System UI) on a host computer scans and
> > > connects to a nearby peer. It makes a guess about the peer device
> > > based on its advertising data. It intends to operate on a specific
> > > GATT profile (not necessarily Battery).
> > > 2. The peer device turns out to be malicious. It runs HOG GATT server
> > > and triggers the host's HOG profile to be active.
> > > 3. The malicious peer device's HOG GATT server can now maliciously
> > > make mouse movements or enter keystrokes to the host.
> >
> > I'm not sure how you would like to prevent that, we could in theory
> > attempt to authorize every single profile before connecting, just like
> > it is done for legacy, but Im sure system would not be asking the user
> > what profiles to connect so they just end up trusting the device,
> > alternatively we could make ConnectProfile to work also for LE so you
> > can provide a UUID and nothing else would be exposed, but note that
> > this guess on the AD may actually be wrong and the device may end up
> > malfunctioning.
> >
> > > In this case the user is considered being attacked, because he/she
> > > doesn't consciously interact with the System UI to connect to a nearby
> > > mouse/keyboard.
> > > My example doesn't have to be HOG. It just happens to be a profile
> > > which is attackable at the moment. My point is that, for applications
> > > it's always safest to turn off all internal profiles to avoid such
> > > incident. There is no use case where applications want to trigger
> > > internal profiles.
> > >
> > > Note 1: By "applications", I mean things like Android apps or
> > > JavaScript apps which are not considered System's Bluetooth UI.
> >
> > Well that doesn't make my point moot, let's say we do enable
> > connecting by UUID and the application try to connect HoG, it could be
> > a malicious application trying to eavesdrop what the user is typing,
> > so this problem of malicious goes both ways Im afraid, besides the
> > performance penalty that one would have if we need to transport HID
> > over D-Bus.
> If an application handles HOG, there will be nothing to eavesdrop
> because that application shouldn't have an access to UHID in the first
> place. If that malicious application had UHID access, that is already
> a problem to begin with regardless of whether there is Bluetooth or
> not. The security of this is handled above the Bluetooth layer. The
> operating system that uses this feature is responsible for higher
> layer security. For operating systems that don't need it I am okay
> with adding an option to disable this feature altogether. But I can
> see that there are systems that need it and I am not convinced that a
> general purpose Bluetooth stack should not support it.

All a malicious application has to do is to subscribe to notification
of HoG characteristic, then any input generate by the device would be
notified back to the application and that doesn't matter if uHID is
accessible or not the application may not even forward the events to
the system, now perhaps you are imagining that applications don't have
direct access to the attribute objects but that would be system
specific which is rather tricky to define.

> >
> > More applications could be involved and then this all becomes a mess
> > if they have to fight over AllowInternalProfiles, so instead of using
> > a theoretical example we better find real apps and devices where
> > conflicts happens and work out case by case, adding ConnectProfile
> > should actually fix most of them if there is a single profile involved
> > by we could also thing about an alternative to connect multiples.
> > There is also the possibility of exposing the btd_service as objects,
> > I've actually had this implemented for the car industry, that way
> > AutoConnect property could actually be controlled on a per service
> > basis instead of having just one switch for everything.
> To be clear, applications do not have direct access to
> AllowInternalProfiles. The higher layer operating system controls it.
> This is just the same case as the org.bluez.Adapter1.Powered property
> and many other examples where applications are not expected to have
> direct control of. Therefore there should be no problem of many things
> fighting over it if used correctly, just like many other properties.
> Again, I am okay with adding an option to disable this for operating
> systems that do not want it.

I see, though you didn't comment on the idea of controlling this on a
per-service basis, not just have everything disabled with
AllowInternalProfiles, note though that there are some profiles we
can't really disable like GAP and GATT as that involves things that
bt_gatt_client itself does need to access in order to work properly.

You can find the service_api commits in here:

https://github.com/Vudentz/BlueZ/commits/service_api

It does allow to control both the auto_connect logic as well a block:

https://github.com/Vudentz/BlueZ/commit/9bd6dce59fe9978b3bf415fe74f89d72254b8075
https://github.com/Vudentz/BlueZ/commit/42a7e479d5beb641a3d94f724a2df60db0f8221c

> Note: I have been using the term "operating system" to refer to high
> level components rather than the kernel.
>
>
> >
> > > >
> > > > Note that we do allow external profiles to be registered with use of:
> > > >
> > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/profile-api.txt
> > > >
> > > > And for GATT:
> > > >
> > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/gatt-api.txt#n366
> > > >
> > > > We could perhaps make the assumption that once an application
> > > > registers itself as supporting a given profile we check if against a
> > > > blacklist of profiles that may have security implications, which
> > > > perhaps could be defined via main.conf or some other file, if that is
> > > > not the case the internal profile can be disabled and the D-Bus object
> > > > would be accessible over D-Bus. Also note that we do offer clients the
> > > > ability to have exclusive access with AcquireWrite and AcquireNotify.
> > > >
> > > > > Therefore I think that this approach, although more complex, has
> > > > > longer lasting benefits. Let me know if you have any objection to
> > > > > having such a feature.
> > > > >
> > > > >
> > > > > On Mon, Jul 13, 2020 at 1:35 PM Luiz Augusto von Dentz
> > > > > <luiz.dentz@gmail.com> wrote:
> > > > > >
> > > > > > Hi Sonny,
> > > > > >
> > > > > > On Mon, Jul 13, 2020 at 1:18 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > > > > > >
> > > > > > > This patch series adds a mechanism for clients to choose whether to
> > > > > > > enable BlueZ internal profiles (e.g. A2DP, Battery) for specific
> > > > > > > devices.
> > > > > > >
> > > > > > > The motivation behind this feature is that some applications (e.g. Web
> > > > > > > Bluetooth or Android apps) need to have control over all remove GATT
> > > > > > > services, like Battery service. With "battery" plugin being enabled on
> > > > > > > BlueZ, it becomes not possible for those apps to work properly because
> > > > > > > BlueZ "hides" the Battery-related attributes from its GATT Client API.
> > > > > > > Disabling the "battery" plugin won't solve the problem either, since we
> > > > > > > do also need to enable the plugin so that we can use org.bluez.Battery1
> > > > > > > API.
> > > > > > >
> > > > > > > The solution that we propose is that clients can choose whether to
> > > > > > > enable internal profiles for each device. Clients know when to enable
> > > > > > > internal profiles (such as when a user chooses to pair/connect via a UI)
> > > > > > > and when to disable internal profiles (such as when the connection is
> > > > > > > initiated by a generic application).
> > > > > >
> > > > > > I wonder if it is not better to just have a flag indicating if the
> > > > > > profile shall claim exclusive access (such as GAP and GATT services),
> > > > > > so profiles that don't set that will have the services exposed so for
> > > > > > battery we can probably just have it exposed by default since it
> > > > > > doesn't appear to would be any conflicts on having it exposed.
> > > > > >
> > > > > > > Sonny Sasaka (3):
> > > > > > >   doc: Add "AllowInternalProfiles" property to org.bluez.Device1
> > > > > > >   device: Add "AllowInternalProfiles" property to org.bluez.Device1
> > > > > > >   client: Add set-allow-internal-profiles command
> > > > > > >
> > > > > > >  client/main.c      | 38 ++++++++++++++++++
> > > > > > >  doc/device-api.txt | 13 +++++++
> > > > > > >  src/device.c       | 96 ++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > >  src/hcid.h         |  2 +
> > > > > > >  src/main.c         | 10 +++++
> > > > > > >  src/main.conf      |  4 ++
> > > > > > >  6 files changed, 163 insertions(+)
> > > > > > >
> > > > > > > --
> > > > > > > 2.26.2
> > > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Luiz Augusto von Dentz
> > > >
> > > >
> > > >
> > > > --
> > > > Luiz Augusto von Dentz
> >
> >
> >
> > --
> > Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 0/3] Per-device option to enable/disable internal profiles

[Sonny Sasaka]

On Wed, Jul 15, 2020 at 9:25 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Sonny,
>
> On Tue, Jul 14, 2020 at 1:55 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> >
> > Hi Luiz,
> >
> > On Mon, Jul 13, 2020 at 10:28 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > Hi Sonny,
> > >
> > > On Mon, Jul 13, 2020 at 4:48 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > > >
> > > > Hi Luiz,
> > > >
> > > > On Mon, Jul 13, 2020 at 3:59 PM Luiz Augusto von Dentz
> > > > <luiz.dentz@gmail.com> wrote:
> > > > >
> > > > > Hi Sonny,
> > > > >
> > > > > On Mon, Jul 13, 2020 at 3:04 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > > > > >
> > > > > > Hi Luiz,
> > > > > >
> > > > > > I considered having such an approach that gives exception to some
> > > > > > profile to not claim exclusive access. However, I think that this
> > > > > > approach has a drawback that it can only be guaranteed to work
> > > > > > correctly for profiles that contain only read-only attributes. Any
> > > > > > profile that contains writable attributes, naturally, cannot be
> > > > > > guaranteed to always work correctly (as is the case with the Battery
> > > > > > profile). Therefore, the usefulness of that feature will be very
> > > > > > limited.
> > > > > >
> > > > > > I also considered the benefits of the AllowInternalProfiles approach:
> > > > > > * Applications can also have control over any profile, not just
> > > > > > Battery profile. For example, if in the future BlueZ has more internal
> > > > > > profiles, like (Blood Pressure Profile or any other profile that may
> > > > > > contain writable attributes), we can guarantee that applications can
> > > > > > still opt to have access to that profile, without relying on a profile
> > > > > > being "safe" to be shared by both BlueZ's internal and external
> > > > > > handlers.
> > > > > > * This has an added security benefit: applications which operate on a
> > > > > > specific GATT profile will not unintentionally activate internal
> > > > > > profiles such as HOG (which is able to hijack input control of the
> > > > > > host). This is the correct and expected behavior of Android apps that
> > > > > > connect over GATT and get access to a GATT profile.
> > > > >
> > > > > Not sure I follow these arguments, it seems AllowInternalProfiles may
> > > > > actually enable hijacking the profiles so I wonder if you got this
> > > > > backwards as we can't let things like HoG be controlled by
> > > > > applications directly it would be too easy to implement something like
> > > > > a keylogger, or perhaps you are suggesting that there is another layer
> > > > > for implementing the profiles? Note that it is intended that plugins
> > > > > shall be used for profiles that need to be integrated system wide,
> > > > > D-Bus interface shall be restricted to only application specific
> > > > > profiles.
> > > >
> > > > I think you misunderstood my point about HOG hijacking. Consider the
> > > > following case:
> > > > 1. A legit application (not System UI) on a host computer scans and
> > > > connects to a nearby peer. It makes a guess about the peer device
> > > > based on its advertising data. It intends to operate on a specific
> > > > GATT profile (not necessarily Battery).
> > > > 2. The peer device turns out to be malicious. It runs HOG GATT server
> > > > and triggers the host's HOG profile to be active.
> > > > 3. The malicious peer device's HOG GATT server can now maliciously
> > > > make mouse movements or enter keystrokes to the host.
> > >
> > > I'm not sure how you would like to prevent that, we could in theory
> > > attempt to authorize every single profile before connecting, just like
> > > it is done for legacy, but Im sure system would not be asking the user
> > > what profiles to connect so they just end up trusting the device,
> > > alternatively we could make ConnectProfile to work also for LE so you
> > > can provide a UUID and nothing else would be exposed, but note that
> > > this guess on the AD may actually be wrong and the device may end up
> > > malfunctioning.
> > >
> > > > In this case the user is considered being attacked, because he/she
> > > > doesn't consciously interact with the System UI to connect to a nearby
> > > > mouse/keyboard.
> > > > My example doesn't have to be HOG. It just happens to be a profile
> > > > which is attackable at the moment. My point is that, for applications
> > > > it's always safest to turn off all internal profiles to avoid such
> > > > incident. There is no use case where applications want to trigger
> > > > internal profiles.
> > > >
> > > > Note 1: By "applications", I mean things like Android apps or
> > > > JavaScript apps which are not considered System's Bluetooth UI.
> > >
> > > Well that doesn't make my point moot, let's say we do enable
> > > connecting by UUID and the application try to connect HoG, it could be
> > > a malicious application trying to eavesdrop what the user is typing,
> > > so this problem of malicious goes both ways Im afraid, besides the
> > > performance penalty that one would have if we need to transport HID
> > > over D-Bus.
> > If an application handles HOG, there will be nothing to eavesdrop
> > because that application shouldn't have an access to UHID in the first
> > place. If that malicious application had UHID access, that is already
> > a problem to begin with regardless of whether there is Bluetooth or
> > not. The security of this is handled above the Bluetooth layer. The
> > operating system that uses this feature is responsible for higher
> > layer security. For operating systems that don't need it I am okay
> > with adding an option to disable this feature altogether. But I can
> > see that there are systems that need it and I am not convinced that a
> > general purpose Bluetooth stack should not support it.
>
> All a malicious application has to do is to subscribe to notification
> of HoG characteristic, then any input generate by the device would be
> notified back to the application and that doesn't matter if uHID is
> accessible or not the application may not even forward the events to
> the system, now perhaps you are imagining that applications don't have
> direct access to the attribute objects but that would be system
> specific which is rather tricky to define.
When the HOG-related GATT object is available on D-Bus, that means the
internal HOG profile is not enabled in the first place, so there is
nothing to sniff anyway. Furthermore, the higher layer operating
system is also responsible to prevent this if it chooses to disable
BlueZ's internal HOG profile handler. If an operating system thinks
that it is safer to always enable internal HOG they can do just that,
as I mentioned I'm willing to add an option to always enable all
internal profiles.

>
> > >
> > > More applications could be involved and then this all becomes a mess
> > > if they have to fight over AllowInternalProfiles, so instead of using
> > > a theoretical example we better find real apps and devices where
> > > conflicts happens and work out case by case, adding ConnectProfile
> > > should actually fix most of them if there is a single profile involved
> > > by we could also thing about an alternative to connect multiples.
> > > There is also the possibility of exposing the btd_service as objects,
> > > I've actually had this implemented for the car industry, that way
> > > AutoConnect property could actually be controlled on a per service
> > > basis instead of having just one switch for everything.
> > To be clear, applications do not have direct access to
> > AllowInternalProfiles. The higher layer operating system controls it.
> > This is just the same case as the org.bluez.Adapter1.Powered property
> > and many other examples where applications are not expected to have
> > direct control of. Therefore there should be no problem of many things
> > fighting over it if used correctly, just like many other properties.
> > Again, I am okay with adding an option to disable this for operating
> > systems that do not want it.
>
> I see, though you didn't comment on the idea of controlling this on a
> per-service basis, not just have everything disabled with
> AllowInternalProfiles, note though that there are some profiles we
> can't really disable like GAP and GATT as that involves things that
> bt_gatt_client itself does need to access in order to work properly.
Right, I missed that GAP should be an exception that it cannot be
disabled, I should've added that exception in my code.

However, it seems that you still don't want a single switch to disable
all internal profiles (even with GAP exception). I'm willing to modify
this feature to be a blocklist of profiles per device (say
BlockedProfiles property on Device1 interface), and this feature can
be disabled altogether (all profiles always enabled) for operating
systems that do not want it.

That idea is also similar to your service_api branch, so I will also
try to port your service_api branch on the master branch. I will test
this and I am okay with using this if it serves our purposes.
I do have a question, though: With this API design, the service
objects are not exposed until a remote profile is detected, and
sometimes a profile is not detected until connection takes place (if
the profile UUID is not in the advertisement). So, how does the BlueZ
client block certain profiles/services before connection takes place?
We can't wait until connection takes place because we already know
that we don't want certain profiles, and if we block a profile after
connection takes place would that work properly? I guess we still need
a way to block certain profiles in the Device1 API, and the blocked
profiles also need to be stored in store_device_info.

>
> You can find the service_api commits in here:
>
> https://github.com/Vudentz/BlueZ/commits/service_api
>
> It does allow to control both the auto_connect logic as well a block:
>
> https://github.com/Vudentz/BlueZ/commit/9bd6dce59fe9978b3bf415fe74f89d72254b8075
> https://github.com/Vudentz/BlueZ/commit/42a7e479d5beb641a3d94f724a2df60db0f8221c
>
> > Note: I have been using the term "operating system" to refer to high
> > level components rather than the kernel.
> >
> >
> > >
> > > > >
> > > > > Note that we do allow external profiles to be registered with use of:
> > > > >
> > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/profile-api.txt
> > > > >
> > > > > And for GATT:
> > > > >
> > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/gatt-api.txt#n366
> > > > >
> > > > > We could perhaps make the assumption that once an application
> > > > > registers itself as supporting a given profile we check if against a
> > > > > blacklist of profiles that may have security implications, which
> > > > > perhaps could be defined via main.conf or some other file, if that is
> > > > > not the case the internal profile can be disabled and the D-Bus object
> > > > > would be accessible over D-Bus. Also note that we do offer clients the
> > > > > ability to have exclusive access with AcquireWrite and AcquireNotify.
> > > > >
> > > > > > Therefore I think that this approach, although more complex, has
> > > > > > longer lasting benefits. Let me know if you have any objection to
> > > > > > having such a feature.
> > > > > >
> > > > > >
> > > > > > On Mon, Jul 13, 2020 at 1:35 PM Luiz Augusto von Dentz
> > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > >
> > > > > > > Hi Sonny,
> > > > > > >
> > > > > > > On Mon, Jul 13, 2020 at 1:18 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > > > > > > >
> > > > > > > > This patch series adds a mechanism for clients to choose whether to
> > > > > > > > enable BlueZ internal profiles (e.g. A2DP, Battery) for specific
> > > > > > > > devices.
> > > > > > > >
> > > > > > > > The motivation behind this feature is that some applications (e.g. Web
> > > > > > > > Bluetooth or Android apps) need to have control over all remove GATT
> > > > > > > > services, like Battery service. With "battery" plugin being enabled on
> > > > > > > > BlueZ, it becomes not possible for those apps to work properly because
> > > > > > > > BlueZ "hides" the Battery-related attributes from its GATT Client API.
> > > > > > > > Disabling the "battery" plugin won't solve the problem either, since we
> > > > > > > > do also need to enable the plugin so that we can use org.bluez.Battery1
> > > > > > > > API.
> > > > > > > >
> > > > > > > > The solution that we propose is that clients can choose whether to
> > > > > > > > enable internal profiles for each device. Clients know when to enable
> > > > > > > > internal profiles (such as when a user chooses to pair/connect via a UI)
> > > > > > > > and when to disable internal profiles (such as when the connection is
> > > > > > > > initiated by a generic application).
> > > > > > >
> > > > > > > I wonder if it is not better to just have a flag indicating if the
> > > > > > > profile shall claim exclusive access (such as GAP and GATT services),
> > > > > > > so profiles that don't set that will have the services exposed so for
> > > > > > > battery we can probably just have it exposed by default since it
> > > > > > > doesn't appear to would be any conflicts on having it exposed.
> > > > > > >
> > > > > > > > Sonny Sasaka (3):
> > > > > > > >   doc: Add "AllowInternalProfiles" property to org.bluez.Device1
> > > > > > > >   device: Add "AllowInternalProfiles" property to org.bluez.Device1
> > > > > > > >   client: Add set-allow-internal-profiles command
> > > > > > > >
> > > > > > > >  client/main.c      | 38 ++++++++++++++++++
> > > > > > > >  doc/device-api.txt | 13 +++++++
> > > > > > > >  src/device.c       | 96 ++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > > >  src/hcid.h         |  2 +
> > > > > > > >  src/main.c         | 10 +++++
> > > > > > > >  src/main.conf      |  4 ++
> > > > > > > >  6 files changed, 163 insertions(+)
> > > > > > > >
> > > > > > > > --
> > > > > > > > 2.26.2
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Luiz Augusto von Dentz
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Luiz Augusto von Dentz
> > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz

Re: [PATCH BlueZ 0/3] Per-device option to enable/disable internal profiles

[Luiz Augusto von Dentz]

Hi Sonny,

On Wed, Jul 15, 2020 at 1:13 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
>
> On Wed, Jul 15, 2020 at 9:25 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Sonny,
> >
> > On Tue, Jul 14, 2020 at 1:55 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > >
> > > Hi Luiz,
> > >
> > > On Mon, Jul 13, 2020 at 10:28 PM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > >
> > > > Hi Sonny,
> > > >
> > > > On Mon, Jul 13, 2020 at 4:48 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > > > >
> > > > > Hi Luiz,
> > > > >
> > > > > On Mon, Jul 13, 2020 at 3:59 PM Luiz Augusto von Dentz
> > > > > <luiz.dentz@gmail.com> wrote:
> > > > > >
> > > > > > Hi Sonny,
> > > > > >
> > > > > > On Mon, Jul 13, 2020 at 3:04 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > > > > > >
> > > > > > > Hi Luiz,
> > > > > > >
> > > > > > > I considered having such an approach that gives exception to some
> > > > > > > profile to not claim exclusive access. However, I think that this
> > > > > > > approach has a drawback that it can only be guaranteed to work
> > > > > > > correctly for profiles that contain only read-only attributes. Any
> > > > > > > profile that contains writable attributes, naturally, cannot be
> > > > > > > guaranteed to always work correctly (as is the case with the Battery
> > > > > > > profile). Therefore, the usefulness of that feature will be very
> > > > > > > limited.
> > > > > > >
> > > > > > > I also considered the benefits of the AllowInternalProfiles approach:
> > > > > > > * Applications can also have control over any profile, not just
> > > > > > > Battery profile. For example, if in the future BlueZ has more internal
> > > > > > > profiles, like (Blood Pressure Profile or any other profile that may
> > > > > > > contain writable attributes), we can guarantee that applications can
> > > > > > > still opt to have access to that profile, without relying on a profile
> > > > > > > being "safe" to be shared by both BlueZ's internal and external
> > > > > > > handlers.
> > > > > > > * This has an added security benefit: applications which operate on a
> > > > > > > specific GATT profile will not unintentionally activate internal
> > > > > > > profiles such as HOG (which is able to hijack input control of the
> > > > > > > host). This is the correct and expected behavior of Android apps that
> > > > > > > connect over GATT and get access to a GATT profile.
> > > > > >
> > > > > > Not sure I follow these arguments, it seems AllowInternalProfiles may
> > > > > > actually enable hijacking the profiles so I wonder if you got this
> > > > > > backwards as we can't let things like HoG be controlled by
> > > > > > applications directly it would be too easy to implement something like
> > > > > > a keylogger, or perhaps you are suggesting that there is another layer
> > > > > > for implementing the profiles? Note that it is intended that plugins
> > > > > > shall be used for profiles that need to be integrated system wide,
> > > > > > D-Bus interface shall be restricted to only application specific
> > > > > > profiles.
> > > > >
> > > > > I think you misunderstood my point about HOG hijacking. Consider the
> > > > > following case:
> > > > > 1. A legit application (not System UI) on a host computer scans and
> > > > > connects to a nearby peer. It makes a guess about the peer device
> > > > > based on its advertising data. It intends to operate on a specific
> > > > > GATT profile (not necessarily Battery).
> > > > > 2. The peer device turns out to be malicious. It runs HOG GATT server
> > > > > and triggers the host's HOG profile to be active.
> > > > > 3. The malicious peer device's HOG GATT server can now maliciously
> > > > > make mouse movements or enter keystrokes to the host.
> > > >
> > > > I'm not sure how you would like to prevent that, we could in theory
> > > > attempt to authorize every single profile before connecting, just like
> > > > it is done for legacy, but Im sure system would not be asking the user
> > > > what profiles to connect so they just end up trusting the device,
> > > > alternatively we could make ConnectProfile to work also for LE so you
> > > > can provide a UUID and nothing else would be exposed, but note that
> > > > this guess on the AD may actually be wrong and the device may end up
> > > > malfunctioning.
> > > >
> > > > > In this case the user is considered being attacked, because he/she
> > > > > doesn't consciously interact with the System UI to connect to a nearby
> > > > > mouse/keyboard.
> > > > > My example doesn't have to be HOG. It just happens to be a profile
> > > > > which is attackable at the moment. My point is that, for applications
> > > > > it's always safest to turn off all internal profiles to avoid such
> > > > > incident. There is no use case where applications want to trigger
> > > > > internal profiles.
> > > > >
> > > > > Note 1: By "applications", I mean things like Android apps or
> > > > > JavaScript apps which are not considered System's Bluetooth UI.
> > > >
> > > > Well that doesn't make my point moot, let's say we do enable
> > > > connecting by UUID and the application try to connect HoG, it could be
> > > > a malicious application trying to eavesdrop what the user is typing,
> > > > so this problem of malicious goes both ways Im afraid, besides the
> > > > performance penalty that one would have if we need to transport HID
> > > > over D-Bus.
> > > If an application handles HOG, there will be nothing to eavesdrop
> > > because that application shouldn't have an access to UHID in the first
> > > place. If that malicious application had UHID access, that is already
> > > a problem to begin with regardless of whether there is Bluetooth or
> > > not. The security of this is handled above the Bluetooth layer. The
> > > operating system that uses this feature is responsible for higher
> > > layer security. For operating systems that don't need it I am okay
> > > with adding an option to disable this feature altogether. But I can
> > > see that there are systems that need it and I am not convinced that a
> > > general purpose Bluetooth stack should not support it.
> >
> > All a malicious application has to do is to subscribe to notification
> > of HoG characteristic, then any input generate by the device would be
> > notified back to the application and that doesn't matter if uHID is
> > accessible or not the application may not even forward the events to
> > the system, now perhaps you are imagining that applications don't have
> > direct access to the attribute objects but that would be system
> > specific which is rather tricky to define.
> When the HOG-related GATT object is available on D-Bus, that means the
> internal HOG profile is not enabled in the first place, so there is
> nothing to sniff anyway. Furthermore, the higher layer operating
> system is also responsible to prevent this if it chooses to disable
> BlueZ's internal HOG profile handler. If an operating system thinks
> that it is safer to always enable internal HOG they can do just that,
> as I mentioned I'm willing to add an option to always enable all
> internal profiles.
>
> >
> > > >
> > > > More applications could be involved and then this all becomes a mess
> > > > if they have to fight over AllowInternalProfiles, so instead of using
> > > > a theoretical example we better find real apps and devices where
> > > > conflicts happens and work out case by case, adding ConnectProfile
> > > > should actually fix most of them if there is a single profile involved
> > > > by we could also thing about an alternative to connect multiples.
> > > > There is also the possibility of exposing the btd_service as objects,
> > > > I've actually had this implemented for the car industry, that way
> > > > AutoConnect property could actually be controlled on a per service
> > > > basis instead of having just one switch for everything.
> > > To be clear, applications do not have direct access to
> > > AllowInternalProfiles. The higher layer operating system controls it.
> > > This is just the same case as the org.bluez.Adapter1.Powered property
> > > and many other examples where applications are not expected to have
> > > direct control of. Therefore there should be no problem of many things
> > > fighting over it if used correctly, just like many other properties.
> > > Again, I am okay with adding an option to disable this for operating
> > > systems that do not want it.
> >
> > I see, though you didn't comment on the idea of controlling this on a
> > per-service basis, not just have everything disabled with
> > AllowInternalProfiles, note though that there are some profiles we
> > can't really disable like GAP and GATT as that involves things that
> > bt_gatt_client itself does need to access in order to work properly.
> Right, I missed that GAP should be an exception that it cannot be
> disabled, I should've added that exception in my code.
>
> However, it seems that you still don't want a single switch to disable
> all internal profiles (even with GAP exception). I'm willing to modify
> this feature to be a blocklist of profiles per device (say
> BlockedProfiles property on Device1 interface), and this feature can
> be disabled altogether (all profiles always enabled) for operating
> systems that do not want it.
>
> That idea is also similar to your service_api branch, so I will also
> try to port your service_api branch on the master branch. I will test
> this and I am okay with using this if it serves our purposes.
> I do have a question, though: With this API design, the service
> objects are not exposed until a remote profile is detected, and
> sometimes a profile is not detected until connection takes place (if
> the profile UUID is not in the advertisement). So, how does the BlueZ
> client block certain profiles/services before connection takes place?
> We can't wait until connection takes place because we already know
> that we don't want certain profiles, and if we block a profile after
> connection takes place would that work properly? I guess we still need
> a way to block certain profiles in the Device1 API, and the blocked
> profiles also need to be stored in store_device_info.

That is a good point, but that would be a problem only for the very
first time you connect since after that the blocked state should be
recovered, we could however work out an API where one can enter the
profiles to connect and to block but usually depends on the services
being resolved so I wonder it wouldn't just be enough to block the
service discovery and in the event the internal profile has already
been connected it would just disconnect, the objects would only be
exported after you had it blocked so that means there should be any
race, but perhaps you want a dedicate Connect method where one can
enumerate the profiles to auto-connect and to block, note though that
it may not be possible to indicate a valid list in case the services
have not been resolved.

> >
> > You can find the service_api commits in here:
> >
> > https://github.com/Vudentz/BlueZ/commits/service_api
> >
> > It does allow to control both the auto_connect logic as well a block:
> >
> > https://github.com/Vudentz/BlueZ/commit/9bd6dce59fe9978b3bf415fe74f89d72254b8075
> > https://github.com/Vudentz/BlueZ/commit/42a7e479d5beb641a3d94f724a2df60db0f8221c
> >
> > > Note: I have been using the term "operating system" to refer to high
> > > level components rather than the kernel.
> > >
> > >
> > > >
> > > > > >
> > > > > > Note that we do allow external profiles to be registered with use of:
> > > > > >
> > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/profile-api.txt
> > > > > >
> > > > > > And for GATT:
> > > > > >
> > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/gatt-api.txt#n366
> > > > > >
> > > > > > We could perhaps make the assumption that once an application
> > > > > > registers itself as supporting a given profile we check if against a
> > > > > > blacklist of profiles that may have security implications, which
> > > > > > perhaps could be defined via main.conf or some other file, if that is
> > > > > > not the case the internal profile can be disabled and the D-Bus object
> > > > > > would be accessible over D-Bus. Also note that we do offer clients the
> > > > > > ability to have exclusive access with AcquireWrite and AcquireNotify.
> > > > > >
> > > > > > > Therefore I think that this approach, although more complex, has
> > > > > > > longer lasting benefits. Let me know if you have any objection to
> > > > > > > having such a feature.
> > > > > > >
> > > > > > >
> > > > > > > On Mon, Jul 13, 2020 at 1:35 PM Luiz Augusto von Dentz
> > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > >
> > > > > > > > Hi Sonny,
> > > > > > > >
> > > > > > > > On Mon, Jul 13, 2020 at 1:18 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > > > > > > > >
> > > > > > > > > This patch series adds a mechanism for clients to choose whether to
> > > > > > > > > enable BlueZ internal profiles (e.g. A2DP, Battery) for specific
> > > > > > > > > devices.
> > > > > > > > >
> > > > > > > > > The motivation behind this feature is that some applications (e.g. Web
> > > > > > > > > Bluetooth or Android apps) need to have control over all remove GATT
> > > > > > > > > services, like Battery service. With "battery" plugin being enabled on
> > > > > > > > > BlueZ, it becomes not possible for those apps to work properly because
> > > > > > > > > BlueZ "hides" the Battery-related attributes from its GATT Client API.
> > > > > > > > > Disabling the "battery" plugin won't solve the problem either, since we
> > > > > > > > > do also need to enable the plugin so that we can use org.bluez.Battery1
> > > > > > > > > API.
> > > > > > > > >
> > > > > > > > > The solution that we propose is that clients can choose whether to
> > > > > > > > > enable internal profiles for each device. Clients know when to enable
> > > > > > > > > internal profiles (such as when a user chooses to pair/connect via a UI)
> > > > > > > > > and when to disable internal profiles (such as when the connection is
> > > > > > > > > initiated by a generic application).
> > > > > > > >
> > > > > > > > I wonder if it is not better to just have a flag indicating if the
> > > > > > > > profile shall claim exclusive access (such as GAP and GATT services),
> > > > > > > > so profiles that don't set that will have the services exposed so for
> > > > > > > > battery we can probably just have it exposed by default since it
> > > > > > > > doesn't appear to would be any conflicts on having it exposed.
> > > > > > > >
> > > > > > > > > Sonny Sasaka (3):
> > > > > > > > >   doc: Add "AllowInternalProfiles" property to org.bluez.Device1
> > > > > > > > >   device: Add "AllowInternalProfiles" property to org.bluez.Device1
> > > > > > > > >   client: Add set-allow-internal-profiles command
> > > > > > > > >
> > > > > > > > >  client/main.c      | 38 ++++++++++++++++++
> > > > > > > > >  doc/device-api.txt | 13 +++++++
> > > > > > > > >  src/device.c       | 96 ++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > > > >  src/hcid.h         |  2 +
> > > > > > > > >  src/main.c         | 10 +++++
> > > > > > > > >  src/main.conf      |  4 ++
> > > > > > > > >  6 files changed, 163 insertions(+)
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > 2.26.2
> > > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > Luiz Augusto von Dentz
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Luiz Augusto von Dentz
> > > >
> > > >
> > > >
> > > > --
> > > > Luiz Augusto von Dentz
> >
> >
> >
> > --
> > Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 0/3] Per-device option to enable/disable internal profiles

[Sonny Sasaka]

Hi Luiz,

On Wed, Jul 15, 2020 at 2:09 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Sonny,
>
> On Wed, Jul 15, 2020 at 1:13 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> >
> > On Wed, Jul 15, 2020 at 9:25 AM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > Hi Sonny,
> > >
> > > On Tue, Jul 14, 2020 at 1:55 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > > >
> > > > Hi Luiz,
> > > >
> > > > On Mon, Jul 13, 2020 at 10:28 PM Luiz Augusto von Dentz
> > > > <luiz.dentz@gmail.com> wrote:
> > > > >
> > > > > Hi Sonny,
> > > > >
> > > > > On Mon, Jul 13, 2020 at 4:48 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > > > > >
> > > > > > Hi Luiz,
> > > > > >
> > > > > > On Mon, Jul 13, 2020 at 3:59 PM Luiz Augusto von Dentz
> > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > >
> > > > > > > Hi Sonny,
> > > > > > >
> > > > > > > On Mon, Jul 13, 2020 at 3:04 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > > > > > > >
> > > > > > > > Hi Luiz,
> > > > > > > >
> > > > > > > > I considered having such an approach that gives exception to some
> > > > > > > > profile to not claim exclusive access. However, I think that this
> > > > > > > > approach has a drawback that it can only be guaranteed to work
> > > > > > > > correctly for profiles that contain only read-only attributes. Any
> > > > > > > > profile that contains writable attributes, naturally, cannot be
> > > > > > > > guaranteed to always work correctly (as is the case with the Battery
> > > > > > > > profile). Therefore, the usefulness of that feature will be very
> > > > > > > > limited.
> > > > > > > >
> > > > > > > > I also considered the benefits of the AllowInternalProfiles approach:
> > > > > > > > * Applications can also have control over any profile, not just
> > > > > > > > Battery profile. For example, if in the future BlueZ has more internal
> > > > > > > > profiles, like (Blood Pressure Profile or any other profile that may
> > > > > > > > contain writable attributes), we can guarantee that applications can
> > > > > > > > still opt to have access to that profile, without relying on a profile
> > > > > > > > being "safe" to be shared by both BlueZ's internal and external
> > > > > > > > handlers.
> > > > > > > > * This has an added security benefit: applications which operate on a
> > > > > > > > specific GATT profile will not unintentionally activate internal
> > > > > > > > profiles such as HOG (which is able to hijack input control of the
> > > > > > > > host). This is the correct and expected behavior of Android apps that
> > > > > > > > connect over GATT and get access to a GATT profile.
> > > > > > >
> > > > > > > Not sure I follow these arguments, it seems AllowInternalProfiles may
> > > > > > > actually enable hijacking the profiles so I wonder if you got this
> > > > > > > backwards as we can't let things like HoG be controlled by
> > > > > > > applications directly it would be too easy to implement something like
> > > > > > > a keylogger, or perhaps you are suggesting that there is another layer
> > > > > > > for implementing the profiles? Note that it is intended that plugins
> > > > > > > shall be used for profiles that need to be integrated system wide,
> > > > > > > D-Bus interface shall be restricted to only application specific
> > > > > > > profiles.
> > > > > >
> > > > > > I think you misunderstood my point about HOG hijacking. Consider the
> > > > > > following case:
> > > > > > 1. A legit application (not System UI) on a host computer scans and
> > > > > > connects to a nearby peer. It makes a guess about the peer device
> > > > > > based on its advertising data. It intends to operate on a specific
> > > > > > GATT profile (not necessarily Battery).
> > > > > > 2. The peer device turns out to be malicious. It runs HOG GATT server
> > > > > > and triggers the host's HOG profile to be active.
> > > > > > 3. The malicious peer device's HOG GATT server can now maliciously
> > > > > > make mouse movements or enter keystrokes to the host.
> > > > >
> > > > > I'm not sure how you would like to prevent that, we could in theory
> > > > > attempt to authorize every single profile before connecting, just like
> > > > > it is done for legacy, but Im sure system would not be asking the user
> > > > > what profiles to connect so they just end up trusting the device,
> > > > > alternatively we could make ConnectProfile to work also for LE so you
> > > > > can provide a UUID and nothing else would be exposed, but note that
> > > > > this guess on the AD may actually be wrong and the device may end up
> > > > > malfunctioning.
> > > > >
> > > > > > In this case the user is considered being attacked, because he/she
> > > > > > doesn't consciously interact with the System UI to connect to a nearby
> > > > > > mouse/keyboard.
> > > > > > My example doesn't have to be HOG. It just happens to be a profile
> > > > > > which is attackable at the moment. My point is that, for applications
> > > > > > it's always safest to turn off all internal profiles to avoid such
> > > > > > incident. There is no use case where applications want to trigger
> > > > > > internal profiles.
> > > > > >
> > > > > > Note 1: By "applications", I mean things like Android apps or
> > > > > > JavaScript apps which are not considered System's Bluetooth UI.
> > > > >
> > > > > Well that doesn't make my point moot, let's say we do enable
> > > > > connecting by UUID and the application try to connect HoG, it could be
> > > > > a malicious application trying to eavesdrop what the user is typing,
> > > > > so this problem of malicious goes both ways Im afraid, besides the
> > > > > performance penalty that one would have if we need to transport HID
> > > > > over D-Bus.
> > > > If an application handles HOG, there will be nothing to eavesdrop
> > > > because that application shouldn't have an access to UHID in the first
> > > > place. If that malicious application had UHID access, that is already
> > > > a problem to begin with regardless of whether there is Bluetooth or
> > > > not. The security of this is handled above the Bluetooth layer. The
> > > > operating system that uses this feature is responsible for higher
> > > > layer security. For operating systems that don't need it I am okay
> > > > with adding an option to disable this feature altogether. But I can
> > > > see that there are systems that need it and I am not convinced that a
> > > > general purpose Bluetooth stack should not support it.
> > >
> > > All a malicious application has to do is to subscribe to notification
> > > of HoG characteristic, then any input generate by the device would be
> > > notified back to the application and that doesn't matter if uHID is
> > > accessible or not the application may not even forward the events to
> > > the system, now perhaps you are imagining that applications don't have
> > > direct access to the attribute objects but that would be system
> > > specific which is rather tricky to define.
> > When the HOG-related GATT object is available on D-Bus, that means the
> > internal HOG profile is not enabled in the first place, so there is
> > nothing to sniff anyway. Furthermore, the higher layer operating
> > system is also responsible to prevent this if it chooses to disable
> > BlueZ's internal HOG profile handler. If an operating system thinks
> > that it is safer to always enable internal HOG they can do just that,
> > as I mentioned I'm willing to add an option to always enable all
> > internal profiles.
> >
> > >
> > > > >
> > > > > More applications could be involved and then this all becomes a mess
> > > > > if they have to fight over AllowInternalProfiles, so instead of using
> > > > > a theoretical example we better find real apps and devices where
> > > > > conflicts happens and work out case by case, adding ConnectProfile
> > > > > should actually fix most of them if there is a single profile involved
> > > > > by we could also thing about an alternative to connect multiples.
> > > > > There is also the possibility of exposing the btd_service as objects,
> > > > > I've actually had this implemented for the car industry, that way
> > > > > AutoConnect property could actually be controlled on a per service
> > > > > basis instead of having just one switch for everything.
> > > > To be clear, applications do not have direct access to
> > > > AllowInternalProfiles. The higher layer operating system controls it.
> > > > This is just the same case as the org.bluez.Adapter1.Powered property
> > > > and many other examples where applications are not expected to have
> > > > direct control of. Therefore there should be no problem of many things
> > > > fighting over it if used correctly, just like many other properties.
> > > > Again, I am okay with adding an option to disable this for operating
> > > > systems that do not want it.
> > >
> > > I see, though you didn't comment on the idea of controlling this on a
> > > per-service basis, not just have everything disabled with
> > > AllowInternalProfiles, note though that there are some profiles we
> > > can't really disable like GAP and GATT as that involves things that
> > > bt_gatt_client itself does need to access in order to work properly.
> > Right, I missed that GAP should be an exception that it cannot be
> > disabled, I should've added that exception in my code.
> >
> > However, it seems that you still don't want a single switch to disable
> > all internal profiles (even with GAP exception). I'm willing to modify
> > this feature to be a blocklist of profiles per device (say
> > BlockedProfiles property on Device1 interface), and this feature can
> > be disabled altogether (all profiles always enabled) for operating
> > systems that do not want it.
> >
> > That idea is also similar to your service_api branch, so I will also
> > try to port your service_api branch on the master branch. I will test
> > this and I am okay with using this if it serves our purposes.
> > I do have a question, though: With this API design, the service
> > objects are not exposed until a remote profile is detected, and
> > sometimes a profile is not detected until connection takes place (if
> > the profile UUID is not in the advertisement). So, how does the BlueZ
> > client block certain profiles/services before connection takes place?
> > We can't wait until connection takes place because we already know
> > that we don't want certain profiles, and if we block a profile after
> > connection takes place would that work properly? I guess we still need
> > a way to block certain profiles in the Device1 API, and the blocked
> > profiles also need to be stored in store_device_info.
>
> That is a good point, but that would be a problem only for the very
> first time you connect since after that the blocked state should be
> recovered, we could however work out an API where one can enter the
> profiles to connect and to block but usually depends on the services
> being resolved so I wonder it wouldn't just be enough to block the
> service discovery and in the event the internal profile has already
> been connected it would just disconnect, the objects would only be
> exported after you had it blocked so that means there should be any
> race, but perhaps you want a dedicate Connect method where one can
> enumerate the profiles to auto-connect and to block, note though that
> it may not be possible to indicate a valid list in case the services
> have not been resolved.
Thanks for the input. In the meantime, I am okay with resolving the
battery profile specifically by giving it an exception from being
claimed internally. I have sent another patch for this.
>
> > >
> > > You can find the service_api commits in here:
> > >
> > > https://github.com/Vudentz/BlueZ/commits/service_api
> > >
> > > It does allow to control both the auto_connect logic as well a block:
> > >
> > > https://github.com/Vudentz/BlueZ/commit/9bd6dce59fe9978b3bf415fe74f89d72254b8075
> > > https://github.com/Vudentz/BlueZ/commit/42a7e479d5beb641a3d94f724a2df60db0f8221c
> > >
> > > > Note: I have been using the term "operating system" to refer to high
> > > > level components rather than the kernel.
> > > >
> > > >
> > > > >
> > > > > > >
> > > > > > > Note that we do allow external profiles to be registered with use of:
> > > > > > >
> > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/profile-api.txt
> > > > > > >
> > > > > > > And for GATT:
> > > > > > >
> > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/gatt-api.txt#n366
> > > > > > >
> > > > > > > We could perhaps make the assumption that once an application
> > > > > > > registers itself as supporting a given profile we check if against a
> > > > > > > blacklist of profiles that may have security implications, which
> > > > > > > perhaps could be defined via main.conf or some other file, if that is
> > > > > > > not the case the internal profile can be disabled and the D-Bus object
> > > > > > > would be accessible over D-Bus. Also note that we do offer clients the
> > > > > > > ability to have exclusive access with AcquireWrite and AcquireNotify.
> > > > > > >
> > > > > > > > Therefore I think that this approach, although more complex, has
> > > > > > > > longer lasting benefits. Let me know if you have any objection to
> > > > > > > > having such a feature.
> > > > > > > >
> > > > > > > >
> > > > > > > > On Mon, Jul 13, 2020 at 1:35 PM Luiz Augusto von Dentz
> > > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > Hi Sonny,
> > > > > > > > >
> > > > > > > > > On Mon, Jul 13, 2020 at 1:18 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> > > > > > > > > >
> > > > > > > > > > This patch series adds a mechanism for clients to choose whether to
> > > > > > > > > > enable BlueZ internal profiles (e.g. A2DP, Battery) for specific
> > > > > > > > > > devices.
> > > > > > > > > >
> > > > > > > > > > The motivation behind this feature is that some applications (e.g. Web
> > > > > > > > > > Bluetooth or Android apps) need to have control over all remove GATT
> > > > > > > > > > services, like Battery service. With "battery" plugin being enabled on
> > > > > > > > > > BlueZ, it becomes not possible for those apps to work properly because
> > > > > > > > > > BlueZ "hides" the Battery-related attributes from its GATT Client API.
> > > > > > > > > > Disabling the "battery" plugin won't solve the problem either, since we
> > > > > > > > > > do also need to enable the plugin so that we can use org.bluez.Battery1
> > > > > > > > > > API.
> > > > > > > > > >
> > > > > > > > > > The solution that we propose is that clients can choose whether to
> > > > > > > > > > enable internal profiles for each device. Clients know when to enable
> > > > > > > > > > internal profiles (such as when a user chooses to pair/connect via a UI)
> > > > > > > > > > and when to disable internal profiles (such as when the connection is
> > > > > > > > > > initiated by a generic application).
> > > > > > > > >
> > > > > > > > > I wonder if it is not better to just have a flag indicating if the
> > > > > > > > > profile shall claim exclusive access (such as GAP and GATT services),
> > > > > > > > > so profiles that don't set that will have the services exposed so for
> > > > > > > > > battery we can probably just have it exposed by default since it
> > > > > > > > > doesn't appear to would be any conflicts on having it exposed.
> > > > > > > > >
> > > > > > > > > > Sonny Sasaka (3):
> > > > > > > > > >   doc: Add "AllowInternalProfiles" property to org.bluez.Device1
> > > > > > > > > >   device: Add "AllowInternalProfiles" property to org.bluez.Device1
> > > > > > > > > >   client: Add set-allow-internal-profiles command
> > > > > > > > > >
> > > > > > > > > >  client/main.c      | 38 ++++++++++++++++++
> > > > > > > > > >  doc/device-api.txt | 13 +++++++
> > > > > > > > > >  src/device.c       | 96 ++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > > > > >  src/hcid.h         |  2 +
> > > > > > > > > >  src/main.c         | 10 +++++
> > > > > > > > > >  src/main.conf      |  4 ++
> > > > > > > > > >  6 files changed, 163 insertions(+)
> > > > > > > > > >
> > > > > > > > > > --
> > > > > > > > > > 2.26.2
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > Luiz Augusto von Dentz
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Luiz Augusto von Dentz
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Luiz Augusto von Dentz
> > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz