USB: disable all RNDIS protocol drivers

  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(-)
  

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