pasta does not correctly handle bind errors with port ranges

pasta does not correctly handle bind errors with port ranges

[Paul Holzinger]

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

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).

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

[Stefano Brivio]

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".

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.

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).

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.

> 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.

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).

-- 
Stefano

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

[Paul Holzinger]

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

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

[Stefano Brivio]

On Mon, 12 Feb 2024 12:45:00 +0100
Paul Holzinger <pholzing@redhat.com> wrote:

> 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.

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.

> It is not even documented in the man page.

Right, yes, we'll need to describe it.

> > 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.

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:

> > 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.

...would probing for the option be a possibility? That is, run 'pasta
--strict-bind', and if it fails, log a warning and continue without the
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 single
error right away), unless the user requests to forward "almost all" the
ports, in which case we could probably warn() with a list of the ports
that we couldn't bind.

> > 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

Reading [1] below, this string is considered confusing too, even though
I feel that hiding it conceptually clashes with the rationale of your
very request. That is, one might argue that the user wanted IPv6 too,
but it's not working, and we should tell the user.

Anyway, we can change it to info() and run pasta with -q / --quiet.
Would that help?

> 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].

Sure, I understand, especially in case of a single port. This is just
the first time somebody reports that.

> > 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

-- 
Stefano

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

[Paul Holzinger]

On 12/02/2024 15:13, Stefano Brivio wrote:
> On Mon, 12 Feb 2024 12:45:00 +0100
> Paul Holzinger <pholzing@redhat.com> wrote:
>
>> 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.
> 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.
>
>> It is not even documented in the man page.
> Right, yes, we'll need to describe it.
>
>>> 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.
> 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:
Yeah I understand that, I guess I just am missing the use case of 
forwarding large port ranges in general as I don't understand how it is 
used in practice. As an application inside the VM/container that just 
binds a port it has no knowledge whenever passt/pasta forwarded the port 
so if it happens to use one that was skipped because it was already used 
on the host it just won't get external connections. I guess the answer 
for that is user error/problem.
>
>>> 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.
> ...would probing for the option be a possibility? That is, run 'pasta
> --strict-bind', and if it fails, log a warning and continue without the
> 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 single
> error right away), unless the user requests to forward "almost all" the
> ports, in which case we could probably warn() with a list of the ports
> that we couldn't bind.

I am fine with an opt in option, it just means that I will need to wait 
a bit until the passt package landed in fedora and our CI images before 
I can implement it. Retrying without that option is a good idea but I 
rather just make a clean requirement of version xxx to not complicate 
the podman code. We are in process of finalizing Podman 5.0 right now so 
if such option could be implemented within the next 2-3 weeks that would 
be great and I can add a note the we require passt version XXX now as 
part of 5.0 and make it the packagers responsibility to provide that.

>
>>> 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
> Reading [1] below, this string is considered confusing too, even though
> I feel that hiding it conceptually clashes with the rationale of your
> very request. That is, one might argue that the user wanted IPv6 too,
> but it's not working, and we should tell the user.
>
> Anyway, we can change it to info() and run pasta with -q / --quiet.
> Would that help?
>
>> 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].
> Sure, I understand, especially in case of a single port. This is just
> the first time somebody reports that.
>
>>> 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

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

[Stefano Brivio]

On Mon, 12 Feb 2024 15:43:00 +0100
Paul Holzinger <pholzing@redhat.com> wrote:

> On 12/02/2024 15:13, Stefano Brivio wrote:
> > On Mon, 12 Feb 2024 12:45:00 +0100
> > Paul Holzinger <pholzing@redhat.com> wrote:
> >  
> >> 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.  
> > 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.
> >  
> >> It is not even documented in the man page.  
> > Right, yes, we'll need to describe it.
> >  
> >>> 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.  
> > 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:  
>
> Yeah I understand that, I guess I just am missing the use case of 
> forwarding large port ranges in general as I don't understand how it is 
> used in practice. As an application inside the VM/container that just 
> binds a port it has no knowledge whenever passt/pasta forwarded the port 
> so if it happens to use one that was skipped because it was already used 
> on the host it just won't get external connections.

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:

  https://kubevirt.io/user-guide/virtual_machines/istio_service_mesh/

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.

> I guess the answer for that is user error/problem.

In some sense, yes. Mind that we have automatic and somewhat abstracted
users, too, which is not so much the focus for Podman.

> >>> 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.  
> > ...would probing for the option be a possibility? That is, run 'pasta
> > --strict-bind', and if it fails, log a warning and continue without the
> > 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 single
> > error right away), unless the user requests to forward "almost all" the
> > ports, in which case we could probably warn() with a list of the ports
> > that we couldn't bind.  
> 
> I am fine with an opt in option, it just means that I will need to wait 
> a bit until the passt package landed in fedora and our CI images before 
> I can implement it. Retrying without that option is a good idea but I 
> rather just make a clean requirement of version xxx to not complicate 
> the podman code. We are in process of finalizing Podman 5.0 right now so 
> if such option could be implemented within the next 2-3 weeks that would 
> be great and I can add a note the we require passt version XXX now as 
> part of 5.0 and make it the packagers responsibility to provide that.

