Message-ID: <20241210.ohC4die2hi8v@digikod.net>
Date: Tue, 10 Dec 2024 19:05:35 +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>, David Laight <David.Laight@...lab.com>
Subject: Re: [RFC PATCH v2 1/8] landlock: Fix non-TCP sockets restriction

On Tue, Dec 10, 2024 at 07:04:15PM +0100, Mickaël Salaün wrote:
> On Mon, Dec 09, 2024 at 01:19:19PM +0300, Mikhail Ivanov wrote:
> > On 12/4/2024 10:35 PM, Mickaël Salaün wrote:
> > > On Wed, Dec 04, 2024 at 08:27:58PM +0100, Mickaël Salaün wrote:
> > > > 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:
> > > > > > Hi Mikhail and Landlock maintainers,
> > > > > > 
> > > > > > +cc MPTCP list.
> > > > > 
> > > > > Thanks, we should include this list in the next series.
> > > > > 
> > > > > > 
> > > > > > On 17/10/2024 13:04, Mikhail Ivanov wrote:
> > > > > > > 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).
> > 
> > You mean that these firewals have protocol granularity (e.g. different
> > restrictions for MPTCP and TCP sockets)?
> 
> Yes, and more importantly they can miss the MTCP semantic and then not
> properly filter such packet, which can be use to escape the network
> policy.  See some issues here:
> https://en.wikipedia.org/wiki/Multipath_TCP
> 
> The point is that we cannot assume anything about other networking
> stacks, and if Landlock can properly differentiate between TCP and MTCP
> (e.g. with new LANDLOCK_ACCESS_NET_CONNECT_MTCP) users of such firewalls
> could still limit the impact of their firewall's bugs.  However, if
> Landlock treats TCP and MTCP the same way, we'll not be able to only
> deny MTCP.  In most use cases, the network policy should treat both TCP
> and MTCP the same way though, but we should let users decide according
> to their context.
> 
> From an implementation point of view, adding MTCP support should be
> simple, mainly tests will grow.

s/MTCP/MPTCP/g of course.

> 
> > 
> > > > 
> > > > Mikhail, could you please send a new patch series containing one patch
> > > > to fix the kernel and another to extend tests?
> > > 
> > > No need to squash them in one, please keep the current split of the test
> > > patches.  However, it would be good to be able to easily backport them,
> > > or at least the most relevant for this fix, which means to avoid
> > > extended refactoring.
> > 
> > No problem, I'll remove the fix of error consistency from this patchset.
> > BTW, what do you think about second and third commits? Should I send the
> > new version of them as well (in separate patch)?
> 
> According to the description, patch 2 may be included in this series if
> it can be tested with any other LSM, but I cannot read these patches:
> https://lore.kernel.org/all/20241017110454.265818-3-ivanov.mikhail1@huawei-partners.com/

Powered by blists - more mailing lists