diff mbox series

[1/1] lsm: adds process attribute getter for Landlock

Message ID 20230302185257.850681-2-enlightened@chromium.org (mailing list archive)
State Handled Elsewhere
Headers show
Series process attribute support for Landlock | expand

Commit Message

Shervin Oloumi March 2, 2023, 6:52 p.m. UTC
From: Shervin Oloumi <enlightened@chromium.org>

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/current to determine whether a given process is
sand-boxed using Landlock.

Adds a new directory for landlock under the process attribute
filesystem, and defines "current" as a read-only process attribute entry
for landlock.

Signed-off-by: Shervin Oloumi <enlightened@chromium.org>
---
 fs/proc/base.c         | 11 +++++++++++
 security/landlock/fs.c | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

Comments

Casey Schaufler March 2, 2023, 8:24 p.m. UTC | #1
On 3/2/2023 10:52 AM, enlightened@chromium.org wrote:
> From: Shervin Oloumi <enlightened@chromium.org>
>
> 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/current to determine whether a given process is
> sand-boxed using Landlock.
>
> Adds a new directory for landlock under the process attribute
> filesystem, and defines "current" as a read-only process attribute entry
> for landlock.

Do not reuse current. Create a new name or, better yet, a landlock
subdirectory.

>
> Signed-off-by: Shervin Oloumi <enlightened@chromium.org>
> ---
>  fs/proc/base.c         | 11 +++++++++++
>  security/landlock/fs.c | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 44 insertions(+)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 9e479d7d202b..3ab29a965911 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", "current", 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..179ba22ce0fc 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -1280,6 +1280,37 @@ static int hook_file_truncate(struct file *const file)
>  	return -EACCES;
>  }
>  
> +/* process attribute interfaces */
> +
> +/**
> + * landlock_getprocattr - Landlock process attribute getter
> + * @p: the object task
> + * @name: the name of the attribute in /proc/.../attr
> + * @value: where to put the result
> + *
> + * Writes the status of landlock to value
> + *
> + * Returns the length of the result inside value
> + */
> +static int landlock_getprocattr(struct task_struct *task, const char *name,
> +				char **value)
> +{
> +	char *val;
> +	int slen;
> +
> +	if (strcmp(name, "current") != 0)
> +		return -EINVAL;
> +
> +	if (landlocked(task))
> +		val = "landlocked:1";
> +	else
> +		val = "landlocked:0";
> +
> +	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 +1333,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)
Günther Noack March 3, 2023, 4:39 p.m. UTC | #2
Hello Shervin!

On Thu, Mar 02, 2023 at 10:52:57AM -0800, enlightened@chromium.org wrote:
> +	if (landlocked(task))
> +		val = "landlocked:1";
> +	else
> +		val = "landlocked:0";

Landlock policies can be stacked on top of each other, similar to
seccomp-bpf.

If a parent process has already enforced a (potentially trivial)
Landlock policy, then you can't tell apart based on this boolean
whether any additional policies are stacked on top. So what does
Chromium do with that information, if the flag is true for all the
involved processes that it manages?

Does this meet the needs of your intended use case?  Should your API
expose more information about the stacked policies, so that it becomes
possible to tell it apart?

Thanks,
–Günther
diff mbox series

Patch

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 9e479d7d202b..3ab29a965911 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", "current", 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..179ba22ce0fc 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1280,6 +1280,37 @@  static int hook_file_truncate(struct file *const file)
 	return -EACCES;
 }
 
+/* process attribute interfaces */
+
+/**
+ * landlock_getprocattr - Landlock process attribute getter
+ * @p: the object task
+ * @name: the name of the attribute in /proc/.../attr
+ * @value: where to put the result
+ *
+ * Writes the status of landlock to value
+ *
+ * Returns the length of the result inside value
+ */
+static int landlock_getprocattr(struct task_struct *task, const char *name,
+				char **value)
+{
+	char *val;
+	int slen;
+
+	if (strcmp(name, "current") != 0)
+		return -EINVAL;
+
+	if (landlocked(task))
+		val = "landlocked:1";
+	else
+		val = "landlocked:0";
+
+	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 +1333,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)