From: "Mickaël Salaün" <mic@digikod.net>
To: TaheraFahimi <fahimitahera@gmail.com>
Cc: Paul Moore <paul@paul-moore.com>, outreachy@lists.linux.dev
Subject: Re: [PATCH] Adding helpers for unix_stream_connect.
Date: Tue, 26 Mar 2024 12:28:36 +0100 [thread overview]
Message-ID: <20240326.Ot8Zooveeh1k@digikod.net> (raw)
In-Reply-To: <ZgD8lTToWEhtNmpg@tahera-OptiPlex-5000>
CCing Outreachy mailing list and Paul.
Related GitHub issue: https://github.com/landlock-lsm/linux/issues/7
On Sun, Mar 24, 2024 at 10:24:53PM -0600, TaheraFahimi wrote:
> Signed-off-by: TaheraFahimi <fahimitahera@gmail.com>
I guess there is a space missing between your first and last name.
> ---
> security/landlock/task.c | 89 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 89 insertions(+)
>
> diff --git a/security/landlock/task.c b/security/landlock/task.c
> index 849f5123610b..10195492c169 100644
> --- a/security/landlock/task.c
> +++ b/security/landlock/task.c
> @@ -14,6 +14,10 @@
> #include <linux/rcupdate.h>
> #include <linux/sched.h>
>
> +/*to use sock structure*/
> +#include <net/sock.h>
> +#include <linux/spinlock.h>
> +
> #include "common.h"
> #include "cred.h"
> #include "ruleset.h"
> @@ -108,9 +112,94 @@ static int hook_ptrace_traceme(struct task_struct *const parent)
> return task_ptrace(parent, current);
> }
>
> +
> +/**
> + * sk_get_peer_cred : Helper Function from socket.c
> + * This helper is used to retrieve the sk_peer credentioals.
> + */
> +static const struct cred *sk_get_peer_cred(struct sock *sk)
> +{
> + const struct cred *cred;
> + spin_lock(&sk->sk_peer_lock);
> + cred = get_cred(sk->sk_peer_cred);
> + spin_unlock(&sk->sk_peer_lock);
> + return cred;
> +}
> +/**
> + * Use domain_scope_le() to scope the access same as ptrace trask.
> + */
> +static bool unix_sock_is_scoped(struct sock *const sock,
> + struct sock *const other)
> +{
> +
> + bool is_scoped = true;
> + /* retrieve the credential of socks.
> + * other->sk_peer_cred shows the listening socket credentials (This credential
> + * is created at the init_cred function and will be coped to
> + * sock->sk_peer_cred later.)
> + * Since sock is the connecting sock, then the credential of current task is equal
> + * to cred_sock.
> + * */
> + const struct cred *cred_other;
> + cred_other = sk_get_peer_cred(other);
> +
> +
> + const struct cred *cred_sock;
> + spin_lock(&sock->sk_peer_lock);
You don't need a lock for get_current_cred(), and sk_peer_lock is
unrelated to current. Moreover, security_unix_stream_connect() is
already called while the lock being held.
> + cred_sock = get_current_cred();
> + spin_unlock(&sock->sk_peer_lock);
You should just use landlock_get_current_domain().
> +
> + printk(KERN_DEBUG "cred uid: %u\n", cred_other->gid);
You can use pr_warn() instead.
> + //printk("the cred_sock kuid is : %llx\n", cred_sock->uid);
> + /* retrieve the landlock_rulesets*/
> +
> + printk("The credentials is retrieved!!\n");
> +
> + const struct landlock_cred_security *dom_parent, *dom_child;
dom_* variables should only be used to refer do a Landlock domain, not
credential.
> + const struct landlock_ruleset *parent, *child;
> + rcu_read_lock();
> + dom_parent = landlock_cred(cred_other);
> + dom_child = landlock_cred(cred_sock);
> + parent = dom_parent->domain;
> + child = dom_child->domain;
> + /* by uncommenting either the following if statement or calling
> + * the domain_scope_le the kernel will not load(make task dead)*/
> + //if (dom_parent)
> + // printk("parent num rules %u\n", parent->num_rules);
> + if (child)
> + printk("child num rules %u\n", child->num_rules);
> + //is_scoped = domain_scope_le(parent, child);
> + rcu_read_unlock();
> +
> +
> + return is_scoped;
> +}
> +/**
> + * unix_stream_connect -
> + */
> +static int task_unix_stream_connect(struct sock *const sock,
> + struct sock *const other,
> + struct sock *const newsk)
> +{
> + /* How do we check if the scoping is optional? */
> + if (unix_sock_is_scoped(sock, other))
> + return 0;
> + return -EPERM;
> +}
> +
> +/**
> + * 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)
> --
> 2.34.1
>
> Hello Mickaël, I made the following changes to have an lsm hook function for "unix_stream_connect". I have an issue, and I was wondering if you could give me a hint. At first, I retrieve the Socks' credentials through "sk_get_peer_cred" and "get_current_cred". As "get_current_cred" gives us the credential related to the sock and "sk_get_peer_cred" gives the sock's peer credential(this credential will be copied to the field sock->sk_peer_cred later). Then I uses both credentials to get their "landlock_cred_security". Since "landlock_cred_security" has a pointer to the "landlock_ruleset", we can pass their rulesets to the "domain_scope_le" (same as ptrace task) to check if it is scoped or not.
Hello Tahera, this approach looks good. However, Landlock credential's
domain may be NULL (when the task is not sandboxed).
> The problem is that If I uncomment the "domain_scope_le", the kernel will not work. I think it can not get the "landlock_cred_security". Thank you :)
"the kernel will not work" could means a lot of things. We need a more
precise description.
I guess this is just a draft and the patch is not ready yet so it's
fine. Before sending a first official version you should follow and read
the referenced links and especially the kernel patch how-to here:
https://lore.kernel.org/outreachy/ZeZzLWtk2nZ1Pcgw@aschofie-mobl2/
For instance, you should run checkpatch.pl
For Landlock-specific patches, they need to be formatted with
clang-format (-i) and pass all the checked done by check-linux.sh
provided here: https://github.com/landlock-lsm/landlock-test-tools
next parent reply other threads:[~2024-03-26 11:28 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <ZgD8lTToWEhtNmpg@tahera-OptiPlex-5000>
2024-03-26 11:28 ` Mickaël Salaün [this message]
2024-03-28 3:46 ` [PATCH] Adding helpers for unix_stream_connect Tahera Fahimi
2024-03-28 14:42 ` Mickaël Salaün
2024-03-30 3:43 ` Tahera Fahimi
2024-04-02 14:35 ` Mickaël Salaün
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=20240326.Ot8Zooveeh1k@digikod.net \
--to=mic@digikod.net \
--cc=fahimitahera@gmail.com \
--cc=outreachy@lists.linux.dev \
--cc=paul@paul-moore.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).