[PATCH 0/3] Allow non-127.0.0.1 loopback address as host resolver

[PATCH 0/3] Allow non-127.0.0.1 loopback address as host resolver

[Stefano Brivio]

This series fixes an issue with DNS resolution reported by Andrea: if
both guest and host use systemd-resolved, reachable at 127.0.0.53, we
need to advertise a reachable address as DNS to the guest, which we
already do, but also to map UDP packets for that address and port 53
to the original host resolver address.

Otherwise, DNS resolution doesn't work at all in the guest, with
default options.

Stefano Brivio (3):
  udp: Actually use host resolver to forward DNS queries
  conf: Split add_dns{4,6}() out of get_dns()
  conf, udp: Allow any loopback address to be used as resolver

 conf.c | 92 ++++++++++++++++++++++++++++++++++++----------------------
 udp.c  | 20 ++++++-------
 2 files changed, 67 insertions(+), 45 deletions(-)

-- 
2.39.1

[PATCH 1/3] udp: Actually use host resolver to forward DNS queries

[Stefano Brivio]

Instead of the address of the first resolver we advertise to
the guest or namespace.

This was one of the intentions behind commit 3a2afde87dd1 ("conf,
udp: Drop mostly duplicated dns_send arrays, rename related fields"),
but I forgot to implement this part. In practice, they are usually
the same thing, unless /etc/resolv.conf points to a loopback address.

Fixes: 3a2afde87dd1 ("conf, udp: Drop mostly duplicated dns_send arrays, rename related fields")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 udp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/udp.c b/udp.c
index c913d27..1d65559 100644
--- a/udp.c
+++ b/udp.c
@@ -867,7 +867,7 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
 		} else if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr,
 					      &c->ip4.dns_match) &&
 			   ntohs(s_in.sin_port) == 53) {
-			s_in.sin_addr = c->ip4.dns[0];
+			s_in.sin_addr = c->ip4.dns_host;
 		}
 	} else {
 		s_in6 = (struct sockaddr_in6) {
@@ -890,7 +890,7 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
 				s_in6.sin6_addr = c->ip6.addr_seen;
 		} else if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.dns_match) &&
 			   ntohs(s_in6.sin6_port) == 53) {
-			s_in6.sin6_addr = c->ip6.dns[0];
+			s_in6.sin6_addr = c->ip6.dns_host;
 		} else if (IN6_IS_ADDR_LINKLOCAL(&s_in6.sin6_addr)) {
 			bind_addr = &c->ip6.addr_ll;
 		}
-- 
@@ -867,7 +867,7 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
 		} else if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr,
 					      &c->ip4.dns_match) &&
 			   ntohs(s_in.sin_port) == 53) {
-			s_in.sin_addr = c->ip4.dns[0];
+			s_in.sin_addr = c->ip4.dns_host;
 		}
 	} else {
 		s_in6 = (struct sockaddr_in6) {
@@ -890,7 +890,7 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
 				s_in6.sin6_addr = c->ip6.addr_seen;
 		} else if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.dns_match) &&
 			   ntohs(s_in6.sin6_port) == 53) {
-			s_in6.sin6_addr = c->ip6.dns[0];
+			s_in6.sin6_addr = c->ip6.dns_host;
 		} else if (IN6_IS_ADDR_LINKLOCAL(&s_in6.sin6_addr)) {
 			bind_addr = &c->ip6.addr_ll;
 		}
-- 
2.39.1

Re: [PATCH 1/3] udp: Actually use host resolver to forward DNS queries

[David Gibson]

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

