Hi,

Binyamin Sharet <bsha...@cisco.com> writes:
>>>>>>> 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?

You mean a ep0 ioctl? Makes sense to me. Then we can add more features
later. Perhaps just add a "Get supported features" IOCTL which returns
a struct with 256 bits (4x uint64_t). I doubt we will ever need that
many bits, but better safe than sorry, I guess.

-- 
balbi

Attachment: signature.asc
Description: PGP signature

Reply via email to