- dev - passt

Alas for CAP_NET_BIND_SERVICE
by David Gibson 17 Oct '22

17 Oct '22

Hi Stefano, I've looked deeper into why giving passt/pasta CAP_NET_BIND_SERVICE isn't working, and I'm afraid I have bad news. We lose CAP_NET_BIND_SERVICE in the initial namespace as soon as we unshare() or setns() into the isolated namespace, and this appears to be intended behaviour. From user_namespaces(7), in the Capabilities section: The child process created by clone(2) with the CLONE_NEWUSER flag starts out with a complete set of capabilities in the new user namespace. Likewise, a process that creates a new user namespace using unshare(2) or joins an existing user namespace using setns(2) gains a full set of capabilities in that namespace. ***On the other hand, that process has no capabilities in the parent (in the case of clone(2)) or previous (in the case of unshare(2) and setns(2)) user namespace, even if the new namespace is created or joined by the root user (i.e., a process with user ID 0 in the root namespace).*** Emphasis (***) mine. Basically, despite the way it's phrased in many places, processes don't have an independent set of capabilities in each userns, they only have a set of capabilities in their current userns. Any capabilities in other namespaces are implied in a pretty much all or nothing way - if the process's UID (the real, init ns one) owns the userns (or one of its ancestors), it gets all caps, otherwise none. cap_capable() has the specific logic in the kernel. So, using CAP_NET_BIND_SERVICE isn't compatible with isolating ourselves in our own userns. At the very least "auto" inbound forwarding of low ports is pretty much off the cards. For forwarding of specific low ports, we could delay our entry into the new userns until we've set up the listening sockets, although it does mean rolling back some of the simplification we gained from the new-style userns handling. Or, we could abandon CAP_NET_BIND_SERVICE, and recommend the net.ipv4.ip_unprivileged_port_start sysctl as the only way to handle low ports in passt. I do see a fair bit of logic in that approach: passt has no meaningful way to limit what users do with the low ports it allows them (indirectly) to bind to, giving passt CAP_NET_BIND_SERVICE is pretty much equivalent to giving any process which can invoke passt CAP_NET_BIND_SERVICE. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson

2 11

all pending patches pushed
by Stefano Brivio 15 Oct '22

15 Oct '22

Up to: 31fc8ed passt.1: Add David to AUTHORS -- Stefano

1 0

[PATCH] conf: Bind inbound ports with CAP_NET_BIND_SERVICE before isolate_user()
by Stefano Brivio 14 Oct '22

14 Oct '22