On Thu, Feb 23, 2023 at 06:07:58PM +0100, Stefano Brivio wrote:
> Instead of the address of the first resolver we advertise to
> the guest or namespace.
> 
> This was one of the intentions behind commit 3a2afde87dd1 ("conf,
> udp: Drop mostly duplicated dns_send arrays, rename related fields"),
> but I forgot to implement this part. In practice, they are usually
> the same thing, unless /etc/resolv.conf points to a loopback address.
> 
> Fixes: 3a2afde87dd1 ("conf, udp: Drop mostly duplicated dns_send arrays, rename related fields")
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

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

> ---
>  udp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/udp.c b/udp.c
> index c913d27..1d65559 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -867,7 +867,7 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
>  		} else if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr,
>  					      &c->ip4.dns_match) &&
>  			   ntohs(s_in.sin_port) == 53) {
> -			s_in.sin_addr = c->ip4.dns[0];
> +			s_in.sin_addr = c->ip4.dns_host;
>  		}
>  	} else {
>  		s_in6 = (struct sockaddr_in6) {
> @@ -890,7 +890,7 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
>  				s_in6.sin6_addr = c->ip6.addr_seen;
>  		} else if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.dns_match) &&
>  			   ntohs(s_in6.sin6_port) == 53) {
> -			s_in6.sin6_addr = c->ip6.dns[0];
> +			s_in6.sin6_addr = c->ip6.dns_host;
>  		} else if (IN6_IS_ADDR_LINKLOCAL(&s_in6.sin6_addr)) {
>  			bind_addr = &c->ip6.addr_ll;
>  		}

-- 
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

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

[PATCH 2/3] conf: Split add_dns{4,6}() out of get_dns()

[Stefano Brivio]

The logic handling which resolvers we add, and whether to add them,
is getting rather cramped in get_dns(): split it into separate
functions.

No functional changes intended.

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

diff --git a/conf.c b/conf.c
index 4dc0660..ed25e35 100644
--- a/conf.c
+++ b/conf.c
@@ -382,6 +382,53 @@ bind_fail:
 	die("Failed to bind any port for '-%c %s', exiting", optname, optarg);
 }
 
+/**
+ * add_dns4() - Possibly add the IPv4 address of a DNS resolver to configuration
+ * @c:		Execution context
+ * @addr:	Address found in /etc/resolv.conf
+ * @conf:	Pointer to reference of current entry in array of IPv4 resolvers
+ */
+static void add_dns4(struct ctx *c, struct in_addr *addr, struct in_addr **conf)
+{
+	/* Guest or container can only access local addresses via redirect */
+	if (IN4_IS_ADDR_LOOPBACK(addr)) {
+		if (!c->no_map_gw) {
+			**conf = c->ip4.gw;
+			(*conf)++;
+		}
+	} else {
+		**conf = *addr;
+		(*conf)++;
+	}
+
+	if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_host))
+		c->ip4.dns_host = *addr;
+}
+
+/**
+ * add_dns6() - Possibly add the IPv6 address of a DNS resolver to configuration
+ * @c:		Execution context
+ * @addr:	Address found in /etc/resolv.conf
+ * @conf:	Pointer to reference of current entry in array of IPv6 resolvers
+ */
+static void add_dns6(struct ctx *c,
+		     struct in6_addr *addr, struct in6_addr **conf)
+{
+	/* Guest or container can only access local addresses via redirect */
+	if (IN6_IS_ADDR_LOOPBACK(addr)) {
+		if (!c->no_map_gw) {
+			memcpy(*conf, &c->ip6.gw, sizeof(**conf));
+			(*conf)++;
+		}
+	} else {
+		memcpy(*conf, addr, sizeof(**conf));
+		(*conf)++;
+	}
+
+	if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_host))
+		c->ip6.dns_host = *addr;
+}
+
 /**
  * get_dns() - Get nameserver addresses from local /etc/resolv.conf
  * @c:		Execution context
@@ -420,44 +467,13 @@ static void get_dns(struct ctx *c)
 
 			if (!dns4_set &&
 			    dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) - 1
-			    && inet_pton(AF_INET, p + 1, &dns4_tmp)) {
-				/* Guest or container can only access local
-				 * addresses via local redirect
-				 */
-				if (IN4_IS_ADDR_LOOPBACK(&dns4_tmp)) {
-					if (!c->no_map_gw) {
-						*dns4 = c->ip4.gw;
-						dns4++;
-					}
-				} else {
-					*dns4 = dns4_tmp;
-					dns4++;
-				}
-
-				if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_host))
-					c->ip4.dns_host = dns4_tmp;
-			}
+			    && inet_pton(AF_INET, p + 1, &dns4_tmp))
+				add_dns4(c, &dns4_tmp, &dns4);
 
 			if (!dns6_set &&
 			    dns6 - &c->ip6.dns[0] < ARRAY_SIZE(c->ip6.dns) - 1
-			    && inet_pton(AF_INET6, p + 1, &dns6_tmp)) {
-				/* Guest or container can only access local
-				 * addresses via local redirect
-				 */
-				if (IN6_IS_ADDR_LOOPBACK(&dns6_tmp)) {
-					if (!c->no_map_gw) {
-						memcpy(dns6, &c->ip6.gw,
-						       sizeof(*dns6));
-						dns6++;
-					}
-				} else {
-					memcpy(dns6, &dns6_tmp, sizeof(*dns6));
-					dns6++;
-				}
-
-				if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_host))
-					c->ip6.dns_host = dns6_tmp;
-			}
+			    && inet_pton(AF_INET6, p + 1, &dns6_tmp))
+				add_dns6(c, &dns6_tmp, &dns6);
 		} else if (!dnss_set && strstr(line, "search ") == line &&
 			   s == c->dns_search) {
 			end = strpbrk(line, "\n");
-- 
@@ -382,6 +382,53 @@ bind_fail:
 	die("Failed to bind any port for '-%c %s', exiting", optname, optarg);
 }
 
