From: "Mickaël Salaün" <mic@digikod.net>
To: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com>
Cc: willemdebruijn.kernel@gmail.com, gnoack3000@gmail.com,
linux-security-module@vger.kernel.org, netdev@vger.kernel.org,
netfilter-devel@vger.kernel.org, yusongping@huawei.com,
artem.kuzin@huawei.com, konstantin.meskhidze@huawei.com,
"Günther Noack" <gnoack@google.com>
Subject: Re: [PATCH 1/2] landlock: Add hook on socket_listen()
Date: Tue, 30 Apr 2024 18:52:12 +0200 [thread overview]
Message-ID: <20240430.beicheugee5T@digikod.net> (raw)
In-Reply-To: <20240425.Soot5eNeexol@digikod.net>
On Tue, Apr 30, 2024 at 03:36:30PM +0200, Mickaël Salaün wrote:
> On Mon, Apr 08, 2024 at 05:47:46PM +0800, Ivanov Mikhail wrote:
> > Make hook for socket_listen(). It will check that the socket protocol is
> > TCP, and if the socket's local port number is 0 (which means,
> > that listen(2) was called without any previous bind(2) call),
> > then listen(2) call will be legitimate only if there is a rule for bind(2)
> > allowing binding to port 0 (or if LANDLOCK_ACCESS_NET_BIND_TCP is not
> > supported by the sandbox).
>
> Thanks for this patch and sorry for the late full review. The code is
> good overall.
>
> We should either consider this patch as a fix or add a new flag/access
> right to Landlock syscalls for compatibility reason. I think this
> should be a fix. Calling listen(2) without a previous call to bind(2)
> is a corner case that we should properly handle. The commit message
> should make that explicit and highlight the goal of the patch: first
> explain why, and then how.
>
> We also need to update the user documentation to explain that
> LANDLOCK_ACCESS_NET_BIND_TCP also handles this case.
>
> >
> > Create a new check_access_socket() function to prevent useless copy paste.
> > It should be called by hook handlers after they perform special checks and
> > calculate socket port value.
>
> You can add this tag:
> Fixes: fff69fb03dde ("landlock: Support network rules with TCP bind and connect")
>
> >
> > Signed-off-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com>
> > Reviewed-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
> > ---
> > security/landlock/net.c | 104 +++++++++++++++++++++++++++++++++-------
> > 1 file changed, 88 insertions(+), 16 deletions(-)
> > + if (inet_sk(sock->sk)->inet_num != 0)
>
> Why do we want to allow listen() on any socket that is binded?
Please ignore this comment. I was initially thinking about a new access
right, which would be good to have anyway, but with another series.
next prev parent reply other threads:[~2024-04-30 16:52 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-08 9:47 [PATCH 0/2] Forbid illegitimate binding via listen(2) Ivanov Mikhail
2024-04-08 9:47 ` [PATCH 1/2] landlock: Add hook on socket_listen() Ivanov Mikhail
2024-04-30 13:36 ` Mickaël Salaün
2024-04-30 16:52 ` Mickaël Salaün [this message]
2024-05-13 12:15 ` Ivanov Mikhail
2024-05-17 15:22 ` Mickaël Salaün
2024-04-08 9:47 ` [PATCH 2/2] selftests/landlock: Create 'listen_zero', 'deny_listen_zero' tests Ivanov Mikhail
2024-04-30 13:36 ` Mickaël Salaün
2024-05-13 12:18 ` Ivanov Mikhail
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240430.beicheugee5T@digikod.net \
--to=mic@digikod.net \
--cc=artem.kuzin@huawei.com \
--cc=gnoack3000@gmail.com \
--cc=gnoack@google.com \
--cc=ivanov.mikhail1@huawei-partners.com \
--cc=konstantin.meskhidze@huawei.com \
--cc=linux-security-module@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=willemdebruijn.kernel@gmail.com \
--cc=yusongping@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).