[PATCH v2] lsm: adds process attribute getter for Landlock

[Casey Schaufler]

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 at chromium.org>
> ---
>  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 */