[PATCH 0/2] Fix --dns-forward with --no-map-gw

[PATCH 0/2] Fix --dns-forward with --no-map-gw

[Stefano Brivio]

Patch 2/2 is the actual fix, patch 1/2 makes it less awkward.

Stefano Brivio (2):
  conf: Split add_dns_resolv() into separate IPv4 and IPv6 versions
  conf: Honour --dns-forward for local resolver even with --no-map-gw

 conf.c | 115 +++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 74 insertions(+), 41 deletions(-)

-- 
2.43.0

[PATCH 1/2] conf: Split add_dns_resolv() into separate IPv4 and IPv6 versions

[Stefano Brivio]

Not really valuable by itself, but dropping one level of nested blocks
makes the next change more convenient.

No functional changes intended.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 conf.c | 101 ++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 60 insertions(+), 41 deletions(-)

diff --git a/conf.c b/conf.c
index 168646f..18ed11c 100644
--- a/conf.c
+++ b/conf.c
@@ -414,6 +414,62 @@ static unsigned add_dns6(struct ctx *c, const struct in6_addr *addr,
 	return 1;
 }
 
+/**
+ * add_dns_resolv4() - Possibly add one IPv4 nameserver from host's resolv.conf
+ * @c:		Execution context
+ * @ns:		Nameserver address
+ * @idx:	Pointer to index of current IPv4 resolver entry, set on return
+ */
+static void add_dns_resolv4(struct ctx *c, struct in_addr *ns, unsigned *idx)
+{
+	if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_host))
+		c->ip4.dns_host = *ns;
+
+	/* Special handling if guest or container can only access local
+	 * addresses via redirect, or if the host gateway is also a resolver and
+	 * we shadow its address
+	 */
+	if (IN4_IS_ADDR_LOOPBACK(ns) ||
+	    IN4_ARE_ADDR_EQUAL(ns, &c->ip4.map_host_loopback)) {
+		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_host_loopback))
+			return;
+
+		*ns = c->ip4.map_host_loopback;
+		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match))
+			c->ip4.dns_match = c->ip4.map_host_loopback;
+	}
+
+	*idx += add_dns4(c, ns, *idx);
+}
+
+/**
+ * add_dns_resolv6() - Possibly add one IPv6 nameserver from host's resolv.conf
+ * @c:		Execution context
+ * @ns:		Nameserver address
+ * @idx:	Pointer to index of current IPv6 resolver entry, set on return
+ */
+static void add_dns_resolv6(struct ctx *c, struct in6_addr *ns, unsigned *idx)
+{
+	if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_host))
+		c->ip6.dns_host = *ns;
+
+	/* Special handling if guest or container can only access local
+	 * addresses via redirect, or if the host gateway is also a resolver and
+	 * we shadow its address
+	 */
+	if (IN6_IS_ADDR_LOOPBACK(ns) ||
+	    IN6_ARE_ADDR_EQUAL(ns, &c->ip6.map_host_loopback)) {
+		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback))
+			return;
+
+		*ns = c->ip6.map_host_loopback;
+		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match))
+			c->ip6.dns_match = c->ip6.map_host_loopback;
+	}
+
+	*idx += add_dns6(c, ns, *idx);
+}
+
 /**
  * add_dns_resolv() - Possibly add ns from host resolv.conf to configuration
  * @c:		Execution context
@@ -430,48 +486,11 @@ static void add_dns_resolv(struct ctx *c, const char *nameserver,
 	struct in6_addr ns6;
 	struct in_addr ns4;
 
-	if (idx4 && inet_pton(AF_INET, nameserver, &ns4)) {
-		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_host))
-			c->ip4.dns_host = ns4;
-
-		/* Special handling if guest or container can only access local
-		 * addresses via redirect, or if the host gateway is also a
-		 * resolver and we shadow its address
-		 */
-		if (IN4_IS_ADDR_LOOPBACK(&ns4) ||
-		    IN4_ARE_ADDR_EQUAL(&ns4, &c->ip4.map_host_loopback)) {
-			if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_host_loopback))
-				return;
-
-			ns4 = c->ip4.map_host_loopback;
-			if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match))
-				c->ip4.dns_match = c->ip4.map_host_loopback;
-		}
-
-		*idx4 += add_dns4(c, &ns4, *idx4);
-	}
-
-	if (idx6 && inet_pton(AF_INET6, nameserver, &ns6)) {
-		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_host))
-			c->ip6.dns_host = ns6;
-
-		/* Special handling if guest or container can only access local
-		 * addresses via redirect, or if the host gateway is also a
-		 * resolver and we shadow its address
-		 */
-		if (IN6_IS_ADDR_LOOPBACK(&ns6) ||
-		    IN6_ARE_ADDR_EQUAL(&ns6, &c->ip6.map_host_loopback)) {
-			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback))
-				return;
+	if (idx4 && inet_pton(AF_INET, nameserver, &ns4))
+		add_dns_resolv4(c, &ns4, idx4);
 
