diff mbox series

[BlueZ,4/5] input: Start the server with sec low and bump it when making the connection

Message ID 20250421111251.108943-5-ludovico.denittis@collabora.com
State New
Headers show
Series Support Sixaxis gamepad with classic bonded only | expand

Commit Message

Ludovico de Nittis April 21, 2025, 11:12 a.m. UTC
BT_IO_SEC_LOW is the only way to allow Sixaxis devices to establish
a connection.

When BlueZ has been compiled with Sixaxis support enabled, we start the
listening input server with BT_IO_SEC_LOW to avoid breaking the Sixaxis
support.
Then, in `hidp_add_connection()`, we check if either
`classic_bonded_only` was disabled or if this device is a Sixaxis. If
neither are true, we bump the security back to BT_IO_SEC_MEDIUM, i.e.
enforcing encryption.

This allows supporting the Sixaxis gamepad without having to change the
classic bonded only option.
---
 profiles/input/device.c | 6 ++++--
 profiles/input/server.c | 7 +++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

Comments

Luiz Augusto von Dentz April 21, 2025, 2:18 p.m. UTC | #1
Hi Ludovico,

On Mon, Apr 21, 2025 at 7:14 AM Ludovico de Nittis
<ludovico.denittis@collabora.com> wrote:
>
> BT_IO_SEC_LOW is the only way to allow Sixaxis devices to establish
> a connection.
>
> When BlueZ has been compiled with Sixaxis support enabled, we start the
> listening input server with BT_IO_SEC_LOW to avoid breaking the Sixaxis
> support.
> Then, in `hidp_add_connection()`, we check if either
> `classic_bonded_only` was disabled or if this device is a Sixaxis. If
> neither are true, we bump the security back to BT_IO_SEC_MEDIUM, i.e.
> enforcing encryption.
>
> This allows supporting the Sixaxis gamepad without having to change the
> classic bonded only option.
> ---
>  profiles/input/device.c | 6 ++++--
>  profiles/input/server.c | 7 +++++++
>  2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/profiles/input/device.c b/profiles/input/device.c
> index 3627573e7..eb2fb5c8e 100644
> --- a/profiles/input/device.c
> +++ b/profiles/input/device.c
> @@ -1088,8 +1088,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));
>
> +       sixaxis_cable_pairing = device_is_sixaxis_cable_pairing(idev->device);
> +
>         /* Make sure the device is bonded if required */
> -       if (classic_bonded_only && !input_device_bonded(idev)) {
> +       if (classic_bonded_only && !sixaxis_cable_pairing && !input_device_bonded(idev)) {
>                 error("Rejected connection from !bonded device %s", idev->path);
>                 goto cleanup;
>         }
> @@ -1098,7 +1100,7 @@ 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) {
> +       if (!sixaxis_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)) {
> diff --git a/profiles/input/server.c b/profiles/input/server.c
> index 79cf08a66..73caeb076 100644
> --- a/profiles/input/server.c
> +++ b/profiles/input/server.c
> @@ -273,6 +273,13 @@ int server_start(const bdaddr_t *src)
>         BtIOSecLevel sec_level = input_get_classic_bonded_only() ?
>                                         BT_IO_SEC_MEDIUM : BT_IO_SEC_LOW;
>
> +#ifdef HAVE_SIXAXIS
> +       /* Lower security to allow the Sixaxis gamepad to connect. */
> +       /* Unless classic bonded only mode is disabled, the security level */
> +       /* will be bumped again for non sixaxis devices in hidp_add_connection() */
> +       sec_level = BT_IO_SEC_LOW;
> +#endif

This is not that great, even if we increase the level at a later stage
there is no reason to use low security while there are no input
devices that require such logic, so I'd keep medium and downgrade the
security only when required (which will probably need to restart the
listening socket.)

> +
>         server = g_new0(struct input_server, 1);
>         bacpy(&server->src, src);
>
> --
> 2.49.0
>
>
diff mbox series

Patch

diff --git a/profiles/input/device.c b/profiles/input/device.c
index 3627573e7..eb2fb5c8e 100644
--- a/profiles/input/device.c
+++ b/profiles/input/device.c
@@ -1088,8 +1088,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));
 
+	sixaxis_cable_pairing = device_is_sixaxis_cable_pairing(idev->device);
+
 	/* Make sure the device is bonded if required */
-	if (classic_bonded_only && !input_device_bonded(idev)) {
+	if (classic_bonded_only && !sixaxis_cable_pairing && !input_device_bonded(idev)) {
 		error("Rejected connection from !bonded device %s", idev->path);
 		goto cleanup;
 	}
@@ -1098,7 +1100,7 @@  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) {
+	if (!sixaxis_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)) {
diff --git a/profiles/input/server.c b/profiles/input/server.c
index 79cf08a66..73caeb076 100644
--- a/profiles/input/server.c
+++ b/profiles/input/server.c
@@ -273,6 +273,13 @@  int server_start(const bdaddr_t *src)
 	BtIOSecLevel sec_level = input_get_classic_bonded_only() ?
 					BT_IO_SEC_MEDIUM : BT_IO_SEC_LOW;
 
+#ifdef HAVE_SIXAXIS
+	/* Lower security to allow the Sixaxis gamepad to connect. */
+	/* Unless classic bonded only mode is disabled, the security level */
+	/* will be bumped again for non sixaxis devices in hidp_add_connection() */
+	sec_level = BT_IO_SEC_LOW;
+#endif
+
 	server = g_new0(struct input_server, 1);
 	bacpy(&server->src, src);