On Sun, Nov 2, 2014 at 4:40 PM, Benjamin Tissoires <benjamin.tissoires@xxxxxxxxx> wrote: > On Sun, Nov 2, 2014 at 6:34 PM, Andy Lutomirski <luto@xxxxxxxxxxxxxx> wrote: >> On Sun, Nov 2, 2014 at 3:01 PM, Benjamin Tissoires >> <benjamin.tissoires@xxxxxxxxx> wrote: >>> On Sun, Nov 2, 2014 at 5:49 PM, Andy Lutomirski <luto@xxxxxxxxxxxxxx> wrote: >>>> On Sun, Nov 2, 2014 at 2:45 PM, Benjamin Tissoires >>>> <benjamin.tissoires@xxxxxxxxx> wrote: >>>>> On Sun, Nov 2, 2014 at 4:40 PM, Jiri Kosina <jkosina@xxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
