[PATCH BlueZ v5 0/6] Support Sixaxis gamepad with classic bonded only

[PATCH BlueZ v5 0/6] Support Sixaxis gamepad with classic bonded only

[Ludovico de Nittis]

This series adds a new "CablePairing" property to allow us to
indentify devices that have been paired using a custom USB cable
cable method and that don't support the canonical bonding with
encryption. With that information, we can dynamically enforce
encryption to drastically reduce the attack surface, compared to just
disabling the "ClassicBondedOnly" property.

The "CablePairing" property is exposed via D-Bus to allow
clients to potentually show this information to end users.

As far as I can tell, starting the listening input server with
BT_IO_SEC_LOW and then bumping it in `hidp_add_connection()` should not
have any negative effect regarding the overall security. However,
please let me know if it turns out not being the case.

Addresses https://github.com/bluez/bluez/issues/1165

Changes in v2:
 - Start the listening input server with BT_IO_SEC_LOW only if we
   actually have a known sixaxis device

Changes in v3:
  - Change the property from being sixaxis specific to a generic
    "CablePairing"
  - Remove the manual validation of Sixaxis HID report descriptor
    because we already replace it with a pre-dermined SDP record
    in `sixaxis.c`

Changes in v4:
  - Instead of setting the server security level in adapter.c, we
    do it when registering the input devices, during probing

Changes in v5:
  - Fix `get_necessary_sec_level()` comment

Ludovico de Nittis (6):
  src: Add new CablePairing property
  client: Print CablePairing property
  sixaxis: Set CablePairing when pairing a Sixaxis with USB
  adapter: Add btd_adapter_has_cable_pairing_devices()
  input: Automatically use sec level low when using a cable paired
    device
  sixaxis: Set security level when adding a sixaxis device

 client/main.c            |  1 +
 doc/org.bluez.Device.rst |  7 +++++
 plugins/sixaxis.c        |  8 ++++-
 profiles/input/device.c  | 20 +++++++++++--
 profiles/input/manager.c |  3 +-
 profiles/input/server.c  | 63 ++++++++++++++++++++++++++++++++++++++--
 profiles/input/server.h  |  3 +-
 src/adapter.c            | 17 +++++++++++
 src/adapter.h            |  1 +
 src/device.c             | 40 +++++++++++++++++++++++++
 src/device.h             |  2 ++
 11 files changed, 157 insertions(+), 8 deletions(-)

-- 
2.49.0

[PATCH BlueZ v5 1/6] src: Add new CablePairing property

[Ludovico de Nittis]

This adds initial support for a new CablePairing property.
The property can be used for devices that are paired using a cable and
don't support the expected bonding (with pairing/encryption), for
example like the Sixaxis gamepads.
---
 doc/org.bluez.Device.rst |  7 +++++++
 src/device.c             | 40 ++++++++++++++++++++++++++++++++++++++++
 src/device.h             |  2 ++
 3 files changed, 49 insertions(+)

diff --git a/doc/org.bluez.Device.rst b/doc/org.bluez.Device.rst
index 13328249b..80501eddd 100644
--- a/doc/org.bluez.Device.rst
+++ b/doc/org.bluez.Device.rst
@@ -279,6 +279,13 @@ boolean LegacyPairing [readonly]
 	Bluetooth 2.1 (or newer) devices that have disabled Extended Inquiry
 	Response support.
 
