From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTP id EBA0E5A027B for ; Wed, 14 Feb 2024 10:16:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1707902182; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=+xGDmQUj57D0nXBaAAC9APnKMRSSs8tlZdcT2mkz+VM=; b=JR+olfaY2OFb925/Jtcs+8uGi2kO9E5C0CTZAhHHlpYgsyDz2YOZlZaGGe17Vso6qXSF7P Na4glXztPV5c5oSWlb7FzdwxK5F/3C914mvfWIVsUQqfSf5Kw4ztZhOcDX4QTHWkr62xpV CmZfk5NiHYY5SXXaJvIIB5SJ79jXJG0= Received: from mail-ej1-f70.google.com (mail-ej1-f70.google.com [209.85.218.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-137-P1KBqbneOQmFJNi4h7orgw-1; Wed, 14 Feb 2024 04:16:20 -0500 X-MC-Unique: P1KBqbneOQmFJNi4h7orgw-1 Received: by mail-ej1-f70.google.com with SMTP id a640c23a62f3a-a357c92f241so309621666b.0 for ; Wed, 14 Feb 2024 01:16:20 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707902179; x=1708506979; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=DCkvlu+sdIUwfU29yxbqObI+1WflJPjYHbeB53jwv2A=; b=k6FYx0f6bm9uc0+BAYwbnnj2/99+gZwn2qhCRY5YAOOjTPGjGUHcsg5awl3f6o/HAL MDrcIiflmUmqobMFN0tspoPN6Acx456NMCKAmLpnh9l77QsjvPPP3LMucEkUkkq1Va5Y kj8ckt4QIinhgNd8FrClkxoJxE+bz+CSA85LOQYxtM6DqIWtYHpYSpW+9a/6FD1uk3eW wzq0shYDJDw5Tl5QeJWgsTo9+66GiEFc4EqFPiXFCmiQmScjjm5IyBxpbbyi9DYNaN+k 03u6OLDTRbZ0FjBkp0vFpxEwmdKrUkATJ/y9TcqdH7u2OuWhCIyfUHF30fpglmKvWuna kcpw== X-Gm-Message-State: AOJu0YwjpnoYGPvGGA/w77zCLz97/uobQ53yamiaQu1h8B03WE8RiYkC 6X/6dlFsvGK2lQp2rqX9DUOluieSm9agUnHjKBCHMxL01n2BlHegwS0TnQpCwIvYZcefuvjqHAP 5sQVagj2IDVasbdycFmh8J0GPmGcwyUrGbuG1pjKRjmywWOhRi+PSg/N4NTb8hCiBUgFQOQq5rk SkP6Vj1iFNC7S71i2sZ9b1dLOQb6TVKelokQA= X-Received: by 2002:a17:906:3b92:b0:a3d:16aa:b62f with SMTP id u18-20020a1709063b9200b00a3d16aab62fmr1244466ejf.71.1707902178791; Wed, 14 Feb 2024 01:16:18 -0800 (PST) X-Google-Smtp-Source: AGHT+IG+JrmKalY/J84SV9adXRGl9ov5mE0w6VexR4OdnZufNkg2fIA7YaCKPiJccYyAfLLKwP7TIg== X-Received: by 2002:a17:906:3b92:b0:a3d:16aa:b62f with SMTP id u18-20020a1709063b9200b00a3d16aab62fmr1244441ejf.71.1707902178187; Wed, 14 Feb 2024 01:16:18 -0800 (PST) Received: from maya.cloud.tilaa.com (maya.cloud.tilaa.com. [164.138.29.33]) by smtp.gmail.com with ESMTPSA id un6-20020a170907cb8600b00a3cfb02c12bsm1550703ejc.79.2024.02.14.01.16.17 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 14 Feb 2024 01:16:17 -0800 (PST) Date: Wed, 14 Feb 2024 10:15:43 +0100 From: Stefano Brivio To: Paul Holzinger Subject: Re: pasta does not correctly handle bind errors with port ranges Message-ID: <20240214101543.6202c69e@elisabeth> In-Reply-To: <20240212175632.5ec08dd6@elisabeth> References: <20240209220939.2f477a76@elisabeth> <20240212151331.277ef9fd@elisabeth> <6f64c0df-3b96-453a-a3a7-8a3fb2c1d70c@redhat.com> <20240212175632.5ec08dd6@elisabeth> Organization: Red Hat X-Mailer: Claws Mail 4.1.1 (GTK 3.24.36; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Message-ID-Hash: KOUVHNFFGYY2F3B76G34JZ6BNIIOKSL7 X-Message-ID-Hash: KOUVHNFFGYY2F3B76G34JZ6BNIIOKSL7 X-MailFrom: sbrivio@redhat.com 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: On Mon, 12 Feb 2024 17:56:32 +0100 Stefano Brivio wrote: > On Mon, 12 Feb 2024 15:43:00 +0100 > Paul Holzinger wrote: >=20 > > On 12/02/2024 15:13, Stefano Brivio wrote: =20 > > > On Mon, 12 Feb 2024 12:45:00 +0100 > > > Paul Holzinger wrote: > > > =20 > > >> Hi Stefano, > > >> > > >> On 09/02/2024 22:09, Stefano Brivio wrote: =20 > > >>> Hi Paul, > > >>> > > >>> On Fri, 9 Feb 2024 17:57:05 +0100 > > >>> Paul Holzinger wrote: > > >>> =20 > > >>>> Hi all, > > >>>> I found some issues with the pasta port binding logic, it does not > > >>>> correctly handle errors when trying to bind a port range. > > >>>> Let's first bind a port so we can force an error condition it: > > >>>> $ nc -l -p 8080 & > > >>>> $ pasta -t 8080=C2=A0 true > > >>>> Failed to bind any port for '-t 8080', exiting <-- fails as expect= ed > > >>>> $ pasta -t 8081 -t 8080=C2=A0 true > > >>>> Failed to bind any port for '-t 8080', exiting <-- here it also fa= ils > > >>>> correctly > > >>>> $ pasta -t 8080-8081=C2=A0 true > > >>>> <-- no error even though pasta could not bind 8080 =20 > > >>> This is actually intended: it only fails if it can't bind *any* por= t in > > >>> a given range, so that users don't have to explicitly exclude ports > > >>> from ranges in case some are already taken, knowingly or not. That'= s > > >>> why the error message says "any port". =20 > > >> Ok but this results in a very inconsistent behavior. I argue this is= not > > >> what a normal user expects at all. =20 > > > This really depends on the user. The main usage behind the current > > > behaviour is the "all" forwarding mode, as originally designed for > > > KubeVirt, now available via libvirt in two forms: -t all / -u all, or > > > giving excluded ranges only (from the man page: "Specifying excluded > > > ranges only implies that all other ports are forwarded."). > > > > > > At least this part, specifically, is an established behaviour and we > > > can't break it. > > > > > > But that's not a substantial conflict with what you're asking for > > > Podman: we can keep this behaviour only if the user asks to forward > > > (almost) "all" the ports (in whatever way). And Podman never asks for > > > that, and probably never will. > > > =20 > > >> It is not even documented in the man page. =20 > > > Right, yes, we'll need to describe it. > > > =20 > > >>> For two ports it probably makes no sense, but for larger ranges > > >>> excluding dozens of ports can get quite annoying for the user. And > > >>> warnings on failed bind() calls could get quite noisy, too. =20 > > >> At the same time this can result in a lot of unexpected problems for > > >> users as it just hides potential problems, assuming 8001-9000 are > > >> already in use then -t 8000-9000 would work and then one might > > >> reasonably expect that if they connect to any of the given ports the= y > > >> talk to the pasta namespace and not something else, this could be a > > >> security concern. I would say in such case yes I want to see a logge= d > > >> error for each individual port. =20 > > > I see, but again, if just port 8005 is not available, you might say > > > that having pasta succeed is a security concern because somebody can > > > "steal" the port, or that having pasta fail is a nuisance because the > > > user probably just wanted to forward the remaining 1000 ports, and > > > graceful degradation would be desirable instead. > > > > > > It's pretty impossible to draw a line in a different way that's not > > > considered inconsistent or inconvenient by some. So: =20 > > > > Yeah I understand that, I guess I just am missing the use case of=20 > > forwarding large port ranges in general as I don't understand how it is= =20 > > used in practice. As an application inside the VM/container that just= =20 > > binds a port it has no knowledge whenever passt/pasta forwarded the por= t=20 > > so if it happens to use one that was skipped because it was already use= d=20 > > on the host it just won't get external connections. =20 >=20 > The "host" could be a very controlled environment, such as a dedicated > network namespace, say a Kubernetes pod. One example I have fresh in > mind, KubeVirt with Istio: >=20 > https://kubevirt.io/user-guide/virtual_machines/istio_service_mesh/ >=20 > Service meshes are usually implemented by / designed for containers. If > you move some parts (implementing a number of servers) to a virtual > machine, they don't expect to get any connection to the ports that are > normally taken on the host, but anything else is legitimate. >=20 > > I guess the answer for that is user error/problem. =20 >=20 > In some sense, yes. Mind that we have automatic and somewhat abstracted > users, too, which is not so much the focus for Podman. >=20 > > >>> If it's a problem for Podman, I can think of two solutions. One wou= ld > > >>> be an option such as --strict-bind or suchlike (better names warmly > > >>> welcome). =20 > > >> It is a big problem for podman as it does not do what users want us = to > > >> do. Also, because podman is smart enough to combine several ports in= to > > >> ranges internally for performance reasons, something like -p 80:80 -= p > > >> 81:81 is always a range for podman thus there is no way for podman u= sers > > >> to avoid hitting this problem. An opt-in option works for me but the= n we > > >> need to bump the minimum pasta requirement version for podman which = is a > > >> bit annoying as I would need to wait until the versions lands in our= CI. =20 > > > ...would probing for the option be a possibility? That is, run 'pasta > > > --strict-bind', and if it fails, log a warning and continue without t= he > > > option. > > > > > > If that's too cumbersome, then I would skip the new option and change > > > it to simply fail if we can't bind a single port (and report the sing= le > > > error right away), unless the user requests to forward "almost all" t= he > > > ports, in which case we could probably warn() with a list of the port= s > > > that we couldn't bind. =20 > >=20 > > I am fine with an opt in option, it just means that I will need to wait= =20 > > a bit until the passt package landed in fedora and our CI images before= =20 > > I can implement it. Retrying without that option is a good idea but I= =20 > > rather just make a clean requirement of version xxx to not complicate= =20 > > the podman code. We are in process of finalizing Podman 5.0 right now s= o=20 > > if such option could be implemented within the next 2-3 weeks that woul= d=20 > > be great and I can add a note the we require passt version XXX now as= =20 > > part of 5.0 and make it the packagers responsibility to provide that. = =20 >=20 > Okay, great. I'll send a patch in a bit. After thinking about it a bit further: adding yet a new option doesn't make a lot of sense if all we want to preserve is the established behaviour for -t / -u all, and while we can document everything, it makes things even more convoluted (and harder to document). In the 'all' case, or with excluded ranges only, we're very likely to fail to bind some ports, and the user can very reasonably expect it, so we can have a different behaviour for those types of port forwarding specifiers and document it. It actually makes the code shorter, perhaps simpler. In any other case, we'll fail as you suggested and report the error. If the user wants to have this same behaviour for -t all, they can also write it as -t 0-65535 -- that's a rather clear indication that they want to bind *all* the ports. With the patch I just posted: $ cat /proc/sys/net/ipv4/ip_unprivileged_port_start=20 21 $ ./passt -t 20 Failed to bind port 20 (Permission denied) for option '-t 20', exiting $ ./passt -t 20-5000 Failed to bind port 20 (Permission denied) for option '-t 20-5000', exiti= ng $ ./passt -t 23 -t 18 Failed to bind port 18 (Permission denied) for option '-t 18', exiting $ ./passt -t 22-23 Failed to bind port 22 (Address already in use) for option '-t 22-23', ex= iting but: $ ./passt -t ~0,~21-65535 Failed to bind any port for '-t ~0,~21-65535', exiting because this approximates the -t all behaviour, minus some ports. Let me know if this works for you. --=20 Stefano