+/**
+ * add_dns4() - Possibly add the IPv4 address of a DNS resolver to configuration
+ * @c:		Execution context
+ * @addr:	Address found in /etc/resolv.conf
+ * @conf:	Pointer to reference of current entry in array of IPv4 resolvers
+ */
+static void add_dns4(struct ctx *c, struct in_addr *addr, struct in_addr **conf)
+{
+	/* Guest or container can only access local addresses via redirect */
+	if (IN4_IS_ADDR_LOOPBACK(addr)) {
+		if (!c->no_map_gw) {
+			**conf = c->ip4.gw;
+			(*conf)++;
+		}
+	} else {
+		**conf = *addr;
+		(*conf)++;
+	}
+
+	if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_host))
+		c->ip4.dns_host = *addr;
+}
+
+/**
+ * add_dns6() - Possibly add the IPv6 address of a DNS resolver to configuration
+ * @c:		Execution context
+ * @addr:	Address found in /etc/resolv.conf
+ * @conf:	Pointer to reference of current entry in array of IPv6 resolvers
+ */
+static void add_dns6(struct ctx *c,
+		     struct in6_addr *addr, struct in6_addr **conf)
+{
+	/* Guest or container can only access local addresses via redirect */
+	if (IN6_IS_ADDR_LOOPBACK(addr)) {
+		if (!c->no_map_gw) {
+			memcpy(*conf, &c->ip6.gw, sizeof(**conf));
+			(*conf)++;
+		}
+	} else {
+		memcpy(*conf, addr, sizeof(**conf));
+		(*conf)++;
+	}
+
+	if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_host))
+		c->ip6.dns_host = *addr;
+}
+
 /**
  * get_dns() - Get nameserver addresses from local /etc/resolv.conf
  * @c:		Execution context
@@ -420,44 +467,13 @@ static void get_dns(struct ctx *c)
 
 			if (!dns4_set &&
 			    dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) - 1
-			    && inet_pton(AF_INET, p + 1, &dns4_tmp)) {
-				/* Guest or container can only access local
-				 * addresses via local redirect
-				 */
-				if (IN4_IS_ADDR_LOOPBACK(&dns4_tmp)) {
-					if (!c->no_map_gw) {
-						*dns4 = c->ip4.gw;
-						dns4++;
-					}
-				} else {
-					*dns4 = dns4_tmp;
-					dns4++;
-				}
-
-				if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_host))
-					c->ip4.dns_host = dns4_tmp;
-			}
+			    && inet_pton(AF_INET, p + 1, &dns4_tmp))
+				add_dns4(c, &dns4_tmp, &dns4);
 
 			if (!dns6_set &&
 			    dns6 - &c->ip6.dns[0] < ARRAY_SIZE(c->ip6.dns) - 1
-			    && inet_pton(AF_INET6, p + 1, &dns6_tmp)) {
-				/* Guest or container can only access local
-				 * addresses via local redirect
-				 */
-				if (IN6_IS_ADDR_LOOPBACK(&dns6_tmp)) {
-					if (!c->no_map_gw) {
-						memcpy(dns6, &c->ip6.gw,
-						       sizeof(*dns6));
-						dns6++;
-					}
-				} else {
-					memcpy(dns6, &dns6_tmp, sizeof(*dns6));
-					dns6++;
-				}
-
-				if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_host))
-					c->ip6.dns_host = dns6_tmp;
-			}
+			    && inet_pton(AF_INET6, p + 1, &dns6_tmp))
+				add_dns6(c, &dns6_tmp, &dns6);
 		} else if (!dnss_set && strstr(line, "search ") == line &&
 			   s == c->dns_search) {
 			end = strpbrk(line, "\n");
-- 
2.39.1

Re: [PATCH 2/3] conf: Split add_dns{4,6}() out of get_dns()

[David Gibson]

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

On Thu, Feb 23, 2023 at 06:07:59PM +0100, Stefano Brivio wrote:
> The logic handling which resolvers we add, and whether to add them,
> is getting rather cramped in get_dns(): split it into separate
> functions.
> 
> No functional changes intended.
> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

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

> ---
>  conf.c | 86 ++++++++++++++++++++++++++++++++++------------------------
>  1 file changed, 51 insertions(+), 35 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 4dc0660..ed25e35 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -382,6 +382,53 @@ bind_fail:
>  	die("Failed to bind any port for '-%c %s', exiting", optname, optarg);
>  }
>  
> +/**
> + * add_dns4() - Possibly add the IPv4 address of a DNS resolver to configuration
> + * @c:		Execution context
> + * @addr:	Address found in /etc/resolv.conf
> + * @conf:	Pointer to reference of current entry in array of IPv4 resolvers
> + */
> +static void add_dns4(struct ctx *c, struct in_addr *addr, struct in_addr **conf)
> +{
> +	/* Guest or container can only access local addresses via redirect */
> +	if (IN4_IS_ADDR_LOOPBACK(addr)) {
> +		if (!c->no_map_gw) {
> +			**conf = c->ip4.gw;
> +			(*conf)++;
> +		}
> +	} else {
> +		**conf = *addr;
> +		(*conf)++;
> +	}
> +
> +	if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_host))
> +		c->ip4.dns_host = *addr;
> +}
> +
> +/**
> + * add_dns6() - Possibly add the IPv6 address of a DNS resolver to configuration
> + * @c:		Execution context
> + * @addr:	Address found in /etc/resolv.conf
> + * @conf:	Pointer to reference of current entry in array of IPv6 resolvers
> + */
> +static void add_dns6(struct ctx *c,
> +		     struct in6_addr *addr, struct in6_addr **conf)
> +{
> +	/* Guest or container can only access local addresses via redirect */
> +	if (IN6_IS_ADDR_LOOPBACK(addr)) {
> +		if (!c->no_map_gw) {
> +			memcpy(*conf, &c->ip6.gw, sizeof(**conf));
> +			(*conf)++;
> +		}
> +	} else {
> +		memcpy(*conf, addr, sizeof(**conf));
> +		(*conf)++;
> +	}
> +
> +	if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_host))
> +		c->ip6.dns_host = *addr;
> +}
> +
>  /**
>   * get_dns() - Get nameserver addresses from local /etc/resolv.conf
>   * @c:		Execution context
> @@ -420,44 +467,13 @@ static void get_dns(struct ctx *c)
>  
>  			if (!dns4_set &&
>  			    dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) - 1
> -			    && inet_pton(AF_INET, p + 1, &dns4_tmp)) {
> -				/* Guest or container can only access local
> -				 * addresses via local redirect
> -				 */
> -				if (IN4_IS_ADDR_LOOPBACK(&dns4_tmp)) {
> -					if (!c->no_map_gw) {
> -						*dns4 = c->ip4.gw;
> -						dns4++;
> -					}
> -				} else {
> -					*dns4 = dns4_tmp;
> -					dns4++;
> -				}
> -
> -				if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_host))
> -					c->ip4.dns_host = dns4_tmp;
> -			}
> +			    && inet_pton(AF_INET, p + 1, &dns4_tmp))
> +				add_dns4(c, &dns4_tmp, &dns4);
>  
>  			if (!dns6_set &&
>  			    dns6 - &c->ip6.dns[0] < ARRAY_SIZE(c->ip6.dns) - 1
> -			    && inet_pton(AF_INET6, p + 1, &dns6_tmp)) {
> -				/* Guest or container can only access local
> -				 * addresses via local redirect
> -				 */
> -				if (IN6_IS_ADDR_LOOPBACK(&dns6_tmp)) {
> -					if (!c->no_map_gw) {
> -						memcpy(dns6, &c->ip6.gw,
> -						       sizeof(*dns6));
> -						dns6++;
> -					}
> -				} else {
> -					memcpy(dns6, &dns6_tmp, sizeof(*dns6));
> -					dns6++;
> -				}
> -
> -				if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_host))
> -					c->ip6.dns_host = dns6_tmp;
> -			}
> +			    && inet_pton(AF_INET6, p + 1, &dns6_tmp))
> +				add_dns6(c, &dns6_tmp, &dns6);
>  		} else if (!dnss_set && strstr(line, "search ") == line &&
>  			   s == c->dns_search) {
>  			end = strpbrk(line, "\n");

-- 
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

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

[PATCH 3/3] conf, udp: Allow any loopback address to be used as resolver

[Stefano Brivio]

Andrea reports that with a Fedora 37 guest running on a Fedora 37
host, both using systemd-resolved, with passt connecting them,
running with default options, DNS queries don't work.

systemd-resolved on the host is reachable only at the loopback
address 127.0.0.53.

We advertise the default gateway address to the guest as resolver,
because our local address is of course unreachable from there, which
means we see DNS queries directed to the default gateway, and we
redirect them to 127.0.0.1. However, systemd-resolved doesn't answer
on 127.0.0.1.

To fix this, set @dns_match to the address of the default gateway,
unless a different resolver address is explicitly configured, so that
we know we explicitly have to map DNS queries, in this case, to the
address of the local resolver.

This means that in udp_tap_handler() we need to check, first, if
the destination address of packets matches @dns_match: even if it's
the address of the local gateway, we want to map that to a specific
address, which isn't necessarily 127.0.0.1.

Do the same for IPv6 for consistency, even though IPv6 defines a
single loopback address.

Reported-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 conf.c |  6 ++++++
 udp.c  | 20 ++++++++++----------
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/conf.c b/conf.c
index ed25e35..37f25d6 100644
--- a/conf.c
+++ b/conf.c
@@ -395,6 +395,9 @@ static void add_dns4(struct ctx *c, struct in_addr *addr, struct in_addr **conf)
 		if (!c->no_map_gw) {
 			**conf = c->ip4.gw;
 			(*conf)++;
+
+			if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match))
+				c->ip4.dns_match = c->ip4.gw;
 		}
 	} else {
 		**conf = *addr;
@@ -419,6 +422,9 @@ static void add_dns6(struct ctx *c,
 		if (!c->no_map_gw) {
 			memcpy(*conf, &c->ip6.gw, sizeof(**conf));
 			(*conf)++;
+
+			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match))
+				memcpy(&c->ip6.dns_match, addr, sizeof(*addr));
 		}
 	} else {
 		memcpy(*conf, addr, sizeof(**conf));
diff --git a/udp.c b/udp.c
index 1d65559..20a9ea0 100644
--- a/udp.c
+++ b/udp.c
@@ -857,17 +857,16 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
 
 		udp_tap_map[V4][src].ts = now->tv_sec;
 
-		if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.gw) &&
-		    !c->no_map_gw) {
+		if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.dns_match) &&
+		    ntohs(s_in.sin_port) == 53) {
+			s_in.sin_addr = c->ip4.dns_host;
+		} else if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.gw) &&
+			   !c->no_map_gw) {
 			if (!(udp_tap_map[V4][dst].flags & PORT_LOCAL) ||
 			    (udp_tap_map[V4][dst].flags & PORT_LOOPBACK))
 				s_in.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
 			else
 				s_in.sin_addr = c->ip4.addr_seen;
-		} else if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr,
-					      &c->ip4.dns_match) &&
-			   ntohs(s_in.sin_port) == 53) {
-			s_in.sin_addr = c->ip4.dns_host;
 		}
 	} else {
 		s_in6 = (struct sockaddr_in6) {
@@ -880,7 +879,11 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
 		sa = (struct sockaddr *)&s_in6;
 		sl = sizeof(s_in6);
 
-		if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.gw) && !c->no_map_gw) {
+		if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.dns_match) &&
+		    ntohs(s_in6.sin6_port) == 53) {
+			s_in6.sin6_addr = c->ip6.dns_host;
+		} else if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.gw) &&
+			   !c->no_map_gw) {
 			if (!(udp_tap_map[V6][dst].flags & PORT_LOCAL) ||
 			    (udp_tap_map[V6][dst].flags & PORT_LOOPBACK))
 				s_in6.sin6_addr = in6addr_loopback;
@@ -888,9 +891,6 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
 				s_in6.sin6_addr = c->ip6.addr;
 			else
 				s_in6.sin6_addr = c->ip6.addr_seen;
-		} else if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.dns_match) &&
-			   ntohs(s_in6.sin6_port) == 53) {
-			s_in6.sin6_addr = c->ip6.dns_host;
 		} else if (IN6_IS_ADDR_LINKLOCAL(&s_in6.sin6_addr)) {
 			bind_addr = &c->ip6.addr_ll;
 		}
