[systemd-devel] Supporting U2F over HID on Linux?

[Andy Lutomirski]

On Sun, Nov 2, 2014 at 4:40 PM, Benjamin Tissoires
<benjamin.tissoires at gmail.com> wrote:
> On Sun, Nov 2, 2014 at 6:34 PM, Andy Lutomirski <luto at amacapital.net> wrote:
>> On Sun, Nov 2, 2014 at 3:01 PM, Benjamin Tissoires
>> <benjamin.tissoires at gmail.com> wrote:
>>> On Sun, Nov 2, 2014 at 5:49 PM, Andy Lutomirski <luto at amacapital.net> wrote:
>>>> On Sun, Nov 2, 2014 at 2:45 PM, Benjamin Tissoires
>>>> <benjamin.tissoires at gmail.com> wrote:
>>>>> On Sun, Nov 2, 2014 at 4:40 PM, Jiri Kosina <jkosina at suse.cz> wrote:
>>>>>> On Sun, 2 Nov 2014, Jiri Kosina wrote:
>>>>>>
>>>>>>> Alternatively, you can just write udev rule which triggers on HID devices,
>>>>>>> issues HIDIOCGRDESC ioctl() on the just-created hidraw node, and decides
>>>>>>> afterwards whether node permissions need to be altered ... right?
>>>>>>
>>>>>> Just to make myself clear here -- this is basically alternative to your
>>>>>> 1st option, but for cases where you want to make yourself independent on
>>>>>> sysfs existence (I don't what is the craziness level of setups you are
>>>>>> considering).
>>>>>>
>>>>>
>>>>> I am a little bit concerned with the user space retrieving the
>>>>> descriptor, parsing it and then deciding how to set the permissions.
>>>>> It's not that it is super complex, but it will duplicate the parsing
>>>>> we already do in the kernel. Wacom tried to do that in their usb
>>>>> driver, and they never managed to fully implement one.
>>>>>
>>>>> I think the HID_GROUP solution is super trivial:
>>>>> - add a match on the U2F input report and set the group to
>>>>> HID_GROUP_U2F (2 lines in hid_scan_input_usage() in hid-core.c)
>>>>> - allow hid-generic to also match HID_GROUP_U2F (1 line in hid-generic.c).
>>>>>
>>>>> Then, the udev rule would be super trivial.
>>>>
>>>> I'm confused.  What would the user-visible effect of this be?  Would
>>>> the hidraw node still show up?  What is user code supposed to match to
>>>> detect a U2F device or to otherwise set permissions?
>>>>
>>>
>>> The only change with what we currently have is that the modalias of
>>> the device will be something like
>>> MODALIAS=hid:b0003gHID_GROUP_U2Fv0000XXXXp0000XXXX. (replace
>>> HID_GROUP_U2F with the proper hex value). So matching against this
>>> modalias is trivial, and you can just put the hidraw node rw for
>>> users.
>>
>> Do we really want udev matching against MODALIAS, and do we really
>> want udev rules to hardcode hid group constants?
>
> That's a good question. I don't think the hid group will change in the
> future and we can guarantee that in the kernel I think.
>
>>
>> I'd like this idea better if we added a HID_GROUP uevent property with
>> a textual description, or perhaps something a little more specific.
>
> This refers back to Jiri's first remark. Adding such a thing is
> doable, but do we really want/need it :/

I would tend to think that designing a HID interface for userspace
consumption might be a hint that it's the right time to give it a
better name than "MODALIAS" :)

--Andy

>
>> The hid group field seems to be used for different types of things.
>
> yes, my proposal is definitively a (ugly) hack around the groups which
> are used to select which hid subdriver we use.
>
>
> An other question which comes to my mind is don't we want logind to
> assign the hidraw node to the proper client? We may still need to tag
> the device somehow, but if logind prevents untrusted application to
> run arbitrary code on the u2f token, that might be a little bit safer
> than allowing anybody to read/write.

We do want it to be assigned to the proper client.  My udev patch
(which will almost certainly not be accepted as-is, but it could be
fixed up) uses the report_descriptor to set ID_SECURITY_TOKEN=1, which
in turn causes an existing rule to set TAG+="uaccess", which causes
logind to do its thing.

>
> Sorry to raise more questions than providing answers.

No problem.

--Andy