[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Zqld+6wj8v0kLMBV@tahera-OptiPlex-5000>
Date: Tue, 30 Jul 2024 15:41:15 -0600
From: Tahera Fahimi <fahimitahera@...il.com>
To: Mickaël Salaün <mic@...ikod.net>
Cc: gnoack@...gle.com, paul@...l-moore.com, jmorris@...ei.org,
serge@...lyn.com, linux-security-module@...r.kernel.org,
linux-kernel@...r.kernel.org, bjorn3_gh@...tonmail.com,
jannh@...gle.com, outreachy@...ts.linux.dev, netdev@...r.kernel.org
Subject: Re: [PATCH v7 1/4] Landlock: Add abstract unix socket connect
restriction
On Tue, Jul 30, 2024 at 06:05:15PM +0200, Mickaël Salaün wrote:
> On Wed, Jul 17, 2024 at 10:15:19PM -0600, Tahera Fahimi wrote:
> > The patch introduces a new "scoped" attribute to the
> > landlock_ruleset_attr that can specify "LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET"
> > to scope abstract unix sockets from connecting to a process outside of
> > the same landlock domain.
> >
> > This patch implement two hooks, "unix_stream_connect" and "unix_may_send" to
> > enforce this restriction.
> >
> > Signed-off-by: Tahera Fahimi <fahimitahera@...il.com>
> >
> > -------
> > v7:
> > - Using socket's file credentials for both connected(STREAM) and
> > non-connected(DGRAM) sockets.
> > - Adding "domain_sock_scope" instead of the domain scoping mechanism used in
> > ptrace ensures that if a server's domain is accessible from the client's
> > domain (where the client is more privileged than the server), the client
> > can connect to the server in all edge cases.
> > - Removing debug codes.
> > v6:
> > - Removing curr_ruleset from landlock_hierarchy, and switching back to use
> > the same domain scoping as ptrace.
> > - code clean up.
> > v5:
> > - Renaming "LANDLOCK_*_ACCESS_SCOPE" to "LANDLOCK_*_SCOPE"
> > - Adding curr_ruleset to hierarachy_ruleset structure to have access from
> > landlock_hierarchy to its respective landlock_ruleset.
> > - Using curr_ruleset to check if a domain is scoped while walking in the
> > hierarchy of domains.
> > - Modifying inline comments.
> > V4:
> > - Rebased on Günther's Patch:
> > https://lore.kernel.org/all/20240610082115.1693267-1-gnoack@google.com/
> > so there is no need for "LANDLOCK_SHIFT_ACCESS_SCOPE", then it is removed.
> > - Adding get_scope_accesses function to check all scoped access masks in a ruleset.
> > - Using file's FD credentials instead of credentials stored in peer_cred
> > for datagram sockets. (see discussion in [1])
> > - Modifying inline comments.
> > V3:
> > - Improving commit description.
> > - Introducing "scoped" attribute to landlock_ruleset_attr for IPC scoping
> > purpose, and adding related functions.
> > - Changing structure of ruleset based on "scoped".
> > - Removing rcu lock and using unix_sk lock instead.
> > - Introducing scoping for datagram sockets in unix_may_send.
> > V2:
> > - Removing wrapper functions
> >
> > [1]https://lore.kernel.org/outreachy/Zmi8Ydz4Z6tYtpY1@tahera-OptiPlex-5000/T/#m8cdf33180d86c7ec22932e2eb4ef7dd4fc94c792
> > -------
> >
> > Signed-off-by: Tahera Fahimi <fahimitahera@...il.com>
> > ---
> > include/uapi/linux/landlock.h | 29 +++++++++
> > security/landlock/limits.h | 3 +
> > security/landlock/ruleset.c | 7 ++-
> > security/landlock/ruleset.h | 23 ++++++-
> > security/landlock/syscalls.c | 14 +++--
> > security/landlock/task.c | 112 ++++++++++++++++++++++++++++++++++
> > 6 files changed, 181 insertions(+), 7 deletions(-)
>
> > diff --git a/security/landlock/task.c b/security/landlock/task.c
> > index 849f5123610b..597d89e54aae 100644
> > --- a/security/landlock/task.c
> > +++ b/security/landlock/task.c
>
> > +static int hook_unix_stream_connect(struct sock *const sock,
> > + struct sock *const other,
> > + struct sock *const newsk)
> > +{
>
> We must check if the unix sockets use an abstract address. We need a
> new test to check against a the three types of addresse: pathname,
> unnamed (socketpair), and abstract. This should result into 6 variants
> because of the stream-oriented or dgram-oriented sockets to check both
> hooks. The test can do 3 checks: without any Landlock domain, with the
> client being sandboxed with a Landlock domain for filesystem
> restrictions only, and with a domain scoped for abstract unix sockets.
correct. Since the current tests check all the possible scenarios for
abstract unix sockets, we only need to add the scenarios where a socket
with pathname or unnamed address and scoped domain wants to connect to a
listening socket. In this case, the access should guaranteed since there
is no scoping mechanism for these sockets yet.
> For pathname you'll need to move the TMP_DIR define from fs_test.c into
> common.h and create a static path for the named socket. A fixture
> teardown should remove the socket and the directory.
Thanks for the help :)
> > + if (sock_is_scoped(other))
> > + return 0;
> > +
> > + return -EPERM;
> > +}
> > +
> > +static int hook_unix_may_send(struct socket *const sock,
> > + struct socket *const other)
> > +{
>
> Same here
>
> > + if (sock_is_scoped(other->sk))
> > + return 0;
> > +
> > + return -EPERM;
> > +}
> > +
> > static struct security_hook_list landlock_hooks[] __ro_after_init = {
> > LSM_HOOK_INIT(ptrace_access_check, hook_ptrace_access_check),
> > LSM_HOOK_INIT(ptrace_traceme, hook_ptrace_traceme),
> > + LSM_HOOK_INIT(unix_stream_connect, hook_unix_stream_connect),
> > + LSM_HOOK_INIT(unix_may_send, hook_unix_may_send),
>
> Future access controls dedicated to pathname unix sockets may need these
> hooks too, but I guess we'll see where tehy fit the best when we'll be
> there.
>
> > };
> >
> > __init void landlock_add_task_hooks(void)
> > --
> > 2.34.1
> >
> >
Powered by blists - more mailing lists