On 08/18/2016 10:44 AM, Felipe Balbi wrote: > Hi, > > Binyamin Sharet <s.binyamin@xxxxxxxxx> writes: >> Felipe, Greg, >> >> You wrote pretty much the same things on two separate threads, >> so I will answer only here... >> >> On Tue, Aug 16, 2016 at 1:51 PM, Felipe Balbi >> <felipe.balbi@xxxxxxxxxxxxxxx> wrote: >>> Hi, >>> >>> Binyamin Sharet <s.binyamin@xxxxxxxxx> writes: >>>>>> 2. At least in my case, where I wan't to use gadgetfs for fuzzing >>>>>> other USB hosts, I >>>>>> can't really fuzz various stages of the enumeration phase, >>>>>> specifically in the case of >>>>>> descriptors that are usually requested at least twice (e.g. >>>>>> configuration descriptor) >>>>> you are not allowed to change your descriptors on the fly. Descriptors >>>>> are static. Configuration descriptor is requested twice for a simple >>>>> reason: Host doesn't know how big a configuration you have; so it asks >>>>> only for the first 9 bytes which will contain bLength. Then host uses >>>>> bLength to figure how big is your configuration and subsequently fetches >>>>> the entire thing. >>>>> >>>> I agree that in normal operation I am not allowed to change the descriptors >>>> on the fly, and as to why the configuration descriptor is requested twice. >>>> >>>> However, this is the exact point of fuzzing, looking for issues with descriptor >>>> parsing and ToC/ToU issues related to the assumptions made based on the >>>> first descriptor (e.g. first 9 bytes of the configuration descriptor). >>> That's an assumption mandated by the spec, though. I wouldn't be >>> surprised if some implementations fail this. Have you found any bug so far? >>> >>>>>>>>> Does a patch to switch the gadgetfs module to "delegate all" sounds reasonable? >>>>>>>>> If so - what's the preferred way to do it? I have a few options in mind: >>>>>>>> Why do you need to delegate Get-Descriptor? The contents of the >>>>>>>> response are entirely dictated by the descriptors provided by the user >>>>>>>> program in the first place. >>>>>>>> >>>>>>>> Set-Configuration _is_ delegated to the user program, although the >>>>>>>> program is not allowed to fail the request. Is that what you want to >>>>>>>> do? >>>>>>>> >>>>>>>>> - module parameter >>>>>>>>> - write some command to the ep0 file >>>>>>>>> - send an ioctl to the ep0 file >>>>>>>>> >>>>>>>>> Any other suggestion? >>>>>>>> I suspect this sort of thing would not be accepted. If Felipe agrees, >>>>>>>> you might as well just keep your changes out-of-tree. >>>>>>> This will just open up a can of worms, I'm afraid. What we have today >>>>>>> can even pass all USBCV tests, we're not breaking that, sorry. >>>>>>> >>>>>> I get your point, what I propose is not to change the default behavior >>>>>> of gadgetfs, >>>>>> but allow it to enter to a special mode by the user. I am aware of the >>>>>> issues that it >>>>>> might raise, and understand your concerns. However, I am asking about >>>>>> modifications in a specific, contained context. I would prefer to have it in the >>>>>> mainline kernel, but if you don't think it fits - I will keep those >>>>>> changes as an >>>>>> out-of-tree module. >>>>> you're gonna have a really hard time getting anything in mainline >>>>> without explaining your goal. All you said is: "I wanna do fuzzying", >>>>> but fuzzying of what exactly? Why do you need to fuzz during >>>>> enumeration? This makes no sense. Gadgets are _not_ allowed to change >>>>> their descriptors and kernel doesn't allow userspace to do that. If you >>>>> wanna change your descriptor, then you _must_ disconnect from the host >>>>> and write brand new descriptors during gadgetfs initialization. >>>>> >>>> Sorry if I wasn't clear enough. We maintain a tool called Umap2, its goal is >>>> to perform security assessment of USB hosts. It does so in multiple ways, >>>> and one of them is fuzzing the USB host by sending invalid/unexpected >>>> packets over USB, including packets in the enumeration phase. There could >>>> be multiple issues with USB host stack during enumeration, and the fuzzing >>>> umap2 performs target those issues. >>> have you found any bugs yet? >>> >>>> However, this kind of operation requires a very low level control over the >>>> traffic, and until now it was done using Facedancer, which is a designated >>>> HW. gadgetfs looked to me like the USB equivalent for "raw socket" as a >>>> USB device, so I thought we could use it to enable umap2 on beaglebone >>>> black and similar boards that run Linux instead of Facedancer. >>>> >>>> I hope this is clearer. If you think that there is some other kernel module >>>> that is better suited to this task, I would be happy to hear about it. >>> The kernel is not exactly meant for pentesting USB host stacks :-) >>> >>> Here's what we can do, for now keep an out-of-tree patch. If/when you >>> find actual bugs, then we can reconsider adding this support if, and >>> only if, it's really difficult to enable (for example, it should depend >>> on EXPERT or something along those lines). We don't want this to ever >>> leak to production systems, so we need to make it really annoying to get >>> enabled. >>> >>> Anyway, first things first, let us know if/when you trigger any bugs >>> with this. >>> >>> -- >>> balbi >> Many USB host implementations, including at least older versions of Linux, >> have bugs in the enumeration phase. While I cannot pinpoint a ToC/ToU >> vulnerability in the configuration descriptor at the moment, I found more than >> a couple of issues with configuration descriptor parsing. I will post them here >> soon, hopefully today. >> >> However, just over the last year multiple USB related CVEs in the Linux kernel >> were published (not by me). >> >> Also, while there might not be a specific ToC/ToU bug in configuration >> descriptor >> parsing in Linux at the moment, there might still be in the future, or >> in a different >> operating system, or in a user application that queries those descriptor. >> My goal is to test all those cases, not just the current Linux kernel. > Fair enough, let's just wrap this "forward everything to userspace" with > a Kconfig choice depending on EXPERT so that it doesn't leak to > production kernel builds. > > Thanks > I think this will cause existing implementation over gadgetfs to fail with this special kernel (as now it will delegate everything all of the time). How about using a ioctl to configure it, but wrapping this ioctl with Kconfig? This way gadgetfs will operate as always unless a user activates "delegate all" in runtime. Sounds reasonable? -- Binyamin Sharet, Cisco, STARE-C -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
