Re: [PATCH v2 01/16] udp: Also bind() connected ports for "splice" forwarding

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 24 Nov 2022 12:16:44 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> pasta handles "spliced" port forwarding by resending datagrams received on
> a bound socket in the init namespace to a connected socket in the guest
> namespace.  This means there are actually three ports associated with each
> "connection".  First there's the source and destination ports of the
> originating datagram.  That's also the destination port of the forwarded
> datagram, but the source port of the forwarded datagram is the kernel
> allocated bound address of the connected socket.
> 
> However, by bind()ing as well as connect()ing the forwarding socket we can
> choose the source port of the forwarded datagrams.  By choosing it to match
> the original source port we remove that surprising third port number and
> no longer need to store port numbers in struct udp_splice_port.

If you wondered, I think the whole connect() with getsockname() thing
without a bind() came from the fundamental misconception I had that you
couldn't connect() a bound socket -- and I didn't quite think of
dropping connect() as you do in 3/16 anyway.

There's one minor problem this introduces: the source port of the
originating datagram now needs to be free in the init namespace. It's
still better than the alternative problem you fix in 16/16, though.

I'm wondering if we could, _once you're done with all this_ (it already
looks complicated enough), revisit the 'goto fail' in
udp_splice_connect() (now udp_splice_new()) when bind() fails, and just
proceed with an ephemeral port then.

Also, I haven't tried, but I'm not sure if this introduces some kind of
DoS possibility: even if pasta forwards a single port, it should be
possible for a remote host to make pasta bind to a large amount of
non-ephemeral ports.

Maybe it would make sense to think of a limit on how many ports a
single peer could cause pasta to bind.

I'm not sure yet how we could track peers without a separate address
storage (even though keeping an LRU array should be feasible) -- the
simpler alternative, limiting bound ports by destination port, would
offer an even more convenient way to a DoS.

On the other hand, this is exceedingly minor I guess. We're binding
ports in the namespace after all, and we can reuse bound sockets.

-- 
Stefano