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

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 3 Nov 2022 14:42:13 +1100
David Gibson <david@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@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