From: Casey Schaufler <casey@schaufler-ca.com>
To: TaheraFahimi <fahimitahera@gmail.com>,
outreachy@lists.linux.dev, "Mickaël Salaün" <mic@digikod.net>,
"Paul Moore" <paul@paul-moore.com>,
"James Morris" <jmorris@namei.org>,
"Serge E. Hallyn" <serge@hallyn.com>,
linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org,
"Casey Schaufler" <casey@schaufler-ca.com>
Subject: Re: [PATCH] landlock: Add abstract unix socket connect restrictions
Date: Thu, 28 Mar 2024 13:31:31 -0700 [thread overview]
Message-ID: <123a6889-4a28-40c4-b322-3537545824e6@schaufler-ca.com> (raw)
In-Reply-To: <ZgXN5fi6A1YQKiAQ@tahera-OptiPlex-5000>
On 3/28/2024 1:07 PM, TaheraFahimi wrote:
> Abstract unix sockets are used for local interprocess communication without
> relying on filesystem. Since landlock has no restriction for connecting to
> a UNIX socket in the abstract namespace, a sandboxed process can connect to
> a socket outside the sandboxed environment. Access to such sockets should
> be scoped the same way ptrace access is limited.
>
> For a landlocked process to be allowed to connect to a target process, it
> must have a subset of the target process’s rules (the connecting socket
> must be in a sub-domain of the listening socket). This patch adds a new
> LSM hook for connect function in unix socket with the related access rights.
>
> Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> ---
> security/landlock/task.c | 70 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 70 insertions(+)
>
> diff --git a/security/landlock/task.c b/security/landlock/task.c
> index 849f5123610b..7f4155fc6174 100644
> --- a/security/landlock/task.c
> +++ b/security/landlock/task.c
> @@ -13,6 +13,7 @@
> #include <linux/lsm_hooks.h>
> #include <linux/rcupdate.h>
> #include <linux/sched.h>
> +#include <net/sock.h>
>
> #include "common.h"
> #include "cred.h"
> @@ -108,9 +109,78 @@ static int hook_ptrace_traceme(struct task_struct *const parent)
> return task_ptrace(parent, current);
> }
>
> +static const struct cred *sk_get_cred(struct sock *sk)
> +{
> + const struct cred *cred = get_cred(sk->sk_peer_cred);
> +
> + if (!cred)
> + return NULL;
This makes no sense. If cred is NULL, why not just return it?
> + return cred;
> +}
This function devolves into a call to get_cred(sk->sk_peer_cred).
What value does it add?
> +
> +static const struct landlock_ruleset *get_current_sock_domain(void)
> +{
> + const struct landlock_ruleset *const dom =
> + landlock_get_current_domain();
> +
> + if (!dom)
> + return NULL;
> +
> + return dom;
> +}
Same here. Just return landlock_get_current_domain().
> +
> +static bool unix_sock_is_scoped(struct sock *const sock,
> + struct sock *const other)
> +{
> + bool is_scoped = true;
> +
> + /* get the ruleset of connecting sock*/
> + const struct landlock_ruleset *const dom_sock =
> + get_current_sock_domain();
> +
> + if (!dom_sock)
> + return true;
> +
> + /* get credential of listening sock*/
> + const struct cred *cred_other = sk_get_cred(other);
> +
> + if (!cred_other)
> + return true;
> +
> + /* retrieve the landlock_rulesets*/
> + const struct landlock_ruleset *dom_parent;
> +
> + rcu_read_lock();
> + dom_parent = landlock_cred(cred_other)->domain;
> + is_scoped = domain_scope_le(dom_parent, dom_sock);
> + rcu_read_unlock();
> +
> + return is_scoped;
> +}
> +
> +static int task_unix_stream_connect(struct sock *const sock,
> + struct sock *const other,
> + struct sock *const newsk)
> +{
> + if (unix_sock_is_scoped(sock, other))
> + return 0;
> + return -EPERM;
> +}
Again, a function that does nothing but wrap another function
adds no value and consumes stack and processing resources.
> +
> +/**
> + * hook_unix_stream_connect
> + */
> +static int hook_unix_stream_connect(struct sock *const sock,
> + struct sock *const other,
> + struct sock *const newsk)
> +{
> + return task_unix_stream_connect(sock, other, newsk);
> +}
> +
> 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),
> };
>
> __init void landlock_add_task_hooks(void)
prev parent reply other threads:[~2024-03-28 20:41 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-28 20:07 [PATCH] landlock: Add abstract unix socket connect restrictions TaheraFahimi
2024-03-28 20:31 ` Casey Schaufler [this message]
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=123a6889-4a28-40c4-b322-3537545824e6@schaufler-ca.com \
--to=casey@schaufler-ca.com \
--cc=fahimitahera@gmail.com \
--cc=jmorris@namei.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=mic@digikod.net \
--cc=outreachy@lists.linux.dev \
--cc=paul@paul-moore.com \
--cc=serge@hallyn.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).