[PATCH 0/7] sixaxis: Don't mark USB plugged device as trusted

[PATCH 0/7] sixaxis: Don't mark USB plugged device as trusted

[Szymon Janc]

Hi,

This serie removes marking USB plugged sixaxis DS3 devices as trusted.
Now agent is asked for service authorization before device is connected
over BT. This is to address conserns about possibly crafted USB devices.

This is implemented in PATCH 1. Rest are fixes for bugs that were uncovered
due this change and client functionality that I've found missing while
testing this.

Comments and testing are welcome.

-- 
BR
Szymon Janc


Szymon Janc (7):
  sixaxis: Don't mark USB plugged device as trusted
  input: Fix crash on authorization reply with first sixaxis connection
  input: Fix check if device is sixaxis in auth_callback
  input: Fix connecting new trusted sixaxis device
  client: Add untrust command
  client: Add block command
  client: Add unblock command

 client/main.c           | 93 +++++++++++++++++++++++++++++++++++++++++++++++++
 plugins/sixaxis.c       |  1 -
 profiles/input/server.c | 37 ++++----------------
 3 files changed, 99 insertions(+), 32 deletions(-)

-- 
1.8.5.2

[PATCH 1/7] sixaxis: Don't mark USB plugged device as trusted

[Szymon Janc]

There were some valid conserns raised against marking plugged device
as trusted. Mainly due to posibility of crafted USB device. With this
patch user will be asked to confirm service connection and device can
be marked as trusted like any other devices.
---
 plugins/sixaxis.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
index 1154c15..4fdeff3 100644
--- a/plugins/sixaxis.c
+++ b/plugins/sixaxis.c
@@ -228,7 +228,6 @@ static bool setup_device(int fd, int index, struct btd_adapter *adapter)
 	btd_device_set_pnpid(device, devices[index].source, devices[index].vid,
 				devices[index].pid, devices[index].version);
 	btd_device_set_temporary(device, FALSE);
-	btd_device_set_trusted(device, TRUE);
 
 	return true;
 }
-- 
1.8.5.2

[PATCH 2/7] input: Fix crash on authorization reply with first sixaxis connection

[Szymon Janc]

Bogus unref in sixaxis_sdp_cb was resulting in NULL pointer dereference when
auth_callback was called.

src/adapter.c:connected_callback() hci0 device 00:06:F7:57:08:9E
    connected eir_len 5
src/device.c:device_set_class() /org/bluez/hci0/dev_00_06_F7_57_08_9E
    0x000508
profiles/input/server.c:connect_event_cb() Incoming connection from
    00:06:F7:57:08:9E on PSM 17
profiles/input/device.c:input_device_set_channel() idev (nil) psm 17
profiles/input/server.c:confirm_event_cb()
src/agent.c:agent_ref() 0x8117eb8: ref=2
src/agent.c:agent_authorize_service() authorize service request was
    sent for /org/bluez/hci0/dev_00_06_F7_57_08_9E
src/device.c:device_probe_profiles() Probing profiles for device
    00:06:F7:57:08:9E
profiles/input/device.c:input_device_register()
    /org/bluez/hci0/dev_00_06_F7_57_08_9E
src/service.c:btd_service_ref() 0x811f580: ref=2
src/service.c:change_state() 0x811f580: device 00:06:F7:57:08:9E
    profile input-hid state changed: unavailable -> disconnected (0)
src/device.c:device_svc_resolved()
    /org/bluez/hci0/dev_00_06_F7_57_08_9E err 0
profiles/input/server.c:sixaxis_sdp_cb() err 0 (Success)
profiles/input/device.c:input_device_set_channel() idev 0x8118568 psm
    17
profiles/input/server.c:connect_event_cb() Incoming connection from
    00:06:F7:57:08:9E on PSM 19
profiles/input/device.c:input_device_set_channel() idev 0x8118568 psm
    19
src/service.c:change_state() 0x811f580: device 00:06:F7:57:08:9E
    profile input-hid state changed: disconnected -> connected (0)
sixaxis: compatible device connected: PLAYSTATION(R)3 Controller
    (054C:0268)
plugins/sixaxis.c:setup_leds() number 2
sixaxis: failed to set LEDS (0 bytes written)
src/agent.c:agent_ref() 0x8117eb8: ref=3
Program received signal SIGSEGV, Segmentation fault.
---
 profiles/input/server.c | 28 ++--------------------------
 1 file changed, 2 insertions(+), 26 deletions(-)

diff --git a/profiles/input/server.c b/profiles/input/server.c
index f6f85a0..f2e5836 100644
--- a/profiles/input/server.c
+++ b/profiles/input/server.c
@@ -68,15 +68,10 @@ struct sixaxis_data {
 	uint16_t psm;
 };
 
