On Sat, Nov 05, 2022 at 08:22:23AM +0100, Stefano Brivio wrote:On Sat, 5 Nov 2022 12:19:55 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:Right, that's what I meant.On Thu, Nov 03, 2022 at 07:42:51AM +0100, Stefano Brivio wrote:Now that we have ip{4,6}.dns_send[], yes.On Thu, 3 Nov 2022 14:42:13 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:*thinks* .. ok, that makes sense. But if that's the case, won't ip4.dns[0] be the only entry in ip4.dns[] we use for anything at all? Can we drop the table and just keep one entry?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.We could rename .dns_send[] back to .dns[] and change the currentRight, I think dns[] is a better name for it..dns[] to .own_dns, or .fwd_dns_target, something like that. Other naming ideas welcome.Yeah, I find the current dns_fwd name not very illuminating either. *thinks* does it even make sense for dns_fwd not to be in dns_send? We're intercepting queries the guest sends to @dns_fwd, so surely we should also be advertising it to the guest. So what about: @dns: Primary DNS server advertised to guest - may be a fake address we'll intercept @dns_extra[]:Additional DNS servers advertised to guest. Must be real servers the guest can address without translation @host_dns: Host side DNS server (may be localhost or another address that's not guest accessible) The DHCP code advertises both @dns and @dns_extra, and that's the *only* place @dns_extra is used. UDP intercepts outbound packets for @dns and redirects them to @host_dns, likewise masquerading inbound packets from @host_dns to appear to have come from @dns.I wanted the change in 2/3 to be simple and fix-like, but I can do this rework soon so that you don't _have_ to. :)-- 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