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

[David Gibson <david@gibson.dropbear.id.au>]

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