2018-11-02 00:19:52

by Andrej Shadura

[permalink] [raw]
Subject: Re: [PATCH] HID: add driver for U2F Zero built-in LED and RNG

Hi everyone,

I’ve got a comment from Nick Kossifidis that I probably shouldn’t set
RNG’s quality to 1024. Adding linux-crypto@ to the loop.

On 23/10/2018 16:46, Andrej Shadura wrote:
> U2F Zero supports custom commands for blinking the LED and getting data
> from the internal hardware RNG. Expose the blinking function as a LED
> device, and the internal hardware RNG as an HWRNG so that it can be used
> to feed the enthropy pool.
>
> Signed-off-by: Andrej Shadura <[email protected]>
> ---
> drivers/hid/Kconfig | 15 ++
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/hid-u2fzero.c | 371 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 388 insertions(+)
> create mode 100644 drivers/hid/hid-u2fzero.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 61e1953ff921..3de6bf202f20 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -975,6 +975,21 @@ config HID_UDRAW_PS3
> Say Y here if you want to use the THQ uDraw gaming tablet for
> the PS3.
>
> +config HID_U2FZERO
> + tristate "U2F Zero LED and RNG support"
> + depends on HID
> + depends on LEDS_CLASS
> + help
> + Support for the LED of the U2F Zero device.
> +
> + U2F Zero supports custom commands for blinking the LED
> + and getting data from the internal hardware RNG.
> + The internal hardware can be used to feed the enthropy pool.
> +
> + U2F Zero only supports blinking its LED, so this driver doesn't
> + allow setting the brightness to anything but 1, which will
> + trigger a single blink and immediately reset to back 0.
> +
> config HID_WACOM
> tristate "Wacom Intuos/Graphire tablet support (USB)"
> depends on USB_HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index bd7ac53b75c5..617b1a928b6a 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -107,6 +107,7 @@ obj-$(CONFIG_HID_THRUSTMASTER) += hid-tmff.o
> obj-$(CONFIG_HID_TIVO) += hid-tivo.o
> obj-$(CONFIG_HID_TOPSEED) += hid-topseed.o
> obj-$(CONFIG_HID_TWINHAN) += hid-twinhan.o
> +obj-$(CONFIG_HID_U2FZERO) += hid-u2fzero.o
> obj-$(CONFIG_HID_UCLOGIC) += hid-uclogic.o
> obj-$(CONFIG_HID_UDRAW_PS3) += hid-udraw-ps3.o
> obj-$(CONFIG_HID_LED) += hid-led.o
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index bc49909aba8e..070a2fcd8a18 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -309,6 +309,7 @@
> #define USB_DEVICE_ID_CYGNAL_RADIO_SI470X 0x818a
> #define USB_DEVICE_ID_FOCALTECH_FTXXXX_MULTITOUCH 0x81b9
> #define USB_DEVICE_ID_CYGNAL_CP2112 0xea90
> +#define USB_DEVICE_ID_U2F_ZERO 0x8acf
>
> #define USB_DEVICE_ID_CYGNAL_RADIO_SI4713 0x8244
>
> diff --git a/drivers/hid/hid-u2fzero.c b/drivers/hid/hid-u2fzero.c
> new file mode 100644
> index 000000000000..bd4077c93c97
> --- /dev/null
> +++ b/drivers/hid/hid-u2fzero.c
> @@ -0,0 +1,371 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * U2F Zero LED and RNG driver
> + *
> + * Copyright 2018 Andrej Shadura <[email protected]>
> + * Loosely based on drivers/hid/hid-led.c
> + * and drivers/usb/misc/chaoskey.c
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation, version 2.
> + */
> +
> +#include <linux/hid.h>
> +#include <linux/hidraw.h>
> +#include <linux/hw_random.h>
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/usb.h>
> +
> +#include "usbhid/usbhid.h"
> +#include "hid-ids.h"
> +
> +#define DRIVER_SHORT "u2fzero"
> +
> +#define HID_REPORT_SIZE 64
> +
> +/* We only use broadcast (CID-less) messages */
> +#define CID_BROADCAST 0xffffffff
> +
> +struct u2f_hid_msg {
> + u32 cid;
> + union {
> + struct {
> + u8 cmd;
> + u8 bcnth;
> + u8 bcntl;
> + u8 data[HID_REPORT_SIZE - 7];
> + } init;
> + struct {
> + u8 seq;
> + u8 data[HID_REPORT_SIZE - 5];
> + } cont;
> + };
> +} __packed;
> +
> +struct u2f_hid_report {
> + u8 report_type;
> + struct u2f_hid_msg msg;
> +} __packed;
> +
> +#define U2F_HID_MSG_LEN(f) (size_t)(((f).init.bcnth << 8) + (f).init.bcntl)
> +
> +/* Custom extensions to the U2FHID protocol */
> +#define U2F_CUSTOM_GET_RNG 0x21
> +#define U2F_CUSTOM_WINK 0x24
> +
> +struct u2fzero_device {
> + struct hid_device *hdev;
> + struct urb *urb; /* URB for the RNG data */
> + struct led_classdev ldev; /* Embedded struct for led */
> + struct hwrng hwrng; /* Embedded struct for hwrng */
> + char *led_name;
> + char *rng_name;
> + u8 *buf_out;
> + u8 *buf_in;
> + struct mutex lock;
> + bool present;
> +};
> +
> +static int u2fzero_send(struct u2fzero_device *dev, struct u2f_hid_report *req)
> +{
> + int ret;
> +
> + mutex_lock(&dev->lock);
> +
> + memcpy(dev->buf_out, req, sizeof(struct u2f_hid_report));
> +
> + ret = hid_hw_output_report(dev->hdev, dev->buf_out,
> + sizeof(struct u2f_hid_msg));
> +
> + mutex_unlock(&dev->lock);
> +
> + if (ret < 0)
> + return ret;
> +
> + return ret == sizeof(struct u2f_hid_msg) ? 0 : -EMSGSIZE;
> +}
> +
> +struct u2fzero_transfer_context {
> + struct completion done;
> + int status;
> +};
> +
> +static void u2fzero_read_callback(struct urb *urb)
> +{
> + struct u2fzero_transfer_context *ctx = urb->context;
> +
> + ctx->status = urb->status;
> + complete(&ctx->done);
> +}
> +
> +static int u2fzero_recv(struct u2fzero_device *dev,
> + struct u2f_hid_report *req,
> + struct u2f_hid_msg *resp)
> +{
> + int ret;
> + struct hid_device *hdev = dev->hdev;
> + struct u2fzero_transfer_context ctx;
> +
> + mutex_lock(&dev->lock);
> +
> + memcpy(dev->buf_out, req, sizeof(struct u2f_hid_report));
> +
> + dev->urb->context = &ctx;
> + init_completion(&ctx.done);
> +
> + ret = usb_submit_urb(dev->urb, GFP_NOIO);
> + if (unlikely(ret)) {
> + hid_err(hdev, "usb_submit_urb failed: %d", ret);
> + goto err;
> + }
> +
> + ret = hid_hw_output_report(dev->hdev, dev->buf_out,
> + sizeof(struct u2f_hid_msg));
> +
> + if (ret < 0) {
> + hid_err(hdev, "hid_hw_output_report failed: %d", ret);
> + goto err;
> + }
> +
> + ret = (wait_for_completion_timeout(
> + &ctx.done, msecs_to_jiffies(USB_CTRL_SET_TIMEOUT)));
> + if (ret < 0) {
> + usb_kill_urb(dev->urb);
> + hid_err(hdev, "urb submission timed out");
> + } else {
> + ret = dev->urb->actual_length;
> + memcpy(resp, dev->buf_in, ret);
> + }
> +
> +err:
> + mutex_unlock(&dev->lock);
> +
> + return ret;
> +}
> +
> +static int u2fzero_blink(struct led_classdev *ldev)
> +{
> + struct u2fzero_device *dev = container_of(ldev,
> + struct u2fzero_device, ldev);
> + struct u2f_hid_report req = {
> + .report_type = 0,
> + .msg.cid = CID_BROADCAST,
> + .msg.init = {
> + .cmd = U2F_CUSTOM_WINK,
> + .bcnth = 0,
> + .bcntl = 0,
> + .data = {0},
> + }
> + };
> + return u2fzero_send(dev, &req);
> +}
> +
> +static int u2fzero_brightness_set(struct led_classdev *ldev,
> + enum led_brightness brightness)
> +{
> + ldev->brightness = LED_OFF;
> + if (brightness)
> + return u2fzero_blink(ldev);
> + else
> + return 0;
> +}
> +
> +static int u2fzero_rng_read(struct hwrng *rng, void *data,
> + size_t max, bool wait)
> +{
> + struct u2fzero_device *dev = container_of(rng,
> + struct u2fzero_device, hwrng);
> + struct u2f_hid_report req = {
> + .report_type = 0,
> + .msg.cid = CID_BROADCAST,
> + .msg.init = {
> + .cmd = U2F_CUSTOM_GET_RNG,
> + .bcnth = 0,
> + .bcntl = 0,
> + .data = {0},
> + }
> + };
> + struct u2f_hid_msg resp;
> + int ret;
> + size_t actual_length;
> +
> + if (!dev->present) {
> + hid_dbg(dev->hdev, "device not present");
> + return 0;
> + }
> +
> + ret = u2fzero_recv(dev, &req, &resp);
> + if (ret < 0)
> + return 0;
> +
> + /* only take the minimum amount of data it is safe to take */
> + actual_length = min3((size_t)ret - offsetof(struct u2f_hid_msg,
> + init.data), U2F_HID_MSG_LEN(resp), max);
> +
> + memcpy(data, resp.init.data, actual_length);
> +
> + return actual_length;
> +}
> +
> +static int u2fzero_init_led(struct u2fzero_device *dev,
> + unsigned int minor)
> +{
> + dev->led_name = devm_kasprintf(&dev->hdev->dev, GFP_KERNEL,
> + "%s%u", DRIVER_SHORT, minor);
> + if (dev->led_name == NULL)
> + return -ENOMEM;
> +
> + dev->ldev.name = dev->led_name;
> + dev->ldev.max_brightness = LED_ON;
> + dev->ldev.flags = LED_HW_PLUGGABLE;
> + dev->ldev.brightness_set_blocking = u2fzero_brightness_set;
> +
> + return devm_led_classdev_register(&dev->hdev->dev, &dev->ldev);
> +}
> +
> +static int u2fzero_init_hwrng(struct u2fzero_device *dev,
> + unsigned int minor)
> +{
> + dev->rng_name = devm_kasprintf(&dev->hdev->dev, GFP_KERNEL,
> + "%s-rng%u", DRIVER_SHORT, minor);
> + if (dev->rng_name == NULL)
> + return -ENOMEM;
> +
> + dev->hwrng.name = dev->rng_name;
> + dev->hwrng.read = u2fzero_rng_read;
> + dev->hwrng.quality = 1024;
> +
> + return devm_hwrng_register(&dev->hdev->dev, &dev->hwrng);
> +}
> +
> +static int u2fzero_fill_in_urb(struct u2fzero_device *dev)
> +{
> + struct hid_device *hdev = dev->hdev;
> + struct usb_device *udev;
> + struct usbhid_device *usbhid = hdev->driver_data;
> + unsigned int pipe_in;
> + struct usb_host_endpoint *ep;
> +
> + if (dev->hdev->bus != BUS_USB)
> + return -EINVAL;
> +
> + udev = hid_to_usb_dev(hdev);
> +
> + if (!usbhid->urbout || !usbhid->urbin)
> + return -ENODEV;
> +
> + ep = usb_pipe_endpoint(udev, usbhid->urbin->pipe);
> + if (!ep)
> + return -ENODEV;
> +
> + dev->urb = usb_alloc_urb(0, GFP_KERNEL);
> + if (!dev->urb)
> + return -ENOMEM;
> +
> + pipe_in = (usbhid->urbin->pipe & ~(3 << 30)) | (PIPE_INTERRUPT << 30);
> +
> + usb_fill_int_urb(dev->urb,
> + udev,
> + pipe_in,
> + dev->buf_in,
> + HID_REPORT_SIZE,
> + u2fzero_read_callback,
> + NULL,
> + ep->desc.bInterval);
> +
> + return 0;
> +}
> +
> +static int u2fzero_probe(struct hid_device *hdev,
> + const struct hid_device_id *id)
> +{
> + struct u2fzero_device *dev;
> + unsigned int minor;
> + int ret;
> +
> + dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
> + if (dev == NULL)
> + return -ENOMEM;
> +
> + dev->buf_out = devm_kmalloc(&hdev->dev,
> + sizeof(struct u2f_hid_report), GFP_KERNEL);
> + if (dev->buf_out == NULL)
> + return -ENOMEM;
> +
> + dev->buf_in = devm_kmalloc(&hdev->dev,
> + sizeof(struct u2f_hid_msg), GFP_KERNEL);
> + if (dev->buf_in == NULL)
> + return -ENOMEM;
> +
> + ret = hid_parse(hdev);
> + if (ret)
> + return ret;
> +
> + dev->hdev = hdev;
> + hid_set_drvdata(hdev, dev);
> + mutex_init(&dev->lock);
> +
> + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> + if (ret)
> + return ret;
> +
> + u2fzero_fill_in_urb(dev);
> +
> + dev->present = true;
> +
> + minor = ((struct hidraw *) hdev->hidraw)->minor;
> +
> + ret = u2fzero_init_led(dev, minor);
> + if (ret) {
> + hid_hw_stop(hdev);
> + return ret;
> + }
> +
> + hid_info(hdev, "U2F Zero LED initialised\n");
> +
> + ret = u2fzero_init_hwrng(dev, minor);
> + if (ret) {
> + hid_hw_stop(hdev);
> + return ret;
> + }
> +
> + hid_info(hdev, "U2F Zero RNG initialised\n");
> +
> + return 0;
> +}
> +
> +static void u2fzero_remove(struct hid_device *hdev)
> +{
> + struct u2fzero_device *dev = hid_get_drvdata(hdev);
> +
> + mutex_lock(&dev->lock);
> + dev->present = false;
> + mutex_unlock(&dev->lock);
> +
> + hid_hw_stop(hdev);
> + usb_poison_urb(dev->urb);
> + usb_free_urb(dev->urb);
> +}
> +
> +static const struct hid_device_id u2fzero_table[] = {
> + { HID_USB_DEVICE(USB_VENDOR_ID_CYGNAL,
> + USB_DEVICE_ID_U2F_ZERO) },
> + { }
> +};
> +MODULE_DEVICE_TABLE(hid, u2fzero_table);
> +
> +static struct hid_driver u2fzero_driver = {
> + .name = "hid-" DRIVER_SHORT,
> + .probe = u2fzero_probe,
> + .remove = u2fzero_remove,
> + .id_table = u2fzero_table,
> +};
> +
> +module_hid_driver(u2fzero_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Andrej Shadura <[email protected]>");
> +MODULE_DESCRIPTION("U2F Zero LED and RNG driver");
>


--
Cheers,
Andrej


2018-11-12 21:10:12

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH] HID: add driver for U2F Zero built-in LED and RNG

