On 5/18/2023 1:45 PM, Shervin Oloumi wrote:
> Adds a new getprocattr hook function to the Landlock LSM, which tracks
> the landlocked state of the process. This is invoked when user-space
> reads /proc/[pid]/attr/domain
Please don't add a Landlock specific entry directly in the attr/
directory. Add it only to attr/landlock.
Also be aware that the LSM maintainer (Paul Moore) wants to move
away from the /proc/.../attr interfaces in favor of a new system call,
which is in review.
> to determine whether a given process is
> sand-boxed using Landlock. When the target process is not sand-boxed,
> the result is "none", otherwise the result is empty, as we still need to
> decide what kind of domain information is best to provide in "domain".
Unless it's too late, you should consider using a term other than "domain".
Domain is used in many contexts already, and your use could be confused
with any number of those.
>
> The hook function also performs an access check. The request is rejected
> if the tracing process is the same as the target process, or if the
> tracing process domain is not an ancestor to the target process domain.
>
> Adds a new directory for landlock under the process attribute
> filesystem, and defines "domain" as a read-only process attribute entry
> for landlock.
>
> Signed-off-by: Shervin Oloumi <enlightened@xxxxxxxxxxxx>
> ---
> fs/proc/base.c | 11 +++++++++++
> security/landlock/fs.c | 38 ++++++++++++++++++++++++++++++++++++++
> security/landlock/fs.h | 1 +
> security/landlock/ptrace.c | 4 ++--
> security/landlock/ptrace.h | 3 +++
> 5 files changed, 55 insertions(+), 2 deletions(-)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 9e479d7d202b..b257ea704666 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2851,6 +2851,13 @@ static const struct pid_entry apparmor_attr_dir_stuff[] = {
> LSM_DIR_OPS(apparmor);
> #endif
>
> +#ifdef CONFIG_SECURITY_LANDLOCK
> +static const struct pid_entry landlock_attr_dir_stuff[] = {
> + ATTR("landlock", "domain", 0444),
> +};
> +LSM_DIR_OPS(landlock);
> +#endif
> +
> static const struct pid_entry attr_dir_stuff[] = {
> ATTR(NULL, "current", 0666),
> ATTR(NULL, "prev", 0444),
> @@ -2866,6 +2873,10 @@ static const struct pid_entry attr_dir_stuff[] = {
> DIR("apparmor", 0555,
> proc_apparmor_attr_dir_inode_ops, proc_apparmor_attr_dir_ops),
> #endif
> +#ifdef CONFIG_SECURITY_LANDLOCK
> + DIR("landlock", 0555,
> + proc_landlock_attr_dir_inode_ops, proc_landlock_attr_dir_ops),
> +#endif
> };
>
> static int proc_attr_dir_readdir(struct file *file, struct dir_context *ctx)
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index adcea0fe7e68..2f8b0837a0fd 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -1280,6 +1280,42 @@ static int hook_file_truncate(struct file *const file)
> return -EACCES;
> }
>
> +/* process attribute interfaces */
> +
> +/**
> + * landlock_getprocattr - Landlock process attribute getter
> + * @task: the object task
> + * @name: the name of the attribute in /proc/.../attr
> + * @value: where to put the result
> + *
> + * Performs access checks and writes any applicable results to value
> + *
> + * Returns the length of the result inside value or an error code
> + */
> +static int landlock_getprocattr(struct task_struct *task, const char *name,
> + char **value)
> +{
> + char *val = "";
> + int slen;
> +
> + // If the tracing process is landlocked, ensure its domain is an
> + // ancestor to the target process domain.
Please read the kernel style documentation. "//" comments are
not used in the kernel.
> + if (landlocked(current))
> + if (current == task || !task_is_scoped(current, task))
> + return -EACCES;
> +
> + // The only supported attribute is "domain".
> + if (strcmp(name, "domain") != 0)
> + return -EINVAL;
> +
> + if (!landlocked(task))
> + val = "none";
> +
> + slen = strlen(val);
> + *value = val;
> + return slen;
> +}
> +
> static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(inode_free_security, hook_inode_free_security),
>
> @@ -1302,6 +1338,8 @@ static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(file_alloc_security, hook_file_alloc_security),
> LSM_HOOK_INIT(file_open, hook_file_open),
> LSM_HOOK_INIT(file_truncate, hook_file_truncate),
> +
> + LSM_HOOK_INIT(getprocattr, landlock_getprocattr),
> };
>
> __init void landlock_add_fs_hooks(void)
> diff --git a/security/landlock/fs.h b/security/landlock/fs.h
> index 488e4813680a..64145e8b5537 100644
> --- a/security/landlock/fs.h
> +++ b/security/landlock/fs.h
> @@ -13,6 +13,7 @@
> #include <linux/init.h>
> #include <linux/rcupdate.h>
>
> +#include "ptrace.h"
> #include "ruleset.h"
> #include "setup.h"
>
> diff --git a/security/landlock/ptrace.c b/security/landlock/ptrace.c
> index 4c5b9cd71286..de943f0f3899 100644
> --- a/security/landlock/ptrace.c
> +++ b/security/landlock/ptrace.c
> @@ -47,8 +47,8 @@ static bool domain_scope_le(const struct landlock_ruleset *const parent,
> return false;
> }
>
> -static bool task_is_scoped(const struct task_struct *const parent,
> - const struct task_struct *const child)
> +const bool task_is_scoped(const struct task_struct *const parent,
> + const struct task_struct *const child)
> {
> bool is_scoped;
> const struct landlock_ruleset *dom_parent, *dom_child;
> diff --git a/security/landlock/ptrace.h b/security/landlock/ptrace.h
> index 265b220ae3bf..c6eb08951fc1 100644
> --- a/security/landlock/ptrace.h
> +++ b/security/landlock/ptrace.h
> @@ -11,4 +11,7 @@
>
> __init void landlock_add_ptrace_hooks(void);
>
> +const bool task_is_scoped(const struct task_struct *const parent,
> + const struct task_struct *const child);
> +
> #endif /* _SECURITY_LANDLOCK_PTRACE_H */
