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 -- balbi
Attachment:
signature.asc
Description: PGP signature
