ip_nonlocal_bind causes havoc with local connection detection

ip_nonlocal_bind causes havoc with local connection detection

[Valtteri Vuorikoski]

If net.ipv4.ip_nonlocal_bind is enabled, the following code in
tcp_conn_from_tap gets very confused:

        if (!bind(s, sa, sl)) {
                tcp_rst(c, conn);       /* Nobody is listening then */
                return;
        }
        if (errno != EADDRNOTAVAIL && errno != EACCES)
                conn_flag(c, conn, LOCAL);

This is especially visible if net.ipv4.ip_unprivileged_port_start is
set to a value lower than the default. For example, if
net.ipv4.ip_unprivileged_port_start=443 and
net.ipv4.ip_nonlocal_bind=1, the bind()==0 branch will be hit for all
outgoing connections going to port 443 because bind() succeeds even
when "sa" contains the remote address, and pretty much nothing will
work.

It might the best to skip the check and marking connections as LOCAL if
net.ipv4.ip_nonlocal_bind is enabled? If that doesn't seem reasonable,
then maybe show a warning at start and/or just document that the
ip_nonlocal_bind setting shouldn't be used with passt?

 -Valtteri
 

Re: ip_nonlocal_bind causes havoc with local connection detection

[Stefano Brivio]

On Tue, 18 Jul 2023 11:14:14 +0300
Valtteri Vuorikoski <vuori@notcom.org> wrote:

> If net.ipv4.ip_nonlocal_bind is enabled, the following code in
> tcp_conn_from_tap gets very confused:
> 
>         if (!bind(s, sa, sl)) {
>                 tcp_rst(c, conn);       /* Nobody is listening then */
>                 return;
>         }
>         if (errno != EADDRNOTAVAIL && errno != EACCES)
>                 conn_flag(c, conn, LOCAL);
> 
> This is especially visible if net.ipv4.ip_unprivileged_port_start is
> set to a value lower than the default. For example, if
> net.ipv4.ip_unprivileged_port_start=443 and
> net.ipv4.ip_nonlocal_bind=1, the bind()==0 branch will be hit for all
> outgoing connections going to port 443 because bind() succeeds even
> when "sa" contains the remote address, and pretty much nothing will
> work.

Ouch, I didn't think about that.

> It might the best to skip the check and marking connections as LOCAL if
> net.ipv4.ip_nonlocal_bind is enabled?

This would mean that if ip_nonlocal_bind is set, we'll always override
the MSS, which would break essentially any non-local connection.

> If that doesn't seem reasonable,
> then maybe show a warning at start and/or just document that the
> ip_nonlocal_bind setting shouldn't be used with passt?

That's not really friendly, nor future-proof:

    https://bugs.passt.top/show_bug.cgi?id=48

I think we should go the relatively hard way of extracting the relevant
logic from procfs_scan_listen(), and understand from there if there's a
local bind for the port at hand.

I'm not sure, then, if we should always use this mechanism, even if
ip_nonlocal_bind isn't set, because bind() gives us a lightweight way to
check for three conditions in one, and we're on a latency-critical path
here, so if this results in more syscalls, I would read from procfs
just in case we really need to.

Feel free to send a patch, or file a bug, or both, or none. :)

-- 
Stefano

Re: ip_nonlocal_bind causes havoc with local connection detection

[Valtteri Vuorikoski]

On Wed, Jul 19, 2023 at 04:10:52PM +0200, Stefano Brivio wrote:
> > If that doesn't seem reasonable,
> > then maybe show a warning at start and/or just document that the
> > ip_nonlocal_bind setting shouldn't be used with passt?
> 
> That's not really friendly, nor future-proof:
> 
>     https://bugs.passt.top/show_bug.cgi?id=48
> 
> I think we should go the relatively hard way of extracting the relevant
> logic from procfs_scan_listen(), and understand from there if there's a
> local bind for the port at hand.
> 
> I'm not sure, then, if we should always use this mechanism, even if
> ip_nonlocal_bind isn't set, because bind() gives us a lightweight way to
> check for three conditions in one, and we're on a latency-critical path
> here, so if this results in more syscalls, I would read from procfs
> just in case we really need to.
> 
> Feel free to send a patch, or file a bug, or both, or none. :)

Thanks for checking this out. But yeah, I looked at the alternatives
a bit and none seemed really appealing. Maybe go for the proc route if
nonlocal binds were enabled at startup?

Luckily for me, it turned out that ip_nonlocal_bind was enabled on
some servers due to a service that had since been removed, so this
time we could solve the problem by just turning the sysctl off. I'll
try to get something into bugzilla for this issue anyway.

 -Valtteri