Even if CAP_NET_BIND_SERVICE is granted, we'll lose the capability in the target user namespace as we isolate the process, which means we're unable to bind to low ports at that point. Bind inbound ports, and only those, before isolate_user(). Keep the handling of outbound ports (for pasta mode only) after the setup of the namespace, because that's where we'll bind them. To this end, initialise the netlink socket for the init namespace before isolate_user() as well, as we actually need to know the addresses of the upstream interface before binding ports, in case they're not explicitly passed by the user. As we now call nl_sock_init() twice, checking its return code from conf() twice looks a bit heavy: make it exit(), instead, as we can't do much if we don't have netlink sockets. While at it: - move the v4_only && v6_only options check just after the first option processing loop, as this is more strictly related to option parsing proper - update the man page, explaining that CAP_NET_BIND_SERVICE is *not* the preferred way to bind ports, because passt and pasta can be abused to allow other processes to make effective usage of it. Add a note about the recommended sysctl instead Reported-by: David Gibson <david(a)gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- This is based on David's patchset "Fixes and cleanups for capability handling". conf.c | 71 +++++++++++++++++++++++++++---------------------------- netlink.c | 20 +++++++++------- netlink.h | 2 +- passt.1 | 45 +++++++++++++++++++++++++++++------ tap.c | 1 + 5 files changed, 87 insertions(+), 52 deletions(-) diff --git a/conf.c b/conf.c index a22ef48..e1f42f1 100644 --- a/conf.c +++ b/conf.c @@ -1530,6 +1530,11 @@ void conf(struct ctx *c, int argc, char **argv) } } while (name != -1); + if (v4_only && v6_only) { + err("Options ipv4-only and ipv6-only are mutually exclusive"); + usage(argv[0]); + } + ret = conf_ugid(runas, &uid, &gid); if (ret) usage(argv[0]); @@ -1539,6 +1544,30 @@ void conf(struct ctx *c, int argc, char **argv) logfile, logsize); } + nl_sock_init(c, false); + if (!v6_only) + c->ifi4 = conf_ip4(ifi, &c->ip4, c->mac); + if (!v4_only) + c->ifi6 = conf_ip6(ifi, &c->ip6, c->mac); + if (!c->ifi4 && !c->ifi6) { + err("External interface not usable"); + exit(EXIT_FAILURE); + } + + /* Inbound port options can be parsed now (after IPv4/IPv6 settings) */ + optind = 1; + do { + struct port_fwd *fwd; + + name = getopt_long(argc, argv, optstring, options, NULL); + + if ((name == 't' && (fwd = &c->tcp.fwd_in)) || + (name == 'u' && (fwd = &c->udp.fwd_in.f))) { + if (!optarg || conf_ports(c, name, optarg, fwd)) + usage(argv[0]); + } + } while (name != -1); + if (c->mode == MODE_PASTA) { if (conf_pasta_ns(&netns_only, userns, netns, optind, argc, argv) < 0) @@ -1561,50 +1590,20 @@ void conf(struct ctx *c, int argc, char **argv) } } - if (nl_sock_init(c)) { - err("Failed to get netlink socket"); - exit(EXIT_FAILURE); - } - - if (v4_only && v6_only) { - err("Options ipv4-only and ipv6-only are mutually exclusive"); - usage(argv[0]); - } - if (!v6_only) - c->ifi4 = conf_ip4(ifi, &c->ip4, c->mac); - if (!v4_only) - c->ifi6 = conf_ip6(ifi, &c->ip6, c->mac); - if (!c->ifi4 && !c->ifi6) { - err("External interface not usable"); - exit(EXIT_FAILURE); - } + if (c->mode == MODE_PASTA) + nl_sock_init(c, true); - /* Now we can process port configuration options */ + /* ...and outbound port options now that namespaces are set up. */ optind = 1; do { - struct port_fwd *fwd = NULL; + struct port_fwd *fwd; name = getopt_long(argc, argv, optstring, options, NULL); - switch (name) { - case 't': - case 'u': - case 'T': - case 'U': - if (name == 't') - fwd = &c->tcp.fwd_in; - else if (name == 'T') - fwd = &c->tcp.fwd_out; - else if (name == 'u') - fwd = &c->udp.fwd_in.f; - else if (name == 'U') - fwd = &c->udp.fwd_out.f; + if ((name == 'T' && (fwd = &c->tcp.fwd_out)) || + (name == 'U' && (fwd = &c->udp.fwd_out.f))) { if (!optarg || conf_ports(c, name, optarg, fwd)) usage(argv[0]); - - break; - default: - break; } } while (name != -1); diff --git a/netlink.c b/netlink.c index 6e5a96b..3ee4d42 100644 --- a/netlink.c +++ b/netlink.c @@ -19,6 +19,7 @@ #include <sys/types.h> #include <limits.h> #include <stdlib.h> +#include <stdbool.h> #include <stdint.h> #include <unistd.h> #include <arpa/inet.h> @@ -71,25 +72,28 @@ ns: } /** - * nl_sock_init() - Call nl_sock_init_do() and check for failures + * nl_sock_init() - Call nl_sock_init_do(), won't return on failure * @c: Execution context - * - * Return: -EIO if sockets couldn't be set up, 0 otherwise + * @ns: Get socket in namespace, not in init */ -int nl_sock_init(const struct ctx *c) +void nl_sock_init(const struct ctx *c, bool ns) { - if (c->mode == MODE_PASTA) { + if (c->mode == MODE_PASTA && ns) { NS_CALL(nl_sock_init_do, c); if (nl_sock_ns == -1) - return -EIO; + goto fail; } else { nl_sock_init_do(NULL); } if (nl_sock == -1) - return -EIO; + goto fail; - return 0; + return; + +fail: + err("Failed to get netlink socket"); + exit(EXIT_FAILURE); } /** diff --git a/netlink.h b/netlink.h index 5ce5037..3c991cc 100644 --- a/netlink.h +++ b/netlink.h @@ -6,7 +6,7 @@ #ifndef NETLINK_H #define NETLINK_H -int nl_sock_init(const struct ctx *c); +void nl_sock_init(const struct ctx *c, bool ns); unsigned int nl_get_ext_if(sa_family_t af); void nl_route(int ns, unsigned int ifi, sa_family_t af, void *gw); void nl_addr(int ns, unsigned int ifi, sa_family_t af, diff --git a/passt.1 b/passt.1 index 7d113f2..2cdba5d 100644 --- a/passt.1 +++ b/passt.1 @@ -771,14 +771,45 @@ possible to bind sockets to foreign addresses. .SS Binding to low numbered ports (well-known or system ports, up to 1023) -If the port forwarding configuration requires binding to port numbers lower than -1024, \fBpasst\fR and \fBpasta\fR will try to bind to them, but will fail if not -running as root, or without the \fICAP_NET_BIND_SERVICE\fR Linux capability, see -\fBservices\fR(5) and \fBcapabilities\fR(7). To grant the -\fICAP_NET_BIND_SERVICE\fR capability to passt, you can issue, as root: +If the port forwarding configuration requires binding to ports with numbers +lower than 1024, \fBpasst\fR and \fBpasta\fR will try to bind to them, but will +fail, unless, either: + +.IP \(bu 2 +the \fIsys.net.ipv4.ip_unprivileged_port_start\fR sysctl is set to the number +of the lowest port \fBpasst\fR and \fBpasta\fR need. For example, as root: + +.nf + sysctl -w net.ipv4.ip_unprivileged_port_start=443 +.fi + +\fBNote\fR: this is the recommended way of enabling \fBpasst\fR and \fBpasta\fR +to bind to ports with numbers below 1024. + +.IP \(bu +or the \fICAP_NET_BIND_SERVICE\fR Linux capability is granted, see +\fBservices\fR(5) and \fBcapabilities\fR(7). + +This is, in general, \fBnot the recommended way\fR, because \fBpasst\fR and +\fBpasta\fR might be used as vector to effectively use this capability from +another process. + +However, if your environment is sufficiently controlled by an LSM (Linux +Security Module) such as \fIAppArmor\fR, \fISELinux\fR, \fISmack\fR or +\fITOMOYO\fR, and no other processes can interact in such a way in virtue of +this, granting this capability to \fBpasst\fR and \fBpasta\fR only can +effectively prevent other processes from utilising it. + +Note that this will not work for automatic detection and forwarding of ports +with \fBpasta\fR, because \fBpasta\fR will relinquish this capability at +runtime. + +To grant this capability, you can issue, as root: + +.nf + setcap 'cap_net_bind_service=+ep' $(which passt) +.fi -.RS -setcap 'cap_net_bind_service=+ep' $(which passt) .RE .SS ICMP/ICMPv6 Echo sockets diff --git a/tap.c b/tap.c index 77e513c..8b6d9bc 100644 --- a/tap.c +++ b/tap.c @@ -30,6 +30,7 @@ #include <sys/stat.h> #include <fcntl.h> #include <sys/uio.h> +#include <stdbool.h> #include <stdlib.h> #include <unistd.h> #include <netinet/ip.h> -- 2.35.1

2 3

[PATCH v2 00/11] Fixes and cleanups for capability handling
by David Gibson 14 Oct '22

14 Oct '22

Our current handling of capabilities isn't quite right. In particular, drop_caps() attempts to remove capabilities from the bounding set, which usually won't work, and even if it does won't have the effect we want. This series corrects that, as well as making some other fixes and cleanups in adjacent code. Changes since v1: * A number of minor style fixes * Should now correctly handle the case where we join an existing userns/netns, or just an existing netns (--netns-only) David Gibson (11): test: Move slower tests to end of test run pasta: More general way of starting spawned shell as a login shell pasta_start_ns() always ends in parent context Remove unhelpful drop_caps() call in pasta_start_ns() isolation: Clarify various self-isolation steps Replace FWRITE with a function isolation: Refactor isolate_user() to allow for a common exit path isolation: Replace drop_caps() with a version that actually does something isolation: Prevent any child processes gaining capabilities isolation: Only configure UID/GID mappings in userns when spawning shell Rename pasta_setup_ns() to pasta_spawn_cmd() conf.c | 5 +- isolation.c | 255 +++++++++++++++++++++++++++++++++++++++++++++------- isolation.h | 9 +- passt.c | 8 +- pasta.c | 72 +++++++++------ pasta.h | 3 +- test/run | 20 ++--- util.c | 33 +++++++ util.h | 13 +-- 9 files changed, 324 insertions(+), 94 deletions(-) -- 2.37.3

1 11

[PATCH] passt.1: Add David to AUTHORS
by Stefano Brivio 14 Oct '22

14 Oct '22

I just realised while reading the man page. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- passt.1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/passt.1 b/passt.1 index 2cdba5d..77f130f 100644 --- a/passt.1 +++ b/passt.1 @@ -859,9 +859,9 @@ TCP Selective Acknowledgment (RFC 2018), as well as Protection Against Wrapped Sequences (PAWS) and Round-Trip Time Measurement (RTTM), both described by RFC 7232, are currently not implemented. -.SH AUTHOR +.SH AUTHORS -Stefano Brivio <sbrivio(a)redhat.com> +Stefano Brivio <sbrivio(a)redhat.com>, David Gibson <david(a)gibson.dropbear.id.au>. .SH REPORTING BUGS -- 2.35.1

2 1

[PATCH 00/10] Fixes and cleanups for capability handling
by David Gibson 14 Oct '22

14 Oct '22

Our current handling of capabilities isn't quite right. In particular, drop_caps() attempts to remove capabilities from the bounding set, which usually won't work, and even if it does won't have the effect we want. This series corrects that, as well as making some other fixes and cleanups in adjacent code. David Gibson (10): test: Move slower tests to end of test run pasta: More general way of starting spawned shell as a login shell pasta_start_ns() always ends in parent context Remove unhelpful drop_caps() call in pasta_start_ns() Clarify various self-isolation steps Replace FWRITE with a function isolation: Replace drop_caps() with a version that actually does something isolation: Prevent any child processes gaining capabilities isolation: Only configure UID/GID mappings in userns when spawning shell Rename pasta_setup_ns() to pasta_spawn_cmd() conf.c | 3 +- isolation.c | 199 ++++++++++++++++++++++++++++++++++++++++++++++------ isolation.h | 6 +- passt.c | 8 +-- pasta.c | 72 +++++++++++-------- pasta.h | 3 +- test/run | 20 +++--- util.c | 33 +++++++++ util.h | 13 +--- 9 files changed, 275 insertions(+), 82 deletions(-) -- 2.37.3

2 32

[PATCH v4] conf, tcp, udp: Allow specification of interface to bind to
by Stefano Brivio 13 Oct '22

13 Oct '22

Since kernel version 5.7, commit c427bfec18f2 ("net: core: enable SO_BINDTODEVICE for non-root users"), we can bind sockets to interfaces, if they haven't been bound yet (as in bind()). Introduce an optional interface specification for forwarded ports, prefixed by %, that can be passed together with an address. Reported use case: running local services that use ports we want to have externally forwarded: https://github.com/containers/podman/issues/14425 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v4: - use interface also in exclude specifications (David Gibson) v3: - escape % characters in usage() formatting v2: - fix check on interface name length (spec - ifname, not ifname - buf) conf.c | 31 +++++++++++++++++++++---------- icmp.c | 4 ++-- passt.1 | 12 ++++++++++-- tcp.c | 27 +++++++++++++++------------ tcp.h | 2 +- udp.c | 35 ++++++++++++++++++----------------- udp.h | 2 +- util.c | 19 ++++++++++++++++++- util.h | 3 ++- 9 files changed, 88 insertions(+), 47 deletions(-) diff --git a/conf.c b/conf.c index 779371f..56f33f0 100644 --- a/conf.c +++ b/conf.c @@ -180,8 +180,8 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg, struct port_fwd *fwd) { char addr_buf[sizeof(struct in6_addr)] = { 0 }, *addr = addr_buf; + char buf[BUFSIZ], *spec, *ifname = NULL, *p; uint8_t exclude[PORT_BITMAP_SIZE] = { 0 }; - char buf[BUFSIZ], *spec, *p; sa_family_t af = AF_UNSPEC; bool exclude_only = true; @@ -209,9 +209,9 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg, for (i = 0; i < PORT_EPHEMERAL_MIN; i++) { if (optname == 't') - tcp_sock_init(c, 0, AF_UNSPEC, NULL, i); + tcp_sock_init(c, 0, AF_UNSPEC, NULL, NULL, i); else if (optname == 'u') - udp_sock_init(c, 0, AF_UNSPEC, NULL, i); + udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL, i); } return 0; @@ -231,6 +231,14 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg, if (optname != 't' && optname != 'u') goto bad; + if ((ifname = strchr(buf, '%'))) { + if (spec - ifname >= IFNAMSIZ - 1) + goto bad; + + *ifname = 0; + ifname++; + } + if (inet_pton(AF_INET, buf, addr)) af = AF_INET; else if (inet_pton(AF_INET6, buf, addr)) @@ -278,9 +286,9 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg, bitmap_set(fwd->map, i); if (optname == 't') - tcp_sock_init(c, 0, af, addr, i); + tcp_sock_init(c, 0, af, addr, ifname, i); else if (optname == 'u') - udp_sock_init(c, 0, af, addr, i); + udp_sock_init(c, 0, af, addr, ifname, i); } return 0; @@ -324,9 +332,9 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg, fwd->delta[i] = mapped_range.first - orig_range.first; if (optname == 't') - tcp_sock_init(c, 0, af, addr, i); + tcp_sock_init(c, 0, af, addr, ifname, i); else if (optname == 'u') - udp_sock_init(c, 0, af, addr, i); + udp_sock_init(c, 0, af, addr, ifname, i); } } while ((p = next_chunk(p, ','))); @@ -720,8 +728,9 @@ static void usage(const char *name) info( " 'all': forward all unbound, non-ephemeral ports"); info( " a comma-separated list, optionally ranged with '-'"); info( " and optional target ports after ':', with optional"); - info( " address specification suffixed by '/'. Ranges can be"); - info( " reduced by excluding ports or ranges prefixed by '~'"); + info( " address specification suffixed by '/' and optional"); + info( " interface prefixed by '%%'. Ranges can be reduced by"); + info( " excluding ports or ranges prefixed by '~'"); info( " Examples:"); info( " -t 22 Forward local port 22 to 22 on guest"); info( " -t 22:23 Forward local port 22 to 23 on guest"); @@ -740,6 +749,7 @@ static void usage(const char *name) exit(EXIT_FAILURE); pasta_opts: + info( " -t, --tcp-ports SPEC TCP port forwarding to namespace"); info( " can be specified multiple times"); info( " SPEC can be:"); @@ -747,7 +757,8 @@ pasta_opts: info( " 'auto': forward all ports currently bound in namespace"); info( " a comma-separated list, optionally ranged with '-'"); info( " and optional target ports after ':', with optional"); - info( " address specification suffixed by '/'. Examples:"); + info( " address specification suffixed by '/' and optional"); + info( " interface prefixed by '%%'. Examples:"); info( " -t 22 Forward local port 22 to port 22 in netns"); info( " -t 22:23 Forward local port 22 to port 23"); info( " -t 22,25 Forward ports 22, 25 to ports 22, 25"); diff --git a/icmp.c b/icmp.c index f02f89f..6bd87fd 100644 --- a/icmp.c +++ b/icmp.c @@ -169,7 +169,7 @@ int icmp_tap_handler(const struct ctx *c, int af, const void *addr, iref.icmp.id = id = ntohs(ih->un.echo.id); if ((s = icmp_id_map[V4][id].sock) <= 0) { - s = sock_l4(c, AF_INET, IPPROTO_ICMP, NULL, id, + s = sock_l4(c, AF_INET, IPPROTO_ICMP, NULL, NULL, id, iref.u32); if (s < 0) goto fail_sock; @@ -207,7 +207,7 @@ int icmp_tap_handler(const struct ctx *c, int af, const void *addr, iref.icmp.id = id = ntohs(ih->icmp6_identifier); if ((s = icmp_id_map[V6][id].sock) <= 0) { - s = sock_l4(c, AF_INET6, IPPROTO_ICMPV6, NULL, id, + s = sock_l4(c, AF_INET6, IPPROTO_ICMPV6, NULL, NULL, id, iref.u32); if (s < 0) goto fail_sock; diff --git a/passt.1 b/passt.1 index 555a50c..7d113f2 100644 --- a/passt.1 +++ b/passt.1 @@ -325,7 +325,8 @@ For low (< 1024) ports, see \fBNOTES\fR. .BR ports A comma-separated list of ports, optionally ranged with \fI-\fR, and, optionally, with target ports after \fI:\fR, if they differ. Specific addresses -can be bound as well, separated by \fI/\fR. Within given ranges, selected ports +can be bound as well, separated by \fI/\fR, and also, since Linux 5.7, limited +to specific interfaces, prefixed by \fI%\fR. Within given ranges, selected ports and ranges can be excluded by an additional specification prefixed by \fI~\fR. Specifying excluded ranges only implies that all other ports are forwarded. Examples: @@ -349,6 +350,9 @@ Forward local ports 22 to 80 to corresponding ports on the guest plus 10 -t 192.0.2.1/22 Forward local port 22, bound to 192.0.2.1, to port 22 on the guest .TP +-t 192.0.2.1%eth0/22 +Forward local port 22, bound to 192.0.2.1 and interface eth0, to port 22 +.TP -t 2000-5000,~3000-3010 Forward local ports 2000 to 5000, but not 3000 to 3010 .TP @@ -399,7 +403,8 @@ periodically derived (every second) from listening sockets reported by .BR ports A comma-separated list of ports, optionally ranged with \fI-\fR, and, optionally, with target ports after \fI:\fR, if they differ. Specific addresses -can be bound as well, separated by \fI/\fR. Within given ranges, selected ports +can be bound as well, separated by \fI/\fR, and also, since Linux 5.7, limited +to specific interfaces, prefixed by \fI%\fR. Within given ranges, selected ports and ranges can be excluded by an additional specification prefixed by \fI~\fR. Specifying excluded ranges only implies that all other ports are forwarded. Examples: @@ -424,6 +429,9 @@ namespace -t 192.0.2.1/22 Forward local port 22, bound to 192.0.2.1, to port 22 in the target namespace .TP +-t 192.0.2.1%eth0/22 +Forward local port 22, bound to 192.0.2.1 and interface eth0, to port 22 +.TP -t 2000-5000,~3000-3010 Forward local ports 2000 to 5000, but not 3000 to 3010 .TP diff --git a/tcp.c b/tcp.c index 830dc88..7e82589 100644 --- a/tcp.c +++ b/tcp.c @@ -3078,10 +3078,11 @@ void tcp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events, * @ns: In pasta mode, if set, bind with loopback address in namespace * @af: Address family to select a specific IP version, or AF_UNSPEC * @addr: Pointer to address for binding, NULL if not configured + * @ifname: Name of interface to bind to, NULL if not configured * @port: Port, host order */ void tcp_sock_init(const struct ctx *c, int ns, sa_family_t af, - const void *addr, in_port_t port) + const void *addr, const char *ifname, in_port_t port) { union tcp_epoll_ref tref = { .tcp.listen = 1 }; const void *bind_addr; @@ -3103,8 +3104,8 @@ void tcp_sock_init(const struct ctx *c, int ns, sa_family_t af, tref.tcp.splice = 0; if (!ns) { - s = sock_l4(c, AF_INET, IPPROTO_TCP, bind_addr, port, - tref.u32); + s = sock_l4(c, AF_INET, IPPROTO_TCP, bind_addr, ifname, + port, tref.u32); if (s >= 0) tcp_sock_set_bufsize(c, s); else @@ -3118,8 +3119,8 @@ void tcp_sock_init(const struct ctx *c, int ns, sa_family_t af, bind_addr = &(uint32_t){ htonl(INADDR_LOOPBACK) }; tref.tcp.splice = 1; - s = sock_l4(c, AF_INET, IPPROTO_TCP, bind_addr, port, - tref.u32); + s = sock_l4(c, AF_INET, IPPROTO_TCP, bind_addr, ifname, + port, tref.u32); if (s >= 0) tcp_sock_set_bufsize(c, s); else @@ -3144,8 +3145,8 @@ void tcp_sock_init(const struct ctx *c, int ns, sa_family_t af, tref.tcp.splice = 0; if (!ns) { - s = sock_l4(c, AF_INET6, IPPROTO_TCP, bind_addr, port, - tref.u32); + s = sock_l4(c, AF_INET6, IPPROTO_TCP, bind_addr, ifname, + port, tref.u32); if (s >= 0) tcp_sock_set_bufsize(c, s); else @@ -3159,8 +3160,8 @@ void tcp_sock_init(const struct ctx *c, int ns, sa_family_t af, bind_addr = &in6addr_loopback; tref.tcp.splice = 1; - s = sock_l4(c, AF_INET6, IPPROTO_TCP, bind_addr, port, - tref.u32); + s = sock_l4(c, AF_INET6, IPPROTO_TCP, bind_addr, ifname, + port, tref.u32); if (s >= 0) tcp_sock_set_bufsize(c, s); else @@ -3193,7 +3194,7 @@ static int tcp_sock_init_ns(void *arg) if (!bitmap_isset(c->tcp.fwd_out.map, port)) continue; - tcp_sock_init(c, 1, AF_UNSPEC, NULL, port); + tcp_sock_init(c, 1, AF_UNSPEC, NULL, NULL, port); } return 0; @@ -3410,7 +3411,8 @@ static int tcp_port_rebind(void *arg) if ((a->c->ifi4 && tcp_sock_ns[port][V4] == -1) || (a->c->ifi6 && tcp_sock_ns[port][V6] == -1)) - tcp_sock_init(a->c, 1, AF_UNSPEC, NULL, port); + tcp_sock_init(a->c, 1, AF_UNSPEC, NULL, NULL, + port); } } else { for (port = 0; port < NUM_PORTS; port++) { @@ -3443,7 +3445,8 @@ static int tcp_port_rebind(void *arg) if ((a->c->ifi4 && tcp_sock_init_ext[port][V4] == -1) || (a->c->ifi6 && tcp_sock_init_ext[port][V6] == -1)) - tcp_sock_init(a->c, 0, AF_UNSPEC, NULL, port); + tcp_sock_init(a->c, 0, AF_UNSPEC, NULL, NULL, + port); } } diff --git a/tcp.h b/tcp.h index 2548d4d..7ba7ab7 100644 --- a/tcp.h +++ b/tcp.h @@ -21,7 +21,7 @@ void tcp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events, int tcp_tap_handler(struct ctx *c, int af, const void *addr, const struct pool *p, const struct timespec *now); void tcp_sock_init(const struct ctx *c, int ns, sa_family_t af, - const void *addr, in_port_t port); + const void *addr, const char *ifname, in_port_t port); int tcp_init(struct ctx *c); void tcp_timer(struct ctx *c, const struct timespec *ts); void tcp_defer_handler(struct ctx *c); diff --git a/udp.c b/udp.c index 5422fdd..cac9c65 100644 --- a/udp.c +++ b/udp.c @@ -1005,7 +1005,7 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr, union udp_epoll_ref uref = { .udp.bound = 1, .udp.port = src }; - s = sock_l4(c, AF_INET, IPPROTO_UDP, NULL, src, + s = sock_l4(c, AF_INET, IPPROTO_UDP, NULL, NULL, src, uref.u32); if (s < 0) return p->count; @@ -1057,8 +1057,8 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr, .udp.v6 = 1, .udp.port = src }; - s = sock_l4(c, AF_INET6, IPPROTO_UDP, bind_addr, src, - uref.u32); + s = sock_l4(c, AF_INET6, IPPROTO_UDP, bind_addr, NULL, + src, uref.u32); if (s < 0) return p->count; @@ -1111,10 +1111,11 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr, * @ns: In pasta mode, if set, bind with loopback address in namespace * @af: Address family to select a specific IP version, or AF_UNSPEC * @addr: Pointer to address for binding, NULL if not configured + * @ifname: Name of interface to bind to, NULL if not configured * @port: Port, host order */ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af, - const void *addr, in_port_t port) + const void *addr, const char *ifname, in_port_t port) { union udp_epoll_ref uref = { .udp.bound = 1 }; const void *bind_addr; @@ -1138,8 +1139,8 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af, if (!ns) { uref.udp.splice = 0; - s = sock_l4(c, AF_INET, IPPROTO_UDP, bind_addr, port, - uref.u32); + s = sock_l4(c, AF_INET, IPPROTO_UDP, bind_addr, ifname, + port, uref.u32); udp_tap_map[V4][uref.udp.port].sock = s; } @@ -1148,16 +1149,16 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af, bind_addr = &(uint32_t){ htonl(INADDR_LOOPBACK) }; uref.udp.splice = UDP_TO_NS; - sock_l4(c, AF_INET, IPPROTO_UDP, bind_addr, port, - uref.u32); + sock_l4(c, AF_INET, IPPROTO_UDP, bind_addr, ifname, + port, uref.u32); } if (ns) { bind_addr = &(uint32_t){ htonl(INADDR_LOOPBACK) }; uref.udp.splice = UDP_TO_INIT; - sock_l4(c, AF_INET, IPPROTO_UDP, bind_addr, port, - uref.u32); + sock_l4(c, AF_INET, IPPROTO_UDP, bind_addr, ifname, + port, uref.u32); } } @@ -1171,8 +1172,8 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af, if (!ns) { uref.udp.splice = 0; - s = sock_l4(c, AF_INET6, IPPROTO_UDP, bind_addr, port, - uref.u32); + s = sock_l4(c, AF_INET6, IPPROTO_UDP, bind_addr, ifname, + port, uref.u32); udp_tap_map[V6][uref.udp.port].sock = s; } @@ -1181,16 +1182,16 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af, bind_addr = &in6addr_loopback; uref.udp.splice = UDP_TO_NS; - sock_l4(c, AF_INET6, IPPROTO_UDP, bind_addr, port, - uref.u32); + sock_l4(c, AF_INET6, IPPROTO_UDP, bind_addr, ifname, + port, uref.u32); } if (ns) { bind_addr = &in6addr_loopback; uref.udp.splice = UDP_TO_INIT; - sock_l4(c, AF_INET6, IPPROTO_UDP, bind_addr, port, - uref.u32); + sock_l4(c, AF_INET6, IPPROTO_UDP, bind_addr, ifname, + port, uref.u32); } } } @@ -1213,7 +1214,7 @@ int udp_sock_init_ns(void *arg) if (!bitmap_isset(c->udp.fwd_out.f.map, dst)) continue; - udp_sock_init(c, 1, AF_UNSPEC, NULL, dst); + udp_sock_init(c, 1, AF_UNSPEC, NULL, NULL, dst); } return 0; diff --git a/udp.h b/udp.h index d14df0a..b4ee8b7 100644 --- a/udp.h +++ b/udp.h @@ -13,7 +13,7 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t events, int udp_tap_handler(struct ctx *c, int af, const void *addr, const struct pool *p, const struct timespec *now); void udp_sock_init(const struct ctx *c, int ns, sa_family_t af, - const void *addr, in_port_t port); + const void *addr, const char *ifname, in_port_t port); int udp_init(struct ctx *c); void udp_timer(struct ctx *c, const struct timespec *ts); void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s, diff --git a/util.c b/util.c index 5b1e08a..76057be 100644 --- a/util.c +++ b/util.c @@ -90,13 +90,15 @@ found: * @af: Address family, AF_INET or AF_INET6 * @proto: Protocol number * @bind_addr: Address for binding, NULL for any + * @ifname: Interface for binding, NULL for any * @port: Port, host order * @data: epoll reference portion for protocol handlers * * Return: newly created socket, -1 on error */ int sock_l4(const struct ctx *c, int af, uint8_t proto, - const void *bind_addr, uint16_t port, uint32_t data) + const void *bind_addr, const char *ifname, uint16_t port, + uint32_t data) { union epoll_ref ref = { .r.proto = proto, .r.p.data = data }; struct sockaddr_in addr4 = { @@ -163,6 +165,21 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto, if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &y, sizeof(y))) debug("Failed to set SO_REUSEADDR on socket %i", fd); + if (ifname) { + /* Supported since kernel version 5.7, commit c427bfec18f2 + * ("net: core: enable SO_BINDTODEVICE for non-root users"). If + * it's unsupported, don't bind the socket at all, because the + * user might rely on this to filter incoming connections. + */ + if (setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, + ifname, strlen(ifname))) { + warn("Can't bind socket for %s port %u to %s, closing", + ip_proto_str[proto], port, ifname); + close(fd); + return -1; + } + } + if (bind(fd, sa, sl) < 0) { /* We'll fail to bind to low ports if we don't have enough * capabilities, and we'll fail to bind on already bound ports, diff --git a/util.h b/util.h index f9a8ec6..7dc3d18 100644 --- a/util.h +++ b/util.h @@ -194,7 +194,8 @@ __attribute__ ((weak)) int ffsl(long int i) { return __builtin_ffsl(i); } char *ipv6_l4hdr(const struct pool *p, int index, size_t offset, uint8_t *proto, size_t *dlen); int sock_l4(const struct ctx *c, int af, uint8_t proto, - const void *bind_addr, uint16_t port, uint32_t data); + const void *bind_addr, const char *ifname, uint16_t port, + uint32_t data); void sock_probe_mem(struct ctx *c); int timespec_diff_ms(const struct timespec *a, const struct timespec *b); void bitmap_set(uint8_t *map, int bit); -- 2.35.1

2 1

[PATCH v2 0/3] Fixes for spliced connections
by Stefano Brivio 13 Oct '22

13 Oct '22

The most pressing problem fixed here is that spliced connections with a remapped destination port would be attempted on the wrong side. This is an old issue as indicated by the Fixes: tag. Patch 1/3 restores sanity in comments before we attempt to fix the issue, patch 2/3 fixes the actual issue, and patch 3/3 introduces a minor rework on top to fix another issue, where if the user explicitly configures a loopback address in a port binding we would still create a non-spliced socket stealing the related connections. v2: - rewrite 2/3 with David's idea of using bit in epoll reference - rebase, and use IPV4_IS_LOOPBACK in tcp_conn_from_sock() too Stefano Brivio (3): tcp, tcp_splice: Adjust comments to current meaning of inbound and outbound tcp, tcp_splice: Fix port remapping for inbound, spliced connections Don't create 'tap' socket for ports that are bound to loopback only tcp.c | 192 +++++++++++++++++++++++++++++++-------------------- tcp.h | 2 + tcp_splice.c | 22 +++--- util.h | 3 + 4 files changed, 137 insertions(+), 82 deletions(-) -- 2.35.1

2 4

[PATCH] util.h: Add missing gcc pragma push before pragma pop
by Stefano Brivio 13 Oct '22

13 Oct '22

While building with clang: ./util.h:176:24: warning: pragma diagnostic pop could not pop, no matching push [-Wunknown-pragmas] Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- util.h | 1 + 1 file changed, 1 insertion(+) diff --git a/util.h b/util.h index b1eadfd..4d1b375 100644 --- a/util.h +++ b/util.h @@ -165,6 +165,7 @@ struct ctx; struct ipv6hdr { +#pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wpedantic" #if __BYTE_ORDER == __BIG_ENDIAN uint8_t version:4, -- 2.35.1

2 1

[PATCH] log.h: Avoid unnecessary GNU extension for token pasting
by Stefano Brivio 13 Oct '22

13 Oct '22

clang says: ./log.h:23:18: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ...but we don't actually need token pasting there. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- log.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/log.h b/log.h index f92394c..5abbfc5 100644 --- a/log.h +++ b/log.h @@ -20,7 +20,7 @@ void trace_init(int enable); #define trace(format, ...) \ do { \ if (log_trace) \ - debug(format, ##__VA_ARGS__); \ + debug(format, __VA_ARGS__); \ } while (0) void __openlog(const char *ident, int option, int facility); -- 2.35.1

2 1