From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 93EB85A026D for ; Sun, 7 Jan 2024 05:28:57 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1704601731; bh=GqIfb/Zo+1skT2+DTDnRAAlO+FoWzv6CRJewnFyP/yY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=KK584KTucAD0Id9ADXTBr7tnkvqIXW54Jg5uGHC/CAgfGJGxUYurUi3GOUVFmty+y HYNDrNNX/1DyoDDbj6gxk4WzVUB8TMbEDqea51srUjU9XK5gt8326MQoYRO+21bQ5o nh2KrmpAtS3VoMNtImJlI/76mYuR+COtLoor027NS5WiKcpiQF4Cgu2pKTvZB35tmz 1TdmEo9kbyZKT84CdG0A6TumRaVlUAoR+bNw/yWiwdI28fEHlTOtSBVp/vDCA2vsPt bRBRtziooBgkN3KtY02ehaNI6LJcSJlTeMUhTI2McgNwVFcg2uLoXlmssSJnThPkJr wymzMOPvJRldA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4T742W5Vzxz4wdC; Sun, 7 Jan 2024 15:28:51 +1100 (AEDT) Date: Sun, 7 Jan 2024 11:37:46 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v2 04/12] icmp: Don't attempt to handle "wrong direction" ping socket traffic Message-ID: References: <20231221065327.1307827-1-david@gibson.dropbear.id.au> <20231221065327.1307827-5-david@gibson.dropbear.id.au> <20240106165911.03b8119a@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="AgnyvVCTgxZH5Ulj" Content-Disposition: inline In-Reply-To: <20240106165911.03b8119a@elisabeth> Message-ID-Hash: P4DGXRNB7BZEUBXVIFEN23DZZFT3JJSN X-Message-ID-Hash: P4DGXRNB7BZEUBXVIFEN23DZZFT3JJSN 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 X-Mailman-Version: 3.3.8 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: --AgnyvVCTgxZH5Ulj Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Jan 06, 2024 at 04:59:11PM +0100, Stefano Brivio wrote: > On Thu, 21 Dec 2023 17:53:19 +1100 > David Gibson wrote: >=20 > > Linux ICMP "ping" sockets are very specific in what they do. They let > > userspace send ping requests (ICMP_ECHO or ICMP6_ECHO_REQUEST), and rec= eive > > matching replies (ICMP_ECHOREPLY or ICMP6_ECHO_REPLY). They don't let = you > > intercept or handle incoming ping requests. >=20 > Right... I don't know exactly what I was trying to do with this, back > then. By the way this is the main reason why it took me a while to > review this series... did I really write all those checks without a > purpose? :) Well, it looks like it. >=20 > Anyway, if you look at ping_err() in net/ipv4/ping.c, you'll see that > among the messages which can be sent back as error (they're received on > the socket causing the error -- same as ICMP messages you get on a UDP > socket for port unreachable), ICMP_ECHO is allowed (see > ping_supported()). >=20 > I think I just used that ping_supported() function to find out which > messages we could get on the socket. But we're not going to get those > anyway. Right. > By the way, ICMP{,V6}_EXT_ECHO{,_REQUEST} support (RFC 8335, PROBE > messages) was added a while ago (kernel commit 08baf54f01f5 "net: add > support for sending RFC 8335 PROBE messages")... we should add that at > some point, it's kind of trivial. Indeed. Want to make a BZ for it so we don't forget? > > In the case of passt/pasta that means we can process echo requests from= tap > > and forward them to a ping socket, then take the replies from the ping > > socket and forward them to tap. We can't do the reverse: take echo > > requests from the host and somehow forward them to the guest. There's > > really no way for something outside to initiate a ping to a passt/pasta > > connected guest and if there was we'd need an entirely different mechan= ism > > to handle it. > >=20 > > However, we have some logic to deal with packets going in that reverse > > direction. Remove it, since it can't ever be used that way. While we'= re > > there use defines for the ICMPv6 types, instead of open coded type valu= es. >=20 > I guess this last sentence only applied to a previous version of your > patch. It doesn't matter so much, but it would be nice to drop if you > re-spin. Uh... no.. that change is right: [snip] > > @@ -222,7 +219,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t p= if, int af, > > if (!ih) > > return 1; > > =20 > > - if (ih->icmp6_type !=3D 128 && ih->icmp6_type !=3D 129) > > + if (ih->icmp6_type !=3D ICMPV6_ECHO_REQUEST) here. --=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 --AgnyvVCTgxZH5Ulj Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmWZ8kUACgkQzQJF27ox 2GdBeQ/+P9/nq3DIU9spKWRO86UURKKQmHqz1zCusREVPFnmOb9FYab1yuj7VS/f j85yF24XIcdxkQmlRygYKAA6wEq8oYB9hKphfd0QTIQ3Lwlo+frFwM1ZEbKlUdQx i/yn1rJzxrE5biX3jswnlH2mj/DL7f3ipCEBVOEW+QddZjNPObT9Csta8rzEmxCr DBeXq1HJBiNhnEaiOxI7/iBWyIhoEw8Tf214ZoqHLGtLKtXTwLZ235HidNjChoT3 QJ6PPZkiIKQRqQMpYVsiea99/pADW6Hh0mrL045+jifNDOf3S6mFKCyINz3jDDYS ai07ztKk9La42kxqS+OruXuvJMxEO3+9hbqxeNlXGVrsLpMLLr+4m8Ts4PHohomJ 0it1H+IV3wckvZpo5fxusqyJM5w1NLSHdBV1Z/ZykFdFeX1n4+00lLvA5SYu63iJ VEZDijiWdpjnwGEQqT75CrfLbnW5uBvOQVuxEqH4bGzSgVBzQthk49fu8Avhl/H6 GOXAIo48JOsAt8pd2uSz6EOarOdSGzaQ7GwFWcCPmcIOQxLFoCuhSLEr8q6SJIQL J+jgD1BOpGze38sCu1SYoBbVk3JawdwGWeK6ZPToMwmyuEIjopSDPgt3IiA5hrUm +er7zxX0UvhiYY3+KcG5/Z/p4LW1x8yY2dM9YakDzxrPqT8wtm4= =WtZ6 -----END PGP SIGNATURE----- --AgnyvVCTgxZH5Ulj--