+boolean CablePairing [readonly]
+```````````````````````````````
+
+	Set to true if the device was cable paired and it doesn't support the
+	canonical bonding with encryption, e.g. the Sixaxis gamepad.
+	If true, BlueZ will establish a connection without enforcing encryption.
+
 string Modalias [readonly, optional]
 ````````````````````````````````````
 
diff --git a/src/device.c b/src/device.c
index b82a905f9..123d44c14 100644
--- a/src/device.c
+++ b/src/device.c
@@ -239,6 +239,7 @@ struct btd_device {
 	GSList		*watches;		/* List of disconnect_data */
 	bool		temporary;
 	bool		connectable;
+	bool		cable_pairing;
 	unsigned int	disconn_timer;
 	unsigned int	discov_timer;
 	unsigned int	temporary_timer;	/* Temporary/disappear timer */
@@ -507,6 +508,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", "CablePairing",
+							device->cable_pairing);
+
 	if (device->wake_override != WAKE_FLAG_DEFAULT) {
 		g_key_file_set_boolean(key_file, "General", "WakeAllowed",
 				       device->wake_override ==
@@ -908,6 +912,11 @@ bool btd_device_is_trusted(struct btd_device *device)
 	return device->trusted;
 }
 
+bool device_is_cable_pairing(struct btd_device *device)
+{
+	return device->cable_pairing;
+}
+
 static gboolean dev_property_get_address(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
@@ -1153,6 +1162,17 @@ static gboolean dev_property_get_legacy(const GDBusPropertyTable *property,
 	return TRUE;
 }
 
+static gboolean dev_property_get_cable_pairing(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct btd_device *device = data;
+	dbus_bool_t val = device->cable_pairing;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &val);
+
+	return TRUE;
+}
+
 static gboolean dev_property_get_rssi(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
@@ -3483,6 +3503,7 @@ static const GDBusPropertyTable device_properties[] = {
 	{ "Trusted", "b", dev_property_get_trusted, dev_property_set_trusted },
 	{ "Blocked", "b", dev_property_get_blocked, dev_property_set_blocked },
 	{ "LegacyPairing", "b", dev_property_get_legacy },
+	{ "CablePairing", "b", dev_property_get_cable_pairing },
 	{ "RSSI", "n", dev_property_get_rssi, NULL, dev_property_exists_rssi },
 	{ "Connected", "b", dev_property_get_connected },
 	{ "UUIDs", "as", dev_property_get_uuids },
@@ -4062,6 +4083,9 @@ next:
 	if (blocked)
 		device_block(device, FALSE);
 
+	device->cable_pairing = g_key_file_get_boolean(key_file, "General",
+							"CablePairing", NULL);
+
 	/* Load device profile list */
 	uuids = g_key_file_get_string_list(key_file, "General", "Services",
 						NULL, NULL);
@@ -6416,6 +6440,22 @@ void device_set_legacy(struct btd_device *device, bool legacy)
 					DEVICE_INTERFACE, "LegacyPairing");
 }
 
+void device_set_cable_pairing(struct btd_device *device, bool cable_pairing)
+{
+	if (!device)
+		return;
+
+	if (device->cable_pairing == cable_pairing)
+		return;
+
+	DBG("setting cable pairing %d", cable_pairing);
+
+	device->cable_pairing = cable_pairing;
+
+	g_dbus_emit_property_changed(dbus_conn, device->path,
+					DEVICE_INTERFACE, "CablePairing");
+}
+
 void device_store_svc_chng_ccc(struct btd_device *device, uint8_t bdaddr_type,
 								uint16_t value)
 {
diff --git a/src/device.h b/src/device.h
index 2e4a9771d..a35bb1386 100644
--- a/src/device.h
+++ b/src/device.h
@@ -94,6 +94,7 @@ bool device_is_connectable(struct btd_device *device);
 bool device_is_paired(struct btd_device *device, uint8_t bdaddr_type);
 bool device_is_bonded(struct btd_device *device, uint8_t bdaddr_type);
 bool btd_device_is_trusted(struct btd_device *device);
+bool device_is_cable_pairing(struct btd_device *device);
 void device_set_paired(struct btd_device *dev, uint8_t bdaddr_type);
 void device_set_unpaired(struct btd_device *dev, uint8_t bdaddr_type);
 void btd_device_set_temporary(struct btd_device *device, bool temporary);
@@ -101,6 +102,7 @@ void btd_device_set_trusted(struct btd_device *device, gboolean trusted);
 void btd_device_set_connectable(struct btd_device *device, bool connectable);
 void device_set_bonded(struct btd_device *device, uint8_t bdaddr_type);
 void device_set_legacy(struct btd_device *device, bool legacy);
+void device_set_cable_pairing(struct btd_device *device, bool cable_pairing);
 void device_set_rssi_with_delta(struct btd_device *device, int8_t rssi,
 							int8_t delta_threshold);
 void device_set_rssi(struct btd_device *device, int8_t rssi);
-- 
2.49.0

RE: Support Sixaxis gamepad with classic bonded only

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]

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

Dear submitter,

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

---Test result---

Test Summary:
CheckPatch                    PENDING   0.27 seconds
GitLint                       PENDING   0.24 seconds
BuildEll                      PASS      21.34 seconds
BluezMake                     PASS      2888.91 seconds
MakeCheck                     PASS      20.90 seconds
MakeDistcheck                 PASS      207.84 seconds
CheckValgrind                 PASS      284.02 seconds
CheckSmatch                   PASS      312.39 seconds
bluezmakeextell               PASS      133.40 seconds
IncrementalBuild              PENDING   0.20 seconds
ScanBuild                     PASS      944.83 seconds

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

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

##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

[PATCH BlueZ v5 2/6] client: Print CablePairing property

[Ludovico de Nittis]

When using the `info` command, include the new CablePairing value.
---
 client/main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/client/main.c b/client/main.c
index 6039aa50c..a2d9d88bf 100644
--- a/client/main.c
+++ b/client/main.c
@@ -1705,6 +1705,7 @@ static void cmd_info(int argc, char *argv[])
 	print_property(proxy, "Connected");
 	print_property(proxy, "WakeAllowed");
 	print_property(proxy, "LegacyPairing");
+	print_property(proxy, "CablePairing");
 	print_uuids(proxy);
 	print_property(proxy, "Modalias");
 	print_property(proxy, "ManufacturerData");
-- 
2.49.0

[PATCH BlueZ v5 3/6] sixaxis: Set CablePairing when pairing a Sixaxis with USB

[Ludovico de Nittis]

Sixaxis gamepads don't support encryption. When doing the USB cable
pairing, set the CablePairing property to keep the connection to the
device unencrypted.
---
 plugins/sixaxis.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
index 3e69f1dd2..ea160c65d 100644
--- a/plugins/sixaxis.c
+++ b/plugins/sixaxis.c
@@ -296,10 +296,13 @@ static void agent_auth_cb(DBusError *derr, void *user_data)
 	remove_device = false;
 	btd_device_set_temporary(closure->device, false);
 
-	if (closure->type == CABLE_PAIRING_SIXAXIS)
+	if (closure->type == CABLE_PAIRING_SIXAXIS) {
 		btd_device_set_record(closure->device, HID_UUID,
 						 SIXAXIS_HID_SDP_RECORD);
 
+		device_set_cable_pairing(closure->device, true);
+	}
+
 	ba2str(&closure->bdaddr, device_addr);
 	ba2str(&central_bdaddr, central_addr);
 	ba2str(adapter_bdaddr, adapter_addr);
-- 
2.49.0

[PATCH BlueZ v5 4/6] adapter: Add btd_adapter_has_cable_pairing_devices()

[Ludovico de Nittis]

Add a function that can be used to know if any of the known devices have
the `CablePaired` property set.
---
 src/adapter.c | 17 +++++++++++++++++
 src/adapter.h |  1 +
 2 files changed, 18 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index c21b38095..fd425e6d2 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -412,6 +412,23 @@ uint16_t btd_adapter_get_index(struct btd_adapter *adapter)
 	return adapter->dev_id;
 }
 
+bool btd_adapter_has_cable_pairing_devices(struct btd_adapter *adapter)
+{
+	GSList *l;
+
+	if (!adapter)
+		return false;
+
+	for (l = adapter->devices; l; l = l->next) {
+		struct btd_device *device = l->data;
+
+		if (device_is_cable_pairing(device))
+			return true;
+	}
+
+	return false;
+}
+
 static gboolean process_auth_queue(gpointer user_data);
 
 static void dev_class_changed_callback(uint16_t index, uint16_t length,
diff --git a/src/adapter.h b/src/adapter.h
index 8dfbe762e..6b2bc28f6 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -30,6 +30,7 @@ struct queue;
 struct btd_adapter *btd_adapter_get_default(void);
 bool btd_adapter_is_default(struct btd_adapter *adapter);
 uint16_t btd_adapter_get_index(struct btd_adapter *adapter);
+bool btd_adapter_has_cable_pairing_devices(struct btd_adapter *adapter);
 
 typedef void (*adapter_cb) (struct btd_adapter *adapter, gpointer user_data);
 
-- 
2.49.0

[PATCH BlueZ v5 5/6] input: Automatically use sec level low when using a cable paired device

[Ludovico de Nittis]

BT_IO_SEC_LOW is the only way to allow devices that use cable pairing to
establish a connection.

This adds the ability to start the listening input server with
BT_IO_SEC_LOW to avoid breaking support for these devices, and then,
in `hidp_add_connection()`, we check if either `classic_bonded_only` was
disabled or if this device has `CablePairing`. If neither are true, we
bump the security back to BT_IO_SEC_MEDIUM, i.e. enforcing encryption.

This allows supporting these devices without having to change the
classic bonded only option.
---
 profiles/input/device.c  | 20 +++++++++++--
 profiles/input/manager.c |  3 +-
 profiles/input/server.c  | 63 ++++++++++++++++++++++++++++++++++++++--
 profiles/input/server.h  |  3 +-
 4 files changed, 82 insertions(+), 7 deletions(-)

diff --git a/profiles/input/device.c b/profiles/input/device.c
index 3627573e7..2fa4e0b82 100644
--- a/profiles/input/device.c
+++ b/profiles/input/device.c
@@ -47,6 +47,7 @@
 
 #include "device.h"
 #include "hidp_defs.h"
+#include "server.h"
 
 #define INPUT_INTERFACE "org.bluez.Input1"
 
@@ -1065,6 +1066,7 @@ static gboolean encrypt_notify(GIOChannel *io, GIOCondition condition,
 static int hidp_add_connection(struct input_device *idev)
 {
 	struct hidp_connadd_req *req;
+	bool cable_pairing;
 	GError *gerr = NULL;
 	int err;
 
@@ -1088,8 +1090,10 @@ static int hidp_add_connection(struct input_device *idev)
 	if (device_name_known(idev->device))
 		device_get_name(idev->device, req->name, sizeof(req->name));
 
+	cable_pairing = device_is_cable_pairing(idev->device);
+
 	/* Make sure the device is bonded if required */
-	if (classic_bonded_only && !input_device_bonded(idev)) {
+	if (!cable_pairing && classic_bonded_only && !input_device_bonded(idev)) {
 		error("Rejected connection from !bonded device %s", idev->path);
 		goto cleanup;
 	}
@@ -1098,7 +1102,9 @@ static int hidp_add_connection(struct input_device *idev)
 	/* Some platforms may choose to require encryption for all devices */
 	/* Note that this only matters for pre 2.1 devices as otherwise the */
 	/* device is encrypted by default by the lower layers */
-	if (classic_bonded_only || idev->type == BT_UHID_KEYBOARD) {
+	/* Don't enforce encryption for cable paired devices because they */
+	/* don't support it */
+	if (!cable_pairing && (classic_bonded_only || idev->type == BT_UHID_KEYBOARD)) {
 		if (!bt_io_set(idev->intr_io, &gerr,
 					BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
 					BT_IO_OPT_INVALID)) {
@@ -1546,6 +1552,16 @@ int input_device_register(struct btd_service *service)
 	btd_service_set_user_data(service, idev);
 	device_set_wake_support(device, true);
 
+	if (device_is_cable_pairing(device)) {
+		struct btd_adapter *adapter = device_get_adapter(device);
+		const bdaddr_t *adapter_bdaddr = btd_adapter_get_address(adapter);
+
+		DBG("This is a cable paired device, setting the listening input "
+			"server security level accordingly");
+
+		server_set_cable_pairing(adapter_bdaddr, true);
+	}
+
 	return 0;
 }
 
diff --git a/profiles/input/manager.c b/profiles/input/manager.c
index d1accc24f..95ca0a7ee 100644
--- a/profiles/input/manager.c
+++ b/profiles/input/manager.c
@@ -33,7 +33,8 @@
 
 static int hid_server_probe(struct btd_profile *p, struct btd_adapter *adapter)
 {
-	return server_start(btd_adapter_get_address(adapter));
+	return server_start(btd_adapter_get_address(adapter),
+				btd_adapter_has_cable_pairing_devices(adapter));
 }
 
 static void hid_server_remove(struct btd_profile *p,
diff --git a/profiles/input/server.c b/profiles/input/server.c
index 79cf08a66..3de2282f6 100644
--- a/profiles/input/server.c
+++ b/profiles/input/server.c
@@ -266,12 +266,23 @@ drop:
 	g_io_channel_shutdown(chan, TRUE, NULL);
 }
 
-int server_start(const bdaddr_t *src)
+static BtIOSecLevel get_necessary_sec_level(bool device_cable_pairing)
+{
+	/* Use lower security to allow the cable paired devices to connect. */
+	/* Unless classic bonded only mode is disabled, the security level */
+	/* will be bumped again for non cable paired devices in */
+	/* hidp_add_connection() */
+	if (device_cable_pairing)
+		return BT_IO_SEC_LOW;
+
+	return input_get_classic_bonded_only() ? BT_IO_SEC_MEDIUM : BT_IO_SEC_LOW;
+}
+
+int server_start(const bdaddr_t *src, bool device_sixaxis_cable_pairing)
 {
 	struct input_server *server;
 	GError *err = NULL;
-	BtIOSecLevel sec_level = input_get_classic_bonded_only() ?
-					BT_IO_SEC_MEDIUM : BT_IO_SEC_LOW;
+	const BtIOSecLevel sec_level = get_necessary_sec_level(device_sixaxis_cable_pairing);
 
 	server = g_new0(struct input_server, 1);
 	bacpy(&server->src, src);
@@ -308,6 +319,52 @@ int server_start(const bdaddr_t *src)
 	return 0;
 }
 
+int server_set_cable_pairing(const bdaddr_t *src, bool device_cable_pairing)
+{
+	struct input_server *server;
+	GSList *l;
+	BtIOSecLevel sec_level;
+	const BtIOSecLevel new_sec_level = get_necessary_sec_level(device_cable_pairing);
+	GError *err = NULL;
+
+	l = g_slist_find_custom(servers, src, server_cmp);
+	if (!l)
+		return -1;
+
+	server = l->data;
+
+	bt_io_get(server->ctrl, &err, BT_IO_OPT_SEC_LEVEL, &sec_level,
+				BT_IO_OPT_INVALID);
+	if (err) {
+		error("%s", err->message);
+		g_error_free(err);
+		return -1;
+	}
+
+	if (sec_level == new_sec_level) {
+		DBG("The listening input server is already using the expected security level");
+		return -1;
+	}
+
+	DBG("Applying the new security level to the listening input server");
+
+	if (!bt_io_set(server->ctrl, &err, BT_IO_OPT_SEC_LEVEL, new_sec_level,
+							BT_IO_OPT_INVALID)) {
+		error("bt_io_set(OPT_SEC_LEVEL): %s", err->message);
+		g_error_free(err);
+		return -1;
+	}
+
+	if (!bt_io_set(server->intr, &err, BT_IO_OPT_SEC_LEVEL, new_sec_level,
+						BT_IO_OPT_INVALID)) {
+		error("bt_io_set(OPT_SEC_LEVEL): %s", err->message);
+		g_error_free(err);
+		return -1;
+	}
+
+	return 0;
+}
+
 void server_stop(const bdaddr_t *src)
 {
 	struct input_server *server;
diff --git a/profiles/input/server.h b/profiles/input/server.h
index 50f4b6135..4ad82c10e 100644
--- a/profiles/input/server.h
+++ b/profiles/input/server.h
@@ -8,5 +8,6 @@
  *
  */
 
-int server_start(const bdaddr_t *src);
+int server_start(const bdaddr_t *src, bool device_cable_pairing);
+int server_set_cable_pairing(const bdaddr_t *src, bool device_cable_pairing);
 void server_stop(const bdaddr_t *src);
-- 
2.49.0

[PATCH BlueZ v5 6/6] sixaxis: Set security level when adding a sixaxis device

[Ludovico de Nittis]

When doing the cable pairing for a sixaxis, we may need to change the
listening input server security level.
This is because sixaxis gamepads can only work with the level
BT_IO_SEC_LOW.
---
 plugins/sixaxis.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
index ea160c65d..1fab8ae59 100644
--- a/plugins/sixaxis.c
+++ b/plugins/sixaxis.c
@@ -36,6 +36,7 @@
 #include "src/plugin.h"
 #include "src/log.h"
 #include "src/shared/util.h"
+#include "profiles/input/server.h"
 #include "profiles/input/sixaxis.h"
 
 struct authentication_closure {
@@ -301,6 +302,8 @@ static void agent_auth_cb(DBusError *derr, void *user_data)
 						 SIXAXIS_HID_SDP_RECORD);
 
 		device_set_cable_pairing(closure->device, true);
+
+		server_set_cable_pairing(adapter_bdaddr, true);
 	}
 
 	ba2str(&closure->bdaddr, device_addr);
-- 
2.49.0

Re: [PATCH BlueZ v5 0/6] Support Sixaxis gamepad with classic bonded only

[patchwork-bot+bluetooth]

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Thu, 24 Apr 2025 18:29:27 +0200 you wrote:
> This series adds a new "CablePairing" property to allow us to
> indentify devices that have been paired using a custom USB cable
> cable method and that don't support the canonical bonding with
> encryption. With that information, we can dynamically enforce
> encryption to drastically reduce the attack surface, compared to just
> disabling the "ClassicBondedOnly" property.
> 
> [...]

Here is the summary with links:
  - [BlueZ,v5,1/6] src: Add new CablePairing property
    (no matching commit)
  - [BlueZ,v5,2/6] client: Print CablePairing property
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=373146be299f
  - [BlueZ,v5,3/6] sixaxis: Set CablePairing when pairing a Sixaxis with USB
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=56516d6cc8d9
  - [BlueZ,v5,4/6] adapter: Add btd_adapter_has_cable_pairing_devices()
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c5dffe0ce2fe
  - [BlueZ,v5,5/6] input: Automatically use sec level low when using a cable paired device
    (no matching commit)
  - [BlueZ,v5,6/6] sixaxis: Set security level when adding a sixaxis device
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=08e327733161

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html