[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZsSYu8kV9l-OTUnF@google.com>
Date: Tue, 20 Aug 2024 15:23:07 +0200
From: "Günther Noack" <gnoack@...gle.com>
To: Mikhail Ivanov <ivanov.mikhail1@...wei-partners.com>
Cc: mic@...ikod.net, willemdebruijn.kernel@...il.com, gnoack3000@...il.com,
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
Subject: Re: [RFC PATCH v2 0/9] Support TCP listen access-control
On Tue, Aug 20, 2024 at 03:11:07PM +0200, Günther Noack wrote:
> Hello!
>
> Thanks for sending v2 of this patchset!
>
> On Wed, Aug 14, 2024 at 11:01:42AM +0800, Mikhail Ivanov wrote:
> > Hello! This is v2 RFC patch dedicated to restriction of listening sockets.
> >
> > It is based on the landlock's mic-next branch on top of 6.11-rc1 kernel
> > version.
> >
> > Description
> > ===========
> > LANDLOCK_ACCESS_NET_BIND_TCP is useful to limit the scope of "bindable"
> > ports to forbid a malicious sandboxed process to impersonate a legitimate
> > server process. However, bind(2) might be used by (TCP) clients to set the
> > source port to a (legitimate) value. Controlling the ports that can be
> > used for listening would allow (TCP) clients to explicitly bind to ports
> > that are forbidden for listening.
> >
> > Such control is implemented with a new LANDLOCK_ACCESS_NET_LISTEN_TCP
> > access right that restricts listening on undesired ports with listen(2).
> >
> > It's worth noticing that this access right doesn't affect changing
> > backlog value using listen(2) on already listening socket. For this case
> > test ipv4_tcp.double_listen is provided.
>
> This is a good catch, btw, that seems like the right thing to do. 👍
>
>
> I am overall happy with this patch set, but left a few remarks in the tests so
> far. There are a few style nits here and there.
>
> A thing that makes me uneasy is that the tests have a lot of logic in
> test_restricted_net_fixture(), where instead of the test logic being
> straightforward, there are conditionals to tell apart different scenarios and
> expect different results. I wish that the style of these tests was more linear.
> This patch set is making it a little bit worse, because the logic in
> test_restricted_net_fixture() increases.
>
> I have also made some restructuring suggestions for the kernel code, in the hope
> that they simplify things. If they don't because I overlooked something, we can
> skip that though.
I missed to mention it -- the documentation in
Documentation/userspace-api/landlock.rst needs updating as well.
—Günther
Powered by blists - more mailing lists