-			ns6 = c->ip6.map_host_loopback;
-
-			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match))
-				c->ip6.dns_match = c->ip6.map_host_loopback;
-		}
-
-		*idx6 += add_dns6(c, &ns6, *idx6);
-	}
+	if (idx6 && inet_pton(AF_INET6, nameserver, &ns6))
+		add_dns_resolv6(c, &ns6, idx6);
 }
 
 /**
-- 
@@ -414,6 +414,62 @@ static unsigned add_dns6(struct ctx *c, const struct in6_addr *addr,
 	return 1;
 }
 
+/**
+ * add_dns_resolv4() - Possibly add one IPv4 nameserver from host's resolv.conf
+ * @c:		Execution context
+ * @ns:		Nameserver address
+ * @idx:	Pointer to index of current IPv4 resolver entry, set on return
+ */
+static void add_dns_resolv4(struct ctx *c, struct in_addr *ns, unsigned *idx)
+{
+	if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_host))
+		c->ip4.dns_host = *ns;
+
+	/* Special handling if guest or container can only access local
+	 * addresses via redirect, or if the host gateway is also a resolver and
+	 * we shadow its address
+	 */
+	if (IN4_IS_ADDR_LOOPBACK(ns) ||
+	    IN4_ARE_ADDR_EQUAL(ns, &c->ip4.map_host_loopback)) {
+		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_host_loopback))
+			return;
+
+		*ns = c->ip4.map_host_loopback;
+		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match))
+			c->ip4.dns_match = c->ip4.map_host_loopback;
+	}
+
+	*idx += add_dns4(c, ns, *idx);
+}
+
+/**
+ * add_dns_resolv6() - Possibly add one IPv6 nameserver from host's resolv.conf
+ * @c:		Execution context
+ * @ns:		Nameserver address
+ * @idx:	Pointer to index of current IPv6 resolver entry, set on return
+ */
+static void add_dns_resolv6(struct ctx *c, struct in6_addr *ns, unsigned *idx)
+{
+	if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_host))
+		c->ip6.dns_host = *ns;
+
+	/* Special handling if guest or container can only access local
+	 * addresses via redirect, or if the host gateway is also a resolver and
+	 * we shadow its address
+	 */
+	if (IN6_IS_ADDR_LOOPBACK(ns) ||
+	    IN6_ARE_ADDR_EQUAL(ns, &c->ip6.map_host_loopback)) {
+		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback))
+			return;
+
+		*ns = c->ip6.map_host_loopback;
+		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match))
+			c->ip6.dns_match = c->ip6.map_host_loopback;
+	}
+
+	*idx += add_dns6(c, ns, *idx);
+}
+
 /**
  * add_dns_resolv() - Possibly add ns from host resolv.conf to configuration
  * @c:		Execution context
@@ -430,48 +486,11 @@ static void add_dns_resolv(struct ctx *c, const char *nameserver,
 	struct in6_addr ns6;
 	struct in_addr ns4;
 
-	if (idx4 && inet_pton(AF_INET, nameserver, &ns4)) {
-		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_host))
-			c->ip4.dns_host = ns4;
-
-		/* Special handling if guest or container can only access local
-		 * addresses via redirect, or if the host gateway is also a
-		 * resolver and we shadow its address
-		 */
-		if (IN4_IS_ADDR_LOOPBACK(&ns4) ||
-		    IN4_ARE_ADDR_EQUAL(&ns4, &c->ip4.map_host_loopback)) {
-			if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_host_loopback))
-				return;
-
-			ns4 = c->ip4.map_host_loopback;
-			if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match))
-				c->ip4.dns_match = c->ip4.map_host_loopback;
-		}
-
-		*idx4 += add_dns4(c, &ns4, *idx4);
-	}
-
-	if (idx6 && inet_pton(AF_INET6, nameserver, &ns6)) {
-		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_host))
-			c->ip6.dns_host = ns6;
-
-		/* Special handling if guest or container can only access local
-		 * addresses via redirect, or if the host gateway is also a
-		 * resolver and we shadow its address
-		 */
-		if (IN6_IS_ADDR_LOOPBACK(&ns6) ||
-		    IN6_ARE_ADDR_EQUAL(&ns6, &c->ip6.map_host_loopback)) {
-			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback))
-				return;
+	if (idx4 && inet_pton(AF_INET, nameserver, &ns4))
+		add_dns_resolv4(c, &ns4, idx4);
 
