This fixes a number of harmless but slightly ugly warts in the ARP resolution code: * Use in4addr_any to represent 0.0.0.0 rather than hand constructing an example. * When comparing am->sip against 0.0.0.0 use sizeof(am->sip) instead of sizeof(am->tip) (same value, but makes more logical sense) * Described the guest's assigned address as such, rather than as "our address" - that's not usually what we mean by "our address" these days * Remove "we might have the same IP address" comment which I can't make sense of in context (possibly it's relating to the statement below, which already has its own comment?) Signed-off-by: David Gibson --- arp.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/arp.c b/arp.c index 53334da..d34b20e 100644 --- a/arp.c +++ b/arp.c @@ -59,14 +59,12 @@ int arp(const struct ctx *c, const struct pool *p) ah->ar_op != htons(ARPOP_REQUEST)) return 1; - /* Discard announcements (but not 0.0.0.0 "probes"): we might have the - * same IP address, hide that. - */ - if (memcmp(am->sip, (unsigned char[4]){ 0 }, sizeof(am->tip)) && + /* Discard announcements, but not 0.0.0.0 "probes" */ + if (memcmp(am->sip, &in4addr_any, sizeof(am->sip)) && !memcmp(am->sip, am->tip, sizeof(am->sip))) return 1; - /* Don't resolve our own address, either. */ + /* Don't resolve guest's assigned address, either. */ if (!memcmp(am->tip, &c->ip4.addr, sizeof(am->tip))) return 1; -- 2.47.0

After running dhclient -6 we expect the DHCPv6 assigned address to be immediately usable. That's true with the Fedora dhclient-script (and the upstream ISC DHCP one), however it's not true with the Debian dhclient-script. The Debian script can complete with the address still in "tentative" state, and the address won't be usable until Duplicate Address Detection (DAD) completes. That's arguably a bug in Debian (see link below), but for the time being we need to work around it anyway. We usually get away with this, because by the time we do anything where the address matters, DAD has completed. However, it's not robust, so we should explicitly wait for DAD to complete when we get an DHCPv6 address. Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1085231 Signed-off-by: David Gibson --- test/passt/dhcp | 2 ++ test/pasta/dhcp | 2 ++ test/two_guests/basic | 3 +++ 3 files changed, 7 insertions(+) diff --git a/test/passt/dhcp b/test/passt/dhcp index e05a4bb..9925ab9 100644 --- a/test/passt/dhcp +++ b/test/passt/dhcp @@ -49,6 +49,8 @@ check [ "__SEARCH__" = "__HOST_SEARCH__" ] test DHCPv6: address guest /sbin/dhclient -6 __IFNAME__ +# Wait for DAD to complete +guest while ip -j -6 addr show tentative | jq -e '.[].addr_info'; do sleep 0.1; done gout ADDR6 ip -j -6 addr show|jq -rM '[.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.prefixlen == 128).local] | .[0]' hout HOST_ADDR6 ip -j -6 addr show|jq -rM '[.[] | select(.ifname == "__HOST_IFNAME6__").addr_info[] | select(.scope == "global" and .deprecated != true).local] | .[0]' check [ "__ADDR6__" = "__HOST_ADDR6__" ] diff --git a/test/pasta/dhcp b/test/pasta/dhcp index 41556b8..d4f3ad5 100644 --- a/test/pasta/dhcp +++ b/test/pasta/dhcp @@ -35,6 +35,8 @@ check [ __MTU__ = 65520 ] test DHCPv6: address ns /sbin/dhclient -6 --no-pid __IFNAME__ +# Wait for DAD to complete +ns while ip -j -6 addr show tentative | jq -e '.[].addr_info'; do sleep 0.1; done hout HOST_IFNAME6 ip -j -6 route show|jq -rM '[.[] | select(.dst == "default").dev] | .[0]' nsout ADDR6 ip -j -6 addr show|jq -rM '[.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.prefixlen == 128).local] | .[0]' hout HOST_ADDR6 ip -j -6 addr show|jq -rM '[.[] | select(.ifname == "__HOST_IFNAME6__").addr_info[] | select(.scope == "global" and .deprecated != true).local] | .[0]' diff --git a/test/two_guests/basic b/test/two_guests/basic index ac50ff8..9ba5efe 100644 --- a/test/two_guests/basic +++ b/test/two_guests/basic @@ -40,6 +40,9 @@ guest1 while ip -j -6 addr show tentative | jq -e '.[].addr_info'; do sleep 0.1; guest2 while ip -j -6 addr show tentative | jq -e '.[].addr_info'; do sleep 0.1; done guest1 /sbin/dhclient -6 __IFNAME1__ guest2 /sbin/dhclient -6 __IFNAME2__ +# Wait for DAD to complete on the DHCP address +guest1 while ip -j -6 addr show tentative | jq -e '.[].addr_info'; do sleep 0.1; done +guest2 while ip -j -6 addr show tentative | jq -e '.[].addr_info'; do sleep 0.1; done g1out ADDR1_6 ip -j -6 addr show|jq -rM '[.[] | select(.ifname == "__IFNAME1__").addr_info[] | select(.prefixlen == 128).local] | .[0]' g2out ADDR2_6 ip -j -6 addr show|jq -rM '[.[] | select(.ifname == "__IFNAME2__").addr_info[] | select(.prefixlen == 128).local] | .[0]' hout HOST_ADDR6 ip -j -6 addr show|jq -rM '[.[] | select(.ifname == "__HOST_IFNAME6__").addr_info[] | select(.scope == "global" and .deprecated != true).local] | .[0]' -- 2.47.0

