Re: [PATCH 3/4] mptcp: handle join requests via pernet listen socket

[Mat Martineau <mathew.j.martineau@linux.intel.com>]

On Wed, 23 Feb 2022, Florian Westphal wrote:

> Currently mptcp adds kernel-based listener socket for all
> netlink-configured mptcp address endpoints.
>
> This has caveats because kernel may interfere with unrelated programs
> that use same address/port pairs.
>
> RFC 8684 says:
> Demultiplexing subflow SYNs MUST be done using the token; this is
> unlike traditional TCP, where the destination port is used for
> demultiplexing SYN packets.  Once a subflow is set up, demultiplexing
> packets is done using the 5-tuple, as in traditional TCP.
>
> This patch deviates from this in that it retains the existing checks of
> verifying the incoming requests destination vs. the list of announced
> addresses.  If the request is to an address that was not assigned, its
> treated like an invalid token, i.e. we send a tcp reset with mptcp
> error specific code is returned.
>
> The checks that do this are moved from subflow specific code to the
> new hook, this allows us to perform the check at an earlier stage.
>
> Furthermore, TCP-only listeners take precedence: An MPTCP peer MUST NOT
> announce addr:port pairs that are already in use by a non-mptcp listener.
>

This rule seems ok to me. But there's a loophole: MPTCP can announce the 
addr:port, then a TCP listener is created for that same addr:port and 
starts rejecting any incoming MP_JOINs.

> This could be changed, but it requires move of mptcp_handle_join() hook
> *before* the tcp port demux, i.e. an additional conditional in hotpath.
>

One possibility that doesn't disturb the hot path too much: Let the TCP 
listener try to handle the SYN. In tcp_parse_options(), add just enough 
code to set a bit in struct tcp_options_received if MP_JOIN is present 
(don't fully parse the MPTCP option). If MP_JOIN is there and MPTCP is 
enabled, return early from tcp_check_req() and have tcp_v4_rcv() treat it 
similar to req_stolen==true. Instead of kicking back to a regular lookup, 
try a token lookup.

> As-is, the additional conditional (syn && !rst && ...) is placed in the
> 'no socket found' path.
>
> The pernet "listening" socket is hidden from userspace, its not part of
> any hashes and not bound to any address/port.
>
> TPROXY-like semantics apply: If tcp demux cannot find a port for a given
> packet, check if the packet is a syn packet with a valid join token.
>
> If so, the pernet listener is returned and tcp processing resumes.
> Otherwise, handling is identical.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> include/net/mptcp.h  |  10 ++
> net/ipv6/tcp_ipv6.c  |  19 ++--
> net/mptcp/ctrl.c     | 229 ++++++++++++++++++++++++++++++++++++++++++-
> net/mptcp/protocol.c |   2 +-
> net/mptcp/protocol.h |   2 +-
> net/mptcp/subflow.c  |   8 +-
> 6 files changed, 251 insertions(+), 19 deletions(-)

--
Mat Martineau
Intel