-			ns6 = c->ip6.map_host_loopback;
-
-			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match))
-				c->ip6.dns_match = c->ip6.map_host_loopback;
-		}
-
-		*idx6 += add_dns6(c, &ns6, *idx6);
-	}
+	if (idx6 && inet_pton(AF_INET6, nameserver, &ns6))
+		add_dns_resolv6(c, &ns6, idx6);
 }
 
 /**
-- 
2.43.0

[PATCH 2/2] conf: Honour --dns-forward for local resolver even with --no-map-gw

[Stefano Brivio]

If the first resolver listed in the host's /etc/resolv.conf is a
loopback address, and --no-map-gw is given, we automatically conclude
that the resolver is not reachable, discard it, and, if it's the only
nameserver listed in /etc/resolv.conf, we'll warn that we:

  Couldn't get any nameserver address

However, this isn't true in a general case: the user might have passed
--dns-forward, and in that case, while we won't map the address of the
default gateway to the host, we're still supposed to map that
particular address. Otherwise, in this common Podman usage:

  pasta --config-net --dns-forward 169.254.1.1 -t none -u none -T none -U none --no-map-gw --netns /run/user/1000/netns/netns-c02a8d8f-6ee3-902e-33c5-317e0f24e0af --map-guest-addr 169.254.1.2

and with a loopback address in /etc/resolv.conf, we'll unexpectedly
refuse to forward DNS queries:

  # nslookup passt.top 169.254.1.1
  ;; connection timed out; no servers could be reached

To fix this, make an exception for --dns-forward: if &c->ip4.dns_match
or &c->ip6.dns_match are set in add_dns_resolv4() / add_dns_resolv6(),
use that address as guest-facing resolver.

We already set 'dns_host' to the address we found in /etc/resolv.conf,
that's correct in this case and it makes us forward queries as
expected.

I'm not changing the man page as the current description of
--dns-forward is already consistent with the new behaviour: there's no
described way in which --no-map-gw should affect it.

Reported-by: Andrew Sayers <andrew-bugs.passt.top@pileofstuff.org>
Link: https://bugs.passt.top/show_bug.cgi?id=111
Suggested-by: Paul Holzinger <pholzing@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 conf.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/conf.c b/conf.c
index 18ed11c..f942851 100644
--- a/conf.c
+++ b/conf.c
@@ -431,12 +431,19 @@ static void add_dns_resolv4(struct ctx *c, struct in_addr *ns, unsigned *idx)
 	 */
 	if (IN4_IS_ADDR_LOOPBACK(ns) ||
 	    IN4_ARE_ADDR_EQUAL(ns, &c->ip4.map_host_loopback)) {
-		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_host_loopback))
-			return;
+		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match)) {
+			if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_host_loopback))
+				return;		/* Address unreachable */
 
