| Message ID | 20221123124620.1387499-1-gregkh@linuxfoundation.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | USB: disable all RNDIS protocol drivers | expand |
On Wed, 2022-11-23 at 13:46 +0100, Greg Kroah-Hartman wrote: > The Microsoft RNDIS protocol is, as designed, insecure and vulnerable on > any system that uses it with untrusted hosts or devices. Because the > protocol is impossible to make secure, just disable all rndis drivers to > prevent anyone from using them again. > Not that I mind disabling these, but is there any more detail available on this pretty broad claim? :) johannes
On Wed, Nov 23, 2022 at 03:20:36PM +0100, Johannes Berg wrote: > On Wed, 2022-11-23 at 13:46 +0100, Greg Kroah-Hartman wrote: > > The Microsoft RNDIS protocol is, as designed, insecure and vulnerable on > > any system that uses it with untrusted hosts or devices. Because the > > protocol is impossible to make secure, just disable all rndis drivers to > > prevent anyone from using them again. > > > > Not that I mind disabling these, but is there any more detail available > on this pretty broad claim? :) I don't want to get into specifics in public any more than the above. The protocol was never designed to be used with untrusted devices. It was created, and we implemented support for it, when we trusted USB devices that we plugged into our systems, AND we trusted the systems we plugged our USB devices into. So at the time, it kind of made sense to create this, and the USB protocol class support that replaced it had not yet been released. As designed, it really can not work at all if you do not trust either the host or the device, due to the way the protocol works. And I can't see how it could be fixed if you wish to remain compliant with the protocol (i.e. still work with Windows XP systems.) Today, with untrusted hosts and devices, it's time to just retire this protcol. As I mentioned in the patch comments, Android disabled this many years ago in their devices, with no loss of functionality. thanks, greg k-h
Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes: > The Microsoft RNDIS protocol is, as designed, insecure and vulnerable on > any system that uses it with untrusted hosts or devices. Because the > protocol is impossible to make secure, just disable all rndis drivers to > prevent anyone from using them again. > > Windows only needed this for XP and newer systems, Windows systems older > than that can use the normal USB class protocols instead, which do not > have these problems. > > Android has had this disabled for many years so there should not be any > real systems that still need this. > > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Eric Dumazet <edumazet@google.com> > Cc: Jakub Kicinski <kuba@kernel.org> > Cc: Paolo Abeni <pabeni@redhat.com> > Cc: Kalle Valo <kvalo@kernel.org> > Cc: Oleksij Rempel <linux@rempel-privat.de> > Cc: "Maciej Żenczykowski" <maze@google.com> > Cc: Neil Armstrong <neil.armstrong@linaro.org> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org> > Cc: Andrzej Pietrasiewicz <andrzejtp2010@gmail.com> > Cc: Jacopo Mondi <jacopo@jmondi.org> > Cc: "Łukasz Stelmach" <l.stelmach@samsung.com> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: linux-usb@vger.kernel.org > Cc: netdev@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-wireless@vger.kernel.org > Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com> > Reported-by: Joseph Tartaro <joseph.tartaro@ioactive.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > Note, I'll submit patches removing the individual drivers for later, but > that is more complex as unwinding the interaction between the CDC > networking and RNDIS drivers is tricky. For now, let's just disable all > of this code as it is not secure. > > I can take this through the USB tree if the networking maintainers have > no objection. I thought I had done this months ago, when the last round > of "there are bugs in the protocol!" reports happened at the end of > 2021, but forgot to do so, my fault. > > drivers/net/usb/Kconfig | 1 + > drivers/net/wireless/Kconfig | 1 + For wireless: Acked-by: Kalle Valo <kvalo@kernel.org> Feel free to take this via your tree.
On Wed, 2022-11-23 at 16:05 +0100, Greg Kroah-Hartman wrote: > On Wed, Nov 23, 2022 at 03:20:36PM +0100, Johannes Berg wrote: > > On Wed, 2022-11-23 at 13:46 +0100, Greg Kroah-Hartman wrote: > > > The Microsoft RNDIS protocol is, as designed, insecure and vulnerable on > > > any system that uses it with untrusted hosts or devices. Because the > > > protocol is impossible to make secure, just disable all rndis drivers to > > > prevent anyone from using them again. > > > > > > > Not that I mind disabling these, but is there any more detail available > > on this pretty broad claim? :) > > I don't want to get into specifics in public any more than the above. Fair. > The protocol was never designed to be used with untrusted devices. It > was created, and we implemented support for it, when we trusted USB > devices that we plugged into our systems, AND we trusted the systems we > plugged our USB devices into. So at the time, it kind of made sense to > create this, and the USB protocol class support that replaced it had not > yet been released. > > As designed, it really can not work at all if you do not trust either > the host or the device, due to the way the protocol works. And I can't > see how it could be fixed if you wish to remain compliant with the > protocol (i.e. still work with Windows XP systems.) I guess I just don't see how a USB-based protocol can be fundamentally insecure (to the host), when the host is always in control over messages and parses their content etc.? I can see this with e.g. firewire which must allow DMA access, and now with Thunderbolt we have the same and ended up with boltd, but USB? > Today, with untrusted hosts and devices, it's time to just retire this > protcol. As I mentioned in the patch comments, Android disabled this > many years ago in their devices, with no loss of functionality. I'm not sure Android counts that much, FWIW, at least for WiFi there really is no good reason to plug in a USB WiFi dongle into an Android phone, and quick googling shows that e.g. Android TV may - depending on build - support/permit RNDIS Ethernet? Anyway, there was probably exactly one RNDIS WiFi dongle from Broadcom (for some kind of console IIRC), so it's not a huge loss. Just having issues with the blanket statement that a USB protocol can be designed as inscure :) johannes
On Wed, 23 Nov 2022 13:46:20 +0100 Greg Kroah-Hartman wrote: > I can take this through the USB tree if the networking maintainers have > no objection. Acked-by: Jakub Kicinski <kuba@kernel.org>
On Wed, Nov 23, 2022 at 4:46 AM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > The Microsoft RNDIS protocol is, as designed, insecure and vulnerable on > any system that uses it with untrusted hosts or devices. Because the > protocol is impossible to make secure, just disable all rndis drivers to > prevent anyone from using them again. > > Windows only needed this for XP and newer systems, Windows systems older > than that can use the normal USB class protocols instead, which do not > have these problems. > > Android has had this disabled for many years so there should not be any > real systems that still need this. > > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Eric Dumazet <edumazet@google.com> > Cc: Jakub Kicinski <kuba@kernel.org> > Cc: Paolo Abeni <pabeni@redhat.com> > Cc: Kalle Valo <kvalo@kernel.org> > Cc: Oleksij Rempel <linux@rempel-privat.de> > Cc: "Maciej Żenczykowski" <maze@google.com> > Cc: Neil Armstrong <neil.armstrong@linaro.org> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org> > Cc: Andrzej Pietrasiewicz <andrzejtp2010@gmail.com> > Cc: Jacopo Mondi <jacopo@jmondi.org> > Cc: "Łukasz Stelmach" <l.stelmach@samsung.com> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: linux-usb@vger.kernel.org > Cc: netdev@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-wireless@vger.kernel.org > Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com> > Reported-by: Joseph Tartaro <joseph.tartaro@ioactive.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > Note, I'll submit patches removing the individual drivers for later, but > that is more complex as unwinding the interaction between the CDC > networking and RNDIS drivers is tricky. For now, let's just disable all > of this code as it is not secure. > > I can take this through the USB tree if the networking maintainers have > no objection. I thought I had done this months ago, when the last round > of "there are bugs in the protocol!" reports happened at the end of > 2021, but forgot to do so, my fault. > > drivers/net/usb/Kconfig | 1 + > drivers/net/wireless/Kconfig | 1 + > drivers/usb/gadget/Kconfig | 4 +--- > drivers/usb/gadget/legacy/Kconfig | 3 +++ > 4 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig > index 4402eedb3d1a..83f9c0632642 100644 > --- a/drivers/net/usb/Kconfig > +++ b/drivers/net/usb/Kconfig > @@ -401,6 +401,7 @@ config USB_NET_MCS7830 > config USB_NET_RNDIS_HOST > tristate "Host for RNDIS and ActiveSync devices" > depends on USB_USBNET > + depends on BROKEN > select USB_NET_CDCETHER > help > This option enables hosting "Remote NDIS" USB networking links, NACK. I'm perfectly okay with disabling the gadget (guest/client/device) side rndis drivers. New devices (ie. phones) moving to newer kernels should simply be switching to the NCM gadget drivers. Especially since AFAICT this won't land until 6.2 and thus will presumably not be in the 6.1 LTS and thus won't even end up in next year's Android 14/U, and instead will only be present on the absolutely freshest Android 15/V devices launching near the end of 2024 (or really in early 2025). Additionally the gadget side upstream RNDIS implementation simply isn't used by some chipset vendors - like Qualcomm (which AFAIK uses an out of tree driver to provide rndis gadget with IPA hardware offload acceleration). However, AFAICT this patch is also disabling *HOST* side RNDIS driver support. ie. the RNDIS driver you'd use on a Linux laptop to usb tether off of an Android phone. AFAICT this will break usb tethering off of the *vast* majority of Android phones - likely including most of those currently being manufactured and sold. The only Android phones I'm actually aware of that have switched to NCM instead of RNDIS for usb tethering are Google Pixel 6+ (ie. 6/6pro/6a/7/7pro). Though it's possible there might be some relatively new hardware from other phone vendors that also uses NCM - I don't track this that closely... I do know Android 13/T doesn't require phones to use NCM for tethering, and I've not heard of any plans to change that with Android 14/U either... Note that NCM isn't natively supported by Windows <10 and it required a fair bit of 'guts' on our side to drop support for usb tethering Windows 8.1 devices prior to Win 8.1 EOL (which is only this coming January). Yes, AFAICT, this patch as currently written will break usb tethering off of a Google Pixel ../3/4/5, and I'd assume any and all qualcomm chipset derived devices, etc... ie. most likely the first of these two and possibly the second are required: CONFIG_USB_NET_RNDIS_HOST=m CONFIG_USB_NET_RNDIS_WLAN=m (AFAIK the rndis host side driver is also used by various cell dongles and portable cell hotspots) [I also don't understand the commit description where it talks about Windows XP - how is XP relevant? AFAIK the issue is with Win<10 not WinXP] > diff --git a/drivers/net/wireless/Kconfig b/drivers/net/wireless/Kconfig > index cb1c15012dd0..f162b25123d7 100644 > --- a/drivers/net/wireless/Kconfig > +++ b/drivers/net/wireless/Kconfig > @@ -81,6 +81,7 @@ config USB_NET_RNDIS_WLAN > tristate "Wireless RNDIS USB support" > depends on USB > depends on CFG80211 > + depends on BROKEN > select USB_NET_DRIVERS > select USB_USBNET > select USB_NET_CDCETHER > diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig > index 4fa2ddf322b4..2c99d4313064 100644 > --- a/drivers/usb/gadget/Kconfig > +++ b/drivers/usb/gadget/Kconfig > @@ -183,9 +183,6 @@ config USB_F_EEM > config USB_F_SUBSET > tristate > > -config USB_F_RNDIS > - tristate > - > config USB_F_MASS_STORAGE > tristate > > @@ -297,6 +294,7 @@ config USB_CONFIGFS_RNDIS > bool "RNDIS" > depends on USB_CONFIGFS > depends on NET > + depends on BROKEN > select USB_U_ETHER > select USB_F_RNDIS > help > diff --git a/drivers/usb/gadget/legacy/Kconfig b/drivers/usb/gadget/legacy/Kconfig > index 0a7b382fbe27..03d6da63edf7 100644 > --- a/drivers/usb/gadget/legacy/Kconfig > +++ b/drivers/usb/gadget/legacy/Kconfig > @@ -153,6 +153,7 @@ config USB_ETH > config USB_ETH_RNDIS > bool "RNDIS support" > depends on USB_ETH > + depends on BROKEN > select USB_LIBCOMPOSITE > select USB_F_RNDIS > default y > @@ -247,6 +248,7 @@ config USB_FUNCTIONFS_ETH > config USB_FUNCTIONFS_RNDIS > bool "Include configuration with RNDIS (Ethernet)" > depends on USB_FUNCTIONFS && NET > + depends on BROKEN > select USB_U_ETHER > select USB_F_RNDIS > help > @@ -427,6 +429,7 @@ config USB_G_MULTI > config USB_G_MULTI_RNDIS > bool "RNDIS + CDC Serial + Storage configuration" > depends on USB_G_MULTI > + depends on BROKEN > select USB_F_RNDIS > default y > help > -- > 2.38.1 > Maciej Żenczykowski, Kernel Networking Developer @ Google
On Tue, Jan 10, 2023 at 3:32 PM Johannes Berg <johannes@sipsolutions.net> wrote: > > On Wed, 2022-11-23 at 16:05 +0100, Greg Kroah-Hartman wrote: > > On Wed, Nov 23, 2022 at 03:20:36PM +0100, Johannes Berg wrote: > > > On Wed, 2022-11-23 at 13:46 +0100, Greg Kroah-Hartman wrote: > > > > The Microsoft RNDIS protocol is, as designed, insecure and vulnerable on > > > > any system that uses it with untrusted hosts or devices. Because the > > > > protocol is impossible to make secure, just disable all rndis drivers to > > > > prevent anyone from using them again. > > > > > > > > > > Not that I mind disabling these, but is there any more detail available > > > on this pretty broad claim? :) > > > > I don't want to get into specifics in public any more than the above. > > Fair. I would guess it's related to?: https://github.com/torvalds/linux/commit/c7dd13805f8b8fc1ce3b6d40f6aff47e66b72ad2 > > > The protocol was never designed to be used with untrusted devices. It > > was created, and we implemented support for it, when we trusted USB > > devices that we plugged into our systems, AND we trusted the systems we > > plugged our USB devices into. So at the time, it kind of made sense to > > create this, and the USB protocol class support that replaced it had not > > yet been released. > > > > As designed, it really can not work at all if you do not trust either > > the host or the device, due to the way the protocol works. And I can't > > see how it could be fixed if you wish to remain compliant with the > > protocol (i.e. still work with Windows XP systems.) Can it be fixed in a way that most RNDIS based modems devices like RNDIS based android tethering work with Linux based hosts still? > > I guess I just don't see how a USB-based protocol can be fundamentally > insecure (to the host), when the host is always in control over messages > and parses their content etc.? > > I can see this with e.g. firewire which must allow DMA access, and now > with Thunderbolt we have the same and ended up with boltd, but USB? > > > Today, with untrusted hosts and devices, it's time to just retire this > > protcol. As I mentioned in the patch comments, Android disabled this > > many years ago in their devices, with no loss of functionality. > > I'm not sure Android counts that much, FWIW, at least for WiFi there > really is no good reason to plug in a USB WiFi dongle into an Android > phone, and quick googling shows that e.g. Android TV may - depending on > build - support/permit RNDIS Ethernet? > > Anyway, there was probably exactly one RNDIS WiFi dongle from Broadcom > (for some kind of console IIRC), so it's not a huge loss. Just having > issues with the blanket statement that a USB protocol can be designed as > inscure :) > > johannes >
On Wednesday 2022-11-23 13:46, Greg Kroah-Hartman wrote: > >The Microsoft RNDIS protocol is, as designed, insecure and vulnerable on >any system that uses it with untrusted hosts or devices. Because the >protocol is impossible to make secure, just disable all rndis drivers to >prevent anyone from using them again. > >Windows only needed this for XP and newer systems, Windows systems older >than that can use the normal USB class protocols instead, which do not >have these problems. In other news, someone just proposed adding "RNDIS" things to UEFI, so now the security problem is added right back into machines but at another layer?! https://edk2.groups.io/g/devel/topic/patch_1_3/95531719
On Wed, Jan 11, 2023 at 02:38:04PM +0100, Jan Engelhardt wrote: > > On Wednesday 2022-11-23 13:46, Greg Kroah-Hartman wrote: > > > >The Microsoft RNDIS protocol is, as designed, insecure and vulnerable on > >any system that uses it with untrusted hosts or devices. Because the > >protocol is impossible to make secure, just disable all rndis drivers to > >prevent anyone from using them again. > > > >Windows only needed this for XP and newer systems, Windows systems older > >than that can use the normal USB class protocols instead, which do not > >have these problems. > > > In other news, someone just proposed adding "RNDIS" things to UEFI, so > now the security problem is added right back into machines but at > another layer?! > > https://edk2.groups.io/g/devel/topic/patch_1_3/95531719 I guess systems that use this will always have to trust that the device plugged into them is "trusted". Seems like an easy way to get access to a "locked down" system if you ever need it :) {sigh} greg k-h
Hi all!! I think the rndis_host USB driver might emit a warning in the dmesg, but disabling the driver wouldn't be a good idea. The TP-Link MR6400 V1 LTE modem and also some ZTE modems integrated in routers do use this protocol. We may also distinguish between these cases and devices you might plug in - as they pose different risk levels. Enrico
On Mon, Jul 03, 2023 at 11:11:57PM +0200, Enrico Mioso wrote: > Hi all!! > > I think the rndis_host USB driver might emit a warning in the dmesg, but disabling the driver wouldn't be a good idea. > The TP-Link MR6400 V1 LTE modem and also some ZTE modems integrated in routers do use this protocol. > > We may also distinguish between these cases and devices you might plug in - as they pose different risk levels. Again, you have to fully trust the other side of an RNDIS connection, any hints on how to have the kernel determine that? thanks, greg k-h
On 04.07.23 08:47, Greg Kroah-Hartman wrote: > On Mon, Jul 03, 2023 at 11:11:57PM +0200, Enrico Mioso wrote: >> Hi all!! >> >> I think the rndis_host USB driver might emit a warning in the dmesg, but disabling the driver wouldn't be a good idea. >> The TP-Link MR6400 V1 LTE modem and also some ZTE modems integrated in routers do use this protocol. >> >> We may also distinguish between these cases and devices you might plug in - as they pose different risk levels. > > Again, you have to fully trust the other side of an RNDIS connection, > any hints on how to have the kernel determine that? Greg, it is a network protocol. So this statement is kind of odd. Are you saying that there are RNDIS messages that cannot be verified for some reason, that still cannot be disclosed? Regards Oliver
On Wed, 2023-07-12 at 11:22 +0200, Oliver Neukum wrote: > > On 04.07.23 08:47, Greg Kroah-Hartman wrote: > > On Mon, Jul 03, 2023 at 11:11:57PM +0200, Enrico Mioso wrote: > > > Hi all!! > > > > > > I think the rndis_host USB driver might emit a warning in the dmesg, but disabling the driver wouldn't be a good idea. > > > The TP-Link MR6400 V1 LTE modem and also some ZTE modems integrated in routers do use this protocol. > > > > > > We may also distinguish between these cases and devices you might plug in - as they pose different risk levels. > > > > Again, you have to fully trust the other side of an RNDIS connection, > > any hints on how to have the kernel determine that? > it is a network protocol. So this statement is kind of odd. > Are you saying that there are RNDIS messages that cannot be verified > for some reason, that still cannot be disclosed? Agree, it's also just a USB device, so no special trickery with DMA, shared buffers, etc. I mean, yeah, the RNDIS code is really old and almost certainly has a severe lack of input validation, but that still doesn't mean it's fundamentally impossible. johannes
On Wed, Jul 12, 2023 at 03:00:55PM +0200, Johannes Berg wrote: > On Wed, 2023-07-12 at 11:22 +0200, Oliver Neukum wrote: > > > > On 04.07.23 08:47, Greg Kroah-Hartman wrote: > > > On Mon, Jul 03, 2023 at 11:11:57PM +0200, Enrico Mioso wrote: > > > > Hi all!! > > > > > > > > I think the rndis_host USB driver might emit a warning in the dmesg, but disabling the driver wouldn't be a good idea. > > > > The TP-Link MR6400 V1 LTE modem and also some ZTE modems integrated in routers do use this protocol. > > > > > > > > We may also distinguish between these cases and devices you might plug in - as they pose different risk levels. > > > > > > Again, you have to fully trust the other side of an RNDIS connection, > > > any hints on how to have the kernel determine that? > > > it is a network protocol. So this statement is kind of odd. > > Are you saying that there are RNDIS messages that cannot be verified > > for some reason, that still cannot be disclosed? > > Agree, it's also just a USB device, so no special trickery with DMA, > shared buffers, etc. > > I mean, yeah, the RNDIS code is really old and almost certainly has a > severe lack of input validation, but that still doesn't mean it's > fundamentally impossible. You all are going to make me have to write some exploits aren't you... Ok, I'll put it on my todo list and do it before submitting this patch again. thanks, greg k-h
On Wed, 2023-07-12 at 18:39 +0200, Greg Kroah-Hartman wrote: > On Wed, Jul 12, 2023 at 03:00:55PM +0200, Johannes Berg wrote: > > On Wed, 2023-07-12 at 11:22 +0200, Oliver Neukum wrote: > > > > > > On 04.07.23 08:47, Greg Kroah-Hartman wrote: > > > > On Mon, Jul 03, 2023 at 11:11:57PM +0200, Enrico Mioso wrote: > > > > > Hi all!! > > > > > > > > > > I think the rndis_host USB driver might emit a warning in the dmesg, but disabling the driver wouldn't be a good idea. > > > > > The TP-Link MR6400 V1 LTE modem and also some ZTE modems integrated in routers do use this protocol. > > > > > > > > > > We may also distinguish between these cases and devices you might plug in - as they pose different risk levels. > > > > > > > > Again, you have to fully trust the other side of an RNDIS connection, > > > > any hints on how to have the kernel determine that? > > > > > it is a network protocol. So this statement is kind of odd. > > > Are you saying that there are RNDIS messages that cannot be verified > > > for some reason, that still cannot be disclosed? > > > > Agree, it's also just a USB device, so no special trickery with DMA, > > shared buffers, etc. > > > > I mean, yeah, the RNDIS code is really old and almost certainly has a > > severe lack of input validation, but that still doesn't mean it's > > fundamentally impossible. > > You all are going to make me have to write some exploits aren't you... This is getting a bit childish. Nobody ever said that wasn't possible, in fact I did say exactly above that I'm sure since it's old and all it lacks input validation. So yeah, I full well believe that you can write exploits for it. All we said is that your statement of "RNDIS is fundamentally unfixable" doesn't make a lot of sense. If this were the case, all USB drivers would have to "trust the other side" as well, right? johannes
Em Tue, 4 Jul 2023 07:47:31 +0100 Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu: > On Mon, Jul 03, 2023 at 11:11:57PM +0200, Enrico Mioso wrote: > > Hi all!! > > > > I think the rndis_host USB driver might emit a warning in the dmesg, but disabling the driver wouldn't be a good idea. > > The TP-Link MR6400 V1 LTE modem and also some ZTE modems integrated in routers do use this protocol. > > > > We may also distinguish between these cases and devices you might plug in - as they pose different risk levels. > > Again, you have to fully trust the other side of an RNDIS connection, > any hints on how to have the kernel determine that? Kernel may not know but the user does. See, when doing a security risk assessment, one needs to evaluate the risks, the costs to implement mitigation issues, and the measures that will be taken. Sometimes, the measure is to just accept the risk, as either the chances to actually happen on a particular scenario is very unlikely, and/or the costs to mitigate are too high. In any case, it should not be up to Kernel developers to do risk assessment, as this has to be checked case by case. For instance I usually disable several the security options on my slow test devices, as the risk to run untrusted code on them while I'm testing a new Kernel I just built is close to zero and doesn't pay off the the extra hours I'll be wasting otherwise. In the specific case of untrusted USB devices, the risk of having USB untrusted sticks connected to my desktop machine is very low, and if a criminal breaks into my house to be close enough to plug an USB device, I would have a lot more to be concerned than just my PC. Granted, the risk is higher on laptops and mobile devices, but still it might be acceptable on some use cases. Maybe a compromise would be to add a modprobe parameter and/or a Kconfig option to allow enabling RDNIS host and RDNIS gadget support at the security options to let the user select what kind of risks he's willing to take. Thanks, Mauro
On Thu, Jul 13, 2023 at 02:28:26AM +0200, Johannes Berg wrote: > On Wed, 2023-07-12 at 18:39 +0200, Greg Kroah-Hartman wrote: > > On Wed, Jul 12, 2023 at 03:00:55PM +0200, Johannes Berg wrote: > > > On Wed, 2023-07-12 at 11:22 +0200, Oliver Neukum wrote: > > > > > > > > On 04.07.23 08:47, Greg Kroah-Hartman wrote: > > > > > On Mon, Jul 03, 2023 at 11:11:57PM +0200, Enrico Mioso wrote: > > > > > > Hi all!! > > > > > > > > > > > > I think the rndis_host USB driver might emit a warning in the dmesg, but disabling the driver wouldn't be a good idea. > > > > > > The TP-Link MR6400 V1 LTE modem and also some ZTE modems integrated in routers do use this protocol. > > > > > > > > > > > > We may also distinguish between these cases and devices you might plug in - as they pose different risk levels. > > > > > > > > > > Again, you have to fully trust the other side of an RNDIS connection, > > > > > any hints on how to have the kernel determine that? > > > > > > > it is a network protocol. So this statement is kind of odd. > > > > Are you saying that there are RNDIS messages that cannot be verified > > > > for some reason, that still cannot be disclosed? > > > > > > Agree, it's also just a USB device, so no special trickery with DMA, > > > shared buffers, etc. > > > > > > I mean, yeah, the RNDIS code is really old and almost certainly has a > > > severe lack of input validation, but that still doesn't mean it's > > > fundamentally impossible. > > > > You all are going to make me have to write some exploits aren't you... > > This is getting a bit childish. Nobody ever said that wasn't possible, > in fact I did say exactly above that I'm sure since it's old and all it > lacks input validation. So yeah, I full well believe that you can write > exploits for it. I wasn't trying to be glib here, sorry if it came across that way. I'll blame the heat... > All we said is that your statement of "RNDIS is fundamentally unfixable" > doesn't make a lot of sense. If this were the case, all USB drivers > would have to "trust the other side" as well, right? No, well, yes. See the zillion patches we have had to apply to the kernel over the years when someone decided that "usb devices are not to be trusted" that syzbot has helped find :) It's not a DMA issue here, it's a "the protocol allows for buffer overflows and does not seem to be able to be verified to prevent this" from what I remember (it's been a year since I looked at this last, details are hazy.) At the time, I didn't see a way that it could be fixed, hence this patch. But yes, details matter, again I'll refrain from submitting this change until I have those details. thanks, greg k-h
On 13.07.23 07:34, Greg Kroah-Hartman wrote: > On Thu, Jul 13, 2023 at 02:28:26AM +0200, Johannes Berg wrote: >> On Wed, 2023-07-12 at 18:39 +0200, Greg Kroah-Hartman wrote: Hi, >> All we said is that your statement of "RNDIS is fundamentally unfixable" >> doesn't make a lot of sense. If this were the case, all USB drivers >> would have to "trust the other side" as well, right? > > No, well, yes. See the zillion patches we have had to apply to the > kernel over the years when someone decided that "usb devices are not to > be trusted" that syzbot has helped find :) Well, there are protocols that are in a sense unfixable. Like, hypothetical example, you allow the execution of postscript code. Hence it is kind of important to keep that distinction. Yes, our attitude here is inconsistent. With the advent of Thunderbolt we should have gone through all PCI drivers and audited them for things malicious devices can do. However, we can wait for Pandora for the purpose of this discussion. > It's not a DMA issue here, it's a "the protocol allows for buffer > overflows and does not seem to be able to be verified to prevent this" > from what I remember (it's been a year since I looked at this last, > details are hazy.) At the time, I didn't see a way that it could be > fixed, hence this patch. That makes sort of sense, but still leaves us with the option of verifying each memcopy for being within allowed buffers. Now, by no means let me stop you from getting into your supervillain outfit and write exploits. But just telling us the rest of the issues would do, though not as well. Regards Oliver
I know the NCM protocol a *lot* better than I do RNDIS, but... RNDIS is just passing around chunks of memory (packets with some metadata) over a usb channel. *Any and all* exploits can be fixed - this isn't a complex DMA level HW problem like pcie or firewire. The trouble is finding the problems (ie. the places where input validation is missing or wrong). Indeed if you can write an exploit, it means you understand the problem well enough to fix it, and indeed fixing it is going to be *much* easier than writing the exploit. (the hard part is finding the problems) The (rndis host) code could probably be audited - the protocol is not (afaik) that complex, nor is the driver all that large. I no longer have the email reporting the problems (deleted in a mass inbox zero purge by mistake), but from what I recall at least a few of them should have been fixable by making types unsigned instead of signed and the like. (ie. adding basic checks for whether values are in range) As for things we can do: - I think we can outright delete Linux' RNDIS gadget side code - that should be half the problem. Why? Because Linux/Mac support better protocols (CDC NCM) and Windows 10+ NCM support exists too. (though the windows driver is afaik a little bit buggier than I'd like...) Android devices (phones, etc) that support RNDIS gadget side don't (AFAIK) use the upstream rndis gadget code anyway, they use out-of-tree versions with offload support (at least afaik that's the case for qualcomm chipsets). Devices without hw reasons (offload) to use RNDIS can just switch to NCM. Deleting it in Linux 6.~5+ doesn't affect older Linux versions anyway, so it doesn't affect any older devices... (Though deleting the code does mean we lose the ability to test linux host side with linux gadget side... I guess you can always just use an old kernel (or even just an old phone) on the gadget side to test that combo...) - I think we could change the RNDIS host side driver to be default disabled (or even experimental) However, be aware people (Linux users wanting to usb tether their laptops off of most Android phones out there) will complain if we do this and distros will end up enabling them anyway. What we should really do is just start finding/fixing the bugs in the rndis_host side. It *cannot* be that hard. If someone re-forwards me the kernel-security report, I promise to send back at least a few fixes...
On Thu, 2023-07-13 at 07:34 +0200, Greg Kroah-Hartman wrote: > I wasn't trying to be glib here, sorry if it came across that way. I'll > blame the heat... No worries. > > All we said is that your statement of "RNDIS is fundamentally unfixable" > > doesn't make a lot of sense. If this were the case, all USB drivers > > would have to "trust the other side" as well, right? > > No, well, yes. See the zillion patches we have had to apply to the > kernel over the years when someone decided that "usb devices are not to > be trusted" that syzbot has helped find :) Sure, I'm well aware of that. But that's also exactly my point - nowhere has anyone previously suggested that the protocol for any of those devices is fundamentally broken and the drivers should be removed. We've fixed those things and moved on. I can even understand the initial reaction of "oh hey this ancient thing is probably not used any more, let's just remove it", but even that's a different reasoning, along the lines of "this has bugs and nobody needs it". Though that nobody uses it has in fact been proven wrong, which is pretty much why we're have this discussion at all. > It's not a DMA issue here, it's a "the protocol allows for buffer > overflows and does not seem to be able to be verified to prevent this" > from what I remember (it's been a year since I looked at this last, > details are hazy.) If you s/be able to be verified/be verified in the code/ I entirely believe it, in fact I think it's quite likely given the age of the code and all. It's just that not being _able_ to verify it seems questionable to me (and you haven't given any reasons), given that it's USB and you always have a full buffer in hand when processing it, at a time where the device can no longer modify it (IOW no TOCTTOU issues either.) (As an aside, I've wondered about TOCTTOU with PCI, given that IOMMUs can and will do lazy unmap ... but that's a different discussion.) > At the time, I didn't see a way that it could be > fixed, hence this patch. Yeah I mean, the code isn't great, even if it's not _that_ much, but all the likely() and things in there don't make it easy to read, and the buffer size handling seems not immediately clear to me. So I probably couldn't fix it quickly either, though I haven't even seen the reports. Maciej seems to think it's fixable, at least. And yeah, we'd want to actually review/audit that, I suppose. So if you'd have said something like Let's disable the RNDIS driver(s) because there are known exploits there, nobody really knows how to fix this, and we need a short-term solution until the issues are public and somebody steps up to fix and maintain it. I'd have much less of a problem with that. That's not _great_, but at least it's honest and realistic. That could give us some time and maybe then we can get the bug reports public once it's no longer an immediate threat for all kernels, and go about fixing it with more time, maybe eventually backporting fixes and reverting the disablement etc. I guess this is why secret bug reports suck so much :-) Thanks, johannes
diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig index 4402eedb3d1a..83f9c0632642 100644 --- a/drivers/net/usb/Kconfig +++ b/drivers/net/usb/Kconfig @@ -401,6 +401,7 @@ config USB_NET_MCS7830 config USB_NET_RNDIS_HOST tristate "Host for RNDIS and ActiveSync devices" depends on USB_USBNET + depends on BROKEN select USB_NET_CDCETHER help This option enables hosting "Remote NDIS" USB networking links, diff --git a/drivers/net/wireless/Kconfig b/drivers/net/wireless/Kconfig index cb1c15012dd0..f162b25123d7 100644 --- a/drivers/net/wireless/Kconfig +++ b/drivers/net/wireless/Kconfig @@ -81,6 +81,7 @@ config USB_NET_RNDIS_WLAN tristate "Wireless RNDIS USB support" depends on USB depends on CFG80211 + depends on BROKEN select USB_NET_DRIVERS select USB_USBNET select USB_NET_CDCETHER diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 4fa2ddf322b4..2c99d4313064 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -183,9 +183,6 @@ config USB_F_EEM config USB_F_SUBSET tristate -config USB_F_RNDIS - tristate - config USB_F_MASS_STORAGE tristate @@ -297,6 +294,7 @@ config USB_CONFIGFS_RNDIS bool "RNDIS" depends on USB_CONFIGFS depends on NET + depends on BROKEN select USB_U_ETHER select USB_F_RNDIS help diff --git a/drivers/usb/gadget/legacy/Kconfig b/drivers/usb/gadget/legacy/Kconfig index 0a7b382fbe27..03d6da63edf7 100644 --- a/drivers/usb/gadget/legacy/Kconfig +++ b/drivers/usb/gadget/legacy/Kconfig @@ -153,6 +153,7 @@ config USB_ETH config USB_ETH_RNDIS bool "RNDIS support" depends on USB_ETH + depends on BROKEN select USB_LIBCOMPOSITE select USB_F_RNDIS default y @@ -247,6 +248,7 @@ config USB_FUNCTIONFS_ETH config USB_FUNCTIONFS_RNDIS bool "Include configuration with RNDIS (Ethernet)" depends on USB_FUNCTIONFS && NET + depends on BROKEN select USB_U_ETHER select USB_F_RNDIS help @@ -427,6 +429,7 @@ config USB_G_MULTI config USB_G_MULTI_RNDIS bool "RNDIS + CDC Serial + Storage configuration" depends on USB_G_MULTI + depends on BROKEN select USB_F_RNDIS default y help
The Microsoft RNDIS protocol is, as designed, insecure and vulnerable on any system that uses it with untrusted hosts or devices. Because the protocol is impossible to make secure, just disable all rndis drivers to prevent anyone from using them again. Windows only needed this for XP and newer systems, Windows systems older than that can use the normal USB class protocols instead, which do not have these problems. Android has had this disabled for many years so there should not be any real systems that still need this. Cc: "David S. Miller" <davem@davemloft.net> Cc: Eric Dumazet <edumazet@google.com> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Paolo Abeni <pabeni@redhat.com> Cc: Kalle Valo <kvalo@kernel.org> Cc: Oleksij Rempel <linux@rempel-privat.de> Cc: "Maciej Żenczykowski" <maze@google.com> Cc: Neil Armstrong <neil.armstrong@linaro.org> Cc: Mauro Carvalho Chehab <mchehab@kernel.org> Cc: Andrzej Pietrasiewicz <andrzejtp2010@gmail.com> Cc: Jacopo Mondi <jacopo@jmondi.org> Cc: "Łukasz Stelmach" <l.stelmach@samsung.com> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: linux-usb@vger.kernel.org Cc: netdev@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: linux-wireless@vger.kernel.org Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com> Reported-by: Joseph Tartaro <joseph.tartaro@ioactive.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- Note, I'll submit patches removing the individual drivers for later, but that is more complex as unwinding the interaction between the CDC networking and RNDIS drivers is tricky. For now, let's just disable all of this code as it is not secure. I can take this through the USB tree if the networking maintainers have no objection. I thought I had done this months ago, when the last round of "there are bugs in the protocol!" reports happened at the end of 2021, but forgot to do so, my fault. drivers/net/usb/Kconfig | 1 + drivers/net/wireless/Kconfig | 1 + drivers/usb/gadget/Kconfig | 4 +--- drivers/usb/gadget/legacy/Kconfig | 3 +++ 4 files changed, 6 insertions(+), 3 deletions(-)