From 7332d2decf819cecc172999de1d8c0d32c18c4e9 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Wed, 28 Apr 2021 14:18:16 -0600 Subject: [PATCH] net/: Add missing packet filtering checks NuttX provides the UDP_BINDTODEVICE socket option. This is a UDP protocol-specific implementation of the semi-standard Linux SO_BINDTODEVICE socket option: "SO_BINDTODEVICE forces packets on the socket to only egress the bound interface, regardless of what the IP routing table would normally choose. Similarly only packets which ingress the bound interface will be received on the socket, packets from other interfaces will not be delivered to the socket." https://codingrelic.geekhold.com/2009/10/code-snippet-sobindtodevice.html If CONFIG_NET_UDP_BINDTODEVICE is selected and a UDP socket is bound to the device, then unrecognized packets UDP packets must not be dropped, but must be forwarded along to the bound socket unconditionally. It the typical case, this should have no impact. It does effect the applications that use DHCP and do select the UDP_BINDTODEVICE socket option. This PR replace existing improper logic in the code and also the improper attempts to fix problems from PR #3601 and PR #3598. Those changes are improper because they expose DHCP appliction dependencies in the OS, breaking modularity and independence of the OS and application. Tested with stm32f4discovery:netnsh with CONFIG_NET_UDP_BINDTODEVICE. A proper DHCP test setup is needed, however. --- include/net/if.h | 6 ++- net/devif/ipv4_input.c | 9 +++- net/devif/ipv6_input.c | 9 +++- net/udp/udp_close.c | 28 ++++++++++++- net/udp/udp_setsockopt.c | 89 ++++++++++++++++++++++++++++++---------- 5 files changed, 115 insertions(+), 26 deletions(-) diff --git a/include/net/if.h b/include/net/if.h index 44f467ae85..f68f9e9f3a 100644 --- a/include/net/if.h +++ b/include/net/if.h @@ -44,6 +44,7 @@ #define IFF_UP (1 << 1) /* Interface is up */ #define IFF_RUNNING (1 << 2) /* Carrier is available */ #define IFF_IPv6 (1 << 3) /* Configured for IPv6 packet (vs ARP or IPv4) */ +#define IFF_BOUND (1 << 4) /* Bound to a socket */ #define IFF_NOARP (1 << 7) /* ARP is not required for this packet */ /* Interface flag helpers */ @@ -51,20 +52,23 @@ #define IFF_SET_DOWN(f) do { (f) |= IFF_DOWN; } while (0) #define IFF_SET_UP(f) do { (f) |= IFF_UP; } while (0) #define IFF_SET_RUNNING(f) do { (f) |= IFF_RUNNING; } while (0) +#define IFF_SET_BOUND(f) do { (f) |= IFF_BOUND; } while (0) #define IFF_SET_NOARP(f) do { (f) |= IFF_NOARP; } while (0) #define IFF_CLR_DOWN(f) do { (f) &= ~IFF_DOWN; } while (0) #define IFF_CLR_UP(f) do { (f) &= ~IFF_UP; } while (0) #define IFF_CLR_RUNNING(f) do { (f) &= ~IFF_RUNNING; } while (0) +#define IFF_CLR_BOUND(f) do { (f) &= ~IFF_BOUND; } while (0) #define IFF_CLR_NOARP(f) do { (f) &= ~IFF_NOARP; } while (0) #define IFF_IS_DOWN(f) (((f) & IFF_DOWN) != 0) #define IFF_IS_UP(f) (((f) & IFF_UP) != 0) #define IFF_IS_RUNNING(f) (((f) & IFF_RUNNING) != 0) +#define IFF_IS_BOUND(f) (((f) & IFF_BOUND) != 0) #define IFF_IS_NOARP(f) (((f) & IFF_NOARP) != 0) /* We only need to manage the IPv6 bit if both IPv6 and IPv4 are supported. - * Otherwise, we can save a few bytes by ignoring it. + * Otherwise, we can save a few bytes by ignoring it. */ #if defined(CONFIG_NET_IPv4) && defined(CONFIG_NET_IPv6) diff --git a/net/devif/ipv4_input.c b/net/devif/ipv4_input.c index e0227a1f99..fa45b930e9 100644 --- a/net/devif/ipv4_input.c +++ b/net/devif/ipv4_input.c @@ -297,7 +297,14 @@ int ipv4_input(FAR struct net_driver_s *dev) } else #endif - if (ipv4->proto != IP_PROTO_UDP) +#if defined(NET_UDP_HAVE_STACK) && defined(CONFIG_NET_UDP_BINDTODEVICE) + /* If the UDP protocol specific socket option UDP_BINDTODEVICE + * is selected, then we must forward all UDP packets to the bound + * socket. + */ + + if (ipv4->proto != IP_PROTO_UDP || !IFF_IS_BOUND(dev->d_flags)) +#endif { /* Not destined for us and not forwardable... Drop the * packet. diff --git a/net/devif/ipv6_input.c b/net/devif/ipv6_input.c index 168cffdbe6..2c42602b37 100644 --- a/net/devif/ipv6_input.c +++ b/net/devif/ipv6_input.c @@ -433,7 +433,14 @@ int ipv6_input(FAR struct net_driver_s *dev) } else #endif - if (nxthdr != IP_PROTO_UDP) +#if defined(NET_UDP_HAVE_STACK) && defined(CONFIG_NET_UDP_BINDTODEVICE) + /* If the UDP protocol specific socket option UDP_BINDTODEVICE + * is selected, then we must forward all UDP packets to the bound + * socket. + */ + + if (nxthdr != IP_PROTO_UDP || !IFF_IS_BOUND(dev->d_flags)) +#endif { /* Not destined for us and not forwardable... * drop the packet. diff --git a/net/udp/udp_close.c b/net/udp/udp_close.c index 2e4722ed15..b2610ce407 100644 --- a/net/udp/udp_close.c +++ b/net/udp/udp_close.c @@ -29,10 +29,14 @@ #include #include +#include + #include +#include #include #include "devif/devif.h" +#include "netdev/netdev.h" #include "udp/udp.h" #include "socket/socket.h" @@ -63,7 +67,7 @@ int udp_close(FAR struct socket *psock) unsigned int timeout = UINT_MAX; int ret; - /* Interrupts are disabled here to avoid race conditions */ + /* Lock the network to avoid race conditions */ net_lock(); @@ -102,6 +106,28 @@ int udp_close(FAR struct socket *psock) nerr("ERROR: udp_txdrain() failed: %d\n", ret); } +#ifdef CONFIG_NET_UDP_BINDTODEVICE + /* Is the socket bound to an interface device */ + + if (conn->boundto != 0) + { + FAR struct net_driver_s *dev; + + /* Yes, get the interface that we are bound do. NULL would indicate + * that the interface no longer exists for some reason. + */ + + dev = netdev_findbyindex(conn->boundto); + if (dev != NULL) + { + /* Clear the interface flag to unbind the device from the socket. + */ + + IFF_CLR_BOUND(dev->d_flags); + } + } +#endif + #ifdef CONFIG_NET_UDP_WRITE_BUFFERS /* Free any semi-permanent write buffer callback in place. */ diff --git a/net/udp/udp_setsockopt.c b/net/udp/udp_setsockopt.c index 880b4d92c9..bd5bd997c8 100644 --- a/net/udp/udp_setsockopt.c +++ b/net/udp/udp_setsockopt.c @@ -29,9 +29,11 @@ #include #include +#include #include #include +#include #include #include "socket/socket.h" @@ -112,31 +114,74 @@ int udp_setsockopt(FAR struct socket *psock, int option, */ case UDP_BINDTODEVICE: /* Bind socket to a specific network device */ - if (value == NULL || value_len == 0 || - (value_len > 0 && ((FAR char *)value)[0] == 0)) - { - conn->boundto = 0; /* This interface is no longer bound */ - ret = OK; - } - else - { - int ifindex; + { + FAR struct net_driver_s *dev; - /* Get the interface index corresponding to the interface name */ + /* Check if we are are unbinding the socket */ - ifindex = netdev_nametoindex(value); - if (ifindex >= 0) - { - DEBUGASSERT(ifindex > 0 && ifindex <= MAX_IFINDEX); - conn->boundto = ifindex; - ret = OK; - } - else - { - ret = ifindex; - } - } + if (value == NULL || value_len == 0 || + (value_len > 0 && ((FAR char *)value)[0] == 0)) + { + /* Just report success if the socket is not bound to an + * interface. + */ + if (conn->boundto != 0) + { + /* Get the interface that we are bound do. NULL would + * indicate that the interface no longer exists for some + * reason. + */ + + dev = netdev_findbyindex(conn->boundto); + if (dev != NULL) + { + /* Clear the interface flag to unbind the device from + * the socket. + */ + + IFF_CLR_BOUND(dev->d_flags); + } + + conn->boundto = 0; /* This interface is no longer bound */ + } + + ret = OK; + } + + /* No, we are binding a socket to the interface. */ + + else + { + /* Find the interface device with this name */ + + dev = netdev_findbyname(value); + if (dev == NULL) + { + ret = -ENODEV; + } + + /* An interface may be bound only to one socket. */ + + else if (IFF_IS_BOUND(dev->d_flags)) + { + ret = -EBUSY; + } + else + { + /* Bind the interface to a socket */ + + IFF_SET_BOUND(dev->d_flags); + + /* Bind the socket to the interface */ + + DEBUGASSERT(dev->d_ifindex > 0 && + dev->d_ifindex <= MAX_IFINDEX); + conn->boundto = dev->d_ifindex; + ret = OK; + } + } + } break; #endif