-		*ns = c->ip4.map_host_loopback;
-		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match))
+			*ns = c->ip4.map_host_loopback;
 			c->ip4.dns_match = c->ip4.map_host_loopback;
+		} else {
+			/* No general host mapping, but requested for DNS
+			 * (--dns-forward and --no-map-gw): advertise resolver
+			 * address from --dns-forward, and map that to loopback
+			 */
+			*ns = c->ip4.dns_match;
+		}
 	}
 
 	*idx += add_dns4(c, ns, *idx);
@@ -459,12 +466,19 @@ static void add_dns_resolv6(struct ctx *c, struct in6_addr *ns, unsigned *idx)
 	 */
 	if (IN6_IS_ADDR_LOOPBACK(ns) ||
 	    IN6_ARE_ADDR_EQUAL(ns, &c->ip6.map_host_loopback)) {
-		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback))
-			return;
+		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match)) {
+			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback))
+				return;		/* Address unreachable */
 
-		*ns = c->ip6.map_host_loopback;
-		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match))
+			*ns = c->ip6.map_host_loopback;
 			c->ip6.dns_match = c->ip6.map_host_loopback;
+		} else {
+			/* No general host mapping, but requested for DNS
+			 * (--dns-forward and --no-map-gw): advertise resolver
+			 * address from --dns-forward, and map that to loopback
+			 */
+			*ns = c->ip6.dns_match;
+		}
 	}
 
 	*idx += add_dns6(c, ns, *idx);
-- 
@@ -431,12 +431,19 @@ static void add_dns_resolv4(struct ctx *c, struct in_addr *ns, unsigned *idx)
 	 */
 	if (IN4_IS_ADDR_LOOPBACK(ns) ||
 	    IN4_ARE_ADDR_EQUAL(ns, &c->ip4.map_host_loopback)) {
-		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_host_loopback))
-			return;
+		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match)) {
+			if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_host_loopback))
+				return;		/* Address unreachable */
 
-		*ns = c->ip4.map_host_loopback;
-		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match))
+			*ns = c->ip4.map_host_loopback;
 			c->ip4.dns_match = c->ip4.map_host_loopback;
+		} else {
+			/* No general host mapping, but requested for DNS
+			 * (--dns-forward and --no-map-gw): advertise resolver
+			 * address from --dns-forward, and map that to loopback
+			 */
+			*ns = c->ip4.dns_match;
+		}
 	}
 
 	*idx += add_dns4(c, ns, *idx);
@@ -459,12 +466,19 @@ static void add_dns_resolv6(struct ctx *c, struct in6_addr *ns, unsigned *idx)
 	 */
 	if (IN6_IS_ADDR_LOOPBACK(ns) ||
 	    IN6_ARE_ADDR_EQUAL(ns, &c->ip6.map_host_loopback)) {
-		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback))
-			return;
+		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match)) {
+			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback))
+				return;		/* Address unreachable */
 
-		*ns = c->ip6.map_host_loopback;
-		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match))
+			*ns = c->ip6.map_host_loopback;
 			c->ip6.dns_match = c->ip6.map_host_loopback;
+		} else {
+			/* No general host mapping, but requested for DNS
+			 * (--dns-forward and --no-map-gw): advertise resolver
+			 * address from --dns-forward, and map that to loopback
+			 */
+			*ns = c->ip6.dns_match;
+		}
 	}
 
 	*idx += add_dns6(c, ns, *idx);
-- 
2.43.0

Re: [PATCH 0/2] Fix --dns-forward with --no-map-gw

[Paul Holzinger]

On 11/04/2025 11:14, Stefano Brivio wrote:
> Patch 2/2 is the actual fix, patch 1/2 makes it less awkward.
>
> Stefano Brivio (2):
>    conf: Split add_dns_resolv() into separate IPv4 and IPv6 versions
>    conf: Honour --dns-forward for local resolver even with --no-map-gw
>
>   conf.c | 115 +++++++++++++++++++++++++++++++++++++--------------------
>   1 file changed, 74 insertions(+), 41 deletions(-)
>
Reviewed-by: Paul Holzinger <pholzing@redhat.com>

