2014-08-22 09:02:58

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCHv2 0/2] Fix accepting not encrypted connections from keyboards

v2 changes:
* checking subclass for the keyboard is done only once, when the SDP is
done. Then the default, minimum security level is stored inside
the device struct.
* fixed typo

Jakub Tyszkowski (2):
android/hid: Reject connections from unknown devices
android/hid: Force encryption for keyboards

android/hidhost.c | 40 +++++++++++++++++++++++++++++++++++++---
1 file changed, 37 insertions(+), 3 deletions(-)

--
1.9.1



2014-08-25 09:38:30

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCHv2 1/2] android/hid: Reject connections from unknown devices

Hi Jakub,

On Fri, Aug 22, 2014 at 12:02 PM, Jakub Tyszkowski
<[email protected]> wrote:
> Don't accept input from not bonded devices.
> ---
> android/hidhost.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/android/hidhost.c b/android/hidhost.c
> index da5f818..07985d8 100644
> --- a/android/hidhost.c
> +++ b/android/hidhost.c
> @@ -1410,6 +1410,20 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
> ba2str(&dst, address);
> DBG("Incoming connection from %s on PSM %d", address, psm);
>
> + if (!bt_device_is_bonded(&dst)) {
> + uint8_t hdr = (HID_MSG_CONTROL | HID_VIRTUAL_CABLE_UNPLUG);
> + int sk = g_io_channel_unix_get_fd(chan);
> +
> + warn("hidhost: Rejected connection from unknown device %s",
> + address);
> +
> + if (write(sk, &hdr, sizeof(hdr)) < 0)
> + error("hidhost: Unable to send virtual cable unplug");
> +
> + g_io_channel_shutdown(chan, TRUE, NULL);
> + return;
> + }
> +
> switch (psm) {
> case L2CAP_PSM_HIDP_CTRL:
> l = g_slist_find_custom(devices, &dst, device_cmp);
> --
> 1.9.1

I think we can a helper function to send HID_VIRTUAL_CABLE_UNPLUG,
that way we can share code with bt_hid_virtual_unplug


--
Luiz Augusto von Dentz

2014-08-22 09:03:00

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCHv2 2/2] android/hid: Force encryption for keyboards

Encryption is mandatory for keyboards. Instead of using hardcoded security
level it's now set per device (with LOW as default) and raised for
keyboards (after the SDP query is done). This level is then used to
establish outgoing connections and raising the security level of the
incomming ones.
---
android/hidhost.c | 26 +++++++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/android/hidhost.c b/android/hidhost.c
index 07985d8..984776a 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -110,6 +110,7 @@ struct hid_device {
uint8_t last_hid_msg;
struct bt_hog *hog;
guint reconnect_id;
+ int sec_level;
};

static int device_cmp(gconstpointer s, gconstpointer user_data)
@@ -162,6 +163,8 @@ static struct hid_device *hid_device_new(const bdaddr_t *addr)
dev = g_new0(struct hid_device, 1);
bacpy(&dev->dst, addr);
dev->state = HAL_HIDHOST_STATE_DISCONNECTED;
+ dev->sec_level = BT_IO_SEC_LOW;
+
devices = g_slist_append(devices, dev);

return dev;
@@ -594,7 +597,7 @@ static void control_connect_cb(GIOChannel *chan, GError *conn_err,
BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
BT_IO_OPT_DEST_BDADDR, &dev->dst,
BT_IO_OPT_PSM, L2CAP_PSM_HIDP_INTR,
- BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+ BT_IO_OPT_SEC_LEVEL, dev->sec_level,
BT_IO_OPT_INVALID);
if (!dev->intr_io) {
error("hidhost: Failed to connect interrupt channel (%s)",
@@ -640,9 +643,14 @@ static void hid_sdp_search_cb(sdp_list_t *recs, int err, gpointer data)
dev->country = data->val.uint8;

data = sdp_data_get(rec, SDP_ATTR_HID_DEVICE_SUBCLASS);
- if (data)
+ if (data) {
dev->subclass = data->val.uint8;

+ /* Encryption is mandatory for keyboards */
+ if (dev->subclass & 0x40)
+ dev->sec_level = BT_IO_SEC_MEDIUM;
+ }
+
data = sdp_data_get(rec, SDP_ATTR_HID_BOOT_DEVICE);
if (data)
dev->boot_dev = data->val.uint8;
@@ -673,6 +681,18 @@ static void hid_sdp_search_cb(sdp_list_t *recs, int err, gpointer data)
}

if (dev->ctrl_io) {
+ /* Raise the security level for this device if needed. */
+ if ((dev->sec_level > BT_IO_SEC_LOW) &&
+ !bt_io_set(dev->ctrl_io, &gerr,
+ BT_IO_OPT_SEC_LEVEL, dev->sec_level,
+ BT_IO_OPT_INVALID)) {
+ error("hidhost: Cannot raise security level: %s",
+ gerr->message);
+ g_error_free(gerr);
+
+ goto fail;
+ }
+
if (uhid_create(dev) < 0)
goto fail;
return;
@@ -682,7 +702,7 @@ static void hid_sdp_search_cb(sdp_list_t *recs, int err, gpointer data)
BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
BT_IO_OPT_DEST_BDADDR, &dev->dst,
BT_IO_OPT_PSM, L2CAP_PSM_HIDP_CTRL,
- BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+ BT_IO_OPT_SEC_LEVEL, dev->sec_level,
BT_IO_OPT_INVALID);
if (gerr) {
error("hidhost: Failed to connect control channel (%s)",
--
1.9.1


2014-08-22 09:02:59

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCHv2 1/2] android/hid: Reject connections from unknown devices

Don't accept input from not bonded devices.
---
android/hidhost.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/android/hidhost.c b/android/hidhost.c
index da5f818..07985d8 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -1410,6 +1410,20 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
ba2str(&dst, address);
DBG("Incoming connection from %s on PSM %d", address, psm);

+ if (!bt_device_is_bonded(&dst)) {
+ uint8_t hdr = (HID_MSG_CONTROL | HID_VIRTUAL_CABLE_UNPLUG);
+ int sk = g_io_channel_unix_get_fd(chan);
+
+ warn("hidhost: Rejected connection from unknown device %s",
+ address);
+
+ if (write(sk, &hdr, sizeof(hdr)) < 0)
+ error("hidhost: Unable to send virtual cable unplug");
+
+ g_io_channel_shutdown(chan, TRUE, NULL);
+ return;
+ }
+
switch (psm) {
case L2CAP_PSM_HIDP_CTRL:
l = g_slist_find_custom(devices, &dst, device_cmp);
--
1.9.1