Re: [PATCH 3/3] udp: Check for answers to forwarded DNS queries before handling local redirects

On Thu, 3 Nov 2022 14:42:13 +1100
David Gibson <david(a)gibson.dropbear.id.au> wrote:

...
  On Thu, Nov 03, 2022 at 12:04:43AM +0100, Stefano
Brivio wrote:
  Now that we allow loopback DNS addresses to be
used as targets for
 forwarding, we need to check if DNS answers come from those targets,
 before deciding to eventually remap traffic for local redirects.
 
 Otherwise, the source address won't match the one configured as
 forwarder, which means that the guest or the container will refuse
 those responses.
 
 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
 ---
  udp.c | 17 ++++++++---------
  1 file changed, 8 insertions(+), 9 deletions(-)
 
 diff --git a/udp.c b/udp.c
 index 4b201d3..7c77e09 100644
 --- a/udp.c
 +++ b/udp.c
 @@ -680,8 +680,10 @@ static void udp_sock_fill_data_v4(const struct ctx *c, int n,
  	src = ntohl(b->s_in.sin_addr.s_addr);
  	src_port = ntohs(b->s_in.sin_port);
  
 -	if (src >> IN_CLASSA_NSHIFT == IN_LOOPBACKNET ||
 -	    src == INADDR_ANY || src == ntohl(c->ip4.addr_seen)) {
 +	if (c->ip4.dns_fwd && src == htonl(c->ip4.dns[0]) && src_port ==
53) {   
 
 I guess this is not a newly introduced bug, but for the case of
 multiple host nameservers, don't you need to check against everything
 in the ip4.dns[] array, not just entry 0? 
No, because that's the only one we're using as target for forwarded
queries -- and DNS answers we want to check here are only the forwarded
ones.

-- 
Stefano