-- 
Paul Holzinger

Re: [PATCH 1/2] conf: Split add_dns_resolv() into separate IPv4 and IPv6 versions

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 5189 bytes --]

On Fri, Apr 11, 2025 at 11:14:38AM +0200, Stefano Brivio wrote:
> Not really valuable by itself, but dropping one level of nested blocks
> makes the next change more convenient.
> 
> No functional changes intended.
> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Not in scope for this code motion, but I did spot another bug here..

> ---
>  conf.c | 101 ++++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 60 insertions(+), 41 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 168646f..18ed11c 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -414,6 +414,62 @@ static unsigned add_dns6(struct ctx *c, const struct in6_addr *addr,
>  	return 1;
>  }
>  
> +/**
> + * add_dns_resolv4() - Possibly add one IPv4 nameserver from host's resolv.conf
> + * @c:		Execution context
> + * @ns:		Nameserver address
> + * @idx:	Pointer to index of current IPv4 resolver entry, set on return
> + */
> +static void add_dns_resolv4(struct ctx *c, struct in_addr *ns, unsigned *idx)
> +{
> +	if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_host))
> +		c->ip4.dns_host = *ns;
> +
> +	/* Special handling if guest or container can only access local
> +	 * addresses via redirect, or if the host gateway is also a resolver and
> +	 * we shadow its address
> +	 */
> +	if (IN4_IS_ADDR_LOOPBACK(ns) ||
> +	    IN4_ARE_ADDR_EQUAL(ns, &c->ip4.map_host_loopback)) {

The second bit here is wrong.  We check if the nameserver address is
the --map-host-loopback address - meaning we can't use it in the
guest - then try to use it in the guest anyway.  That path should
instead return, like the ns == 127.0.0.1 && map_host_loopback ==
0.0.0.0 case.

> +		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_host_loopback))
> +			return;
> +
> +		*ns = c->ip4.map_host_loopback;
> +		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match))
> +			c->ip4.dns_match = c->ip4.map_host_loopback;
> +	}
> +
> +	*idx += add_dns4(c, ns, *idx);
> +}
> +
> +/**
> + * add_dns_resolv6() - Possibly add one IPv6 nameserver from host's resolv.conf
> + * @c:		Execution context
> + * @ns:		Nameserver address
> + * @idx:	Pointer to index of current IPv6 resolver entry, set on return
> + */
> +static void add_dns_resolv6(struct ctx *c, struct in6_addr *ns, unsigned *idx)
> +{
> +	if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_host))
> +		c->ip6.dns_host = *ns;
> +
> +	/* Special handling if guest or container can only access local
> +	 * addresses via redirect, or if the host gateway is also a resolver and
> +	 * we shadow its address
> +	 */
> +	if (IN6_IS_ADDR_LOOPBACK(ns) ||
> +	    IN6_ARE_ADDR_EQUAL(ns, &c->ip6.map_host_loopback)) {

Same bug for IPv6.

> +		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback))
> +			return;
> +
> +		*ns = c->ip6.map_host_loopback;
> +		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match))
> +			c->ip6.dns_match = c->ip6.map_host_loopback;
> +	}
> +
> +	*idx += add_dns6(c, ns, *idx);
> +}
> +
>  /**
>   * add_dns_resolv() - Possibly add ns from host resolv.conf to configuration
>   * @c:		Execution context
> @@ -430,48 +486,11 @@ static void add_dns_resolv(struct ctx *c, const char *nameserver,
>  	struct in6_addr ns6;
>  	struct in_addr ns4;
>  
> -	if (idx4 && inet_pton(AF_INET, nameserver, &ns4)) {
> -		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_host))
> -			c->ip4.dns_host = ns4;
> -
> -		/* Special handling if guest or container can only access local
> -		 * addresses via redirect, or if the host gateway is also a
> -		 * resolver and we shadow its address
> -		 */
> -		if (IN4_IS_ADDR_LOOPBACK(&ns4) ||
> -		    IN4_ARE_ADDR_EQUAL(&ns4, &c->ip4.map_host_loopback)) {
> -			if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_host_loopback))
> -				return;
> -
> -			ns4 = c->ip4.map_host_loopback;
> -			if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match))
> -				c->ip4.dns_match = c->ip4.map_host_loopback;
> -		}
> -
> -		*idx4 += add_dns4(c, &ns4, *idx4);
> -	}
> -
> -	if (idx6 && inet_pton(AF_INET6, nameserver, &ns6)) {
> -		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_host))
> -			c->ip6.dns_host = ns6;
> -
> -		/* Special handling if guest or container can only access local
> -		 * addresses via redirect, or if the host gateway is also a
> -		 * resolver and we shadow its address
> -		 */
> -		if (IN6_IS_ADDR_LOOPBACK(&ns6) ||
> -		    IN6_ARE_ADDR_EQUAL(&ns6, &c->ip6.map_host_loopback)) {
> -			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback))
> -				return;
> +	if (idx4 && inet_pton(AF_INET, nameserver, &ns4))
> +		add_dns_resolv4(c, &ns4, idx4);
>  
> -			ns6 = c->ip6.map_host_loopback;
> -
> -			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match))
> -				c->ip6.dns_match = c->ip6.map_host_loopback;
> -		}
> -
> -		*idx6 += add_dns6(c, &ns6, *idx6);
> -	}
> +	if (idx6 && inet_pton(AF_INET6, nameserver, &ns6))
> +		add_dns_resolv6(c, &ns6, idx6);
>  }
>  
>  /**

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] conf: Honour --dns-forward for local resolver even with --no-map-gw

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4194 bytes --]

On Fri, Apr 11, 2025 at 11:14:39AM +0200, Stefano Brivio wrote:
> If the first resolver listed in the host's /etc/resolv.conf is a
> loopback address, and --no-map-gw is given, we automatically conclude
> that the resolver is not reachable, discard it, and, if it's the only
> nameserver listed in /etc/resolv.conf, we'll warn that we:
> 
>   Couldn't get any nameserver address
> 
> However, this isn't true in a general case: the user might have passed
> --dns-forward, and in that case, while we won't map the address of the
> default gateway to the host, we're still supposed to map that
> particular address. Otherwise, in this common Podman usage:
> 
>   pasta --config-net --dns-forward 169.254.1.1 -t none -u none -T none -U none --no-map-gw --netns /run/user/1000/netns/netns-c02a8d8f-6ee3-902e-33c5-317e0f24e0af --map-guest-addr 169.254.1.2
> 
> and with a loopback address in /etc/resolv.conf, we'll unexpectedly
> refuse to forward DNS queries:
> 
>   # nslookup passt.top 169.254.1.1
>   ;; connection timed out; no servers could be reached
> 
> To fix this, make an exception for --dns-forward: if &c->ip4.dns_match
> or &c->ip6.dns_match are set in add_dns_resolv4() / add_dns_resolv6(),
> use that address as guest-facing resolver.
> 
> We already set 'dns_host' to the address we found in /etc/resolv.conf,
> that's correct in this case and it makes us forward queries as
> expected.
> 
> I'm not changing the man page as the current description of
> --dns-forward is already consistent with the new behaviour: there's no
> described way in which --no-map-gw should affect it.
> 
> Reported-by: Andrew Sayers <andrew-bugs.passt.top@pileofstuff.org>
> Link: https://bugs.passt.top/show_bug.cgi?id=111
> Suggested-by: Paul Holzinger <pholzing@redhat.com>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  conf.c | 30 ++++++++++++++++++++++--------
>  1 file changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 18ed11c..f942851 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -431,12 +431,19 @@ static void add_dns_resolv4(struct ctx *c, struct in_addr *ns, unsigned *idx)
>  	 */
>  	if (IN4_IS_ADDR_LOOPBACK(ns) ||
>  	    IN4_ARE_ADDR_EQUAL(ns, &c->ip4.map_host_loopback)) {
> -		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_host_loopback))
> -			return;
> +		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match)) {
> +			if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_host_loopback))
> +				return;		/* Address unreachable */
>  
> -		*ns = c->ip4.map_host_loopback;
> -		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match))
> +			*ns = c->ip4.map_host_loopback;
>  			c->ip4.dns_match = c->ip4.map_host_loopback;
> +		} else {
> +			/* No general host mapping, but requested for DNS
> +			 * (--dns-forward and --no-map-gw): advertise resolver
> +			 * address from --dns-forward, and map that to loopback
> +			 */
> +			*ns = c->ip4.dns_match;
> +		}
>  	}
>  
>  	*idx += add_dns4(c, ns, *idx);
> @@ -459,12 +466,19 @@ static void add_dns_resolv6(struct ctx *c, struct in6_addr *ns, unsigned *idx)
>  	 */
>  	if (IN6_IS_ADDR_LOOPBACK(ns) ||
>  	    IN6_ARE_ADDR_EQUAL(ns, &c->ip6.map_host_loopback)) {
> -		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback))
> -			return;
> +		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match)) {
> +			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback))
> +				return;		/* Address unreachable */
>  
> -		*ns = c->ip6.map_host_loopback;
> -		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match))
> +			*ns = c->ip6.map_host_loopback;
>  			c->ip6.dns_match = c->ip6.map_host_loopback;
> +		} else {
> +			/* No general host mapping, but requested for DNS
> +			 * (--dns-forward and --no-map-gw): advertise resolver
> +			 * address from --dns-forward, and map that to loopback
> +			 */
> +			*ns = c->ip6.dns_match;
> +		}
>  	}
>  
>  	*idx += add_dns6(c, ns, *idx);

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] conf: Split add_dns_resolv() into separate IPv4 and IPv6 versions