This section didn't mention the effect of the --map-host-loopback option which now alters this behaviour. Update it accordingly. It used "local addresses" to mean specifically 127.0.0.0/8 and ::1. However, "local" could also refer to link-local addresses or to addresses of any scope which happen to be configured on the host. Use "loopback address" to be more precise about this. Signed-off-by: David Gibson --- passt.1 | 54 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/passt.1 b/passt.1 index c573788..46100e2 100644 --- a/passt.1 +++ b/passt.1 @@ -882,38 +882,40 @@ root@localhost's password: .SH NOTES -.SS Handling of traffic with local destination and source addresses - -Both \fBpasst\fR and \fBpasta\fR can bind on ports with a local address, -depending on the configuration. Local destination or source addresses need to be -changed before packets are delivered to the guest or target namespace: most -operating systems would drop packets received from non-loopback interfaces with -local addresses, and it would also be impossible for guest or target namespace -to route answers back. - -For convenience, and somewhat arbitrarily, the source address on these packets -is translated to the address of the default IPv4 or IPv6 gateway (if any) -- -this is known to be an existing, valid address on the same subnet. - -Loopback destination addresses are instead translated to the observed external -address of the guest or target namespace. For IPv6 packets, if usage of a -link-local address by guest or namespace has ever been observed, and the -original destination address is also a link-local address, the observed -link-local address is used. Otherwise, the observed global address is used. For -both IPv4 and IPv6, if no addresses have been seen yet, the configured addresses -will be used instead. +.SS Handling of traffic with loopback destination and source addresses + +Both \fBpasst\fR and \fBpasta\fR can bind on ports with a loopback +address (127.0.0.0/8 or ::1), depending on the configuration. Loopback +destination or source addresses need to be changed before packets are +delivered to the guest or target namespace: most operating systems +would drop packets received with loopback addresses on non-loopback +interfaces, and it would also be impossible for guest or target +namespace to route answers back. + +For convenience, the source address on these packets is translated to +the address specified by the \fB\-\-map-host-loopback\fR option. If +not specified this defaults, somewhat arbitrarily, to the address of +default IPv4 or IPv6 gateway (if any) -- this is known to be an +existing, valid address on the same subnet. If \fB\-\-no-map-gw\fR or +\fB\-\-map-host-loopback none\fR are specified this translation is +disabled and packets with loopback addresses are simply dropped. + +Loopback destination addresses are translated to the observed external +address of the guest or target namespace. For IPv6, the observed +link-local address is used if the translated source address is +link-local, otherwise the observed global address is used. For both +IPv4 and IPv6, if no addresses have been seen yet, the configured +addresses will be used instead. For example, if \fBpasst\fR or \fBpasta\fR receive a connection from 127.0.0.1, with destination 127.0.0.10, and the default IPv4 gateway is 192.0.2.1, while the last observed source address from guest or namespace is 192.0.2.2, this will be translated to a connection from 192.0.2.1 to 192.0.2.2. -Similarly, for traffic coming from guest or namespace, packets with destination -address corresponding to the default gateway will have their destination address -translated to a loopback address, if and only if a packet, in the opposite -direction, with a loopback destination or source address, port-wise matching for -UDP, or connection-wise for TCP, has been recently forwarded to guest or -namespace. This behaviour can be disabled with \-\-no\-map\-gw. +Similarly, for traffic coming from guest or namespace, packets with +destination address corresponding to the \fB\-\-map-host-loopback\fR +address will have their destination address translated to a loopback +address. .SS Handling of local traffic in pasta -- 2.47.0

