Re: pasta does not correctly handle bind errors with port ranges

[Paul Holzinger <pholzing@redhat.com>]

Hi Stefano,

On 09/02/2024 22:09, Stefano Brivio wrote:
> Hi Paul,
>
> On Fri, 9 Feb 2024 17:57:05 +0100
> Paul Holzinger <pholzing@redhat.com> wrote:
>
>> 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  true
>> Failed to bind any port for '-t 8080', exiting <-- fails as expected
>> $ pasta -t 8081 -t 8080  true
>> Failed to bind any port for '-t 8080', exiting <-- here it also fails
>> correctly
>> $ pasta -t 8080-8081  true
>> <-- no error even though pasta could not bind 8080
> This is actually intended: it only fails if it can't bind *any* port 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".
Ok but this results in a very inconsistent behavior. I argue this is not 
what a normal user expects at all. It is not even documented in the man 
page.
>
> 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.
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 they 
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 logged 
error for each individual port.
>
> If it's a problem for Podman, I can think of two solutions. One would
> be an option such as --strict-bind or suchlike (better names warmly
> welcome).
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 into 
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 users 
to avoid hitting this problem. An opt-in option works for me but then 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.
> Another idea would be that the back-end in Podman passes ranges as
> single ports... but then the command line might explode and that's
> not ideal for users, either. I'd rather favour the extra option.
Yeah that works but for large ranges it would not be as performant and 
there is the risk of hitting ARG_MAX (although I understand it is 
unlikely to be real problem given one would need to give millions of 
ports to hit it).
>
>> Also besides this I find the error message less than ideal. It missing
>> the errno from the bind syscall so important context gets lost (i.e.
>> Address already in use vs Permission denied).
> The problem is that we might fail to bind multiple ports, so there
> isn't necessarily a single bind() error. But if we go with
> --strict-bind, we could report the first error (including return code
> from the system call) and exit right away.

I am fine with this, exit on the first error is what all our other 
network tools do.
But the behavior also exists for even a single port today, for reference 
the error messages today:

rootlessport:

$ podman run -p 53:53 --rm quay.io/libpod/testimage:20221018
Error: rootlessport cannot expose privileged port 53, you can add 
'net.ipv4.ip_unprivileged_port_start=53' to /etc/sysctl.conf (currently 
1024), or choose a larger port number (>= 1024): listen tcp 0.0.0.0:53: 
bind: permission denied
$ podman run -p 8000:8000 --network slirp4netns  --rm 
quay.io/libpod/testimage:20221018
Error: rootlessport listen tcp 0.0.0.0:8000: bind: address already in use

pasta:

$ podman run -p 53:53 --network pasta  --rm 
quay.io/libpod/testimage:20221018
Error: pasta failed with exit code 1:
No external routable interface for IPv6
Failed to bind any port for '-t 53-53:53-53', exiting
$ podman run -p 8000:8000 --network pasta  --rm 
quay.io/libpod/testimage:20221018
Error: pasta failed with exit code 1:
No external routable interface for IPv6
Failed to bind any port for '-t 8000-8000:8000-8000', exiting

I think the answer of what is more helpful to users is obvious and that 
isn't just my opinion[1].


>
> Let me know if any of this would address your problem, I can write a
> patch in the next days in case (or feel free to submit one).
>
[1] https://github.com/containers/podman/pull/21563#issuecomment-1937024642

--
Paul