On Thu, 1 Nov 2018, Andrej Shadura wrote:

> Hi everyone,
>
> I’ve got a comment from Nick Kossifidis that I probably shouldn’t set
> RNG’s quality to 1024. Adding linux-crypto@ to the loop.

So, what was this about? Is there any resolution to it? :)

Thanks.

>
> On 23/10/2018 16:46, Andrej Shadura wrote:
> > U2F Zero supports custom commands for blinking the LED and getting data
> > from the internal hardware RNG. Expose the blinking function as a LED
> > device, and the internal hardware RNG as an HWRNG so that it can be used
> > to feed the enthropy pool.
> >
> > Signed-off-by: Andrej Shadura <[email protected]>
> > ---
> > drivers/hid/Kconfig | 15 ++
> > drivers/hid/Makefile | 1 +
> > drivers/hid/hid-ids.h | 1 +
> > drivers/hid/hid-u2fzero.c | 371 ++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 388 insertions(+)
> > create mode 100644 drivers/hid/hid-u2fzero.c
> >
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index 61e1953ff921..3de6bf202f20 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -975,6 +975,21 @@ config HID_UDRAW_PS3
> > Say Y here if you want to use the THQ uDraw gaming tablet for
> > the PS3.
> >
> > +config HID_U2FZERO
> > + tristate "U2F Zero LED and RNG support"
> > + depends on HID
> > + depends on LEDS_CLASS
> > + help
> > + Support for the LED of the U2F Zero device.
> > +
> > + U2F Zero supports custom commands for blinking the LED
> > + and getting data from the internal hardware RNG.
> > + The internal hardware can be used to feed the enthropy pool.
> > +
> > + U2F Zero only supports blinking its LED, so this driver doesn't
> > + allow setting the brightness to anything but 1, which will
> > + trigger a single blink and immediately reset to back 0.
> > +
> > config HID_WACOM
> > tristate "Wacom Intuos/Graphire tablet support (USB)"
> > depends on USB_HID
> > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> > index bd7ac53b75c5..617b1a928b6a 100644
> > --- a/drivers/hid/Makefile
> > +++ b/drivers/hid/Makefile
> > @@ -107,6 +107,7 @@ obj-$(CONFIG_HID_THRUSTMASTER) += hid-tmff.o
> > obj-$(CONFIG_HID_TIVO) += hid-tivo.o
> > obj-$(CONFIG_HID_TOPSEED) += hid-topseed.o
> > obj-$(CONFIG_HID_TWINHAN) += hid-twinhan.o
> > +obj-$(CONFIG_HID_U2FZERO) += hid-u2fzero.o
> > obj-$(CONFIG_HID_UCLOGIC) += hid-uclogic.o
> > obj-$(CONFIG_HID_UDRAW_PS3) += hid-udraw-ps3.o
> > obj-$(CONFIG_HID_LED) += hid-led.o
> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > index bc49909aba8e..070a2fcd8a18 100644
> > --- a/drivers/hid/hid-ids.h
> > +++ b/drivers/hid/hid-ids.h
> > @@ -309,6 +309,7 @@
> > #define USB_DEVICE_ID_CYGNAL_RADIO_SI470X 0x818a
> > #define USB_DEVICE_ID_FOCALTECH_FTXXXX_MULTITOUCH 0x81b9
> > #define USB_DEVICE_ID_CYGNAL_CP2112 0xea90
> > +#define USB_DEVICE_ID_U2F_ZERO 0x8acf
> >
> > #define USB_DEVICE_ID_CYGNAL_RADIO_SI4713 0x8244
> >
> > diff --git a/drivers/hid/hid-u2fzero.c b/drivers/hid/hid-u2fzero.c
> > new file mode 100644
> > index 000000000000..bd4077c93c97
> > --- /dev/null
> > +++ b/drivers/hid/hid-u2fzero.c
> > @@ -0,0 +1,371 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * U2F Zero LED and RNG driver
> > + *
> > + * Copyright 2018 Andrej Shadura <[email protected]>
> > + * Loosely based on drivers/hid/hid-led.c
> > + * and drivers/usb/misc/chaoskey.c
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation, version 2.
> > + */
> > +
> > +#include <linux/hid.h>
> > +#include <linux/hidraw.h>
> > +#include <linux/hw_random.h>
> > +#include <linux/leds.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/usb.h>
> > +
> > +#include "usbhid/usbhid.h"
> > +#include "hid-ids.h"
> > +
> > +#define DRIVER_SHORT "u2fzero"
> > +
> > +#define HID_REPORT_SIZE 64
> > +
> > +/* We only use broadcast (CID-less) messages */
> > +#define CID_BROADCAST 0xffffffff
> > +
> > +struct u2f_hid_msg {
> > + u32 cid;
> > + union {
> > + struct {
> > + u8 cmd;
> > + u8 bcnth;
> > + u8 bcntl;
> > + u8 data[HID_REPORT_SIZE - 7];
> > + } init;
> > + struct {
> > + u8 seq;
> > + u8 data[HID_REPORT_SIZE - 5];
> > + } cont;
> > + };
> > +} __packed;
> > +
> > +struct u2f_hid_report {
> > + u8 report_type;
> > + struct u2f_hid_msg msg;
> > +} __packed;
> > +
> > +#define U2F_HID_MSG_LEN(f) (size_t)(((f).init.bcnth << 8) + (f).init.bcntl)
> > +
> > +/* Custom extensions to the U2FHID protocol */
> > +#define U2F_CUSTOM_GET_RNG 0x21
> > +#define U2F_CUSTOM_WINK 0x24
> > +
> > +struct u2fzero_device {
> > + struct hid_device *hdev;
> > + struct urb *urb; /* URB for the RNG data */
> > + struct led_classdev ldev; /* Embedded struct for led */
> > + struct hwrng hwrng; /* Embedded struct for hwrng */
> > + char *led_name;
> > + char *rng_name;
> > + u8 *buf_out;
> > + u8 *buf_in;
> > + struct mutex lock;
> > + bool present;
> > +};
> > +
> > +static int u2fzero_send(struct u2fzero_device *dev, struct u2f_hid_report *req)
> > +{
> > + int ret;
> > +
> > + mutex_lock(&dev->lock);
> > +
> > + memcpy(dev->buf_out, req, sizeof(struct u2f_hid_report));
> > +
> > + ret = hid_hw_output_report(dev->hdev, dev->buf_out,
> > + sizeof(struct u2f_hid_msg));
> > +
> > + mutex_unlock(&dev->lock);
> > +
> > + if (ret < 0)
> > + return ret;
> > +
> > + return ret == sizeof(struct u2f_hid_msg) ? 0 : -EMSGSIZE;
> > +}
> > +
> > +struct u2fzero_transfer_context {
> > + struct completion done;
> > + int status;
> > +};
> > +
> > +static void u2fzero_read_callback(struct urb *urb)
> > +{
> > + struct u2fzero_transfer_context *ctx = urb->context;
> > +
> > + ctx->status = urb->status;
> > + complete(&ctx->done);
> > +}
> > +
> > +static int u2fzero_recv(struct u2fzero_device *dev,
> > + struct u2f_hid_report *req,
> > + struct u2f_hid_msg *resp)
> > +{
> > + int ret;
> > + struct hid_device *hdev = dev->hdev;
> > + struct u2fzero_transfer_context ctx;
> > +
> > + mutex_lock(&dev->lock);
> > +
> > + memcpy(dev->buf_out, req, sizeof(struct u2f_hid_report));
> > +
> > + dev->urb->context = &ctx;
> > + init_completion(&ctx.done);
> > +
> > + ret = usb_submit_urb(dev->urb, GFP_NOIO);
> > + if (unlikely(ret)) {
> > + hid_err(hdev, "usb_submit_urb failed: %d", ret);
> > + goto err;
> > + }
> > +
> > + ret = hid_hw_output_report(dev->hdev, dev->buf_out,
> > + sizeof(struct u2f_hid_msg));
> > +
> > + if (ret < 0) {
> > + hid_err(hdev, "hid_hw_output_report failed: %d", ret);
> > + goto err;
> > + }
> > +
> > + ret = (wait_for_completion_timeout(
> > + &ctx.done, msecs_to_jiffies(USB_CTRL_SET_TIMEOUT)));
> > + if (ret < 0) {
> > + usb_kill_urb(dev->urb);
> > + hid_err(hdev, "urb submission timed out");
> > + } else {
> > + ret = dev->urb->actual_length;
> > + memcpy(resp, dev->buf_in, ret);
> > + }
> > +
> > +err:
> > + mutex_unlock(&dev->lock);
> > +
> > + return ret;
> > +}
> > +
> > +static int u2fzero_blink(struct led_classdev *ldev)
> > +{
> > + struct u2fzero_device *dev = container_of(ldev,
> > + struct u2fzero_device, ldev);
> > + struct u2f_hid_report req = {
> > + .report_type = 0,
> > + .msg.cid = CID_BROADCAST,
> > + .msg.init = {
> > + .cmd = U2F_CUSTOM_WINK,
> > + .bcnth = 0,
> > + .bcntl = 0,
> > + .data = {0},
> > + }
> > + };
> > + return u2fzero_send(dev, &req);
> > +}
> > +
> > +static int u2fzero_brightness_set(struct led_classdev *ldev,
> > + enum led_brightness brightness)
> > +{
> > + ldev->brightness = LED_OFF;
> > + if (brightness)
> > + return u2fzero_blink(ldev);
> > + else
> > + return 0;
> > +}
> > +
> > +static int u2fzero_rng_read(struct hwrng *rng, void *data,
> > + size_t max, bool wait)
> > +{
> > + struct u2fzero_device *dev = container_of(rng,
> > + struct u2fzero_device, hwrng);
> > + struct u2f_hid_report req = {
> > + .report_type = 0,
> > + .msg.cid = CID_BROADCAST,
> > + .msg.init = {
> > + .cmd = U2F_CUSTOM_GET_RNG,
> > + .bcnth = 0,
> > + .bcntl = 0,
> > + .data = {0},
> > + }
> > + };
> > + struct u2f_hid_msg resp;
> > + int ret;
> > + size_t actual_length;
> > +
> > + if (!dev->present) {
> > + hid_dbg(dev->hdev, "device not present");
> > + return 0;
> > + }
> > +
> > + ret = u2fzero_recv(dev, &req, &resp);
> > + if (ret < 0)
> > + return 0;
> > +
> > + /* only take the minimum amount of data it is safe to take */
> > + actual_length = min3((size_t)ret - offsetof(struct u2f_hid_msg,
> > + init.data), U2F_HID_MSG_LEN(resp), max);
> > +
> > + memcpy(data, resp.init.data, actual_length);
> > +
> > + return actual_length;
> > +}
> > +
> > +static int u2fzero_init_led(struct u2fzero_device *dev,
> > + unsigned int minor)
> > +{
> > + dev->led_name = devm_kasprintf(&dev->hdev->dev, GFP_KERNEL,
> > + "%s%u", DRIVER_SHORT, minor);
> > + if (dev->led_name == NULL)
> > + return -ENOMEM;
> > +
> > + dev->ldev.name = dev->led_name;
> > + dev->ldev.max_brightness = LED_ON;
> > + dev->ldev.flags = LED_HW_PLUGGABLE;
> > + dev->ldev.brightness_set_blocking = u2fzero_brightness_set;
> > +
> > + return devm_led_classdev_register(&dev->hdev->dev, &dev->ldev);
> > +}
> > +
> > +static int u2fzero_init_hwrng(struct u2fzero_device *dev,
> > + unsigned int minor)
> > +{
> > + dev->rng_name = devm_kasprintf(&dev->hdev->dev, GFP_KERNEL,
> > + "%s-rng%u", DRIVER_SHORT, minor);
> > + if (dev->rng_name == NULL)
> > + return -ENOMEM;
> > +
> > + dev->hwrng.name = dev->rng_name;
> > + dev->hwrng.read = u2fzero_rng_read;
> > + dev->hwrng.quality = 1024;
> > +
> > + return devm_hwrng_register(&dev->hdev->dev, &dev->hwrng);
> > +}
> > +
> > +static int u2fzero_fill_in_urb(struct u2fzero_device *dev)
> > +{
> > + struct hid_device *hdev = dev->hdev;
> > + struct usb_device *udev;
> > + struct usbhid_device *usbhid = hdev->driver_data;
> > + unsigned int pipe_in;
> > + struct usb_host_endpoint *ep;
> > +
> > + if (dev->hdev->bus != BUS_USB)
> > + return -EINVAL;
> > +
> > + udev = hid_to_usb_dev(hdev);
> > +
> > + if (!usbhid->urbout || !usbhid->urbin)
> > + return -ENODEV;
> > +
> > + ep = usb_pipe_endpoint(udev, usbhid->urbin->pipe);
> > + if (!ep)
> > + return -ENODEV;
> > +
> > + dev->urb = usb_alloc_urb(0, GFP_KERNEL);
> > + if (!dev->urb)
> > + return -ENOMEM;
> > +
> > + pipe_in = (usbhid->urbin->pipe & ~(3 << 30)) | (PIPE_INTERRUPT << 30);
> > +
> > + usb_fill_int_urb(dev->urb,
> > + udev,
> > + pipe_in,
> > + dev->buf_in,
> > + HID_REPORT_SIZE,
> > + u2fzero_read_callback,
> > + NULL,
> > + ep->desc.bInterval);
> > +
> > + return 0;
> > +}
> > +
> > +static int u2fzero_probe(struct hid_device *hdev,
> > + const struct hid_device_id *id)
> > +{
> > + struct u2fzero_device *dev;
> > + unsigned int minor;
> > + int ret;
> > +
> > + dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
> > + if (dev == NULL)
> > + return -ENOMEM;
> > +
> > + dev->buf_out = devm_kmalloc(&hdev->dev,
> > + sizeof(struct u2f_hid_report), GFP_KERNEL);
> > + if (dev->buf_out == NULL)
> > + return -ENOMEM;
> > +
> > + dev->buf_in = devm_kmalloc(&hdev->dev,
> > + sizeof(struct u2f_hid_msg), GFP_KERNEL);
> > + if (dev->buf_in == NULL)
> > + return -ENOMEM;
> > +
> > + ret = hid_parse(hdev);
> > + if (ret)
> > + return ret;
> > +
> > + dev->hdev = hdev;
> > + hid_set_drvdata(hdev, dev);
> > + mutex_init(&dev->lock);
> > +
> > + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> > + if (ret)
> > + return ret;
> > +
> > + u2fzero_fill_in_urb(dev);
> > +
> > + dev->present = true;
> > +
> > + minor = ((struct hidraw *) hdev->hidraw)->minor;
> > +
> > + ret = u2fzero_init_led(dev, minor);
> > + if (ret) {
> > + hid_hw_stop(hdev);
> > + return ret;
> > + }
> > +
> > + hid_info(hdev, "U2F Zero LED initialised\n");
> > +
> > + ret = u2fzero_init_hwrng(dev, minor);
> > + if (ret) {
> > + hid_hw_stop(hdev);
> > + return ret;
> > + }
> > +
> > + hid_info(hdev, "U2F Zero RNG initialised\n");
> > +
> > + return 0;
> > +}
> > +
> > +static void u2fzero_remove(struct hid_device *hdev)
> > +{
> > + struct u2fzero_device *dev = hid_get_drvdata(hdev);
> > +
> > + mutex_lock(&dev->lock);
> > + dev->present = false;
> > + mutex_unlock(&dev->lock);
> > +
> > + hid_hw_stop(hdev);
> > + usb_poison_urb(dev->urb);
> > + usb_free_urb(dev->urb);
> > +}
> > +
> > +static const struct hid_device_id u2fzero_table[] = {
> > + { HID_USB_DEVICE(USB_VENDOR_ID_CYGNAL,
> > + USB_DEVICE_ID_U2F_ZERO) },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(hid, u2fzero_table);
> > +
> > +static struct hid_driver u2fzero_driver = {
> > + .name = "hid-" DRIVER_SHORT,
> > + .probe = u2fzero_probe,
> > + .remove = u2fzero_remove,
> > + .id_table = u2fzero_table,
> > +};
> > +
> > +module_hid_driver(u2fzero_driver);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Andrej Shadura <[email protected]>");
> > +MODULE_DESCRIPTION("U2F Zero LED and RNG driver");
> >
>
>
> --
> Cheers,
> Andrej
>

--
Jiri Kosina
SUSE Labs

2018-11-15 04:37:25

by Andrej Shadura

[permalink] [raw]
Subject: Re: [PATCH] HID: add driver for U2F Zero built-in LED and RNG

On 12/11/2018 03:17, Jiri Kosina wrote:
> On Thu, 1 Nov 2018, Andrej Shadura wrote:
>
>> Hi everyone,
>>
>> I’ve got a comment from Nick Kossifidis that I probably shouldn’t set
>> RNG’s quality to 1024. Adding linux-crypto@ to the loop.
>
> So, what was this about? Is there any resolution to it? :)