Okay, great. I'll send a patch in a bit.

-- 
Stefano

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

[Stefano Brivio]

On Mon, 12 Feb 2024 17:56:32 +0100
Stefano Brivio <sbrivio@redhat.com> wrote:

> On Mon, 12 Feb 2024 15:43:00 +0100
> Paul Holzinger <pholzing@redhat.com> wrote:
> 
> > On 12/02/2024 15:13, Stefano Brivio wrote:  
> > > On Mon, 12 Feb 2024 12:45:00 +0100
> > > Paul Holzinger <pholzing@redhat.com> wrote:
> > >    
> > >> 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.    
> > > 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.
> > >    
> > >> It is not even documented in the man page.    
> > > Right, yes, we'll need to describe it.
> > >    
> > >>> 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.    
> > > 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:    
> >
> > Yeah I understand that, I guess I just am missing the use case of 
> > forwarding large port ranges in general as I don't understand how it is 
> > used in practice. As an application inside the VM/container that just 
> > binds a port it has no knowledge whenever passt/pasta forwarded the port 
> > so if it happens to use one that was skipped because it was already used 
> > on the host it just won't get external connections.  
> 
> 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:
> 
>   https://kubevirt.io/user-guide/virtual_machines/istio_service_mesh/
> 
> 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.
> 
> > I guess the answer for that is user error/problem.  
> 
> In some sense, yes. Mind that we have automatic and somewhat abstracted
> users, too, which is not so much the focus for Podman.
> 
> > >>> 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.    
> > > ...would probing for the option be a possibility? That is, run 'pasta
> > > --strict-bind', and if it fails, log a warning and continue without the
> > > 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 single
> > > error right away), unless the user requests to forward "almost all" the
> > > ports, in which case we could probably warn() with a list of the ports
> > > that we couldn't bind.    
> > 
> > I am fine with an opt in option, it just means that I will need to wait 
> > a bit until the passt package landed in fedora and our CI images before 
> > I can implement it. Retrying without that option is a good idea but I 
> > rather just make a clean requirement of version xxx to not complicate 
> > the podman code. We are in process of finalizing Podman 5.0 right now so 
> > if such option could be implemented within the next 2-3 weeks that would 
> > be great and I can add a note the we require passt version XXX now as 
> > part of 5.0 and make it the packagers responsibility to provide that.  
> 
> 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 
  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', exiting
  $ ./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', exiting

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.

-- 
Stefano

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

[Paul Holzinger]

On 14/02/2024 10:15, Stefano Brivio wrote:
> On Mon, 12 Feb 2024 17:56:32 +0100
> Stefano Brivio <sbrivio@redhat.com> wrote:
>
>> On Mon, 12 Feb 2024 15:43:00 +0100
>> Paul Holzinger <pholzing@redhat.com> wrote:
>>
>>> On 12/02/2024 15:13, Stefano Brivio wrote:
>>>> On Mon, 12 Feb 2024 12:45:00 +0100
>>>> Paul Holzinger <pholzing@redhat.com> wrote:
>>>>     
>>>>> 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.
>>>> 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.
>>>>     
>>>>> It is not even documented in the man page.
>>>> Right, yes, we'll need to describe it.
>>>>     
>>>>>> 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.
>>>> 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:
>>> Yeah I understand that, I guess I just am missing the use case of
>>> forwarding large port ranges in general as I don't understand how it is
>>> used in practice. As an application inside the VM/container that just
>>> binds a port it has no knowledge whenever passt/pasta forwarded the port
>>> so if it happens to use one that was skipped because it was already used
>>> on the host it just won't get external connections.
>> 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:
>>
>>    https://kubevirt.io/user-guide/virtual_machines/istio_service_mesh/
>>
>> 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.
>>
>>> I guess the answer for that is user error/problem.
>> In some sense, yes. Mind that we have automatic and somewhat abstracted
>> users, too, which is not so much the focus for Podman.
>>
>>>>>> 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.
>>>> ...would probing for the option be a possibility? That is, run 'pasta
>>>> --strict-bind', and if it fails, log a warning and continue without the
>>>> 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 single
>>>> error right away), unless the user requests to forward "almost all" the
>>>> ports, in which case we could probably warn() with a list of the ports
>>>> that we couldn't bind.
>>> I am fine with an opt in option, it just means that I will need to wait
>>> a bit until the passt package landed in fedora and our CI images before
>>> I can implement it. Retrying without that option is a good idea but I
>>> rather just make a clean requirement of version xxx to not complicate
>>> the podman code. We are in process of finalizing Podman 5.0 right now so
>>> if such option could be implemented within the next 2-3 weeks that would
>>> be great and I can add a note the we require passt version XXX now as
>>> part of 5.0 and make it the packagers responsibility to provide that.
>> 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
>    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', exiting
>    $ ./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', exiting
>
> 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.
Sure this works for me, thanks.