Message-ID: <20250129.Oo1xou8ieche@digikod.net>
Date: Wed, 29 Jan 2025 15:51:37 +0100
From: Mickaël Salaün <mic@...ikod.net>
To: Mikhail Ivanov <ivanov.mikhail1@...wei-partners.com>
Cc: Matthieu Baerts <matttbe@...nel.org>, gnoack@...gle.com, 
	willemdebruijn.kernel@...il.com, matthieu@...fet.re, linux-security-module@...r.kernel.org, 
	netdev@...r.kernel.org, netfilter-devel@...r.kernel.org, yusongping@...wei.com, 
	artem.kuzin@...wei.com, konstantin.meskhidze@...wei.com, 
	MPTCP Linux <mptcp@...ts.linux.dev>, linux-nfs@...r.kernel.org, Paul Moore <paul@...l-moore.com>
Subject: Re: [RFC PATCH v2 1/8] landlock: Fix non-TCP sockets restriction

On Wed, Jan 29, 2025 at 02:47:19PM +0300, Mikhail Ivanov wrote:
> On 1/29/2025 2:33 PM, Matthieu Baerts wrote:
> > On 29/01/2025 12:02, Mikhail Ivanov wrote:
> > > On 1/29/2025 1:25 PM, Matthieu Baerts wrote:
> > > > Hi Mikhail,
> > > > 
> > > > On 29/01/2025 10:52, Mikhail Ivanov wrote:
> > > > > On 1/28/2025 9:14 PM, Matthieu Baerts wrote:
> > > > > > Hi Mikhail,
> > > > > > 
> > > > > > Sorry, I didn't follow all the discussions in this thread, but here are
> > > > > > some comments, hoping this can help to clarify the MPTCP case.
> > > > > 
> > > > > Thanks a lot for sharing your knowledge, Matthieu!
> > > > > 
> > > > > > 
> > > > > > On 28/01/2025 11:56, Mikhail Ivanov wrote:
> > > > > > > On 1/27/2025 10:48 PM, Mickaël Salaün wrote:
> > > > > > 
> > > > > > (...)
> > > > > > 
> > > > > > > > I'm a bit worried that we miss some of these places (now or in future
> > > > > > > > kernel versions).  We'll need a new LSM hook for that.
> > > > > > > > 
> > > > > > > > Could you list the current locations?
> > > > > > > 
> > > > > > > Currently, I know only about TCP-related transformations:
> > > > > > > 
> > > > > > > * SMC can fallback to TCP during connection. TCP connection is used
> > > > > > >      (1) to exchange CLC control messages in default case and (2)
> > > > > > > for the
> > > > > > >      communication in the case of fallback. If socket was connected or
> > > > > > >      connection failed, socket can not be reconnected again. There
> > > > > > > is no
> > > > > > >      existing security hook to control the fallback case,
> > > > > > > 
> > > > > > > * MPTCP uses TCP for communication between two network interfaces
> > > > > > > in the
> > > > > > >      default case and can fallback to plain TCP if remote peer does not
> > > > > > >      support MPTCP. AFAICS, there is also no security hook to
> > > > > > > control the
> > > > > > >      fallback transformation,
> > > > > > 
> > > > > > There are security hooks to control the path creation, but not to
> > > > > > control the "fallback transformation".
> > > > > > 
> > > > > > Technically, with MPTCP, the userspace will create an IPPROTO_MPTCP
> > > > > > socket. This is only used "internally": to communicate between the
> > > > > > userspace and the kernelspace, but not directly used between network
> > > > > > interfaces. This "external" communication is done via one or multiple
> > > > > > kernel TCP sockets carrying extra TCP options for the mapping. The
> > > > > > userspace cannot directly control these sockets created by the kernel.
> > > > > > 
> > > > > > In case of fallback, the kernel TCP socket "simply" drop the extra TCP
> > > > > > options needed for MPTCP, and carry on like normal TCP. So on the wire
> > > > > > and in the Linux network stack, it is the same TCP connection, without
> > > > > > the MPTCP options in the TCP header. The userspace continue to
> > > > > > communicate with the same socket.
> > > > > > 
> > > > > > I'm not sure if there is a need to block the fallback: it means only
> > > > > > one
> > > > > > path can be used at a time.

Thanks Matthieu.

So user space needs to specific IPPROTO_MPTCP to use MPTCP, but on the
network this socket can translate to "augmented" or plain TCP.

>From Landlock point of view, what matters is to have a consistent policy
that maps to user space code.  The fear was that a malicious user space
that is only allowed to use MPTCP could still transform an MPTCP socket
to a TCP socket, while it wasn't allowed to create a TCP socket in the
first place.  I now think this should not be an issue because:
1. MPTCP is kind of a superset of TCP
2. user space legitimately using MPTCP should not get any error related
   to a Landlock policy because of TCP/any automatic fallback.  To say
   it another way, such fallback is independent of user space requests
   and may not be predicted because it is related to the current network
   path.  This follows the principle of least astonishment (at least
   from user space point of view).

So, if I understand correctly, this should be simple for the Landlock
socket creation control:  we only check socket properties at creation
time and we ignore potential fallbacks.  This should be documented
though.

