Hi Paul,
Thanks for the review and comments! Answers inline:
On Wed, 23 Feb 2022 13:45:02 +0100
Paul Holzinger <pholzing(a)redhat.com> wrote:
Hi Stefano,
This looks very cool. Improved ipv6 support and better performance is
definitely something I would like to have.
I took a quick look at your patch and it looks great. It looks simple to
support for us.
Nice!
Although I have some comments, we should not allow the automatic
port
forwarding mode (at least no by default). It is just too simple to create a
DOS attack on the host with this.
I was also a bit undecided about this one: on one hand, it makes things
rather convenient and "seamless", on the other hand, sure, that.
Perhaps it would be reasonable to make it non-default in the options
passed by Podman ("-t none -u none" if no ports are passed), and keep
it the default in pasta (it saves some typing).
Another idea... I had a half-request in the past to add another
configuration option restricting the range of ports that would be
automatically detected.
Perhaps we could also allow restricting the amount of ports (say, five)
that can be bound automatically. Would something like that preferable as
default?
Also it looks like the loopback interface is shared between host and
container ns. I do not understand why I can see the binded loopback ports
in the netns.
The interface isn't really shared, but yes, ports can be directly bound
both ways.
The reason is to implement faster port forwarding (like the rootlesskit
port handler does) without L4 <-> L2 <-> L4 translation: in that case,
a pair of splice() syscalls (recvmmsg()/sendmmsg() for UDP) directly
between the sockets is enough.
I guess that shouldn't be the default either, at least in Podman (-P
none -U none will disable that).
This seems dangerous and causes other problems because I
cannot bind ports in the netns when they are already used on the host. IMO
access to the hosts loopback adapter should not be allowed at all, if users
need this it should be a separate option like slirp4netns has.
So, by making both mechanisms not the default, the command like to turn
them back on (both TCP and UDP) would be like:
podman -net=pasta:-t,auto,-u,auto,-T,auto,-U,auto
(-t, -u bind from init to ns, and -T, -U bind from ns to init)... any
different preference? Actually, I think this might be less prone to
mistakes than a blanket "all-auto" option.
--
Stefano