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 D5A9D5A0082 for ; Thu, 3 Nov 2022 04:42:36 +0100 (CET) Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4N2qMY4rHmz4xGQ; Thu, 3 Nov 2022 14:42:33 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1667446953; bh=yhCc/Dc/vTpTuYjrrrYRuIHIuYWlx7whtjZKh9EeKNo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=aXhAhA+dYVzIy5Y8/nXfAkWmzpwDzFjz/ioa70EFJ5vVFnLp3QjI5YOZJ6135XZ6z j80ukDpsiLZks6To6WCTAMiWmbiFACFm11hKkCozQzjtVbqzw5IdFWIFT5bW2vmX6p qNx6MDJ/uYHP2fAwAgusYtdOceMTW26DhhYZ08lk= Date: Thu, 3 Nov 2022 14:42:13 +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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="zTdkncamjfjn3goN" Content-Disposition: inline In-Reply-To: <20221102230443.377446-4-sbrivio@redhat.com> Message-ID-Hash: S2OWXKTSOFLJIRNMEHDF7VHFSJXW6S75 X-Message-ID-Hash: S2OWXKTSOFLJIRNMEHDF7VHFSJXW6S75 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: --zTdkncamjfjn3goN Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. >=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 struct 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_port =3D= =3D 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? > + b->iph.saddr =3D c->ip4.dns_fwd; > + } else if (IPV4_IS_LOOPBACK(src) || src =3D=3D INADDR_ANY || > + src =3D=3D ntohl(c->ip4.addr_seen)) { > b->iph.saddr =3D c->ip4.gw; > udp_tap_map[V4][src_port].ts =3D now->tv_sec; > udp_tap_map[V4][src_port].flags |=3D PORT_LOCAL; > @@ -692,9 +694,6 @@ static void udp_sock_fill_data_v4(const struct ctx *c= , int n, > udp_tap_map[V4][src_port].flags |=3D PORT_LOOPBACK; > =20 > bitmap_set(udp_act[V4][UDP_ACT_TAP], src_port); > - } else if (c->ip4.dns_fwd && > - src =3D=3D htonl(c->ip4.dns[0]) && src_port =3D=3D 53) { > - b->iph.saddr =3D c->ip4.dns_fwd; > } else { > b->iph.saddr =3D b->s_in.sin_addr.s_addr; > } > @@ -770,6 +769,10 @@ static void udp_sock_fill_data_v6(const struct ctx *= c, int n, > if (IN6_IS_ADDR_LINKLOCAL(src)) { > b->ip6h.daddr =3D c->ip6.addr_ll_seen; > b->ip6h.saddr =3D b->s_in6.sin6_addr; > + } else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_fwd) && > + IN6_ARE_ADDR_EQUAL(src, &c->ip6.dns[0]) && src_port =3D=3D 53) { > + b->ip6h.daddr =3D c->ip6.addr_seen; > + b->ip6h.saddr =3D c->ip6.dns_fwd; > } else if (IN6_IS_ADDR_LOOPBACK(src) || > IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr_seen) || > IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr)) { > @@ -794,10 +797,6 @@ static void udp_sock_fill_data_v6(const struct ctx *= c, int n, > udp_tap_map[V6][src_port].flags &=3D ~PORT_GUA; > =20 > bitmap_set(udp_act[V6][UDP_ACT_TAP], src_port); > - } else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_fwd) && > - IN6_ARE_ADDR_EQUAL(src, &c->ip6.dns[0]) && src_port =3D=3D 53) { > - b->ip6h.daddr =3D c->ip6.addr_seen; > - b->ip6h.saddr =3D c->ip6.dns_fwd; > } else { > b->ip6h.daddr =3D c->ip6.addr_seen; > b->ip6h.saddr =3D b->s_in6.sin6_addr; --=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 --zTdkncamjfjn3goN Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEoULxWu4/Ws0dB+XtgypY4gEwYSIFAmNjOI4ACgkQgypY4gEw YSIl4xAAqiHyIyjYdVkdzvdl20UDd1WATy811OPehwm9JMbr2lAm+xhdZimTsncu Rg8iMZyNmcknJk8ZL25tj+cMRie/d5Y+km4gTy/G/SIfcHuZkEDm2Bj0kUrwSq8a 5V3U5iRajhp3ngAgPK2F3QU8wXgPhCYIqvalz48MW0ierLWiw3xyQlDh7A0ryQks iOy6WDYZ1ibD8PC0WDBYQ5eJvPH4WnpxrT/6Rov2pq6yUNBye48b+7OveqJ3oCmc j0ZhR825f8fymN5N9vXrfK39szSLIwp0zAyI5Dsfog4CS0Gga8alxzZenvcQOfuQ i5vEeha4bRoJok+Hcq2jGujh11uMukk2mD9sYUsBjhCapMJtFDBljZo50WPuZ3Kj PiliK/pYZnKj4plgpig5hAV+cTgcOsKCx26pABUKJHq32YKIyun65xTaF8EziiDX +uxT64/46b3OsCo4pt0TLOpZywRnlz8luQ1DmQQut6ZdVSiHTEn4zQJGAMNf6q71 jg8q34fXjIJ3+AlolxTq2ZdytmN5ZP1DUBNL5wsjUj6oXzfcUyboeeP6oDRGl/NT WVFTYkhK7nIMJKfkgcl3z28J/m7Z5n7Lmniha34HotPnPu31treniwlDnyZ/yQwh /9CSRpv5uqxh41l/4qinfyKQk4z78IiNSzGq+hirvj71poqHkDE= =7zM2 -----END PGP SIGNATURE----- --zTdkncamjfjn3goN--