-- 
@@ -857,17 +857,16 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
 
 		udp_tap_map[V4][src].ts = now->tv_sec;
 
-		if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.gw) &&
-		    !c->no_map_gw) {
+		if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.dns_match) &&
+		    ntohs(s_in.sin_port) == 53) {
+			s_in.sin_addr = c->ip4.dns_host;
+		} else if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.gw) &&
+			   !c->no_map_gw) {
 			if (!(udp_tap_map[V4][dst].flags & PORT_LOCAL) ||
 			    (udp_tap_map[V4][dst].flags & PORT_LOOPBACK))
 				s_in.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
 			else
 				s_in.sin_addr = c->ip4.addr_seen;
-		} else if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr,
-					      &c->ip4.dns_match) &&
-			   ntohs(s_in.sin_port) == 53) {
-			s_in.sin_addr = c->ip4.dns_host;
 		}
 	} else {
 		s_in6 = (struct sockaddr_in6) {
@@ -880,7 +879,11 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
 		sa = (struct sockaddr *)&s_in6;
 		sl = sizeof(s_in6);
 
-		if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.gw) && !c->no_map_gw) {
+		if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.dns_match) &&
+		    ntohs(s_in6.sin6_port) == 53) {
+			s_in6.sin6_addr = c->ip6.dns_host;
+		} else if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.gw) &&
+			   !c->no_map_gw) {
 			if (!(udp_tap_map[V6][dst].flags & PORT_LOCAL) ||
 			    (udp_tap_map[V6][dst].flags & PORT_LOOPBACK))
 				s_in6.sin6_addr = in6addr_loopback;
@@ -888,9 +891,6 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
 				s_in6.sin6_addr = c->ip6.addr;
 			else
 				s_in6.sin6_addr = c->ip6.addr_seen;
-		} else if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.dns_match) &&
-			   ntohs(s_in6.sin6_port) == 53) {
-			s_in6.sin6_addr = c->ip6.dns_host;
 		} else if (IN6_IS_ADDR_LINKLOCAL(&s_in6.sin6_addr)) {
 			bind_addr = &c->ip6.addr_ll;
 		}
