On Fri, Oct 18, 2024 at 08:08:12PM +0200, Mickaël Salaün wrote:

On Thu, Oct 17, 2024 at 02:59:48PM +0200, Matthieu Baerts wrote:

quoted

Hi Mikhail and Landlock maintainers,

+cc MPTCP list.

Thanks, we should include this list in the next series.

quoted

On 17/10/2024 13:04, Mikhail Ivanov wrote:

quoted

Do not check TCP access right if socket protocol is not IPPROTO_TCP.
LANDLOCK_ACCESS_NET_BIND_TCP and LANDLOCK_ACCESS_NET_CONNECT_TCP
should not restrict bind(2) and connect(2) for non-TCP protocols
(SCTP, MPTCP, SMC).

Thank you for the patch!

I'm part of the MPTCP team, and I'm wondering if MPTCP should not be
treated like TCP here. MPTCP is an extension to TCP: on the wire, we can
see TCP packets with extra TCP options. On Linux, there is indeed a
dedicated MPTCP socket (IPPROTO_MPTCP), but that's just internal,
because we needed such dedicated socket to talk to the userspace.

I don't know Landlock well, but I think it is important to know that an
MPTCP socket can be used to discuss with "plain" TCP packets: the kernel
will do a fallback to "plain" TCP if MPTCP is not supported by the other
peer or by a middlebox. It means that with this patch, if TCP is blocked
by Landlock, someone can simply force an application to create an MPTCP
socket -- e.g. via LD_PRELOAD -- and bypass the restrictions. It will
certainly work, even when connecting to a peer not supporting MPTCP.

Please note that I'm not against this modification -- especially here
when we remove restrictions around MPTCP sockets :) -- I'm just saying
it might be less confusing for users if MPTCP is considered as being
part of TCP. A bit similar to what someone would do with a firewall: if
TCP is blocked, MPTCP is blocked as well.

Good point!  I don't know well MPTCP but I think you're right.  Given
it's close relationship with TCP and the fallback mechanism, it would
make sense for users to not make a difference and it would avoid bypass
of misleading restrictions.  Moreover the Landlock rules are simple and
only control TCP ports, not peer addresses, which seems to be the main
evolution of MPTCP.

Thinking more about this, this makes sense from the point of view of the
network stack, but looking at external (potentially bogus) firewalls or
malware detection systems, it is something different.  If we don't
provide a way for users to differenciate the control of SCTP from TCP,
malicious use of SCTP could still bypass this kind of bogus security
appliances.  It would then be safer to stick to the protocol semantic by
clearly differenciating TCP from MPTCP (or any other protocol).

Mikhail, could you please send a new patch series containing one patch
to fix the kernel and another to extend tests?  We should also include
this rationale in the commit message.

quoted

I understand that a future goal might probably be to have dedicated
restrictions for MPTCP and the other stream protocols (and/or for all
stream protocols like it was before this patch), but in the meantime, it
might be less confusing considering MPTCP as being part of TCP (I'm not
sure about the other stream protocols).

We need to take a closer look at the other stream protocols indeed.

It would be nice to add support for MPTCP too, but this will be treated
as a new Landlock feature (with a proper ABI bump).

quoted

quoted

sk_is_tcp() is used for this to check address family of the socket
before doing INET-specific address length validation. This is required
for error consistency.

Closes: https://github.com/landlock-lsm/linux/issues/40
Fixes: fff69fb03dde ("landlock: Support network rules with TCP bind and connect")