Re: [PATCH v2 1/4] Landlock: Add signal control
From: Jann Horn <jannh@google.com>
Date: 2024-08-06 18:56:53
Also in:
lkml, netdev
On Tue, Aug 6, 2024 at 8:11 PM Tahera Fahimi [off-list ref] wrote:
Currently, a sandbox process is not restricted to send a signal (e.g. SIGKILL) to a process outside of the sandbox environment. Ability to sending a signal for a sandboxed process should be scoped the same way abstract unix sockets are scoped. Therefore, we extend "scoped" field in a ruleset with "LANDLOCK_SCOPED_SIGNAL" to specify that a ruleset will deny sending any signal from within a sandbox process to its parent(i.e. any parent sandbox or non-sandboxed procsses).
[...]
quoted hunk ↗ jump to hunk
diff --git a/security/landlock/task.c b/security/landlock/task.c index 7e8579ebae83..a73cff27bb91 100644 --- a/security/landlock/task.c +++ b/security/landlock/task.c@@ -261,11 +261,54 @@ static int hook_unix_may_send(struct socket *const sock, return -EPERM; } +static int hook_task_kill(struct task_struct *const p, + struct kernel_siginfo *const info, const int sig, + const struct cred *const cred) +{ + bool is_scoped; + const struct landlock_ruleset *target_dom; + + /* rcu is already locked */ + target_dom = landlock_get_task_domain(p); + if (cred) + /* dealing with USB IO */ + is_scoped = domain_IPC_scope(landlock_cred(cred)->domain, + target_dom, + LANDLOCK_SCOPED_SIGNAL); + else + is_scoped = domain_IPC_scope(landlock_get_current_domain(), + target_dom, + LANDLOCK_SCOPED_SIGNAL);
This might be a bit more concise if you turn it into something like:
/* only USB IO supplies creds */
cred = cred ?: current_cred();
is_scoped = domain_IPC_scope(landlock_cred(cred)->domain,
target_dom, LANDLOCK_SCOPED_SIGNAL);
but that's just a question of style, feel free to keep it as-is
depending on what you prefer.
+ if (is_scoped)
+ return 0;
+
+ return -EPERM;
+}
+
+static int hook_file_send_sigiotask(struct task_struct *tsk,
+ struct fown_struct *fown, int signum)
+{
+ bool is_scoped;
+ const struct landlock_ruleset *dom, *target_dom;
+ struct task_struct *result = get_pid_task(fown->pid, fown->pid_type);I'm not an expert on how the fowner stuff works, but I think this will probably give you "result = NULL" if the file owner PID has already exited, and then the following landlock_get_task_domain() would probably crash? But I'm not entirely sure about how this works. I think the intended way to use this hook would be to instead use the "file_set_fowner" hook to record the owning domain (though the setup for that is going to be kind of a pain...), see the Smack and SELinux definitions of that hook. Or alternatively maybe it would be even nicer to change the fown_struct to record a cred* instead of a uid and euid and then use the domain from those credentials for this hook... I'm not sure which of those would be easier.
+ /* rcu is already locked! */ + dom = landlock_get_task_domain(result); + target_dom = landlock_get_task_domain(tsk); + is_scoped = domain_IPC_scope(dom, target_dom, LANDLOCK_SCOPED_SIGNAL); + put_task_struct(result); + if (is_scoped) + return 0; + return -EPERM; +}