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