[PATCH v2 0/2] Allow users to give source address with --gateway also for IPv6

[PATCH v2 0/2] Allow users to give source address with --gateway also for IPv6

[Stefano Brivio]

...just like it happens with IPv4. This offers a solution for
https://bugs.passt.top/show_bug.cgi?id=217, as Podman can specify
--address and --gateway for IPv6, and provide a unique local address,
without the need to introduce other options.

Further options might be more appropriate, strictly speaking, but
as the future perspective is to eventually generalise NAT tables,
it would be nice to avoid them right now.

v2: In 2/2, use the address passed as --gateway as source for inbound
    NAT from the host also when we're not in local mode

Stefano Brivio (2):
  conf: Honour --address, --gateway, --netmask in local mode as well
  conf, fwd: Prefer same-scope address as inbound source address from
    host

 conf.c  | 25 +++++++++++++++++++++----
 fwd.c   |  6 +++++-
 passt.h |  2 ++
 3 files changed, 28 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH v2 1/2] conf: Honour --address, --gateway, --netmask in local mode as well

[Stefano Brivio]

When I implemented local mode in 14b84a7f077e ("treewide: Introduce
'local mode' for disconnected setups"), I didn't consider the
possibility that, also in that case, the user might want to override
addresses, default gateway or netmask, even though I expressly
mentioned this in the man page:

  In this case, **unless configured otherwise**, they will assign the
  IPv4 link-local address 169.254.2.1 to the guest or target
  namespace, and no IPv6 address.

Fix this by checking if an address, gateway, or netmask length was
explicitly set by the user, before overriding them with the default
parameters for local mode.

This might lead to invalid configurations where we won't be able to
set the default gateway passed by the user, but we print a warning
message, and we assume users know what they're doing in that case.

Link: https://bugs.passt.top/show_bug.cgi?id=217
Fixes: 14b84a7f077e ("treewide: Introduce 'local mode' for disconnected setups")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
v2: No changes

 conf.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/conf.c b/conf.c
index 2223604..8205dbf 100644
--- a/conf.c
+++ b/conf.c
@@ -435,9 +435,16 @@ static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4)
  */
 static void conf_ip4_local(struct ip4_ctx *ip4)
 {
-	ip4->addr_seen = ip4->addr = IP4_LL_GUEST_ADDR;
-	ip4->our_tap_addr = ip4->guest_gw = IP4_LL_GUEST_GW;
-	ip4->prefix_len = IP4_LL_PREFIX_LEN;
+	if (IN4_IS_ADDR_UNSPECIFIED(&ip4->addr))
+		ip4->addr = IP4_LL_GUEST_ADDR;
+	ip4->addr_seen = ip4->addr;
+
+	if (IN4_IS_ADDR_UNSPECIFIED(&ip4->guest_gw))
+		ip4->guest_gw = IP4_LL_GUEST_GW;
+	ip4->our_tap_addr = ip4->guest_gw;
+
+	if (!ip4->prefix_len)
+		ip4->prefix_len = IP4_LL_PREFIX_LEN;
 
 	ip4->no_copy_addrs = ip4->no_copy_routes = true;
 }
@@ -497,7 +504,13 @@ static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6)
  */
 static void conf_ip6_local(struct ip6_ctx *ip6)
 {
-	ip6->our_tap_ll = ip6->guest_gw = IP6_LL_GUEST_GW;
+	if (IN6_IS_ADDR_UNSPECIFIED(&ip6->guest_gw))
+		ip6->guest_gw = IP6_LL_GUEST_GW;
+
+	if (IN6_IS_ADDR_LINKLOCAL(&ip6->guest_gw))
+		ip6->our_tap_ll = ip6->guest_gw;
+	else
+		ip6->our_tap_ll = IP6_LL_GUEST_GW;
 
 	ip6->no_copy_addrs = ip6->no_copy_routes = true;
 }
-- 
2.43.0

[PATCH v2 2/2] conf, fwd: Prefer same-scope address as inbound source address from host

[Stefano Brivio]

We might have situations, such as the one described in
https://bugs.passt.top/show_bug.cgi?id=217, where using a link-local
address as source in a given namespace doesn't guarantee that we can
reach the intended destination, because, for instance, the inbound
traffic we forward is in turn forwarded to a different interface, such
as a bridge.