As an example, if a Landlock policies only allows MPTCP: socket(...,
IPPROTO_MPTCP) should be allowed and any legitimate use of the returned
socket (according to MPTCP) should be allowed, including TCP fallback.
However, socket(..., IPPROTO_TCP/0), should only be allowed if TCP is
explicitly allowed.  This means that we might end up with an MPTCP
socket only using TCP, which is OK.

I guess this should be the same for other protocols, except if user
space can explicitly transform a specific socket type to use an
*arbitrary* protocol, but I think this is not possible.

> > > > > 
> > > > > You mean that users always rely on a plain TCP communication in the case
> > > > > the connection of MPTCP multipath communication fails?
> > > > 
> > > > Yes, that's the same TCP connection, just without extra bit to be able
> > > > to use multiple TCP connections associated to the same MPTCP one.
> > > 
> > > Indeed, so MPTCP communication should be restricted the same way as TCP.
> > > AFAICS this should be intuitive for MPTCP users and it'll be better
> > > to let userland define this dependency.
> > 
> > Yes, I think that would make more sense.
> > 
> > I guess we can look at MPTCP as TCP with extra features.
> 
> Yeap
> 
> > 
> > So if TCP is blocked, MPTCP should be blocked as well. (And eventually
> > having the possibility to block only TCP but not MPTCP and the opposite,
> > but that's a different topic: a possible new feature, but not a bug-fix)
> What do you mean by the "bug fix"?
> 
> > 
> > > > > > > * IPv6 -> IPv4 transformation for TCP and UDP sockets withon
> > > > > > >      IPV6_ADDRFORM. Can be controlled with setsockopt() security hook.

According to the man page: "It is allowed only for IPv6 sockets that are
connected and bound to a v4-mapped-on-v6 address."

This compatibility feature makes sense from user space point of view and
should not result in an error because of Landlock.

> > > > > > > 
> > > > > > > As I said before, I wonder if user may want to use SMC or MPTCP and
> > > > > > > deny
> > > > > > > TCP communication, since he should rely on fallback transformation
> > > > > > > during the connection in the common case. It may be unexpected for
> > > > > > > connect(2) to fail during the fallback due to security politics.
> > > > > > 
> > > > > > With MPTCP, fallbacks can happen at the beginning of a connection, when
> > > > > > there is only one path. This is done after the userspace's
> > > > > > connect(). If

A remaining question is then, can we repurpose an MPTCP socket that did
fallback to TCP, to (re)connect to another destination (this time
directly with TCP)?

I guess this is possible.  If it is the case, I think it should be OK
anyway.  That could be used by an attacker, but that should not give
more access because of the MPTCP fallback mechanism anyway.  We should
see MPTCP as a superset of TCP.  At the end, security policy is in the
hands of user space.

> > > > > > the fallback is blocked, I guess the userspace will get the same errors
> > > > > > as when an open connection is reset.
> > > > > 
> > > > > In the case of blocking due to security policy, userspace should get
> > > > > -EACESS. I mean, the user might not expect the fallback path to be
> > > > > blocked during the connection if he has allowed only MPTCP communication
> > > > > using the Landlock policy.
> > > > 
> > > > A "fallback" can happen on different occasions as mentioned in the
> > > > RFC8684 [1], e.g.
> > > > 
> > > > - The client asks to use MPTCP, but the other peer doesn't support it:
> > > > 
> > > >     Client                Server
> > > >     |     SYN + MP_CAPABLE     |
> > > >     |------------------------->|
> > > >     |         SYN/ACK          |
> > > >     |<-------------------------|  => Fallback on the client side
> > > >     |           ACK            |
> > > >     |------------------------->|
> > > > 
> > > > - A middle box doesn't touch the 3WHS, but intercept the communication
> > > > just after:
> > > > 
> > > >     Client                Server
> > > >     |     SYN + MP_CAPABLE     |
> > > >     |------------------------->|
> > > >     |   SYN/ACK + MP_CAPABLE   |
> > > >     |<-------------------------|
> > > >     |     ACK + MP_CAPABLE     |
> > > >     |------------------------->|
> > > >     |        DSS + data        | => but the server doesn't receive the DSS
> > > >     |------------------------->| => So fallback on the server side
> > > >     |           ACK            |
> > > >     |<-------------------------| => Fallback on the client side
> > > > 
> > > > - etc.
> > > > 
> > > > So the connect(), even in blocking mode, can be OK, but the "fallback"
> > > > will happen later.
> > > 
> > > Thanks! Theoretical "socket transformation" control should cover all
> > > these cases.
> > > 
> > > You mean that it might be reasonable for a Landlock policy to block
> > > MPTCP fallback when establishing first sublflow (when client does not
> > > receive MP_CAPABLE)?
> > 
> > Personally, I don't even know if there is really a need for such
> > policies. The fallback is there not to block a connection if the other
> > peer doesn't support MPTCP, or if a middlebox decides to mess-up with
> > MPTCP options. So instead of an error, the connection continues but is
> > "degraded" by not being able to create multiple paths later on.

I agree, this kind of compatibility feature should not be denied.

> > 
> > Maybe best to wait for a concrete use-case before implementing this?
> 
> Ok, got it! I agree that such policies does not seem to be useful.
> 
> > 
> > (...)
> > 
> > Cheers,
> > Matt
> 

Powered by blists - more mailing lists