From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 59F725A0082 for ; Mon, 7 Nov 2022 02:09:16 +0100 (CET) Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4N5Cmk3BKZz4xx0; Mon, 7 Nov 2022 12:09:10 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1667783350; bh=WrRqiumpVd834H702Jcxrd399PYvHcOK9nsSCWgbjzE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=AiEUS3030VcInH8M1bPvuA5pfNDsK+Go6PtdnFIF3dQV3lTwKlSi654jcNvi7nOGb ptYSxEIN3f60ZVuCwoo+l6oTpfGIC8Gvy5UI7RM6Q+86fMEA5tJoP5SUHSeXo3OMrZ 88fPEA8xk46cs236tZs1PzbGHtJWTKNFS9G8gXiI= Date: Mon, 7 Nov 2022 12:08:59 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 3/3] udp: Check for answers to forwarded DNS queries before handling local redirects Message-ID: References: <20221102230443.377446-1-sbrivio@redhat.com> <20221102230443.377446-4-sbrivio@redhat.com> <20221103074251.60b2898b@elisabeth> <20221105082223.4a447ac5@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Ml1JdUtIFyg/51zx" Content-Disposition: inline In-Reply-To: <20221105082223.4a447ac5@elisabeth> Message-ID-Hash: COGET7IJPHZBZXQV24LJYMK7O2CK74XT X-Message-ID-Hash: COGET7IJPHZBZXQV24LJYMK7O2CK74XT X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, Paul Holzinger X-Mailman-Version: 3.3.3 Precedence: list List-Id: Development discussion and patches for passt Archived-At: <> Archived-At: List-Archive: <> List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --Ml1JdUtIFyg/51zx Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Nov 05, 2022 at 08:22:23AM +0100, Stefano Brivio wrote: > On Sat, 5 Nov 2022 12:19:55 +1100 > David Gibson wrote: >=20 > > On Thu, Nov 03, 2022 at 07:42:51AM +0100, Stefano Brivio wrote: > > > On Thu, 3 Nov 2022 14:42:13 +1100 > > > David Gibson wrote: > > > =20 > > > > On Thu, Nov 03, 2022 at 12:04:43AM +0100, Stefano Brivio wrote: =20 > > > > > Now that we allow loopback DNS addresses to be used as targets for > > > > > forwarding, we need to check if DNS answers come from those targe= ts, > > > > > before deciding to eventually remap traffic for local redirects. > > > > >=20 > > > > > Otherwise, the source address won't match the one configured as > > > > > forwarder, which means that the guest or the container will refuse > > > > > those responses. > > > > >=20 > > > > > Signed-off-by: Stefano Brivio > > > > > --- > > > > > udp.c | 17 ++++++++--------- > > > > > 1 file changed, 8 insertions(+), 9 deletions(-) > > > > >=20 > > > > > 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 stru= ct ctx *c, int n, > > > > > src =3D ntohl(b->s_in.sin_addr.s_addr); > > > > > src_port =3D ntohs(b->s_in.sin_port); > > > > > =20 > > > > > - if (src >> IN_CLASSA_NSHIFT =3D=3D IN_LOOPBACKNET || > > > > > - src =3D=3D INADDR_ANY || src =3D=3D ntohl(c->ip4.addr_seen)= ) { > > > > > + if (c->ip4.dns_fwd && src =3D=3D htonl(c->ip4.dns[0]) && src_po= rt =3D=3D 53) { =20 > > > >=20 > > > > I guess this is not a newly introduced bug, but for the case of > > > > multiple host nameservers, don't you need to check against everythi= ng > > > > in the ip4.dns[] array, not just entry 0? =20 > > >=20 > > > 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 forward= ed > > > ones. =20 > >=20 > > *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? >=20 > Now that we have ip{4,6}.dns_send[], yes. Right, that's what I meant. > We could rename .dns_send[] back to .dns[] and change the current Right, 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. :) >=20 --=20 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 --Ml1JdUtIFyg/51zx Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEoULxWu4/Ws0dB+XtgypY4gEwYSIFAmNoWpIACgkQgypY4gEw YSLrARAAsOa1KeTgmFJ5a7BunbEDUyfNrBGfNJ4CNmXZCF1hxo+641SnvsUphCKF Mrv4YEdolVc3Q93eIxUK4hxWM1boeK9kp8IfI5tr46DeOOXHMFsvxdxiJLwPqk8c EHgDONi1mMIZMc7vO5AEkbM0F0/ieynyE7Zf7VkbmQZCI9yPLTqFgjSZZ2ZITc3z 0OgzweC8ZcK5R59ZOOxdS/maqXd9RjN9Ke798BsvQlNpw4up8JZVtzuob2bMMASc fiUMOgXw3ZtV/t7jrRvj3ej4w73odFbllk0wkojyoMEpTcgn8UXAoVYow9fVAqTA PzpQI2oO6tImIFclIsdb9iCILGVm9aekeEgPrm6+UxYt/26sUWmB8GvSCySzRiRi Y/NS9+ckB9yYNGElVncFZXabrJ+g/BE6S1o66Wm5y5V0RbwXuYD6Y+OBcahRcr9D jNFfaDS5NBR/Z3AzRFD0JFmjk4JmQglsTvDkw/CO61vchNQmbWVDHV7SRp2HcGUv vO9Q+R2vxneyPulL6jkNM2LB5j/bPSTFuwCzt66XAilvauDUe10qj3lLKL1v14ed r1rEROTd3FUG8azO7yaWhrYldl3tyf8IO+YJr8Ekiwdu9hdcSDKw2S+BQ2OBhq8i 1H5yvkIm5ftjKqkR5xcqOpdEep/pdSFF20894cBeErOtIm+61C8= =h9yT -----END PGP SIGNATURE----- --Ml1JdUtIFyg/51zx--