In that case, the assumption from 9618d247006a ("ndp, dhcpv6, tcp,
udp: Always use link-local as source if gateway isn't") isn't a safe
one: the user might have specified a valid gateway address, matching
the scope of the destination address, but we won't use it as address
of last resort, and prefer a link-local address with a mismatch in
scope instead.

So, if the user specifies a given default gateway address for IPv6,
note that as 'our_tap_addr', like we would do with with IPv4, and
stick to Rule 2 of RFC 6724, Section 5, when selecting a source
address, by preferring an address with the same scope, if available.

Note that this preference will be applied only if, at the point of
the source address selection, the destination address is already
known, which, in general, only happens if there's an explicit mapping
rule specifying the target destination address.

Otherwise, if there's no explicit rule picking the destination
address, we select the destination address after selecting the source
address. This should probably be improved at some point, to select
matching address pairs (when a compatible one is available) instead of
sequentially picking source and destination addresses. That appears to
be beyond the scope of this patch, though.

Reported-by: Paul Holzinger <pholzing@redhat.com>
Link: https://bugs.passt.top/show_bug.cgi?id=217
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
v2: Use the address passed as --gateway as source for inbound NAT from
the host regardless of local mode, see:

  https://bugs.passt.top/show_bug.cgi?id=217#c5

 conf.c  | 8 ++++++--
 fwd.c   | 6 +++++-
 passt.h | 2 ++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/conf.c b/conf.c
index 8205dbf..faf2681 100644
--- a/conf.c
+++ b/conf.c
@@ -490,6 +490,8 @@ static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6)
 
 	if (IN6_IS_ADDR_LINKLOCAL(&ip6->guest_gw))
 		ip6->our_tap_ll = ip6->guest_gw;
+	else
+		ip6->our_tap_addr = ip6->guest_gw;
 
 	if (IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) ||
 	    IN6_IS_ADDR_UNSPECIFIED(&ip6->our_tap_ll))
@@ -507,10 +509,12 @@ static void conf_ip6_local(struct ip6_ctx *ip6)
 	if (IN6_IS_ADDR_UNSPECIFIED(&ip6->guest_gw))
 		ip6->guest_gw = IP6_LL_GUEST_GW;
 
-	if (IN6_IS_ADDR_LINKLOCAL(&ip6->guest_gw))
+	if (IN6_IS_ADDR_LINKLOCAL(&ip6->guest_gw)) {
 		ip6->our_tap_ll = ip6->guest_gw;
-	else
+	} else {
+		ip6->our_tap_addr = ip6->guest_gw;
 		ip6->our_tap_ll = IP6_LL_GUEST_GW;
+	}
 
 	ip6->no_copy_addrs = ip6->no_copy_routes = true;
 }
diff --git a/fwd.c b/fwd.c
index 7152169..4ba0af3 100644
--- a/fwd.c
+++ b/fwd.c
@@ -1090,7 +1090,11 @@ uint8_t fwd_nat_from_host(const struct ctx *c,
 				return PIF_NONE;
 			tgt->oaddr = inany_from_v4(c->ip4.our_tap_addr);
 		} else {
-			tgt->oaddr.a6 = c->ip6.our_tap_ll;
+			if (inany_is_linklocal6(&tgt->eaddr) ||
+			    IN6_IS_ADDR_UNSPECIFIED(&c->ip6.our_tap_addr))
+				tgt->oaddr.a6 = c->ip6.our_tap_ll;
+			else
+				tgt->oaddr.a6 = c->ip6.our_tap_addr;
 		}
 	}
 	tgt->oport = ini->eport;
diff --git a/passt.h b/passt.h
index a61baca..51ccd4f 100644
--- a/passt.h
+++ b/passt.h
@@ -121,6 +121,7 @@ struct ip4_ctx {
  * @dns:		DNS addresses for DHCPv6 and NDP
  * @dns_match:		Forward DNS query if sent to this address
  * @our_tap_ll:		Link-local IPv6 address for passt's use on tap
+ * @our_tap_addr:	Non-LL IPv6 address for passt's use on tap (if any)
  * @dns_host:		Use this DNS on the host for forwarding
  * @addr_out:		Optional source address for outbound traffic
  * @ifname_out:		Optional interface name to bind outbound sockets to
@@ -140,6 +141,7 @@ struct ip6_ctx {
 	struct in6_addr dns[MAXNS];
 	struct in6_addr dns_match;
 	struct in6_addr our_tap_ll;
+	struct in6_addr our_tap_addr;
 
 	/* PIF_HOST addresses */
 	struct in6_addr dns_host;
-- 
2.43.0