From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id F15C65A004F for ; Thu, 11 Jul 2024 04:52:40 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1720666357; bh=bQcUpa1QDl5p3Uw6G0xuR80OAfvhaLyFiRIFcH4c4qs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=DbT9OS+9lnN68kDP5l5e0KOq6trKB+GM5Kx4zEkPMkKwcgyaKyMXA06+sCzx2sFi3 NQl1B04bNf4KOEq81zUXQfqvYw+cUpsrevGspi1sKCQrTndhNWV48swfyBcmkdEy0d sOtUkh8DjcHEE5LvFLXfcjtVaM0StkvUfkMmt0DIyQUCmfTYYvhm66MeW3TSvh93nf oGzIglU+s6AJQmpv+eqVSTSu3BC/pigHB5Rrv5/Md+58h71hobjXlqlgh/Yi6f98A8 22gJbXiL7NK6G2NTQPYYso7vZJrIul5egES0CscbFAcSgcMJvq6jzGyAXN1meP1hpu pjGWembbuwYkA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4WKK5d5rN3z4wnx; Thu, 11 Jul 2024 12:52:37 +1000 (AEST) Date: Thu, 11 Jul 2024 12:48:48 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v7 21/27] udp: Handle "spliced" datagrams with per-flow sockets Message-ID: References: <20240705020724.3447719-1-david@gibson.dropbear.id.au> <20240705020724.3447719-22-david@gibson.dropbear.id.au> <20240710003233.1d9937fd@elisabeth> <20240710191316.53b7ac5d@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="VgarDYJcnYm9ubOS" Content-Disposition: inline In-Reply-To: <20240710191316.53b7ac5d@elisabeth> Message-ID-Hash: K753II2JWZXJW55U5JTXLGYAZW2Z53CU X-Message-ID-Hash: K753II2JWZXJW55U5JTXLGYAZW2Z53CU 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, jmaloy@redhat.com 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: --VgarDYJcnYm9ubOS Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jul 10, 2024 at 07:13:26PM +0200, Stefano Brivio wrote: > On Wed, 10 Jul 2024 10:23:14 +1000 > David Gibson wrote: >=20 > > On Wed, Jul 10, 2024 at 12:32:33AM +0200, Stefano Brivio wrote: > > > On Fri, 5 Jul 2024 12:07:18 +1000 > > > David Gibson wrote: [snip] > > > > + * need to. > > > > + */ > > > > + /* cppcheck-suppress nullPointer */ > > > > + while (recv(uflow->s[TGTSIDE], NULL, 0, MSG_DONTWAIT) >=3D 0) > > > > + ; =20 > > >=20 > > > Could a local attacker (another user) attempt to use this for denial = of > > > service? =20 > >=20 > > Ah, interesting question. > >=20 > > > Of course, somebody could flood us anyway and we would get and handle > > > all the events that that causes. But this case is different because we > > > could get stuck for an unlimited amount of time without serving other > > > sockets at all. =20 > >=20 > > Right. > >=20 > > > If that's a possibility, perhaps a limit for this loop (a maximum > > > amount of recv()) tries would be a good idea. I'm not sure how we sho= uld > > > handle the case where we exceed the threshold. =20 > >=20 > > We could fail to create the flow. That would limit the damage, but > > see below. > >=20 > > > Another one, which adds some complexity, but looks more correct to me, > > > would be to try a single recv() call, and if we get data from it, fail > > > creating the new flow entirely. =20 > >=20 > > Right. I also considered a single recvmmsg(); the difficulty with > > that is making suitable arrays for it, since the normal ones may be in > > use at this point. >=20 > For UDP, we don't need to make the buffers large enough to fit packets > into them, see udp(7): >=20 > All receive operations return only one packet. When the packet is > smaller than the passed buffer, only that much data is returned; when it > is bigger, the packet is truncated and the MSG_TRUNC flag is set. >=20 > so probably 1024 bytes (1 * UIO_MAXIOV) on the stack would be enough... > or maybe we can even pass 0 as size and NULL buffers? We can (see doc/platform-requirements/recv-zero.c). And even if we couldn't, we could use the same 1 byte buffer for all the datagrams. We basically just need the actual msghdr array. > If recvmmsg() returns UIO_MAXIOV, then something weird is going on and > we can abort the "connection" attempt. Seems reasonable; I've coded something up. > > This removes the potential DoS of wedging passt completely, but raises > > the possibility of a different one. Could an attacker - not > > necessarily on the same host, but network-closer than the guest's > > intended endpoint - prevent the guest from connecting to a particular > > service by spamming fake reply packets here. >=20 > Oops, I didn't think about that, that's also concerning. With my > suggestion above, 1024 packets is all an attacker would need. >=20 > But maybe there's a safe way to deal with this that's still simpler > than a separate handler: using recvmmsg(), going through msg_name of > the (truncated) messages we received with it, and trying to draw some > conclusion about what kind of attack we're dealing with from there. I'm > not sure exactly which conclusion, though. >=20 > > I believe it would need to anticipate the guest's source port to do so, > > which might be good enough. >=20 > Linux does source port randomisation for UDP starting from 2.6.24, so > that should be good enough, but given that we try to preserve the > source port from the guest, we might make a guest that doesn't do port > randomisation (albeit unlikely to be found) vulnerable to this. Right. It feels like we're at least not making the guest dramatically more vulnerable to something here, so I'm inclined to treat it as good enough for now. --=20 David Gibson (he or they) | 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 --VgarDYJcnYm9ubOS Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmaPSA8ACgkQzQJF27ox 2Gdkfg//b8yT6pD73Vu0A0UMuDm06JRr+rFjXcnlC4VqmyNz3iANIZZPdI8uY80s usFUXFcg5k1JbZ8eI2EYmHgBoiVJTAB0ZIjBvGCFZ2ZW8q4PS0FnNEI649yWHoqd h/RrjvBM7urUnv6SdjhfcnGszp9XxnJ7ukXMFmOhnAeEcCDEId+b93QMzqzE94J9 35d4Fu+Y3t8XNtaCg0v6KC5fle09VK0qsnJxGdhUx1mTtdhTB6rAtLo8QZqshv3Z 6s00WBi2+pM+LlDK4saLMbSCyr3EWUgR6qz5dr7Sdzlm6m8pHJpKGc6eNo3orZ9o 6u0iFV305D4FWr7cV2YxbDC0PmZzYGceeraTImWPIUBSE6j73IqLDNN53nsRlTKi zuJ64Fl+zeSDmtwphQZA+DiDvrVd+udUjGvBSMdXoRzu+CX6xyA+uX7TBHT7DGxk XPTp38HYH3LHiDhn4XrDHkilDDZZIsxRUZLZbxkpvZ3K9zQKW48x9BMG76PoJwYY Zh3FsWE4yVDlflRcM30RQZ2iY1+NfHcf2DA5WJ//SOO/qduiqGSzvUOcJUY2JaJm 8Mw/ecIqsdp0kwxKA2UwThnOjfF0dGaxBjeToxVLVt9DCxaNumz2uh65oFZKvm9Q yr6mvsNgssD9HJW9k8Jqc/1jJ5X+jFwSUCgVKJ0EqVriLIAki7k= =ulbG -----END PGP SIGNATURE----- --VgarDYJcnYm9ubOS--