-static void connect_event_cb(GIOChannel *chan, GError *err, gpointer data);
-
 static void sixaxis_sdp_cb(struct btd_device *dev, int err, void *user_data)
 {
 	struct sixaxis_data *data = user_data;
-	struct input_server *server;
-	GError *gerr = NULL;
 	const bdaddr_t *src;
-	GSList *l;
 
 	DBG("err %d (%s)", err, strerror(-err));
 
@@ -85,29 +80,10 @@ static void sixaxis_sdp_cb(struct btd_device *dev, int err, void *user_data)
 
 	src = btd_adapter_get_address(device_get_adapter(dev));
 
-	l = g_slist_find_custom(servers, src, server_cmp);
-	if (!l)
-		goto fail;
-
-	server = l->data;
-
-	err = input_device_set_channel(src, device_get_address(dev),
-							data->psm, data->chan);
-	if (err < 0)
+	if (input_device_set_channel(src, device_get_address(dev), data->psm,
+								data->chan) < 0)
 		goto fail;
 
-	if (server->confirm) {
-		if (!bt_io_accept(server->confirm, connect_event_cb, server,
-								NULL, &gerr)) {
-			error("bt_io_accept: %s", gerr->message);
-			g_error_free(gerr);
-			goto fail;
-		}
-
-		g_io_channel_unref(server->confirm);
-		server->confirm = NULL;
-	}
-
 	g_io_channel_unref(data->chan);
 	g_free(data);
 
-- 
1.8.5.2

[PATCH 3/7] input: Fix check if device is sixaxis in auth_callback

[Szymon Janc]

We need to accept connection if idev is not present but device is sixaxis.
This fix not doing so for sixaxis devices.
---
 profiles/input/server.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/profiles/input/server.c b/profiles/input/server.c
index f2e5836..d85d6a9 100644
--- a/profiles/input/server.c
+++ b/profiles/input/server.c
@@ -204,7 +204,7 @@ static void auth_callback(DBusError *derr, void *user_data)
 		goto reject;
 	}
 