So far, not really. I talked to Keith Packard regarding a similar
setting in his ChaosKey driver, and I understand his opinion is that it
is appropriate there since he’s convinced about the quality of the
hardware he designed. I’m not sure what exactly I should set it to here.

>>
>> On 23/10/2018 16:46, Andrej Shadura wrote:
>>> U2F Zero supports custom commands for blinking the LED and getting data
>>> from the internal hardware RNG. Expose the blinking function as a LED
>>> device, and the internal hardware RNG as an HWRNG so that it can be used
>>> to feed the enthropy pool.
>>>
>>> Signed-off-by: Andrej Shadura <[email protected]>
>>> ---
>>> drivers/hid/Kconfig | 15 ++
>>> drivers/hid/Makefile | 1 +
>>> drivers/hid/hid-ids.h | 1 +
>>> drivers/hid/hid-u2fzero.c | 371 ++++++++++++++++++++++++++++++++++++++
>>> 4 files changed, 388 insertions(+)
>>> create mode 100644 drivers/hid/hid-u2fzero.c
>>>
>>> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>>> index 61e1953ff921..3de6bf202f20 100644
>>> --- a/drivers/hid/Kconfig
>>> +++ b/drivers/hid/Kconfig
>>> @@ -975,6 +975,21 @@ config HID_UDRAW_PS3
>>> Say Y here if you want to use the THQ uDraw gaming tablet for
>>> the PS3.
>>>
>>> +config HID_U2FZERO
>>> + tristate "U2F Zero LED and RNG support"
>>> + depends on HID
>>> + depends on LEDS_CLASS
>>> + help
>>> + Support for the LED of the U2F Zero device.
>>> +
>>> + U2F Zero supports custom commands for blinking the LED
>>> + and getting data from the internal hardware RNG.
>>> + The internal hardware can be used to feed the enthropy pool.
>>> +
>>> + U2F Zero only supports blinking its LED, so this driver doesn't
>>> + allow setting the brightness to anything but 1, which will
>>> + trigger a single blink and immediately reset to back 0.
>>> +
>>> config HID_WACOM
>>> tristate "Wacom Intuos/Graphire tablet support (USB)"
>>> depends on USB_HID
>>> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
>>> index bd7ac53b75c5..617b1a928b6a 100644
>>> --- a/drivers/hid/Makefile
>>> +++ b/drivers/hid/Makefile
>>> @@ -107,6 +107,7 @@ obj-$(CONFIG_HID_THRUSTMASTER) += hid-tmff.o
>>> obj-$(CONFIG_HID_TIVO) += hid-tivo.o
>>> obj-$(CONFIG_HID_TOPSEED) += hid-topseed.o
>>> obj-$(CONFIG_HID_TWINHAN) += hid-twinhan.o
>>> +obj-$(CONFIG_HID_U2FZERO) += hid-u2fzero.o
>>> obj-$(CONFIG_HID_UCLOGIC) += hid-uclogic.o
>>> obj-$(CONFIG_HID_UDRAW_PS3) += hid-udraw-ps3.o
>>> obj-$(CONFIG_HID_LED) += hid-led.o
>>> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
>>> index bc49909aba8e..070a2fcd8a18 100644
>>> --- a/drivers/hid/hid-ids.h
>>> +++ b/drivers/hid/hid-ids.h
>>> @@ -309,6 +309,7 @@
>>> #define USB_DEVICE_ID_CYGNAL_RADIO_SI470X 0x818a
>>> #define USB_DEVICE_ID_FOCALTECH_FTXXXX_MULTITOUCH 0x81b9
>>> #define USB_DEVICE_ID_CYGNAL_CP2112 0xea90
>>> +#define USB_DEVICE_ID_U2F_ZERO 0x8acf
>>>
>>> #define USB_DEVICE_ID_CYGNAL_RADIO_SI4713 0x8244
>>>
>>> diff --git a/drivers/hid/hid-u2fzero.c b/drivers/hid/hid-u2fzero.c
>>> new file mode 100644
>>> index 000000000000..bd4077c93c97
>>> --- /dev/null
>>> +++ b/drivers/hid/hid-u2fzero.c
>>> @@ -0,0 +1,371 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * U2F Zero LED and RNG driver
>>> + *
>>> + * Copyright 2018 Andrej Shadura <[email protected]>
>>> + * Loosely based on drivers/hid/hid-led.c
>>> + * and drivers/usb/misc/chaoskey.c
>>> + *
>>> + * This program is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU General Public License as
>>> + * published by the Free Software Foundation, version 2.
>>> + */
>>> +
>>> +#include <linux/hid.h>
>>> +#include <linux/hidraw.h>
>>> +#include <linux/hw_random.h>
>>> +#include <linux/leds.h>
>>> +#include <linux/module.h>
>>> +#include <linux/mutex.h>
>>> +#include <linux/usb.h>
>>> +
>>> +#include "usbhid/usbhid.h"
>>> +#include "hid-ids.h"
>>> +
>>> +#define DRIVER_SHORT "u2fzero"
>>> +
>>> +#define HID_REPORT_SIZE 64
>>> +
>>> +/* We only use broadcast (CID-less) messages */
>>> +#define CID_BROADCAST 0xffffffff
>>> +
>>> +struct u2f_hid_msg {
>>> + u32 cid;
>>> + union {
>>> + struct {
>>> + u8 cmd;
>>> + u8 bcnth;
>>> + u8 bcntl;
>>> + u8 data[HID_REPORT_SIZE - 7];
>>> + } init;
>>> + struct {
>>> + u8 seq;
>>> + u8 data[HID_REPORT_SIZE - 5];
>>> + } cont;
>>> + };
>>> +} __packed;
>>> +
>>> +struct u2f_hid_report {
>>> + u8 report_type;
>>> + struct u2f_hid_msg msg;
>>> +} __packed;
>>> +
>>> +#define U2F_HID_MSG_LEN(f) (size_t)(((f).init.bcnth << 8) + (f).init.bcntl)
>>> +
>>> +/* Custom extensions to the U2FHID protocol */
>>> +#define U2F_CUSTOM_GET_RNG 0x21
>>> +#define U2F_CUSTOM_WINK 0x24
>>> +
>>> +struct u2fzero_device {
>>> + struct hid_device *hdev;
>>> + struct urb *urb; /* URB for the RNG data */
>>> + struct led_classdev ldev; /* Embedded struct for led */
>>> + struct hwrng hwrng; /* Embedded struct for hwrng */
>>> + char *led_name;
>>> + char *rng_name;
>>> + u8 *buf_out;
>>> + u8 *buf_in;
>>> + struct mutex lock;
>>> + bool present;
>>> +};
>>> +
>>> +static int u2fzero_send(struct u2fzero_device *dev, struct u2f_hid_report *req)
>>> +{
>>> + int ret;
>>> +
>>> + mutex_lock(&dev->lock);
>>> +
>>> + memcpy(dev->buf_out, req, sizeof(struct u2f_hid_report));
>>> +
>>> + ret = hid_hw_output_report(dev->hdev, dev->buf_out,
>>> + sizeof(struct u2f_hid_msg));
>>> +
>>> + mutex_unlock(&dev->lock);
>>> +
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + return ret == sizeof(struct u2f_hid_msg) ? 0 : -EMSGSIZE;
>>> +}
>>> +
>>> +struct u2fzero_transfer_context {
>>> + struct completion done;
>>> + int status;
>>> +};
>>> +
>>> +static void u2fzero_read_callback(struct urb *urb)
>>> +{
>>> + struct u2fzero_transfer_context *ctx = urb->context;
>>> +
>>> + ctx->status = urb->status;
>>> + complete(&ctx->done);
>>> +}
>>> +
>>> +static int u2fzero_recv(struct u2fzero_device *dev,
>>> + struct u2f_hid_report *req,
>>> + struct u2f_hid_msg *resp)
>>> +{
>>> + int ret;
>>> + struct hid_device *hdev = dev->hdev;
>>> + struct u2fzero_transfer_context ctx;
>>> +
>>> + mutex_lock(&dev->lock);
>>> +
>>> + memcpy(dev->buf_out, req, sizeof(struct u2f_hid_report));
>>> +
>>> + dev->urb->context = &ctx;
>>> + init_completion(&ctx.done);
>>> +
>>> + ret = usb_submit_urb(dev->urb, GFP_NOIO);
>>> + if (unlikely(ret)) {
>>> + hid_err(hdev, "usb_submit_urb failed: %d", ret);
>>> + goto err;
>>> + }
>>> +
>>> + ret = hid_hw_output_report(dev->hdev, dev->buf_out,
>>> + sizeof(struct u2f_hid_msg));
>>> +
>>> + if (ret < 0) {
>>> + hid_err(hdev, "hid_hw_output_report failed: %d", ret);
>>> + goto err;
>>> + }
>>> +
>>> + ret = (wait_for_completion_timeout(
>>> + &ctx.done, msecs_to_jiffies(USB_CTRL_SET_TIMEOUT)));
>>> + if (ret < 0) {
>>> + usb_kill_urb(dev->urb);
>>> + hid_err(hdev, "urb submission timed out");
>>> + } else {
>>> + ret = dev->urb->actual_length;
>>> + memcpy(resp, dev->buf_in, ret);
>>> + }
>>> +
>>> +err:
>>> + mutex_unlock(&dev->lock);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int u2fzero_blink(struct led_classdev *ldev)
>>> +{
>>> + struct u2fzero_device *dev = container_of(ldev,
>>> + struct u2fzero_device, ldev);
>>> + struct u2f_hid_report req = {
>>> + .report_type = 0,
>>> + .msg.cid = CID_BROADCAST,
>>> + .msg.init = {
>>> + .cmd = U2F_CUSTOM_WINK,
>>> + .bcnth = 0,
>>> + .bcntl = 0,
>>> + .data = {0},
>>> + }
>>> + };
>>> + return u2fzero_send(dev, &req);
>>> +}
>>> +
>>> +static int u2fzero_brightness_set(struct led_classdev *ldev,
>>> + enum led_brightness brightness)
>>> +{
>>> + ldev->brightness = LED_OFF;
>>> + if (brightness)
>>> + return u2fzero_blink(ldev);
>>> + else
>>> + return 0;
>>> +}
>>> +
>>> +static int u2fzero_rng_read(struct hwrng *rng, void *data,
>>> + size_t max, bool wait)
>>> +{
>>> + struct u2fzero_device *dev = container_of(rng,
>>> + struct u2fzero_device, hwrng);
>>> + struct u2f_hid_report req = {
>>> + .report_type = 0,
>>> + .msg.cid = CID_BROADCAST,
>>> + .msg.init = {
>>> + .cmd = U2F_CUSTOM_GET_RNG,
>>> + .bcnth = 0,
>>> + .bcntl = 0,
>>> + .data = {0},
>>> + }
>>> + };
>>> + struct u2f_hid_msg resp;
>>> + int ret;
>>> + size_t actual_length;
>>> +
>>> + if (!dev->present) {
>>> + hid_dbg(dev->hdev, "device not present");
>>> + return 0;
>>> + }
>>> +
>>> + ret = u2fzero_recv(dev, &req, &resp);
>>> + if (ret < 0)
>>> + return 0;
>>> +
>>> + /* only take the minimum amount of data it is safe to take */
>>> + actual_length = min3((size_t)ret - offsetof(struct u2f_hid_msg,
>>> + init.data), U2F_HID_MSG_LEN(resp), max);
>>> +
>>> + memcpy(data, resp.init.data, actual_length);
>>> +
>>> + return actual_length;
>>> +}
>>> +
>>> +static int u2fzero_init_led(struct u2fzero_device *dev,
>>> + unsigned int minor)
>>> +{
>>> + dev->led_name = devm_kasprintf(&dev->hdev->dev, GFP_KERNEL,
>>> + "%s%u", DRIVER_SHORT, minor);
>>> + if (dev->led_name == NULL)
>>> + return -ENOMEM;
>>> +
>>> + dev->ldev.name = dev->led_name;
>>> + dev->ldev.max_brightness = LED_ON;
>>> + dev->ldev.flags = LED_HW_PLUGGABLE;
>>> + dev->ldev.brightness_set_blocking = u2fzero_brightness_set;
>>> +
>>> + return devm_led_classdev_register(&dev->hdev->dev, &dev->ldev);
>>> +}
>>> +
>>> +static int u2fzero_init_hwrng(struct u2fzero_device *dev,
>>> + unsigned int minor)
>>> +{
>>> + dev->rng_name = devm_kasprintf(&dev->hdev->dev, GFP_KERNEL,
>>> + "%s-rng%u", DRIVER_SHORT, minor);
>>> + if (dev->rng_name == NULL)
>>> + return -ENOMEM;
>>> +
>>> + dev->hwrng.name = dev->rng_name;
>>> + dev->hwrng.read = u2fzero_rng_read;
>>> + dev->hwrng.quality = 1024;
>>> +
>>> + return devm_hwrng_register(&dev->hdev->dev, &dev->hwrng);
>>> +}
>>> +
>>> +static int u2fzero_fill_in_urb(struct u2fzero_device *dev)
>>> +{
>>> + struct hid_device *hdev = dev->hdev;
>>> + struct usb_device *udev;
>>> + struct usbhid_device *usbhid = hdev->driver_data;
>>> + unsigned int pipe_in;
>>> + struct usb_host_endpoint *ep;
>>> +
>>> + if (dev->hdev->bus != BUS_USB)
>>> + return -EINVAL;
>>> +
>>> + udev = hid_to_usb_dev(hdev);
>>> +
>>> + if (!usbhid->urbout || !usbhid->urbin)
>>> + return -ENODEV;
>>> +
>>> + ep = usb_pipe_endpoint(udev, usbhid->urbin->pipe);
>>> + if (!ep)
>>> + return -ENODEV;
>>> +
>>> + dev->urb = usb_alloc_urb(0, GFP_KERNEL);
>>> + if (!dev->urb)
>>> + return -ENOMEM;
>>> +
>>> + pipe_in = (usbhid->urbin->pipe & ~(3 << 30)) | (PIPE_INTERRUPT << 30);
>>> +
>>> + usb_fill_int_urb(dev->urb,
>>> + udev,
>>> + pipe_in,
>>> + dev->buf_in,
>>> + HID_REPORT_SIZE,
>>> + u2fzero_read_callback,
>>> + NULL,
>>> + ep->desc.bInterval);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int u2fzero_probe(struct hid_device *hdev,
>>> + const struct hid_device_id *id)
>>> +{
>>> + struct u2fzero_device *dev;
>>> + unsigned int minor;
>>> + int ret;
>>> +
>>> + dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
>>> + if (dev == NULL)
>>> + return -ENOMEM;
>>> +
>>> + dev->buf_out = devm_kmalloc(&hdev->dev,
>>> + sizeof(struct u2f_hid_report), GFP_KERNEL);
>>> + if (dev->buf_out == NULL)
>>> + return -ENOMEM;
>>> +
>>> + dev->buf_in = devm_kmalloc(&hdev->dev,
>>> + sizeof(struct u2f_hid_msg), GFP_KERNEL);
>>> + if (dev->buf_in == NULL)
>>> + return -ENOMEM;
>>> +
>>> + ret = hid_parse(hdev);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + dev->hdev = hdev;
>>> + hid_set_drvdata(hdev, dev);
>>> + mutex_init(&dev->lock);
>>> +
>>> + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + u2fzero_fill_in_urb(dev);
>>> +
>>> + dev->present = true;
>>> +
>>> + minor = ((struct hidraw *) hdev->hidraw)->minor;
>>> +
>>> + ret = u2fzero_init_led(dev, minor);
>>> + if (ret) {
>>> + hid_hw_stop(hdev);
>>> + return ret;
>>> + }
>>> +
>>> + hid_info(hdev, "U2F Zero LED initialised\n");
>>> +
>>> + ret = u2fzero_init_hwrng(dev, minor);
>>> + if (ret) {
>>> + hid_hw_stop(hdev);
>>> + return ret;
>>> + }
>>> +
>>> + hid_info(hdev, "U2F Zero RNG initialised\n");
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void u2fzero_remove(struct hid_device *hdev)
>>> +{
>>> + struct u2fzero_device *dev = hid_get_drvdata(hdev);
>>> +
>>> + mutex_lock(&dev->lock);
>>> + dev->present = false;
>>> + mutex_unlock(&dev->lock);
>>> +
>>> + hid_hw_stop(hdev);
>>> + usb_poison_urb(dev->urb);
>>> + usb_free_urb(dev->urb);
>>> +}
>>> +
>>> +static const struct hid_device_id u2fzero_table[] = {
>>> + { HID_USB_DEVICE(USB_VENDOR_ID_CYGNAL,
>>> + USB_DEVICE_ID_U2F_ZERO) },
>>> + { }
>>> +};
>>> +MODULE_DEVICE_TABLE(hid, u2fzero_table);
>>> +
>>> +static struct hid_driver u2fzero_driver = {
>>> + .name = "hid-" DRIVER_SHORT,
>>> + .probe = u2fzero_probe,
>>> + .remove = u2fzero_remove,
>>> + .id_table = u2fzero_table,
>>> +};
>>> +
>>> +module_hid_driver(u2fzero_driver);
>>> +
>>> +MODULE_LICENSE("GPL");
>>> +MODULE_AUTHOR("Andrej Shadura <[email protected]>");
>>> +MODULE_DESCRIPTION("U2F Zero LED and RNG driver");
>>>
>>
>>
>> --
>> Cheers,
>> Andrej
>>
>


--
Cheers,
Andrej

2018-11-15 05:13:12

by Andrej Shadura

[permalink] [raw]
Subject: Re: [PATCH] HID: add driver for U2F Zero built-in LED and RNG

On 14/11/2018 10:32, Andrej Shadura wrote:
> On 12/11/2018 03:17, Jiri Kosina wrote:
>> On Thu, 1 Nov 2018, Andrej Shadura wrote:
>>
>>> Hi everyone,
>>>
>>> I’ve got a comment from Nick Kossifidis that I probably shouldn’t set
>>> RNG’s quality to 1024. Adding linux-crypto@ to the loop.
>>
>> So, what was this about? Is there any resolution to it? :)
>
> So far, not really. I talked to Keith Packard regarding a similar
> setting in his ChaosKey driver, and I understand his opinion is that it
> is appropriate there since he’s convinced about the quality of the
> hardware he designed. I’m not sure what exactly I should set it to here.

Just talked to Theodore Ts'o about this, it seems that it doesn’t really
matter that much what to set it to, since this subsystem apparently will
be reworked soon, and setting it to a fair value of 0 will apparently
make it not feed the entropy pool at all, and with a non-zero value only
one device with the highest value will be used. I’m tempted to resubmit
the patch with 0 as the default (as Nick suggested) so that pro users
can toggle it later from the userspace, but it doesn’t have the
opportunity to potentially poison the entropy pool if it’s compromised.

Conor (cc'ed), out of curiosity, could you please post some info on how
the hardware RNG is implemented in U2F Zero?

>>> On 23/10/2018 16:46, Andrej Shadura wrote:
>>>> U2F Zero supports custom commands for blinking the LED and getting data
>>>> from the internal hardware RNG. Expose the blinking function as a LED
>>>> device, and the internal hardware RNG as an HWRNG so that it can be used
>>>> to feed the enthropy pool.
>>>>
>>>> Signed-off-by: Andrej Shadura <[email protected]>
>>>> ---
>>>> drivers/hid/Kconfig | 15 ++
>>>> drivers/hid/Makefile | 1 +
>>>> drivers/hid/hid-ids.h | 1 +
>>>> drivers/hid/hid-u2fzero.c | 371 ++++++++++++++++++++++++++++++++++++++
>>>> 4 files changed, 388 insertions(+)
>>>> create mode 100644 drivers/hid/hid-u2fzero.
--
Cheers,
Andrej

2018-11-17 20:10:07

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [PATCH] HID: add driver for U2F Zero built-in LED and RNG

Στις Τετ, 14 Νοε 2018 στις 8:33 μ.μ., ο/η Andrej Shadura
<[email protected]> έγραψε:
>
> On 12/11/2018 03:17, Jiri Kosina wrote:
> > On Thu, 1 Nov 2018, Andrej Shadura wrote:
> >
> >> Hi everyone,
> >>
> >> I’ve got a comment from Nick Kossifidis that I probably shouldn’t set
> >> RNG’s quality to 1024. Adding linux-crypto@ to the loop.
> >
> > So, what was this about? Is there any resolution to it? :)
>
> So far, not really. I talked to Keith Packard regarding a similar
> setting in his ChaosKey driver, and I understand his opinion is that it
> is appropriate there since he’s convinced about the quality of the
> hardware he designed. I’m not sure what exactly I should set it to here.
>

The issue is not how good the ChaosKey is but how sure he is that what
gets plugged in is indeed a ChaosKey and not something else that e.g.
outputs only 0s. I suggest that all removable hwrngs are zero-credit
by default, those that will use them will most probably be ok with
changing a setting, verifying in a sense that they are aware of what's
plugged in.


--
GPG ID: 0xEE878588
As you read this post global entropy rises. Have Fun ;-)
Nick

2018-11-17 20:36:07

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [PATCH] HID: add driver for U2F Zero built-in LED and RNG

Στις Τετ, 14 Νοε 2018 στις 9:08 μ.μ., ο/η Andrej Shadura
<[email protected]> έγραψε:
>
> On 14/11/2018 10:32, Andrej Shadura wrote:
> > On 12/11/2018 03:17, Jiri Kosina wrote:
> >> On Thu, 1 Nov 2018, Andrej Shadura wrote:
> >>
> >>> Hi everyone,
> >>>
> >>> I’ve got a comment from Nick Kossifidis that I probably shouldn’t set
> >>> RNG’s quality to 1024. Adding linux-crypto@ to the loop.
> >>
> >> So, what was this about? Is there any resolution to it? :)
> >
> > So far, not really. I talked to Keith Packard regarding a similar
> > setting in his ChaosKey driver, and I understand his opinion is that it
> > is appropriate there since he’s convinced about the quality of the
> > hardware he designed. I’m not sure what exactly I should set it to here.
>
> Just talked to Theodore Ts'o about this, it seems that it doesn’t really
> matter that much what to set it to, since this subsystem apparently will
> be reworked soon, and setting it to a fair value of 0 will apparently
> make it not feed the entropy pool at all, and with a non-zero value only
> one device with the highest value will be used. I’m tempted to resubmit
> the patch with 0 as the default (as Nick suggested) so that pro users
> can toggle it later from the userspace, but it doesn’t have the
> opportunity to potentially poison the entropy pool if it’s compromised.
>

I think that's better, good to know the subsystem is being reworked.
BTW I brought up some issues regarding the quality parameter and
how it worked some years ago:
https://www.spinics.net/lists/linux-crypto/msg17476.html

I haven't monitored the progress on this one but I recently found
this patch from Theodore that adds an option to skip the CPU's
built-in hwrng (RDRAND).
https://patchwork.kernel.org/patch/10531149/

Since we allow flexibility for the CPU's build-in hwrng, I believe
it's appropriate to at least allow the same level of flexibility for
removable devices as well, even better if the setting can be
modified at runtime as you mention.

--
GPG ID: 0xEE878588
As you read this post global entropy rises. Have Fun ;-)
Nick