In pasta mode, where addressing permits we "splice" connections, forwarding directly from host socket to guest/container socket without any L2 or L3 processing. This gives us a very large performance improvement when it's possible. Since the traffic is from a local socket within the guest, it will go over the guest's 'lo' interface, and accordingly we set the guest side address to be the loopback address. However this has a surprising side effect: sometimes guests will run services that are only supposed to be used within the guest and are therefore bound to only 127.0.0.1 and/or ::1. pasta's forwarding exposes those services to the host, which isn't generally what we want. Correct this by instead forwarding inbound "splice" flows to the guest's external address. Link: https://github.com/containers/podman/issues/24045 Signed-off-by: David Gibson --- conf.c | 9 +++++++++ fwd.c | 31 +++++++++++++++++++++++-------- passt.1 | 23 +++++++++++++++++++---- passt.h | 2 ++ 4 files changed, 53 insertions(+), 12 deletions(-) diff --git a/conf.c b/conf.c index c631019..b3b5342 100644 --- a/conf.c +++ b/conf.c @@ -912,6 +912,9 @@ pasta_opts: " -U, --udp-ns SPEC UDP port forwarding to init namespace\n" " SPEC is as described above\n" " default: auto\n" + " --host-lo-to-ns-lo DEPRECATED:\n" + " Translate host-loopback forwards to\n" + " namespace loopback\n" " --userns NSPATH Target user namespace to join\n" " --netns PATH|NAME Target network namespace to join\n" " --netns-only Don't join existing user namespace\n" @@ -1289,6 +1292,7 @@ void conf(struct ctx *c, int argc, char **argv) {"netns-only", no_argument, NULL, 20 }, {"map-host-loopback", required_argument, NULL, 21 }, {"map-guest-addr", required_argument, NULL, 22 }, + {"host-lo-to-ns-lo", no_argument, NULL, 23 }, {"dns-host", required_argument, NULL, 24 }, { 0 }, }; @@ -1467,6 +1471,11 @@ void conf(struct ctx *c, int argc, char **argv) conf_nat(optarg, &c->ip4.map_guest_addr, &c->ip6.map_guest_addr, NULL); break; + case 23: + if (c->mode != MODE_PASTA) + die("--host-lo-to-ns-lo is for pasta mode only"); + c->host_lo_to_ns_lo = 1; + break; case 24: if (inet_pton(AF_INET6, optarg, &c->ip6.dns_host) && !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_host)) diff --git a/fwd.c b/fwd.c index a505098..c71f5e1 100644 --- a/fwd.c +++ b/fwd.c @@ -447,20 +447,35 @@ uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto, (proto == IPPROTO_TCP || proto == IPPROTO_UDP)) { /* spliceable */ - /* Preserve the specific loopback adddress used, but let the - * kernel pick a source port on the target side + /* The traffic will go over the guest's 'lo' interface, but by + * default use its external address, so we don't inadvertently + * expose services that listen only on the guest's loopback + * address. That can be overridden by --host-lo-to-ns-lo which + * will instead forward to the loopback address in the guest. + * + * In either case, let the kernel pick the source address to + * match. */ - tgt->oaddr = ini->eaddr; + if (inany_v4(&ini->eaddr)) { + if (c->host_lo_to_ns_lo) + tgt->eaddr = inany_loopback4; + else + tgt->eaddr = inany_from_v4(c->ip4.addr_seen); + tgt->oaddr = inany_any4; + } else { + if (c->host_lo_to_ns_lo) + tgt->eaddr = inany_loopback6; + else + tgt->eaddr.a6 = c->ip6.addr_seen; + tgt->oaddr = inany_any6; + } + + /* Let the kernel pick source port */ tgt->oport = 0; if (proto == IPPROTO_UDP) /* But for UDP preserve the source port */ tgt->oport = ini->eport; - if (inany_v4(&ini->eaddr)) - tgt->eaddr = inany_loopback4; - else - tgt->eaddr = inany_loopback6; - return PIF_SPLICE; } diff --git a/passt.1 b/passt.1 index 46100e2..f084978 100644 --- a/passt.1 +++ b/passt.1 @@ -605,6 +605,13 @@ Configure UDP port forwarding from target namespace to init namespace. Default is \fBauto\fR. +.TP +.BR \-\-host-lo-to-ns-lo " " (DEPRECATED) +If specified, connections forwarded with \fB\-t\fR and \fB\-u\fR from +the host's loopback address will appear on the loopback address in the +guest as well. Without this option such forwarded packets will appear +to come from the guest's public address. + .TP .BR \-\-userns " " \fIspec Target user namespace to join, as a path. If PID is given, without this option, @@ -893,8 +900,9 @@ interfaces, and it would also be impossible for guest or target namespace to route answers back. For convenience, the source address on these packets is translated to -the address specified by the \fB\-\-map-host-loopback\fR option. If -not specified this defaults, somewhat arbitrarily, to the address of +the address specified by the \fB\-\-map-host-loopback\fR option (with +some exceptions in pasta mode, see next section below). If not +specified this defaults, somewhat arbitrarily, to the address of default IPv4 or IPv6 gateway (if any) -- this is known to be an existing, valid address on the same subnet. If \fB\-\-no-map-gw\fR or \fB\-\-map-host-loopback none\fR are specified this translation is @@ -931,8 +939,15 @@ and the new socket using the \fBsplice\fR(2) system call, and for UDP, a pair of \fBrecvmmsg\fR(2) and \fBsendmmsg\fR(2) system calls deals with packet transfers. -This bypass only applies to local connections and traffic, because it's not -possible to bind sockets to foreign addresses. +Because it's not possible to bind sockets to foreign addresses, this +bypass only applies to local connections and traffic. It also means +that the address translation differs slightly from passt mode. +Connections from loopback to loopback on the host will appear to come +from the target namespace's public address within the guest, unless +\fB\-\-host-lo-to-ns-lo\fR is specified, in which case they will +appear to come from loopback in the namespace as well. The latter +behaviour used to be the default, but is usually undesirable, since it +can unintentionally expose namespace local services to the host. .SS Binding to low numbered ports (well-known or system ports, up to 1023) diff --git a/passt.h b/passt.h index 4908ed9..72c7f72 100644 --- a/passt.h +++ b/passt.h @@ -225,6 +225,7 @@ struct ip6_ctx { * @no_dhcpv6: Disable DHCPv6 server * @no_ndp: Disable NDP handler altogether * @no_ra: Disable router advertisements + * @host_lo_to_ns_lo: Map host loopback addresses to ns loopback addresses * @freebind: Allow binding of non-local addresses for forwarding * @low_wmem: Low probed net.core.wmem_max * @low_rmem: Low probed net.core.rmem_max @@ -285,6 +286,7 @@ struct ctx { int no_dhcpv6; int no_ndp; int no_ra; + int host_lo_to_ns_lo; int freebind; int low_wmem; -- 2.47.0