-- 
2.39.1

Re: [PATCH 3/3] conf, udp: Allow any loopback address to be used as resolver

[David Gibson]

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

On Thu, Feb 23, 2023 at 06:08:00PM +0100, Stefano Brivio wrote:
> Andrea reports that with a Fedora 37 guest running on a Fedora 37
> host, both using systemd-resolved, with passt connecting them,
> running with default options, DNS queries don't work.
> 
> systemd-resolved on the host is reachable only at the loopback
> address 127.0.0.53.
> 
> We advertise the default gateway address to the guest as resolver,
> because our local address is of course unreachable from there, which
> means we see DNS queries directed to the default gateway, and we
> redirect them to 127.0.0.1. However, systemd-resolved doesn't answer
> on 127.0.0.1.
> 
> To fix this, set @dns_match to the address of the default gateway,
> unless a different resolver address is explicitly configured, so that
> we know we explicitly have to map DNS queries, in this case, to the
> address of the local resolver.
> 
> This means that in udp_tap_handler() we need to check, first, if
> the destination address of packets matches @dns_match: even if it's
> the address of the local gateway, we want to map that to a specific
> address, which isn't necessarily 127.0.0.1.
> 
> Do the same for IPv6 for consistency, even though IPv6 defines a
> single loopback address.
> 
> Reported-by: Andrea Bolognani <abologna@redhat.com>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

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