-	if (!input_device_exists(&src, &dst) && dev_is_sixaxis(&src, &dst))
+	if (!input_device_exists(&src, &dst) && !dev_is_sixaxis(&src, &dst))
 		return;
 
 	if (!bt_io_accept(server->confirm, connect_event_cb, server,
-- 
1.8.5.2

[PATCH 4/7] input: Fix connecting new trusted sixaxis device

[Szymon Janc]

If sixaxis device was trusted before first connection over Bluetooth
connection might get accepted before device services were discovered.
This results in conection to PSM19 not being added to idev. To fix
this channel for PSM19 is also added to SDP complete callback so that
both channels are added.
---
 profiles/input/server.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/profiles/input/server.c b/profiles/input/server.c
index d85d6a9..3814eaf 100644
--- a/profiles/input/server.c
+++ b/profiles/input/server.c
@@ -101,9 +101,6 @@ static void sixaxis_browse_sdp(const bdaddr_t *src, const bdaddr_t *dst,
 	struct btd_device *device;
 	struct sixaxis_data *data;
 
-	if (psm != L2CAP_PSM_HIDP_CTRL)
-		return;
-
 	device = btd_adapter_find_device(adapter_find(src), dst);
 	if (!device)
 		return;
@@ -112,7 +109,9 @@ static void sixaxis_browse_sdp(const bdaddr_t *src, const bdaddr_t *dst,
 	data->chan = g_io_channel_ref(chan);
 	data->psm = psm;
 
-	device_discover_services(device);
+	if (psm == L2CAP_PSM_HIDP_CTRL)
+		device_discover_services(device);
+
 	device_wait_for_svc_complete(device, sixaxis_sdp_cb, data);
 }
 
-- 
1.8.5.2

[PATCH 5/7] client: Add untrust command

[Szymon Janc]

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

diff --git a/client/main.c b/client/main.c
index ebc85c6..a6e7906 100644
--- a/client/main.c
+++ b/client/main.c
@@ -851,6 +851,35 @@ static void cmd_trust(const char *arg)
 	g_free(str);
 }
 
+static void cmd_untrust(const char *arg)
+{
+	GDBusProxy *proxy;
+	dbus_bool_t trusted;
+	char *str;
+
+	if (!arg || !strlen(arg)) {
+		rl_printf("Missing device address argument\n");
+		return;
+	}
+
+	proxy = find_proxy_by_address(dev_list, arg);
+	if (!proxy) {
+		rl_printf("Device %s not available\n", arg);
+		return;
+	}
+
+	trusted = FALSE;
+
+	str = g_strdup_printf("%s untrust", arg);
+
+	if (g_dbus_proxy_set_property_basic(proxy, "Trusted",
+					DBUS_TYPE_BOOLEAN, &trusted,
+					generic_callback, str, g_free) == TRUE)
+		return;
+
+	g_free(str);
+}
+
 static void remove_device_reply(DBusMessage *message, void *user_data)
 {
 	DBusError error;
@@ -1088,6 +1117,8 @@ static const struct {
 							dev_generator },
 	{ "trust",        "<dev>",    cmd_trust, "Trust device",
 							dev_generator },
+	{ "untrust",      "<dev>",    cmd_untrust, "Untrust device",
+							dev_generator },
 	{ "remove",       "<dev>",    cmd_remove, "Remove device",
 							dev_generator },
 	{ "connect",      "<dev>",    cmd_connect, "Connect device",
-- 
1.8.5.2

[PATCH 6/7] client: Add block command

[Szymon Janc]

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

diff --git a/client/main.c b/client/main.c
index a6e7906..9f9231a 100644
--- a/client/main.c
+++ b/client/main.c
@@ -880,6 +880,35 @@ static void cmd_untrust(const char *arg)
 	g_free(str);
 }
 
+static void cmd_block(const char *arg)
+{
+	GDBusProxy *proxy;
+	dbus_bool_t blocked;
+	char *str;
+
+	if (!arg || !strlen(arg)) {
+		rl_printf("Missing device address argument\n");
+		return;
+	}
+
+	proxy = find_proxy_by_address(dev_list, arg);
+	if (!proxy) {
+		rl_printf("Device %s not available\n", arg);
+		return;
+	}
+
+	blocked = TRUE;
+
+	str = g_strdup_printf("%s block", arg);
+
+	if (g_dbus_proxy_set_property_basic(proxy, "Blocked",
+					DBUS_TYPE_BOOLEAN, &blocked,
+					generic_callback, str, g_free) == TRUE)
+		return;
+
+	g_free(str);
+}
+
 static void remove_device_reply(DBusMessage *message, void *user_data)
 {
 	DBusError error;
@@ -1119,6 +1148,8 @@ static const struct {
 							dev_generator },
 	{ "untrust",      "<dev>",    cmd_untrust, "Untrust device",
 							dev_generator },
+	{ "block",        "<dev>",    cmd_block, "Block device",
+								dev_generator },
 	{ "remove",       "<dev>",    cmd_remove, "Remove device",
 							dev_generator },
 	{ "connect",      "<dev>",    cmd_connect, "Connect device",
-- 
1.8.5.2

[PATCH 7/7] client: Add unblock command

[Szymon Janc]

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

diff --git a/client/main.c b/client/main.c
index 9f9231a..5791e95 100644
--- a/client/main.c
+++ b/client/main.c
@@ -909,6 +909,35 @@ static void cmd_block(const char *arg)
 	g_free(str);
 }
 
+static void cmd_unblock(const char *arg)
+{
+	GDBusProxy *proxy;
+	dbus_bool_t blocked;
+	char *str;
+
+	if (!arg || !strlen(arg)) {
+		rl_printf("Missing device address argument\n");
+		return;
+	}
+
+	proxy = find_proxy_by_address(dev_list, arg);
+	if (!proxy) {
+		rl_printf("Device %s not available\n", arg);
+		return;
+	}
+
+	blocked = FALSE;
+
+	str = g_strdup_printf("%s unblock", arg);
+
+	if (g_dbus_proxy_set_property_basic(proxy, "Blocked",
+					DBUS_TYPE_BOOLEAN, &blocked,
+					generic_callback, str, g_free) == TRUE)
+		return;
+
+	g_free(str);
+}
+
 static void remove_device_reply(DBusMessage *message, void *user_data)
 {
 	DBusError error;
@@ -1150,6 +1179,8 @@ static const struct {
 							dev_generator },
 	{ "block",        "<dev>",    cmd_block, "Block device",
 								dev_generator },
+	{ "unblock",      "<dev>",    cmd_unblock, "Unblock device",
+								dev_generator },
 	{ "remove",       "<dev>",    cmd_remove, "Remove device",
 							dev_generator },
 	{ "connect",      "<dev>",    cmd_connect, "Connect device",
-- 
1.8.5.2

Re: [PATCH 0/7] sixaxis: Don't mark USB plugged device as trusted

[Johan Hedberg]

Hi Szymon,

On Wed, Jan 08, 2014, Szymon Janc wrote:
> This serie removes marking USB plugged sixaxis DS3 devices as trusted.
> Now agent is asked for service authorization before device is connected
> over BT. This is to address conserns about possibly crafted USB devices.
> 
> This is implemented in PATCH 1. Rest are fixes for bugs that were uncovered
> due this change and client functionality that I've found missing while
> testing this.
> 
> Comments and testing are welcome.
> 
> -- 
> BR
> Szymon Janc
> 
> 
> Szymon Janc (7):
>   sixaxis: Don't mark USB plugged device as trusted
>   input: Fix crash on authorization reply with first sixaxis connection
>   input: Fix check if device is sixaxis in auth_callback
>   input: Fix connecting new trusted sixaxis device
>   client: Add untrust command
>   client: Add block command
>   client: Add unblock command
> 
>  client/main.c           | 93 +++++++++++++++++++++++++++++++++++++++++++++++++
>  plugins/sixaxis.c       |  1 -
>  profiles/input/server.c | 37 ++++----------------
>  3 files changed, 99 insertions(+), 32 deletions(-)

All patches have been applied. Thanks.

Johan