[Stefano Brivio]

On Mon, 14 Apr 2025 12:05:30 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Apr 11, 2025 at 11:14:38AM +0200, Stefano Brivio wrote:
> > Not really valuable by itself, but dropping one level of nested blocks
> > makes the next change more convenient.
> > 
> > No functional changes intended.
> > 
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>  
> 
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> 
> Not in scope for this code motion, but I did spot another bug here..
> 
> > ---
> >  conf.c | 101 ++++++++++++++++++++++++++++++++++-----------------------
> >  1 file changed, 60 insertions(+), 41 deletions(-)
> > 
> > diff --git a/conf.c b/conf.c
> > index 168646f..18ed11c 100644
> > --- a/conf.c
> > +++ b/conf.c
> > @@ -414,6 +414,62 @@ static unsigned add_dns6(struct ctx *c, const struct in6_addr *addr,
> >  	return 1;
> >  }
> >  
> > +/**
> > + * add_dns_resolv4() - Possibly add one IPv4 nameserver from host's resolv.conf
> > + * @c:		Execution context
> > + * @ns:		Nameserver address
> > + * @idx:	Pointer to index of current IPv4 resolver entry, set on return
> > + */
> > +static void add_dns_resolv4(struct ctx *c, struct in_addr *ns, unsigned *idx)
> > +{
> > +	if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_host))
> > +		c->ip4.dns_host = *ns;
> > +
> > +	/* Special handling if guest or container can only access local
> > +	 * addresses via redirect, or if the host gateway is also a resolver and
> > +	 * we shadow its address
> > +	 */
> > +	if (IN4_IS_ADDR_LOOPBACK(ns) ||
> > +	    IN4_ARE_ADDR_EQUAL(ns, &c->ip4.map_host_loopback)) {  
> 
> The second bit here is wrong.  We check if the nameserver address is
> the --map-host-loopback address - meaning we can't use it in the
> guest - then try to use it in the guest anyway.  That path should
> instead return, like the ns == 127.0.0.1 && map_host_loopback ==
> 0.0.0.0 case.

I'm not sure why we can't use the --map-host-loopback address in the
guest. DNS traffic has the priority, right?

Together with 2/2, do you still see an issue?

That is, if there's no --dns-forward and no address maps to a local
resolver, there's no way to reach the local resolver and we return, but
in any other case, I guess we can pick that address.

-- 
Stefano