> ---
>  conf.c |  6 ++++++
>  udp.c  | 20 ++++++++++----------
>  2 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index ed25e35..37f25d6 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -395,6 +395,9 @@ static void add_dns4(struct ctx *c, struct in_addr *addr, struct in_addr **conf)
>  		if (!c->no_map_gw) {
>  			**conf = c->ip4.gw;
>  			(*conf)++;
> +
> +			if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match))
> +				c->ip4.dns_match = c->ip4.gw;
>  		}
>  	} else {
>  		**conf = *addr;
> @@ -419,6 +422,9 @@ static void add_dns6(struct ctx *c,
>  		if (!c->no_map_gw) {
>  			memcpy(*conf, &c->ip6.gw, sizeof(**conf));
>  			(*conf)++;
> +
> +			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match))
> +				memcpy(&c->ip6.dns_match, addr, sizeof(*addr));
>  		}
>  	} else {
>  		memcpy(*conf, addr, sizeof(**conf));
> diff --git a/udp.c b/udp.c
> index 1d65559..20a9ea0 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -857,17 +857,16 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
>  
>  		udp_tap_map[V4][src].ts = now->tv_sec;
>  
> -		if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.gw) &&
> -		    !c->no_map_gw) {
> +		if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.dns_match) &&
> +		    ntohs(s_in.sin_port) == 53) {
> +			s_in.sin_addr = c->ip4.dns_host;
> +		} else if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.gw) &&
> +			   !c->no_map_gw) {
>  			if (!(udp_tap_map[V4][dst].flags & PORT_LOCAL) ||
>  			    (udp_tap_map[V4][dst].flags & PORT_LOOPBACK))
>  				s_in.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
>  			else
>  				s_in.sin_addr = c->ip4.addr_seen;
> -		} else if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr,
> -					      &c->ip4.dns_match) &&
> -			   ntohs(s_in.sin_port) == 53) {
> -			s_in.sin_addr = c->ip4.dns_host;
>  		}
>  	} else {
>  		s_in6 = (struct sockaddr_in6) {
> @@ -880,7 +879,11 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
>  		sa = (struct sockaddr *)&s_in6;
>  		sl = sizeof(s_in6);
>  
> -		if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.gw) && !c->no_map_gw) {
> +		if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.dns_match) &&
> +		    ntohs(s_in6.sin6_port) == 53) {
> +			s_in6.sin6_addr = c->ip6.dns_host;
> +		} else if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.gw) &&
> +			   !c->no_map_gw) {
>  			if (!(udp_tap_map[V6][dst].flags & PORT_LOCAL) ||
>  			    (udp_tap_map[V6][dst].flags & PORT_LOOPBACK))
>  				s_in6.sin6_addr = in6addr_loopback;
> @@ -888,9 +891,6 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr,
>  				s_in6.sin6_addr = c->ip6.addr;
>  			else
>  				s_in6.sin6_addr = c->ip6.addr_seen;
> -		} else if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.dns_match) &&
> -			   ntohs(s_in6.sin6_port) == 53) {
> -			s_in6.sin6_addr = c->ip6.dns_host;
>  		} else if (IN6_IS_ADDR_LINKLOCAL(&s_in6.sin6_addr)) {
>  			bind_addr = &c->ip6.addr_ll;
>  		}

-- 
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

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

Re: [PATCH 0/3] Allow non-127.0.0.1 loopback address as host resolver

[Andrea Bolognani]

On Thu, Feb 23, 2023 at 06:07:57PM +0100, Stefano Brivio wrote:
> This series fixes an issue with DNS resolution reported by Andrea: if
> both guest and host use systemd-resolved, reachable at 127.0.0.53, we
> need to advertise a reachable address as DNS to the guest, which we
> already do, but also to map UDP packets for that address and port 53
> to the original host resolver address.
>
> Otherwise, DNS resolution doesn't work at all in the guest, with
> default options.
>
> Stefano Brivio (3):
>   udp: Actually use host resolver to forward DNS queries
>   conf: Split add_dns{4,6}() out of get_dns()
>   conf, udp: Allow any loopback address to be used as resolver

I haven't really looked at the changes, nor do I have any expectation
that I would understand them if I did, but applying this series takes
things from "kinda works, in some cases, at glacial speed, after
tweaking the configuration manually" to "works out of the box, with
reasonable speed and no noticeable issues" on my setup.

Thanks a lot for looking into it!

Tested-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization