| Message ID | 1469777680-3687-6-git-send-email-elena.reshetova@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, This again is completely unacceptable. > + * Copyright (c) 2016, Intel Corporation You know full well several functions in this code are completely copy+pasted from grsecurity, keeping unnecessary types, same exact variable names, same exact casts, same exact code structure. In other cases you've simply renamed variables. Last time I had to mention this, the entirety of my RANDSTRUCT plugin had been copy+pasted by an Intel employee with Intel's copyright placed alone on the top of it. Why does Intel have such a problem with plagiarism? Yes, there are some original changes in here, but this doesn't seem to have been tested at all -- I see obvious ways of bypassing it, and some of the checks that have been modified (aka single lines that weren't copy+pasted) are now wrong and simply won't work at all. I'm sure Kees will save the day though by repeating the flaw I just mentioned on IRC. For the rest, Intel will have to do some actual original work for a change. -Brad > +#include <linux/lsm_hooks.h> > +#include <linux/sysctl.h> > +#include <linux/fs_struct.h> > +#include <linux/nsproxy.h> > +#include <linux/pid_namespace.h> > +#include <linux/printk.h> > +#include "../fs/mount.h" > + > +/* describe a hardchroot info for a task */ > +struct hardchroot_info { > + struct task_struct *task; > + struct dentry *dentry; > + bool invalid; > + struct list_head node; > + struct rcu_head rcu; > +}; > + > +static LIST_HEAD(hardchroot_infos); > +static DEFINE_SPINLOCK(hardchroot_infos_lock); > + > +static void hardchroot_info_cleanup(struct work_struct *work); > +static DECLARE_WORK(hardchroot_info_work, hardchroot_info_cleanup); > + > +/** > + * hardchroot_info_cleanup - remove invalid entries from > + * the hardchroot info list. > + */ > +static void hardchroot_info_cleanup(struct work_struct *work) > +{ > + struct hardchroot_info *entry; > + > + spin_lock(&hardchroot_infos_lock); > + rcu_read_lock(); > + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { > + if (entry->invalid) { > + list_del_rcu(&entry->node); > + kfree_rcu(entry, rcu); > + } > + } > + rcu_read_unlock(); > + spin_unlock(&hardchroot_infos_lock); > +} > + > +/** > + * hardchroot_info_add - add/replace > + * @task: the task_struct of the process entering chroot > + * @dentry: the chroot dentry > + * > + * Returns 0 if info was added, -ve on error. > + */ > +static int hardchroot_info_add(struct task_struct *task, > + struct dentry *dentry) > +{ > + struct hardchroot_info *entry; > + struct hardchroot_info *added; > + > + added = kmalloc(sizeof(*added), GFP_KERNEL); > + if (!added) > + return -ENOMEM; > + > + added->task = task; > + added->dentry = dentry; > + added->invalid = false; > + > + spin_lock(&hardchroot_infos_lock); > + rcu_read_lock(); > + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { > + if (entry->invalid) > + continue; > + if (entry->task == task) { > + list_replace_rcu(&entry->node, &added->node); > + kfree_rcu(entry, rcu); > + goto out; > + } > + } > + > + list_add_rcu(&added->node, &hardchroot_infos); > + > +out: > + rcu_read_unlock(); > + spin_unlock(&hardchroot_infos_lock); > + return 0; > +} > + > +/** > + * hardchroot_info_del - remove hardchroot info for a given task > + * @task: remove any relation where task matches > + * @dentry: remove any relation where dentry matches > + */ > +static void hardchroot_info_del(struct task_struct *task, > + struct dentry *dentry) > +{ > + struct hardchroot_info *entry; > + bool marked = false; > + > + rcu_read_lock(); > + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { > + if (entry->invalid) > + continue; > + if (entry->task == task || > + (dentry && entry->dentry == dentry)) { > + entry->invalid = true; > + marked = true; > + } > + } > + rcu_read_unlock(); > + > + if (marked) > + schedule_work(&hardchroot_info_work); > +} > + > +/** > + * hardchroot_task_free - remove task from exception list > + * @task: task being removed > + */ > +void hardchroot_task_free(struct task_struct *task) > +{ > + hardchroot_info_del(task, NULL); > +} > + > +/** > + * task_is_descendant - walk up a process family tree looking for a match > + * The function is taken from Yama LSM > + * @parent: the process to compare against while walking up from child > + * @child: the process to start from while looking upwards for parent > + * > + * Returns 1 if child is a descendant of parent, 0 if not. > + */ > +static int task_is_descendant(struct task_struct *parent, > + struct task_struct *child) > +{ > + int rc = 0; > + struct task_struct *walker = child; > + > + if (!parent || !child) > + return 0; > + > + rcu_read_lock(); > + if (!thread_group_leader(parent)) > + parent = rcu_dereference(parent->group_leader); > + while (walker->pid > 0) { > + if (!thread_group_leader(walker)) > + walker = rcu_dereference(walker->group_leader); > + if (walker == parent) { > + rc = 1; > + break; > + } > + walker = rcu_dereference(walker->real_parent); > + } > + rcu_read_unlock(); > + > + return rc; > +} > + > +/** > + * is_process_chrooted - process is inside chroot > + * @task: the task_struct of the process to be checked > + * > + * Returns 1 if task is inside chroot. > + */ > +static int is_process_chrooted(struct task_struct *task) > +{ > + int rc = 0; > + struct hardchroot_info *entry; > + > + rcu_read_lock(); > + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { > + if (entry->invalid) > + continue; > + if ((entry->task == task) || > + (task_is_descendant(entry->task, task))) { > + rc = 1; > + pr_info("HCRT: pid %d is already chrooted\n", > + task_pid_nr(entry->task)); > + break; > + } > + } > + rcu_read_unlock(); > + return rc; > +} > + > +/** > + * is_same_root - check if two tasks share the same root > + * @task1: the task_struct of the first task to be checked > + * @task2: the task_struct of the second task to be checked > + * > + * Returns 1 if tasks share the same root. > + */ > +static int is_same_root(struct task_struct *task1, struct task_struct *task2) > +{ > + int rc = 0; > + struct hardchroot_info *entry; > + struct dentry *dentry1 = NULL; > + struct dentry *dentry2 = NULL; > + > + rcu_read_lock(); > + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { > + if (entry->invalid) > + continue; > + if (entry->task == task1) > + dentry1 = entry->dentry; > + if (entry->task == task2) > + dentry2 = entry->dentry; > + } > + if (dentry1 && (dentry1 == dentry2)) { > + rc = 1; > + pr_info("HCRT: pids %d and %d have the same root\n", > + task_pid_nr(task1), task_pid_nr(task2)); > + } > + rcu_read_unlock(); > + return rc; > +} > + > +/** > + * is_inside_chroot - check if dentry and mount > + * are inside the current process fs root > + * @u_dentry: dentry to be checked > + * @u_mnt: mnt to be checked > + * > + * Returns 1 if dentry and mount are under fs root. > + */ > +int is_inside_chroot(const struct dentry *u_dentry, > + const struct vfsmount *u_mnt) > +{ > + struct path path; > + struct path currentroot; > + int ret = 0; > + > + path.dentry = (struct dentry *)u_dentry; > + path.mnt = (struct vfsmount *)u_mnt; > + get_fs_root(current->fs, ¤troot); > + if (path_is_under(&path, ¤troot)) > + ret = 1; > + else > + pr_info("HCRT: dentry %lu is outside current task %d root\n", > + d_backing_inode(u_dentry)->i_ino, > + task_pid_nr(current)); > + path_put(¤troot); > + return ret; > +} > + > +/** > + * hardchroot_path_chroot - validate chroot entry > + * @path contains the path structure. > + * > + * Returns 0 if chroot is allowed, -ve on error. > + */ > +static int hardchroot_path_chroot(const struct path *path) > +{ > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, chroot: inode %lu and pid %d\n", > + d_backing_inode(path->dentry)->i_ino, > + task_pid_nr(myself)); > + > + get_task_struct(myself); > + if (is_process_chrooted(myself) && > + !is_inside_chroot(path->dentry, path->mnt)) { > + put_task_struct(myself); > + pr_info("HCRT, chroot denied: for inode %lu and pid %d\n", > + d_backing_inode(path->dentry)->i_ino, > + task_pid_nr(myself)); > + return -EACCES; > + } > + > + if (task_pid_nr(myself) > 1 && > + path->dentry != init_task.fs->root.dentry && > + path->dentry != myself->nsproxy->mnt_ns->root->mnt.mnt_root) { > + /* task is attempting to chroot, add it to the list */ > + rc = hardchroot_info_add(myself, path->dentry); > + pr_info("HCRT, chroot: adding %d to chrooted task list\n", > + task_pid_nr(myself)); > + } > + > + /* set the current working directory of all newly-chrooted > + * processes to the the root directory of the chroot > + */ > + set_fs_pwd(myself->fs, path); > + put_task_struct(myself); > + > + return rc; > +} > + > +/** > + * hardchroot_task_unshare - check if process is > + * allowed to unshare its namespaces > + * @unshare_flags flags > + * @new_fs contains the new fs_struct if created. > + * @new_fd contains the new files_struct if created. > + * @new_creds contains the new cred if created. > + * @new_nsproxy contains the new nsproxy if created. > + * > + * Returns 0 if unshare is allowed, -ve on error. > + */ > +static int hardchroot_task_unshare(unsigned long unshare_flags, > + const struct fs_struct *new_fs, > + const struct files_struct *new_fd, > + const struct cred *new_cred, > + const struct nsproxy *new_nsproxy) > +{ > + int rc = 0; > + struct task_struct *myself = current; > + const struct nsproxy *tnsproxy = new_nsproxy; > + > + pr_info("HCRT, unshare: unshare_flags %lu and pid %d\n", > + unshare_flags, task_pid_nr(myself)); > + if (new_fs) > + pr_info("HCRT, unshare: new_fs->root.dentry inode%lu\n", > + d_backing_inode(new_fs->root.dentry)->i_ino); > + > + if (!tnsproxy) > + tnsproxy = myself->nsproxy; > + > + if (new_fs && task_pid_nr(myself) > 1 && > + new_fs->root.dentry != init_task.fs->root.dentry && > + new_fs->root.dentry != tnsproxy->mnt_ns->root->mnt.mnt_root) { > + rc = hardchroot_info_add(myself, new_fs->root.dentry); > + pr_info("HCRT, unshare: adding %d to chrooted task list\n", > + task_pid_nr(myself)); > + } > + > + return rc; > +} > + > +/** > + * hardchroot_sb_unsharefs - check if process is > + * allowed to unshare fs_struct > + * @path contains the path for the new root structure. > + * > + * Returns 0 if unsharefs is allowed, -ve on error. > + */ > +static int hardchroot_sb_unsharefs(const struct path *path) > +{ > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, unsharefs: inode %lu and pid %d\n", > + d_backing_inode(path->dentry)->i_ino, > + task_pid_nr(myself)); > + > + if (task_pid_nr(myself) > 1 && > + path->dentry != init_task.fs->root.dentry && > + path->dentry != myself->nsproxy->mnt_ns->root->mnt.mnt_root) { > + rc = hardchroot_info_add(myself, path->dentry); > + pr_info("HCRT, unsharefs: adding %d to chrooted task list\n", > + task_pid_nr(myself)); > + } > + > + return rc; > +} > + > +/** > + * hardchroot_path_chmod - validate if chmod is allowed > + * @mnt contains the vfsmnt structure. > + * @mode contains DAC's mode > + * > + * Returns 0 if allowed, -ve on error. > + */ > +static int hardchroot_path_chmod(const struct path *path, umode_t mode) > +{ > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, chmod: inode %lu, pid %d\n", > + d_backing_inode(path->dentry)->i_ino, > + task_pid_nr(myself)); > + > + /* allow chmod +s on directories, but not files */ > + if (!S_ISDIR(path->dentry->d_inode->i_mode) && ((mode & S_ISUID) || > + ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP))) && > + is_process_chrooted(myself)) { > + pr_info("HCRT, chmod denied: inode %lu, pid %d\n", > + d_backing_inode(path->dentry)->i_ino, > + task_pid_nr(myself)); > + return -EACCES; > + } > + > + return rc; > + > +} > + > +/** > + * hardchroot_path_fchdir - validate if fchdir is allowed > + * @path: contains the path structure > + * > + * Returns 0 if allowed, -ve on error. > + */ > +static int hardchroot_path_fchdir(const struct path *path) > +{ > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, fchdir: pid %d, path %lu", > + task_pid_nr(myself), > + d_backing_inode(path->dentry)->i_ino); > + > + if (!is_process_chrooted(myself)) > + return rc; > + if (!is_inside_chroot(path->dentry, path->mnt)) { > + pr_info("HCRT, fchdir denied: pid %d, path %lu", > + task_pid_nr(myself), > + d_backing_inode(path->dentry)->i_ino); > + return -EACCES; > + } > + > + return rc; > +} > + > +/** > + * hardchroot_path_fhandle - validate if converting > + * handle to path is allowed > + * @path: contains the path structure > + * > + * Returns 0 if allowed, -ve on error. > + */ > +static int hardchroot_path_fhandle(const struct path *path) > +{ > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, fhandle: pid %d, path %lu", > + task_pid_nr(myself), > + d_backing_inode(path->dentry)->i_ino); > + > + if (is_process_chrooted(myself)) { > + pr_info("HCRT, fhandle denied: pid %d, path %lu", > + task_pid_nr(myself), > + d_backing_inode(path->dentry)->i_ino); > + return -EACCES; > + } > + > + return rc; > +} > + > +/** > + * hardchroot_task_setnice - check if setting nice is allowed > + * @task contains the task_struct of process. > + * @nice contains the new nice value. > + * > + * Return 0 if allowed, -ve on error. > + */ > +static int hardchroot_task_setnice(struct task_struct *task, int nice) > +{ > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, setnice: current %d, nice %d, for pid %d and current nice %d\n", > + task_pid_nr(myself), nice, > + task_pid_nr(task), task_nice(task)); > + > + if (is_process_chrooted(myself) && (nice < task_nice(task))) { > + pr_info("HCRT, setnice denied: current %d, nice %d, for pid %d and current nice %d\n", > + task_pid_nr(myself), nice, > + task_pid_nr(task), task_nice(task)); > + return -EACCES; > + } > + return rc; > +} > + > +/** > + * hardchroot_path_mknod - check if mknod is allowed > + * @dir contains the path structure of parent of the new file. > + * @dentry contains the dentry structure of the new file. > + * @mode contains the mode of the new file. > + * @dev contains the undecoded device number. Use new_decode_dev() to get > + * the decoded device number. > + * > + * Return 0 if allowed, -ve on error. > + */ > +static int hardchroot_path_mknod(const struct path *dir, struct dentry *dentry, > + umode_t mode, unsigned int dev) > +{ > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, mknod: dir %lu, mode %d pid %d\n", > + d_backing_inode(dir->dentry)->i_ino, > + mode, task_pid_nr(myself)); > + > + if (!S_ISFIFO(mode) && !S_ISREG(mode) && is_process_chrooted(myself)) { > + pr_info("HCRT, mknod denied: dir %lu, mode %d pid %d\n", > + d_backing_inode(dir->dentry)->i_ino, > + mode, task_pid_nr(myself)); > + return -EACCES; > + } > + return rc; > +} > + > +/** > + * hardchroot_sb_mount - check if mount is allowed > + * @dev_name contains the name for object being mounted. > + * @path contains the path for mount point object. > + * @type contains the filesystem type. > + * @flags contains the mount flags. > + * @data contains the filesystem-specific data. > + * > + * Return 0 if allowed, -ve on error. > + */ > +static int hardchroot_sb_mount(const char *dev_name, const struct path *path, > + const char *type, unsigned long flags, void *data) > +{ > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, mount: dev %s, inode %lu, flags %lu pid %d\n", > + dev_name, d_backing_inode(path->dentry)->i_ino, > + flags, task_pid_nr(myself)); > + > + if (is_process_chrooted(myself)) { > + pr_info("HCRT, mount denied: dev %s, inode %lu, flags %lu, pid %d\n", > + dev_name, d_backing_inode(path->dentry)->i_ino, > + flags, task_pid_nr(myself)); > + return -EACCES; > + } > + return rc; > +} > + > +/** > + * hardchroot_sb_pivotroot - check if pivotroot is allowed > + * @old_path contains the path for the new location of the > + * current root (put_old). > + * @new_path contains the path for the new root (new_root). > + * > + * Return 0 if allowed, -ve on error. > + */ > +static int hardchroot_sb_pivotroot(const struct path *old_path, > + const struct path *new_path) > +{ > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, pivotroot: old %lu, new %lu, pid %d\n", > + d_backing_inode(old_path->dentry)->i_ino, > + d_backing_inode(new_path->dentry)->i_ino, > + task_pid_nr(myself)); > + > + if (is_process_chrooted(myself)) { > + pr_info("HCRT, pivotroot denied: old %lu, new %lu, pid %d\n", > + d_backing_inode(old_path->dentry)->i_ino, > + d_backing_inode(new_path->dentry)->i_ino, > + task_pid_nr(myself)); > + return -EACCES; > + } > + return rc; > +} > + > +/** > + * hardchroot_shm_shmat - check if shmat is allowed > + * @shp contains the shared memory structure to be modified. > + * @shmaddr contains the address to attach memory region to. > + * @shmflg contains the operational flags. > + * > + * Return 0 if allowed, -ve on error. > + */ > +int hardchroot_shm_shmat(struct shmid_kernel *shp, char __user *shmaddr, > + int shmflg) > +{ > + int rc = 0; > + struct task_struct *p; > + struct task_struct *myself = current; > + u64 st; > + time_t ct; > + > + pr_info("HCRT, shmat: shp %d, shmflg %d, pid %d\n", > + shp->shm_perm.id, shmflg, > + task_pid_nr(myself)); > + > + if (likely(!is_process_chrooted(myself))) > + return rc; > + > + rcu_read_lock(); > + read_lock(&tasklist_lock); > + > + p = find_task_by_vpid(shp->shm_cprid); > + if (p) { > + st = p->start_time; > + ct = shp->shm_ctim; > + if (time_before_eq((unsigned long)st, (unsigned long)ct)) { > + if (is_same_root(myself, p)) > + goto allow; > + else { > + read_unlock(&tasklist_lock); > + rcu_read_unlock(); > + pr_info("HCRT, shmat denied: shp %d, shmflg %d, pid %d\n", > + shp->shm_perm.id, shmflg, > + task_pid_nr(myself)); > + return -EACCES; > + } > + } > + /* creator exited, pid reuse, fall through to next check */ > + } > + p = find_task_by_vpid(shp->shm_lprid); > + if (p) { > + if (unlikely(!is_same_root(myself, p))) { > + read_unlock(&tasklist_lock); > + rcu_read_unlock(); > + pr_info("HCRT, shmat denied: shp %d, shmflg %d, pid %d\n", > + shp->shm_perm.id, shmflg, > + task_pid_nr(myself)); > + return -EACCES; > + } > + } > + > +allow: > + read_unlock(&tasklist_lock); > + rcu_read_unlock(); > + > + return rc; > + > + > +} > + > +static struct security_hook_list hardchroot_hooks[] = { > + LSM_HOOK_INIT(path_chroot, hardchroot_path_chroot), > + LSM_HOOK_INIT(path_chmod, hardchroot_path_chmod), > + LSM_HOOK_INIT(path_mknod, hardchroot_path_mknod), > + LSM_HOOK_INIT(path_fchdir, hardchroot_path_fchdir), > + LSM_HOOK_INIT(path_fhandle, hardchroot_path_fhandle), > + LSM_HOOK_INIT(sb_mount, hardchroot_sb_mount), > + LSM_HOOK_INIT(sb_pivotroot, hardchroot_sb_pivotroot), > + LSM_HOOK_INIT(sb_unsharefs, hardchroot_sb_unsharefs), > + LSM_HOOK_INIT(task_setnice, hardchroot_task_setnice), > + LSM_HOOK_INIT(task_free, hardchroot_task_free), > + LSM_HOOK_INIT(task_unshare, hardchroot_task_unshare), > + LSM_HOOK_INIT(shm_shmat, hardchroot_shm_shmat), > +}; > + > +static inline void hardchroot_init_sysctl(void) { } > + > +void __init hardchroot_add_hooks(void) > +{ > + pr_info("Hardchroot: Getting stronger.\n"); > + security_add_hooks(hardchroot_hooks, ARRAY_SIZE(hardchroot_hooks)); > + hardchroot_init_sysctl(); > +} > diff --git a/security/security.c b/security/security.c > index 95487b9..ff65f06 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -61,6 +61,7 @@ int __init security_init(void) > capability_add_hooks(); > yama_add_hooks(); > loadpin_add_hooks(); > + hardchroot_add_hooks(); > > /* > * Load all the remaining security modules. > -- > 1.9.1
>This again is completely unacceptable. >> + * Copyright (c) 2016, Intel Corporation >You know full well several functions in this code are completely >copy+pasted from grsecurity, keeping unnecessary types, same exact >variable names, same exact casts, same exact code structure. In other cases you've simply renamed variables. Last time I had to mention this, the entirety of my RANDSTRUCT plugin had been copy+pasted by an Intel employee with Intel's copyright >placed alone on the top of it. >Why does Intel have such a problem with plagiarism? I am not sure how to handle the copyright statement in this case. Part of code comes from your code (and acknowledgment provided), task management done based on Yama (also acknowledged), part of code is modified, new and etc. I will ask lawyers on this to clarify since this is not really my choice. I am not interested to take credit for this, I was just interested to make the feature work inside LSM and be accepted to upstream. >Yes, there are some original changes in here, but this doesn't seem to have been tested at all -- I see obvious ways of bypassing it, and some of the checks that have been modified (aka single lines that weren't copy+pasted) are now wrong and simply >won't work at all. It was actually tested, I won't send untested code for review. I have tested this on Ubuntu and Fedora, as well as with Ubuntu standard chroot. I don't have any automatic tests done yet and there are couple of corner cases that I omitted now, as I don't mark process from init_mount_tree(), don't restrict filename lookup yet, don't cover mount with MS_MOVE flag and etc. This is beginning of the work to bring all these features in place, and I figured I would get initial comments earlier than later on this. >I'm sure Kees will save the day though by repeating the flaw I just mentioned on IRC. For the rest, Intel will have to do some actual original work for a change. This was actually the main point of sending these patches now, I want feedback and I am grateful for it. So, thank you! Anything you can point out is valuable. > +#include <linux/lsm_hooks.h> > +#include <linux/sysctl.h> > +#include <linux/fs_struct.h> > +#include <linux/nsproxy.h> > +#include <linux/pid_namespace.h> > +#include <linux/printk.h> > +#include "../fs/mount.h" > + > +/* describe a hardchroot info for a task */ struct hardchroot_info { > + struct task_struct *task; > + struct dentry *dentry; > + bool invalid; > + struct list_head node; > + struct rcu_head rcu; > +}; > + > +static LIST_HEAD(hardchroot_infos); > +static DEFINE_SPINLOCK(hardchroot_infos_lock); > + > +static void hardchroot_info_cleanup(struct work_struct *work); static > +DECLARE_WORK(hardchroot_info_work, hardchroot_info_cleanup); > + > +/** > + * hardchroot_info_cleanup - remove invalid entries from > + * the hardchroot info list. > + */ > +static void hardchroot_info_cleanup(struct work_struct *work) { > + struct hardchroot_info *entry; > + > + spin_lock(&hardchroot_infos_lock); > + rcu_read_lock(); > + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { > + if (entry->invalid) { > + list_del_rcu(&entry->node); > + kfree_rcu(entry, rcu); > + } > + } > + rcu_read_unlock(); > + spin_unlock(&hardchroot_infos_lock); > +} > + > +/** > + * hardchroot_info_add - add/replace > + * @task: the task_struct of the process entering chroot > + * @dentry: the chroot dentry > + * > + * Returns 0 if info was added, -ve on error. > + */ > +static int hardchroot_info_add(struct task_struct *task, > + struct dentry *dentry) > +{ > + struct hardchroot_info *entry; > + struct hardchroot_info *added; > + > + added = kmalloc(sizeof(*added), GFP_KERNEL); > + if (!added) > + return -ENOMEM; > + > + added->task = task; > + added->dentry = dentry; > + added->invalid = false; > + > + spin_lock(&hardchroot_infos_lock); > + rcu_read_lock(); > + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { > + if (entry->invalid) > + continue; > + if (entry->task == task) { > + list_replace_rcu(&entry->node, &added->node); > + kfree_rcu(entry, rcu); > + goto out; > + } > + } > + > + list_add_rcu(&added->node, &hardchroot_infos); > + > +out: > + rcu_read_unlock(); > + spin_unlock(&hardchroot_infos_lock); > + return 0; > +} > + > +/** > + * hardchroot_info_del - remove hardchroot info for a given task > + * @task: remove any relation where task matches > + * @dentry: remove any relation where dentry matches */ static void > +hardchroot_info_del(struct task_struct *task, > + struct dentry *dentry) > +{ > + struct hardchroot_info *entry; > + bool marked = false; > + > + rcu_read_lock(); > + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { > + if (entry->invalid) > + continue; > + if (entry->task == task || > + (dentry && entry->dentry == dentry)) { > + entry->invalid = true; > + marked = true; > + } > + } > + rcu_read_unlock(); > + > + if (marked) > + schedule_work(&hardchroot_info_work); > +} > + > +/** > + * hardchroot_task_free - remove task from exception list > + * @task: task being removed > + */ > +void hardchroot_task_free(struct task_struct *task) { > + hardchroot_info_del(task, NULL); > +} > + > +/** > + * task_is_descendant - walk up a process family tree looking for a > +match > + * The function is taken from Yama LSM > + * @parent: the process to compare against while walking up from > +child > + * @child: the process to start from while looking upwards for parent > + * > + * Returns 1 if child is a descendant of parent, 0 if not. > + */ > +static int task_is_descendant(struct task_struct *parent, > + struct task_struct *child) > +{ > + int rc = 0; > + struct task_struct *walker = child; > + > + if (!parent || !child) > + return 0; > + > + rcu_read_lock(); > + if (!thread_group_leader(parent)) > + parent = rcu_dereference(parent->group_leader); > + while (walker->pid > 0) { > + if (!thread_group_leader(walker)) > + walker = rcu_dereference(walker->group_leader); > + if (walker == parent) { > + rc = 1; > + break; > + } > + walker = rcu_dereference(walker->real_parent); > + } > + rcu_read_unlock(); > + > + return rc; > +} > + > +/** > + * is_process_chrooted - process is inside chroot > + * @task: the task_struct of the process to be checked > + * > + * Returns 1 if task is inside chroot. > + */ > +static int is_process_chrooted(struct task_struct *task) { > + int rc = 0; > + struct hardchroot_info *entry; > + > + rcu_read_lock(); > + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { > + if (entry->invalid) > + continue; > + if ((entry->task == task) || > + (task_is_descendant(entry->task, task))) { > + rc = 1; > + pr_info("HCRT: pid %d is already chrooted\n", > + task_pid_nr(entry->task)); > + break; > + } > + } > + rcu_read_unlock(); > + return rc; > +} > + > +/** > + * is_same_root - check if two tasks share the same root > + * @task1: the task_struct of the first task to be checked > + * @task2: the task_struct of the second task to be checked > + * > + * Returns 1 if tasks share the same root. > + */ > +static int is_same_root(struct task_struct *task1, struct task_struct > +*task2) { > + int rc = 0; > + struct hardchroot_info *entry; > + struct dentry *dentry1 = NULL; > + struct dentry *dentry2 = NULL; > + > + rcu_read_lock(); > + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { > + if (entry->invalid) > + continue; > + if (entry->task == task1) > + dentry1 = entry->dentry; > + if (entry->task == task2) > + dentry2 = entry->dentry; > + } > + if (dentry1 && (dentry1 == dentry2)) { > + rc = 1; > + pr_info("HCRT: pids %d and %d have the same root\n", > + task_pid_nr(task1), task_pid_nr(task2)); > + } > + rcu_read_unlock(); > + return rc; > +} > + > +/** > + * is_inside_chroot - check if dentry and mount > + * are inside the current process fs root > + * @u_dentry: dentry to be checked > + * @u_mnt: mnt to be checked > + * > + * Returns 1 if dentry and mount are under fs root. > + */ > +int is_inside_chroot(const struct dentry *u_dentry, > + const struct vfsmount *u_mnt) > +{ > + struct path path; > + struct path currentroot; > + int ret = 0; > + > + path.dentry = (struct dentry *)u_dentry; > + path.mnt = (struct vfsmount *)u_mnt; > + get_fs_root(current->fs, ¤troot); > + if (path_is_under(&path, ¤troot)) > + ret = 1; > + else > + pr_info("HCRT: dentry %lu is outside current task %d root\n", > + d_backing_inode(u_dentry)->i_ino, > + task_pid_nr(current)); > + path_put(¤troot); > + return ret; > +} > + > +/** > + * hardchroot_path_chroot - validate chroot entry > + * @path contains the path structure. > + * > + * Returns 0 if chroot is allowed, -ve on error. > + */ > +static int hardchroot_path_chroot(const struct path *path) { > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, chroot: inode %lu and pid %d\n", > + d_backing_inode(path->dentry)->i_ino, > + task_pid_nr(myself)); > + > + get_task_struct(myself); > + if (is_process_chrooted(myself) && > + !is_inside_chroot(path->dentry, path->mnt)) { > + put_task_struct(myself); > + pr_info("HCRT, chroot denied: for inode %lu and pid %d\n", > + d_backing_inode(path->dentry)->i_ino, > + task_pid_nr(myself)); > + return -EACCES; > + } > + > + if (task_pid_nr(myself) > 1 && > + path->dentry != init_task.fs->root.dentry && > + path->dentry != myself->nsproxy->mnt_ns->root->mnt.mnt_root) { > + /* task is attempting to chroot, add it to the list */ > + rc = hardchroot_info_add(myself, path->dentry); > + pr_info("HCRT, chroot: adding %d to chrooted task list\n", > + task_pid_nr(myself)); > + } > + > + /* set the current working directory of all newly-chrooted > + * processes to the the root directory of the chroot > + */ > + set_fs_pwd(myself->fs, path); > + put_task_struct(myself); > + > + return rc; > +} > + > +/** > + * hardchroot_task_unshare - check if process is > + * allowed to unshare its namespaces > + * @unshare_flags flags > + * @new_fs contains the new fs_struct if created. > + * @new_fd contains the new files_struct if created. > + * @new_creds contains the new cred if created. > + * @new_nsproxy contains the new nsproxy if created. > + * > + * Returns 0 if unshare is allowed, -ve on error. > + */ > +static int hardchroot_task_unshare(unsigned long unshare_flags, > + const struct fs_struct *new_fs, > + const struct files_struct *new_fd, > + const struct cred *new_cred, > + const struct nsproxy *new_nsproxy) > +{ > + int rc = 0; > + struct task_struct *myself = current; > + const struct nsproxy *tnsproxy = new_nsproxy; > + > + pr_info("HCRT, unshare: unshare_flags %lu and pid %d\n", > + unshare_flags, task_pid_nr(myself)); > + if (new_fs) > + pr_info("HCRT, unshare: new_fs->root.dentry inode%lu\n", > + d_backing_inode(new_fs->root.dentry)->i_ino); > + > + if (!tnsproxy) > + tnsproxy = myself->nsproxy; > + > + if (new_fs && task_pid_nr(myself) > 1 && > + new_fs->root.dentry != init_task.fs->root.dentry && > + new_fs->root.dentry != tnsproxy->mnt_ns->root->mnt.mnt_root) { > + rc = hardchroot_info_add(myself, new_fs->root.dentry); > + pr_info("HCRT, unshare: adding %d to chrooted task list\n", > + task_pid_nr(myself)); > + } > + > + return rc; > +} > + > +/** > + * hardchroot_sb_unsharefs - check if process is > + * allowed to unshare fs_struct > + * @path contains the path for the new root structure. > + * > + * Returns 0 if unsharefs is allowed, -ve on error. > + */ > +static int hardchroot_sb_unsharefs(const struct path *path) { > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, unsharefs: inode %lu and pid %d\n", > + d_backing_inode(path->dentry)->i_ino, > + task_pid_nr(myself)); > + > + if (task_pid_nr(myself) > 1 && > + path->dentry != init_task.fs->root.dentry && > + path->dentry != myself->nsproxy->mnt_ns->root->mnt.mnt_root) { > + rc = hardchroot_info_add(myself, path->dentry); > + pr_info("HCRT, unsharefs: adding %d to chrooted task list\n", > + task_pid_nr(myself)); > + } > + > + return rc; > +} > + > +/** > + * hardchroot_path_chmod - validate if chmod is allowed > + * @mnt contains the vfsmnt structure. > + * @mode contains DAC's mode > + * > + * Returns 0 if allowed, -ve on error. > + */ > +static int hardchroot_path_chmod(const struct path *path, umode_t > +mode) { > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, chmod: inode %lu, pid %d\n", > + d_backing_inode(path->dentry)->i_ino, > + task_pid_nr(myself)); > + > + /* allow chmod +s on directories, but not files */ > + if (!S_ISDIR(path->dentry->d_inode->i_mode) && ((mode & S_ISUID) || > + ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP))) && > + is_process_chrooted(myself)) { > + pr_info("HCRT, chmod denied: inode %lu, pid %d\n", > + d_backing_inode(path->dentry)->i_ino, > + task_pid_nr(myself)); > + return -EACCES; > + } > + > + return rc; > + > +} > + > +/** > + * hardchroot_path_fchdir - validate if fchdir is allowed > + * @path: contains the path structure > + * > + * Returns 0 if allowed, -ve on error. > + */ > +static int hardchroot_path_fchdir(const struct path *path) { > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, fchdir: pid %d, path %lu", > + task_pid_nr(myself), > + d_backing_inode(path->dentry)->i_ino); > + > + if (!is_process_chrooted(myself)) > + return rc; > + if (!is_inside_chroot(path->dentry, path->mnt)) { > + pr_info("HCRT, fchdir denied: pid %d, path %lu", > + task_pid_nr(myself), > + d_backing_inode(path->dentry)->i_ino); > + return -EACCES; > + } > + > + return rc; > +} > + > +/** > + * hardchroot_path_fhandle - validate if converting > + * handle to path is allowed > + * @path: contains the path structure > + * > + * Returns 0 if allowed, -ve on error. > + */ > +static int hardchroot_path_fhandle(const struct path *path) { > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, fhandle: pid %d, path %lu", > + task_pid_nr(myself), > + d_backing_inode(path->dentry)->i_ino); > + > + if (is_process_chrooted(myself)) { > + pr_info("HCRT, fhandle denied: pid %d, path %lu", > + task_pid_nr(myself), > + d_backing_inode(path->dentry)->i_ino); > + return -EACCES; > + } > + > + return rc; > +} > + > +/** > + * hardchroot_task_setnice - check if setting nice is allowed > + * @task contains the task_struct of process. > + * @nice contains the new nice value. > + * > + * Return 0 if allowed, -ve on error. > + */ > +static int hardchroot_task_setnice(struct task_struct *task, int > +nice) { > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, setnice: current %d, nice %d, for pid %d and current nice %d\n", > + task_pid_nr(myself), nice, > + task_pid_nr(task), task_nice(task)); > + > + if (is_process_chrooted(myself) && (nice < task_nice(task))) { > + pr_info("HCRT, setnice denied: current %d, nice %d, for pid %d and current nice %d\n", > + task_pid_nr(myself), nice, > + task_pid_nr(task), task_nice(task)); > + return -EACCES; > + } > + return rc; > +} > + > +/** > + * hardchroot_path_mknod - check if mknod is allowed > + * @dir contains the path structure of parent of the new file. > + * @dentry contains the dentry structure of the new file. > + * @mode contains the mode of the new file. > + * @dev contains the undecoded device number. Use new_decode_dev() to > +get > + * the decoded device number. > + * > + * Return 0 if allowed, -ve on error. > + */ > +static int hardchroot_path_mknod(const struct path *dir, struct dentry *dentry, > + umode_t mode, unsigned int dev) > +{ > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, mknod: dir %lu, mode %d pid %d\n", > + d_backing_inode(dir->dentry)->i_ino, > + mode, task_pid_nr(myself)); > + > + if (!S_ISFIFO(mode) && !S_ISREG(mode) && is_process_chrooted(myself)) { > + pr_info("HCRT, mknod denied: dir %lu, mode %d pid %d\n", > + d_backing_inode(dir->dentry)->i_ino, > + mode, task_pid_nr(myself)); > + return -EACCES; > + } > + return rc; > +} > + > +/** > + * hardchroot_sb_mount - check if mount is allowed > + * @dev_name contains the name for object being mounted. > + * @path contains the path for mount point object. > + * @type contains the filesystem type. > + * @flags contains the mount flags. > + * @data contains the filesystem-specific data. > + * > + * Return 0 if allowed, -ve on error. > + */ > +static int hardchroot_sb_mount(const char *dev_name, const struct path *path, > + const char *type, unsigned long flags, void *data) { > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, mount: dev %s, inode %lu, flags %lu pid %d\n", > + dev_name, d_backing_inode(path->dentry)->i_ino, > + flags, task_pid_nr(myself)); > + > + if (is_process_chrooted(myself)) { > + pr_info("HCRT, mount denied: dev %s, inode %lu, flags %lu, pid %d\n", > + dev_name, d_backing_inode(path->dentry)->i_ino, > + flags, task_pid_nr(myself)); > + return -EACCES; > + } > + return rc; > +} > + > +/** > + * hardchroot_sb_pivotroot - check if pivotroot is allowed > + * @old_path contains the path for the new location of the > + * current root (put_old). > + * @new_path contains the path for the new root (new_root). > + * > + * Return 0 if allowed, -ve on error. > + */ > +static int hardchroot_sb_pivotroot(const struct path *old_path, > + const struct path *new_path) > +{ > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, pivotroot: old %lu, new %lu, pid %d\n", > + d_backing_inode(old_path->dentry)->i_ino, > + d_backing_inode(new_path->dentry)->i_ino, > + task_pid_nr(myself)); > + > + if (is_process_chrooted(myself)) { > + pr_info("HCRT, pivotroot denied: old %lu, new %lu, pid %d\n", > + d_backing_inode(old_path->dentry)->i_ino, > + d_backing_inode(new_path->dentry)->i_ino, > + task_pid_nr(myself)); > + return -EACCES; > + } > + return rc; > +} > + > +/** > + * hardchroot_shm_shmat - check if shmat is allowed > + * @shp contains the shared memory structure to be modified. > + * @shmaddr contains the address to attach memory region to. > + * @shmflg contains the operational flags. > + * > + * Return 0 if allowed, -ve on error. > + */ > +int hardchroot_shm_shmat(struct shmid_kernel *shp, char __user *shmaddr, > + int shmflg) > +{ > + int rc = 0; > + struct task_struct *p; > + struct task_struct *myself = current; > + u64 st; > + time_t ct; > + > + pr_info("HCRT, shmat: shp %d, shmflg %d, pid %d\n", > + shp->shm_perm.id, shmflg, > + task_pid_nr(myself)); > + > + if (likely(!is_process_chrooted(myself))) > + return rc; > + > + rcu_read_lock(); > + read_lock(&tasklist_lock); > + > + p = find_task_by_vpid(shp->shm_cprid); > + if (p) { > + st = p->start_time; > + ct = shp->shm_ctim; > + if (time_before_eq((unsigned long)st, (unsigned long)ct)) { > + if (is_same_root(myself, p)) > + goto allow; > + else { > + read_unlock(&tasklist_lock); > + rcu_read_unlock(); > + pr_info("HCRT, shmat denied: shp %d, shmflg %d, pid %d\n", > + shp->shm_perm.id, shmflg, > + task_pid_nr(myself)); > + return -EACCES; > + } > + } > + /* creator exited, pid reuse, fall through to next check */ > + } > + p = find_task_by_vpid(shp->shm_lprid); > + if (p) { > + if (unlikely(!is_same_root(myself, p))) { > + read_unlock(&tasklist_lock); > + rcu_read_unlock(); > + pr_info("HCRT, shmat denied: shp %d, shmflg %d, pid %d\n", > + shp->shm_perm.id, shmflg, > + task_pid_nr(myself)); > + return -EACCES; > + } > + } > + > +allow: > + read_unlock(&tasklist_lock); > + rcu_read_unlock(); > + > + return rc; > + > + > +} > + > +static struct security_hook_list hardchroot_hooks[] = { > + LSM_HOOK_INIT(path_chroot, hardchroot_path_chroot), > + LSM_HOOK_INIT(path_chmod, hardchroot_path_chmod), > + LSM_HOOK_INIT(path_mknod, hardchroot_path_mknod), > + LSM_HOOK_INIT(path_fchdir, hardchroot_path_fchdir), > + LSM_HOOK_INIT(path_fhandle, hardchroot_path_fhandle), > + LSM_HOOK_INIT(sb_mount, hardchroot_sb_mount), > + LSM_HOOK_INIT(sb_pivotroot, hardchroot_sb_pivotroot), > + LSM_HOOK_INIT(sb_unsharefs, hardchroot_sb_unsharefs), > + LSM_HOOK_INIT(task_setnice, hardchroot_task_setnice), > + LSM_HOOK_INIT(task_free, hardchroot_task_free), > + LSM_HOOK_INIT(task_unshare, hardchroot_task_unshare), > + LSM_HOOK_INIT(shm_shmat, hardchroot_shm_shmat), }; > + > +static inline void hardchroot_init_sysctl(void) { } > + > +void __init hardchroot_add_hooks(void) { > + pr_info("Hardchroot: Getting stronger.\n"); > + security_add_hooks(hardchroot_hooks, ARRAY_SIZE(hardchroot_hooks)); > + hardchroot_init_sysctl(); > +} > diff --git a/security/security.c b/security/security.c index > 95487b9..ff65f06 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -61,6 +61,7 @@ int __init security_init(void) > capability_add_hooks(); > yama_add_hooks(); > loadpin_add_hooks(); > + hardchroot_add_hooks(); > > /* > * Load all the remaining security modules. > -- > 1.9.1
>This again is completely unacceptable. >> + * Copyright (c) 2016, Intel Corporation And actually while I am waiting for an answer from our lawyers, how would you want such cases to be handled to make sure this doesn't happen again? Do you want to be in copyright also or? Please suggest, it would help greatly!
On Fri, Jul 29, 2016 at 10:34:40AM +0300, Elena Reshetova wrote: > This adds a new Hardchroot LSM that is intended to make > classical chroot more secure. It is based on > GRKERNSEC_CHROOT feature with necessary changes needed to > make it fit inside LSM. Currently not all GRKERNSEC_CHROOT > features are supported, but support is planned to be added > on granular basis. > > The credits for feature itself should go to the original > authors of GRKERNSEC_CHROOT. Since there is no way to share > security metadata between LSMs yet, the Hardchroot info task > management is done based on Yama LSM. When support is added, > the required info can be stored as part of task struct and it > can drastically simplify the internal management. I really don't like this series. First off: On Linux, as far as I know, chroots were never meant to be a security feature, and chroot "jails" break in a number of different ways. A lot of effort went into making bind mounts actually secure with reasonable performance, and I doubt that something like this can provide anything close to that, at least not without gigantic runtime overhead. Instead of making people believe that it's now okay to use chroot for security, I would very much prefer to keep the "never use this for security purposes" warning in the chroot() manpage and encourage people to use namespaces with bind mounts instead. I think that a somewhat more formal specification of what this is supposed to be good for might help. As far as I can tell, you haven't done anything to restrict the access to unix domain sockets or ptrace, which are probably the main tricks people would actually use to break out of a chroot "jail". Instead, you add support for some weird edgecase things like "if you are privileged and inside a chroot, you can renice processes outside the chroot". > Note: this feature currently prevents rtkit daemon from > normal operation. And it probably also prevents people from using chroot() for what it's actually intended to do. > +/** > + * is_process_chrooted - process is inside chroot > + * @task: the task_struct of the process to be checked > + * > + * Returns 1 if task is inside chroot. > + */ > +static int is_process_chrooted(struct task_struct *task) > +{ > + int rc = 0; > + struct hardchroot_info *entry; > + > + rcu_read_lock(); > + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { Wait, what? Is this really a function that has execution time linear in the number of running processes and that is called for every fchdir()? That seems slow. Did you benchmark this on a system with lots of processes? > +/** > + * is_same_root - check if two tasks share the same root > + * @task1: the task_struct of the first task to be checked > + * @task2: the task_struct of the second task to be checked > + * > + * Returns 1 if tasks share the same root. > + */ > +static int is_same_root(struct task_struct *task1, struct task_struct *task2) > +{ > + int rc = 0; > + struct hardchroot_info *entry; > + struct dentry *dentry1 = NULL; > + struct dentry *dentry2 = NULL; > + > + rcu_read_lock(); > + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { > + if (entry->invalid) > + continue; > + if (entry->task == task1) > + dentry1 = entry->dentry; > + if (entry->task == task2) > + dentry2 = entry->dentry; > + } > + if (dentry1 && (dentry1 == dentry2)) { > + rc = 1; > + pr_info("HCRT: pids %d and %d have the same root\n", > + task_pid_nr(task1), task_pid_nr(task2)); > + } You're comparing dentries, not paths? So using the same directory through different bind mounts as root directories will still count as "same root" even though for chroot breakouts, it's like having different roots? > +/** > + * hardchroot_path_chroot - validate chroot entry > + * @path contains the path structure. > + * > + * Returns 0 if chroot is allowed, -ve on error. > + */ > +static int hardchroot_path_chroot(const struct path *path) > +{ > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, chroot: inode %lu and pid %d\n", > + d_backing_inode(path->dentry)->i_ino, > + task_pid_nr(myself)); > + > + get_task_struct(myself); > + if (is_process_chrooted(myself) && > + !is_inside_chroot(path->dentry, path->mnt)) { > + put_task_struct(myself); > + pr_info("HCRT, chroot denied: for inode %lu and pid %d\n", > + d_backing_inode(path->dentry)->i_ino, > + task_pid_nr(myself)); > + return -EACCES; > + } > + > + if (task_pid_nr(myself) > 1 && > + path->dentry != init_task.fs->root.dentry && > + path->dentry != myself->nsproxy->mnt_ns->root->mnt.mnt_root) { > + /* task is attempting to chroot, add it to the list */ > + rc = hardchroot_info_add(myself, path->dentry); > + pr_info("HCRT, chroot: adding %d to chrooted task list\n", > + task_pid_nr(myself)); > + } > + > + /* set the current working directory of all newly-chrooted > + * processes to the the root directory of the chroot > + */ > + set_fs_pwd(myself->fs, path); That seems pretty broken, considering the possibility of open file descriptors pointing to the old root or so. And again, chroot only works if the current process is privileged. If it is, there are probably other ways to break out. > +/** > + * hardchroot_task_unshare - check if process is > + * allowed to unshare its namespaces > + * @unshare_flags flags > + * @new_fs contains the new fs_struct if created. > + * @new_fd contains the new files_struct if created. > + * @new_creds contains the new cred if created. > + * @new_nsproxy contains the new nsproxy if created. > + * > + * Returns 0 if unshare is allowed, -ve on error. > + */ > +static int hardchroot_task_unshare(unsigned long unshare_flags, > + const struct fs_struct *new_fs, > + const struct files_struct *new_fd, > + const struct cred *new_cred, > + const struct nsproxy *new_nsproxy) > +{ > + int rc = 0; > + struct task_struct *myself = current; > + const struct nsproxy *tnsproxy = new_nsproxy; > + > + pr_info("HCRT, unshare: unshare_flags %lu and pid %d\n", > + unshare_flags, task_pid_nr(myself)); > + if (new_fs) > + pr_info("HCRT, unshare: new_fs->root.dentry inode%lu\n", > + d_backing_inode(new_fs->root.dentry)->i_ino); > + > + if (!tnsproxy) > + tnsproxy = myself->nsproxy; > + > + if (new_fs && task_pid_nr(myself) > 1 && > + new_fs->root.dentry != init_task.fs->root.dentry && Hm? You're accessing the fs_struct of init_task without any explicit locking? > +/** > + * hardchroot_shm_shmat - check if shmat is allowed > + * @shp contains the shared memory structure to be modified. > + * @shmaddr contains the address to attach memory region to. > + * @shmflg contains the operational flags. > + * > + * Return 0 if allowed, -ve on error. > + */ > +int hardchroot_shm_shmat(struct shmid_kernel *shp, char __user *shmaddr, > + int shmflg) > +{ > + int rc = 0; > + struct task_struct *p; > + struct task_struct *myself = current; > + u64 st; > + time_t ct; > + > + pr_info("HCRT, shmat: shp %d, shmflg %d, pid %d\n", > + shp->shm_perm.id, shmflg, > + task_pid_nr(myself)); > + > + if (likely(!is_process_chrooted(myself))) > + return rc; > + > + rcu_read_lock(); > + read_lock(&tasklist_lock); > + > + p = find_task_by_vpid(shp->shm_cprid); Not exactly your fault, but this is broken when PID namespaces are involved.
On 7/29/2016 11:53 AM, Jann Horn wrote: > On Fri, Jul 29, 2016 at 10:34:40AM +0300, Elena Reshetova wrote: >> This adds a new Hardchroot LSM that is intended to make >> classical chroot more secure. It is based on >> GRKERNSEC_CHROOT feature with necessary changes needed to >> make it fit inside LSM. Currently not all GRKERNSEC_CHROOT >> features are supported, but support is planned to be added >> on granular basis. >> >> The credits for feature itself should go to the original >> authors of GRKERNSEC_CHROOT. Since there is no way to share >> security metadata between LSMs yet, the Hardchroot info task >> management is done based on Yama LSM. When support is added, >> the required info can be stored as part of task struct and it >> can drastically simplify the internal management. > I really don't like this series. > > First off: On Linux, as far as I know, chroots were never meant > to be a security feature, This is a common misconception. When chroot was introduced circa 1979 (the exact date is subject to interpretation and your skill with sccs) security, especially in the form of protecting the system from accidental corruption, was an important concern. > and chroot "jails" break in a number > of different ways. All of which were introduced after the fact, and most of which have been introduced in spite of the objections of the security community. Even sockets, which are the biggest single breakage (followed closely by the process namespace and SVIPC) came along well after chroot and really should have taken the "root" into account. > A lot of effort went into making bind mounts > actually secure with reasonable performance, and I doubt that > something like this can provide anything close to that, at least > not without gigantic runtime overhead. Instead of making people > believe that it's now okay to use chroot for security, I would > very much prefer to keep the "never use this for security > purposes" warning in the chroot() manpage and encourage people > to use namespaces with bind mounts instead. There is merit to the argument that namespaces are better than chroot jails. Nonetheless, we're all aware of just how much legacy code we're going to have to deal with for the next forever, and some of that can benefit from this work. > > I think that a somewhat more formal specification of what this > is supposed to be good for might help. > > As far as I can tell, you haven't done anything to restrict > the access to unix domain sockets or ptrace, which are probably > the main tricks people would actually use to break out of a > chroot "jail". Instead, you add support for some weird edgecase > things like "if you are privileged and inside a chroot, you can > renice processes outside the chroot". > > >> Note: this feature currently prevents rtkit daemon from >> normal operation. > And it probably also prevents people from using chroot() for > what it's actually intended to do. > >> +/** >> + * is_process_chrooted - process is inside chroot >> + * @task: the task_struct of the process to be checked >> + * >> + * Returns 1 if task is inside chroot. >> + */ >> +static int is_process_chrooted(struct task_struct *task) >> +{ >> + int rc = 0; >> + struct hardchroot_info *entry; >> + >> + rcu_read_lock(); >> + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { > Wait, what? Is this really a function that has execution time linear > in the number of running processes and that is called for every > fchdir()? That seems slow. Did you benchmark this on a system with > lots of processes? > > >> +/** >> + * is_same_root - check if two tasks share the same root >> + * @task1: the task_struct of the first task to be checked >> + * @task2: the task_struct of the second task to be checked >> + * >> + * Returns 1 if tasks share the same root. >> + */ >> +static int is_same_root(struct task_struct *task1, struct task_struct *task2) >> +{ >> + int rc = 0; >> + struct hardchroot_info *entry; >> + struct dentry *dentry1 = NULL; >> + struct dentry *dentry2 = NULL; >> + >> + rcu_read_lock(); >> + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { >> + if (entry->invalid) >> + continue; >> + if (entry->task == task1) >> + dentry1 = entry->dentry; >> + if (entry->task == task2) >> + dentry2 = entry->dentry; >> + } >> + if (dentry1 && (dentry1 == dentry2)) { >> + rc = 1; >> + pr_info("HCRT: pids %d and %d have the same root\n", >> + task_pid_nr(task1), task_pid_nr(task2)); >> + } > You're comparing dentries, not paths? So using the same directory > through different bind mounts as root directories will still count > as "same root" even though for chroot breakouts, it's like having > different roots? > > >> +/** >> + * hardchroot_path_chroot - validate chroot entry >> + * @path contains the path structure. >> + * >> + * Returns 0 if chroot is allowed, -ve on error. >> + */ >> +static int hardchroot_path_chroot(const struct path *path) >> +{ >> + int rc = 0; >> + struct task_struct *myself = current; >> + >> + pr_info("HCRT, chroot: inode %lu and pid %d\n", >> + d_backing_inode(path->dentry)->i_ino, >> + task_pid_nr(myself)); >> + >> + get_task_struct(myself); >> + if (is_process_chrooted(myself) && >> + !is_inside_chroot(path->dentry, path->mnt)) { >> + put_task_struct(myself); >> + pr_info("HCRT, chroot denied: for inode %lu and pid %d\n", >> + d_backing_inode(path->dentry)->i_ino, >> + task_pid_nr(myself)); >> + return -EACCES; >> + } >> + >> + if (task_pid_nr(myself) > 1 && >> + path->dentry != init_task.fs->root.dentry && >> + path->dentry != myself->nsproxy->mnt_ns->root->mnt.mnt_root) { >> + /* task is attempting to chroot, add it to the list */ >> + rc = hardchroot_info_add(myself, path->dentry); >> + pr_info("HCRT, chroot: adding %d to chrooted task list\n", >> + task_pid_nr(myself)); >> + } >> + >> + /* set the current working directory of all newly-chrooted >> + * processes to the the root directory of the chroot >> + */ >> + set_fs_pwd(myself->fs, path); > That seems pretty broken, considering the possibility of open > file descriptors pointing to the old root or so. > > And again, chroot only works if the current process is privileged. > If it is, there are probably other ways to break out. > > >> +/** >> + * hardchroot_task_unshare - check if process is >> + * allowed to unshare its namespaces >> + * @unshare_flags flags >> + * @new_fs contains the new fs_struct if created. >> + * @new_fd contains the new files_struct if created. >> + * @new_creds contains the new cred if created. >> + * @new_nsproxy contains the new nsproxy if created. >> + * >> + * Returns 0 if unshare is allowed, -ve on error. >> + */ >> +static int hardchroot_task_unshare(unsigned long unshare_flags, >> + const struct fs_struct *new_fs, >> + const struct files_struct *new_fd, >> + const struct cred *new_cred, >> + const struct nsproxy *new_nsproxy) >> +{ >> + int rc = 0; >> + struct task_struct *myself = current; >> + const struct nsproxy *tnsproxy = new_nsproxy; >> + >> + pr_info("HCRT, unshare: unshare_flags %lu and pid %d\n", >> + unshare_flags, task_pid_nr(myself)); >> + if (new_fs) >> + pr_info("HCRT, unshare: new_fs->root.dentry inode%lu\n", >> + d_backing_inode(new_fs->root.dentry)->i_ino); >> + >> + if (!tnsproxy) >> + tnsproxy = myself->nsproxy; >> + >> + if (new_fs && task_pid_nr(myself) > 1 && >> + new_fs->root.dentry != init_task.fs->root.dentry && > Hm? You're accessing the fs_struct of init_task without any explicit locking? > > >> +/** >> + * hardchroot_shm_shmat - check if shmat is allowed >> + * @shp contains the shared memory structure to be modified. >> + * @shmaddr contains the address to attach memory region to. >> + * @shmflg contains the operational flags. >> + * >> + * Return 0 if allowed, -ve on error. >> + */ >> +int hardchroot_shm_shmat(struct shmid_kernel *shp, char __user *shmaddr, >> + int shmflg) >> +{ >> + int rc = 0; >> + struct task_struct *p; >> + struct task_struct *myself = current; >> + u64 st; >> + time_t ct; >> + >> + pr_info("HCRT, shmat: shp %d, shmflg %d, pid %d\n", >> + shp->shm_perm.id, shmflg, >> + task_pid_nr(myself)); >> + >> + if (likely(!is_process_chrooted(myself))) >> + return rc; >> + >> + rcu_read_lock(); >> + read_lock(&tasklist_lock); >> + >> + p = find_task_by_vpid(shp->shm_cprid); > Not exactly your fault, but this is broken when PID namespaces are involved. -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 29, 2016 at 12:20:56PM -0700, Casey Schaufler wrote: > On 7/29/2016 11:53 AM, Jann Horn wrote: > > On Fri, Jul 29, 2016 at 10:34:40AM +0300, Elena Reshetova wrote: > >> This adds a new Hardchroot LSM that is intended to make > >> classical chroot more secure. It is based on > >> GRKERNSEC_CHROOT feature with necessary changes needed to > >> make it fit inside LSM. Currently not all GRKERNSEC_CHROOT > >> features are supported, but support is planned to be added > >> on granular basis. > >> > >> The credits for feature itself should go to the original > >> authors of GRKERNSEC_CHROOT. Since there is no way to share > >> security metadata between LSMs yet, the Hardchroot info task > >> management is done based on Yama LSM. When support is added, > >> the required info can be stored as part of task struct and it > >> can drastically simplify the internal management. > > I really don't like this series. > > > > First off: On Linux, as far as I know, chroots were never meant > > to be a security feature, > > This is a common misconception. When chroot was introduced circa 1979 > (the exact date is subject to interpretation and your skill with sccs) > security, especially in the form of protecting the system from > accidental corruption, was an important concern. I'm explicitly talking about the situation *on Linux*. I don't know much about old UNIX variants, and I don't think that they are very relevant here - IMO, what matters here are what chroot() was designed for *on Linux* and how it was treated during the development of the kernel, because that is what influences how easy it is going to be to add that stuff to Linux today. And when you look at Linux 0.10, you'll see that already back then, sys_chroot() just updated current->root; sending signals to other processes, setting the system time and so on just did UID checks. > > and chroot "jails" break in a number > > of different ways. > > All of which were introduced after the fact, and most of which > have been introduced in spite of the objections of the security > community. Even sockets, which are the biggest single breakage > (followed closely by the process namespace and SVIPC) came along > well after chroot and really should have taken the "root" into > account. Namespaces on Linux actually take chroots into account - you can't create a new namespace if you're unprivileged and inside a chroot, see commit 3151527ee0. I'm not sure whether that was added before or after unprivileged user namespaces were enabled. > > A lot of effort went into making bind mounts > > actually secure with reasonable performance, and I doubt that > > something like this can provide anything close to that, at least > > not without gigantic runtime overhead. Instead of making people > > believe that it's now okay to use chroot for security, I would > > very much prefer to keep the "never use this for security > > purposes" warning in the chroot() manpage and encourage people > > to use namespaces with bind mounts instead. > > There is merit to the argument that namespaces are better than > chroot jails. Nonetheless, we're all aware of just how much > legacy code we're going to have to deal with for the next > forever, and some of that can benefit from this work. Eh. For that, you could also make a shim that turns chroot into namespace creation automatically - either as a libc feature or as a personality flag in the kernel. The biggest issue with this would probably be dealing with multithreaded processes that call chroot() while being multithreaded - in that case, a personality flag would have the advantage of allowing the kernel to have a variant of unshare() that synchronizes new user and mount namespaces across all threads. That approach would probably be less of a maintenance and performance burden and have less security issues popping up over time compared to attempting to have two orthogonal filesystem sandboxing implementations.
On 7/29/2016 1:53 PM, Jann Horn wrote: > On Fri, Jul 29, 2016 at 12:20:56PM -0700, Casey Schaufler wrote: >> On 7/29/2016 11:53 AM, Jann Horn wrote: >>> On Fri, Jul 29, 2016 at 10:34:40AM +0300, Elena Reshetova wrote: >>>> This adds a new Hardchroot LSM that is intended to make >>>> classical chroot more secure. It is based on >>>> GRKERNSEC_CHROOT feature with necessary changes needed to >>>> make it fit inside LSM. Currently not all GRKERNSEC_CHROOT >>>> features are supported, but support is planned to be added >>>> on granular basis. >>>> >>>> The credits for feature itself should go to the original >>>> authors of GRKERNSEC_CHROOT. Since there is no way to share >>>> security metadata between LSMs yet, the Hardchroot info task >>>> management is done based on Yama LSM. When support is added, >>>> the required info can be stored as part of task struct and it >>>> can drastically simplify the internal management. >>> I really don't like this series. >>> >>> First off: On Linux, as far as I know, chroots were never meant >>> to be a security feature, >> This is a common misconception. When chroot was introduced circa 1979 >> (the exact date is subject to interpretation and your skill with sccs) >> security, especially in the form of protecting the system from >> accidental corruption, was an important concern. > I'm explicitly talking about the situation *on Linux*. I don't > know much about old UNIX variants, and I don't think that they > are very relevant here - IMO, what matters here are what chroot() > was designed for *on Linux* and how it was treated during the > development of the kernel, because that is what influences how > easy it is going to be to add that stuff to Linux today. Sorry, but you can't separate the Linux behavior from the UNIX behavior. The Linux behavior came from the UNIX behavior. Although I wasn't in Finland at the time, I think that it's pretty safe to conclude that the "design" of chroot for Linux was pretty well limited to duplicating what it did on UNIX. > And when you look at Linux 0.10, you'll see that already back > then, sys_chroot() just updated current->root; sending signals > to other processes, setting the system time and so on just did > UID checks. > > >>> and chroot "jails" break in a number >>> of different ways. >> All of which were introduced after the fact, and most of which >> have been introduced in spite of the objections of the security >> community. Even sockets, which are the biggest single breakage >> (followed closely by the process namespace and SVIPC) came along >> well after chroot and really should have taken the "root" into >> account. > Namespaces on Linux actually take chroots into account - you can't > create a new namespace if you're unprivileged and inside a chroot, > see commit 3151527ee0. I'm not sure whether that was added before > or after unprivileged user namespaces were enabled. > > >>> A lot of effort went into making bind mounts >>> actually secure with reasonable performance, and I doubt that >>> something like this can provide anything close to that, at least >>> not without gigantic runtime overhead. Instead of making people >>> believe that it's now okay to use chroot for security, I would >>> very much prefer to keep the "never use this for security >>> purposes" warning in the chroot() manpage and encourage people >>> to use namespaces with bind mounts instead. >> There is merit to the argument that namespaces are better than >> chroot jails. Nonetheless, we're all aware of just how much >> legacy code we're going to have to deal with for the next >> forever, and some of that can benefit from this work. > Eh. For that, you could also make a shim that turns chroot into > namespace creation automatically Right. Why carry a tent when you can pull a 24' Airsteam trailer? :) > - either as a libc feature or > as a personality flag in the kernel. The biggest issue with this > would probably be dealing with multithreaded processes that call > chroot() while being multithreaded - in that case, a personality > flag would have the advantage of allowing the kernel to have a > variant of unshare() that synchronizes new user and mount > namespaces across all threads. > > That approach would probably be less of a maintenance and > performance burden and have less security issues popping up over > time compared to attempting to have two orthogonal filesystem > sandboxing implementations. -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 29, 2016 at 02:10:31PM -0700, Casey Schaufler wrote: > On 7/29/2016 1:53 PM, Jann Horn wrote: > > On Fri, Jul 29, 2016 at 12:20:56PM -0700, Casey Schaufler wrote: > >> On 7/29/2016 11:53 AM, Jann Horn wrote: [...] > > And when you look at Linux 0.10, you'll see that already back > > then, sys_chroot() just updated current->root; sending signals > > to other processes, setting the system time and so on just did > > UID checks. > > > > > >>> and chroot "jails" break in a number > >>> of different ways. > >> All of which were introduced after the fact, and most of which > >> have been introduced in spite of the objections of the security > >> community. Even sockets, which are the biggest single breakage > >> (followed closely by the process namespace and SVIPC) came along > >> well after chroot and really should have taken the "root" into > >> account. > > Namespaces on Linux actually take chroots into account - you can't > > create a new namespace if you're unprivileged and inside a chroot, > > see commit 3151527ee0. I'm not sure whether that was added before > > or after unprivileged user namespaces were enabled. > > > > > >>> A lot of effort went into making bind mounts > >>> actually secure with reasonable performance, and I doubt that > >>> something like this can provide anything close to that, at least > >>> not without gigantic runtime overhead. Instead of making people > >>> believe that it's now okay to use chroot for security, I would > >>> very much prefer to keep the "never use this for security > >>> purposes" warning in the chroot() manpage and encourage people > >>> to use namespaces with bind mounts instead. > >> There is merit to the argument that namespaces are better than > >> chroot jails. Nonetheless, we're all aware of just how much > >> legacy code we're going to have to deal with for the next > >> forever, and some of that can benefit from this work. > > Eh. For that, you could also make a shim that turns chroot into > > namespace creation automatically > > Right. Why carry a tent when you can pull a 24' Airsteam trailer? :) Because that "tent" would be a lot of messy parts all over your house while the "Airsteam trailer" could maybe be parked outside and wouldn't be in the way every time you want to make coffee? And besides, since lots of people are going to build their own trailers, it might make sense to have your own so that you can show people what a proper trailer looks like. (And if multithreaded namespace creation is needed for this, it might be nice to have anyway, considering that people want to use namespaces from languages like Go, where they currently have to implement the unsharing in C before the runtime starts up - I think that was e.g. the case in subgraph's oz. This could probably be implemented similar to grsecurity's GRKERNSEC_SETXID. I think this isn't exactly a priority at the moment, but as people start using memory-safe languages with GC for low-level stuff like this, it might become more important - I don't know.)
First of all, thank you very much for the review! I am still learning with many things and your feedback is very much appreciated! >First off: On Linux, as far as I know, chroots were never meant to be a security feature, and chroot "jails" break in a number of different ways. A lot of effort went into making bind mounts actually secure with reasonable performance, and I doubt that >something like this can provide anything close to that, at least not without gigantic runtime overhead. Instead of making people believe that it's now okay to use chroot for security, I would very much prefer to keep the "never use this for security >purposes" warning in the chroot() manpage and encourage people to use namespaces with bind mounts instead. I am aware that chroot jail should not be used for security purposes, but reality is that it is *used* in many places at least with some intention of getting a better security. I don't believe there is any person nowadays who believes that chroot is secure, but because it is super light-weight (even compare with namespaces setup) and because of legacy concerns, it is still there. I don't also claim that this LSM make chroot *fully* secure (as you mentioned below some important features are still missing) and it would never be as strong as full namespace-based solution, but practical problem with namespaces is that it isn't that easy to setup them up properly and possibility of making configuration mistake is still quite high. About the performance overhead, I actually don't believe it would be that gigantic (especially when we get proper LSM stacking in place that eliminates the need to store internal list and do searches), but of course any LSM has performance impact, especially with the hooks on the filesystem. And don't take me wrong, I do personally like namespaces very much. Just usecases I am trying to address with this are very different. > I think that a somewhat more formal specification of what this is supposed to be good for might help. Yes, I think this make sense. It would clarify on when you should use this (in cases when you do need to use chroot only but want to make it a degree better). >As far as I can tell, you haven't done anything to restrict the access to unix domain sockets or ptrace, which are probably the main tricks people would actually use to break out of a chroot "jail". Instead, you add support for some weird edgecase things like >"if you are privileged and inside a chroot, you can renice processes outside the chroot". Supporting unix domain sockets is in plans, I was looking into all GRKERNSEC features and working them one by one. I first processed all features that can be easily moved into existing LSM hooks, and nice was one of them. Then I started adding new hooks for the rest of features. But at some point I just exceeded my threshold of confidence with all the changes and hooks I am making (unix domain sockets might need further new hooks), so I wanted to bring this out for review earlier than later. What about ptrace? I am assume that we have Yama in place for this :) > Note: this feature currently prevents rtkit daemon from > normal operation. >And it probably also prevents people from using chroot() for what it's actually intended to do. I would not go as far as that. How many cases apart from rtkit daemon who sets priorities for other processes you know that would suffer from this behaviour? Again, this was an easy feature that fits into existing hook and can be fully removed if people don't see a value in it. It is certainly not the main part of it. > +/** > + * is_process_chrooted - process is inside chroot > + * @task: the task_struct of the process to be checked > + * > + * Returns 1 if task is inside chroot. > + */ > +static int is_process_chrooted(struct task_struct *task) { > + int rc = 0; > + struct hardchroot_info *entry; > + > + rcu_read_lock(); > + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { >Wait, what? Is this really a function that has execution time linear in the number of running processes and that is called for every fchdir()? That seems slow. Did you benchmark this on a system with lots of processes? I haven't done any benchmarking on this yet, but remember that hardchroot_infos will be quite a short list, which equals number of processes that explicitly entered chroot (not their children, only first parent process to do it). In Ubuntu and Fedora for example, after the full boot, I only had 3 such processes in the list, so this gives the size of list I am expecting: 10-20 items absolutely max (normally under 10), not hundreds. So, list iteration is quite fast. So, it doesn't depend on the number of system processes, but it does depend on number of processes that entered chroot. > +/** > + * is_same_root - check if two tasks share the same root > + * @task1: the task_struct of the first task to be checked > + * @task2: the task_struct of the second task to be checked > + * > + * Returns 1 if tasks share the same root. > + */ > +static int is_same_root(struct task_struct *task1, struct task_struct > +*task2) { > + int rc = 0; > + struct hardchroot_info *entry; > + struct dentry *dentry1 = NULL; > + struct dentry *dentry2 = NULL; > + > + rcu_read_lock(); > + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { > + if (entry->invalid) > + continue; > + if (entry->task == task1) > + dentry1 = entry->dentry; > + if (entry->task == task2) > + dentry2 = entry->dentry; > + } > + if (dentry1 && (dentry1 == dentry2)) { > + rc = 1; > + pr_info("HCRT: pids %d and %d have the same root\n", > + task_pid_nr(task1), task_pid_nr(task2)); > + } >You're comparing dentries, not paths? So using the same directory through different bind mounts as root directories will still count as "same root" even though for chroot breakouts, it's like having different roots? Bind mounts are tricky in this case. From one side yes, it is yes like having two different roots, but then how to handle legitimate cases when people bind mount things into chroot to be used there? Blocking them would make bind mounts not supported inside chroot, which is not smth that should be done IMO. > +/** > + * hardchroot_path_chroot - validate chroot entry > + * @path contains the path structure. > + * > + * Returns 0 if chroot is allowed, -ve on error. > + */ > +static int hardchroot_path_chroot(const struct path *path) { > + int rc = 0; > + struct task_struct *myself = current; > + > + pr_info("HCRT, chroot: inode %lu and pid %d\n", > + d_backing_inode(path->dentry)->i_ino, > + task_pid_nr(myself)); > + > + get_task_struct(myself); > + if (is_process_chrooted(myself) && > + !is_inside_chroot(path->dentry, path->mnt)) { > + put_task_struct(myself); > + pr_info("HCRT, chroot denied: for inode %lu and pid %d\n", > + d_backing_inode(path->dentry)->i_ino, > + task_pid_nr(myself)); > + return -EACCES; > + } > + > + if (task_pid_nr(myself) > 1 && > + path->dentry != init_task.fs->root.dentry && > + path->dentry != myself->nsproxy->mnt_ns->root->mnt.mnt_root) { > + /* task is attempting to chroot, add it to the list */ > + rc = hardchroot_info_add(myself, path->dentry); > + pr_info("HCRT, chroot: adding %d to chrooted task list\n", > + task_pid_nr(myself)); > + } > + > + /* set the current working directory of all newly-chrooted > + * processes to the the root directory of the chroot > + */ > + set_fs_pwd(myself->fs, path); >That seems pretty broken, considering the possibility of open file descriptors pointing to the old root or so. Again, the main use case here is that we have a good process that enters chroot to minimize the damage in case it gets exploited some later time in life. A good process is supposed to close its file descriptors pointing to the old root before entering chroot, isn't it? >And again, chroot only works if the current process is privileged. >If it is, there are probably other ways to break out. You need to have privilege to *enter* chroot, but if you are good process, you will drop the privilege after you did your setup. > +/** > + * hardchroot_task_unshare - check if process is > + * allowed to unshare its namespaces > + * @unshare_flags flags > + * @new_fs contains the new fs_struct if created. > + * @new_fd contains the new files_struct if created. > + * @new_creds contains the new cred if created. > + * @new_nsproxy contains the new nsproxy if created. > + * > + * Returns 0 if unshare is allowed, -ve on error. > + */ > +static int hardchroot_task_unshare(unsigned long unshare_flags, > + const struct fs_struct *new_fs, > + const struct files_struct *new_fd, > + const struct cred *new_cred, > + const struct nsproxy *new_nsproxy) > +{ > + int rc = 0; > + struct task_struct *myself = current; > + const struct nsproxy *tnsproxy = new_nsproxy; > + > + pr_info("HCRT, unshare: unshare_flags %lu and pid %d\n", > + unshare_flags, task_pid_nr(myself)); > + if (new_fs) > + pr_info("HCRT, unshare: new_fs->root.dentry inode%lu\n", > + d_backing_inode(new_fs->root.dentry)->i_ino); > + > + if (!tnsproxy) > + tnsproxy = myself->nsproxy; > + > + if (new_fs && task_pid_nr(myself) > 1 && > + new_fs->root.dentry != init_task.fs->root.dentry && >Hm? You're accessing the fs_struct of init_task without any explicit locking? I am quite new to the kernel development and locking. I was reading a lot recently about locking to understand where it is needed and where not, but mistakes are obviously still coming. I guess I would need to do smth like: task_lock(&init_task) before and task_unlock(&init_task) after. > +/** > + * hardchroot_shm_shmat - check if shmat is allowed > + * @shp contains the shared memory structure to be modified. > + * @shmaddr contains the address to attach memory region to. > + * @shmflg contains the operational flags. > + * > + * Return 0 if allowed, -ve on error. > + */ > +int hardchroot_shm_shmat(struct shmid_kernel *shp, char __user *shmaddr, > + int shmflg) > +{ > + int rc = 0; > + struct task_struct *p; > + struct task_struct *myself = current; > + u64 st; > + time_t ct; > + > + pr_info("HCRT, shmat: shp %d, shmflg %d, pid %d\n", > + shp->shm_perm.id, shmflg, > + task_pid_nr(myself)); > + > + if (likely(!is_process_chrooted(myself))) > + return rc; > + > + rcu_read_lock(); > + read_lock(&tasklist_lock); > + > + p = find_task_by_vpid(shp->shm_cprid); >Not exactly your fault, but this is broken when PID namespaces are involved. Yes, this has been bothering me also... Grsecurity has their own version of this function (find_task_by_vpid_unrestricted) instead that tried to do global search, I guess I should think on doing smth similar, but I am wondering if this can be done better. I will look into this for the next version.
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index f30cf47..c87fcf7 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1930,5 +1930,10 @@ void __init loadpin_add_hooks(void); #else static inline void loadpin_add_hooks(void) { }; #endif +#ifdef CONFIG_SECURITY_HARDCHROOT +void __init hardchroot_add_hooks(void); +#else +static inline void hardchroot_add_hooks(void) { }; +#endif #endif /* ! __LINUX_LSM_HOOKS_H */ diff --git a/security/Kconfig b/security/Kconfig index 176758c..cd7821d 100644 --- a/security/Kconfig +++ b/security/Kconfig @@ -124,6 +124,7 @@ source security/tomoyo/Kconfig source security/apparmor/Kconfig source security/loadpin/Kconfig source security/yama/Kconfig +source security/hardchroot/Kconfig source security/integrity/Kconfig diff --git a/security/Makefile b/security/Makefile index f2d71cd..b102b60 100644 --- a/security/Makefile +++ b/security/Makefile @@ -9,6 +9,7 @@ subdir-$(CONFIG_SECURITY_TOMOYO) += tomoyo subdir-$(CONFIG_SECURITY_APPARMOR) += apparmor subdir-$(CONFIG_SECURITY_YAMA) += yama subdir-$(CONFIG_SECURITY_LOADPIN) += loadpin +subdir-$(CONFIG_SECURITY_HARDCHROOT)+= hardchroot # always enable default capabilities obj-y += commoncap.o @@ -25,6 +26,7 @@ obj-$(CONFIG_SECURITY_APPARMOR) += apparmor/ obj-$(CONFIG_SECURITY_YAMA) += yama/ obj-$(CONFIG_SECURITY_LOADPIN) += loadpin/ obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o +obj-$(CONFIG_SECURITY_HARDCHROOT) += hardchroot/ # Object integrity file lists subdir-$(CONFIG_INTEGRITY) += integrity diff --git a/security/hardchroot/Kconfig b/security/hardchroot/Kconfig new file mode 100644 index 0000000..a20d758 --- /dev/null +++ b/security/hardchroot/Kconfig @@ -0,0 +1,10 @@ +config SECURITY_HARDCHROOT + bool "Hardchroot support" + depends on SECURITY + select SECURITY_PATH + default n + help + This selects Hardchroot LSM, which makes a traditional Linux chroot + environment stronger. Like capabilities, this security module + stacks with other LSMs. + If you are unsure how to answer this question, answer N. diff --git a/security/hardchroot/Makefile b/security/hardchroot/Makefile new file mode 100644 index 0000000..b03461b --- /dev/null +++ b/security/hardchroot/Makefile @@ -0,0 +1,3 @@ +obj-$(CONFIG_SECURITY_HARDCHROOT) := hardchroot.o + +hardchroot-y := hardchroot_lsm.o diff --git a/security/hardchroot/hardchroot_lsm.c b/security/hardchroot/hardchroot_lsm.c new file mode 100644 index 0000000..01aa95c --- /dev/null +++ b/security/hardchroot/hardchroot_lsm.c @@ -0,0 +1,654 @@ +/* + * Hardchroot Linux Security Module + * + * Author: Elena Reshetova <elena.reshetova@intel.com> + * + * Done based on GRSECURITY_CHROOT feature. + * + * Management of task-related chroot information + * is done based on Yama ptrace relationship management + * + * Copyright (c) 2016, Intel Corporation + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2, as + * published by the Free Software Foundation. + * + */ + +#include <linux/lsm_hooks.h> +#include <linux/sysctl.h> +#include <linux/fs_struct.h> +#include <linux/nsproxy.h> +#include <linux/pid_namespace.h> +#include <linux/printk.h> +#include "../fs/mount.h" + +/* describe a hardchroot info for a task */ +struct hardchroot_info { + struct task_struct *task; + struct dentry *dentry; + bool invalid; + struct list_head node; + struct rcu_head rcu; +}; + +static LIST_HEAD(hardchroot_infos); +static DEFINE_SPINLOCK(hardchroot_infos_lock); + +static void hardchroot_info_cleanup(struct work_struct *work); +static DECLARE_WORK(hardchroot_info_work, hardchroot_info_cleanup); + +/** + * hardchroot_info_cleanup - remove invalid entries from + * the hardchroot info list. + */ +static void hardchroot_info_cleanup(struct work_struct *work) +{ + struct hardchroot_info *entry; + + spin_lock(&hardchroot_infos_lock); + rcu_read_lock(); + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { + if (entry->invalid) { + list_del_rcu(&entry->node); + kfree_rcu(entry, rcu); + } + } + rcu_read_unlock(); + spin_unlock(&hardchroot_infos_lock); +} + +/** + * hardchroot_info_add - add/replace + * @task: the task_struct of the process entering chroot + * @dentry: the chroot dentry + * + * Returns 0 if info was added, -ve on error. + */ +static int hardchroot_info_add(struct task_struct *task, + struct dentry *dentry) +{ + struct hardchroot_info *entry; + struct hardchroot_info *added; + + added = kmalloc(sizeof(*added), GFP_KERNEL); + if (!added) + return -ENOMEM; + + added->task = task; + added->dentry = dentry; + added->invalid = false; + + spin_lock(&hardchroot_infos_lock); + rcu_read_lock(); + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { + if (entry->invalid) + continue; + if (entry->task == task) { + list_replace_rcu(&entry->node, &added->node); + kfree_rcu(entry, rcu); + goto out; + } + } + + list_add_rcu(&added->node, &hardchroot_infos); + +out: + rcu_read_unlock(); + spin_unlock(&hardchroot_infos_lock); + return 0; +} + +/** + * hardchroot_info_del - remove hardchroot info for a given task + * @task: remove any relation where task matches + * @dentry: remove any relation where dentry matches + */ +static void hardchroot_info_del(struct task_struct *task, + struct dentry *dentry) +{ + struct hardchroot_info *entry; + bool marked = false; + + rcu_read_lock(); + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { + if (entry->invalid) + continue; + if (entry->task == task || + (dentry && entry->dentry == dentry)) { + entry->invalid = true; + marked = true; + } + } + rcu_read_unlock(); + + if (marked) + schedule_work(&hardchroot_info_work); +} + +/** + * hardchroot_task_free - remove task from exception list + * @task: task being removed + */ +void hardchroot_task_free(struct task_struct *task) +{ + hardchroot_info_del(task, NULL); +} + +/** + * task_is_descendant - walk up a process family tree looking for a match + * The function is taken from Yama LSM + * @parent: the process to compare against while walking up from child + * @child: the process to start from while looking upwards for parent + * + * Returns 1 if child is a descendant of parent, 0 if not. + */ +static int task_is_descendant(struct task_struct *parent, + struct task_struct *child) +{ + int rc = 0; + struct task_struct *walker = child; + + if (!parent || !child) + return 0; + + rcu_read_lock(); + if (!thread_group_leader(parent)) + parent = rcu_dereference(parent->group_leader); + while (walker->pid > 0) { + if (!thread_group_leader(walker)) + walker = rcu_dereference(walker->group_leader); + if (walker == parent) { + rc = 1; + break; + } + walker = rcu_dereference(walker->real_parent); + } + rcu_read_unlock(); + + return rc; +} + +/** + * is_process_chrooted - process is inside chroot + * @task: the task_struct of the process to be checked + * + * Returns 1 if task is inside chroot. + */ +static int is_process_chrooted(struct task_struct *task) +{ + int rc = 0; + struct hardchroot_info *entry; + + rcu_read_lock(); + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { + if (entry->invalid) + continue; + if ((entry->task == task) || + (task_is_descendant(entry->task, task))) { + rc = 1; + pr_info("HCRT: pid %d is already chrooted\n", + task_pid_nr(entry->task)); + break; + } + } + rcu_read_unlock(); + return rc; +} + +/** + * is_same_root - check if two tasks share the same root + * @task1: the task_struct of the first task to be checked + * @task2: the task_struct of the second task to be checked + * + * Returns 1 if tasks share the same root. + */ +static int is_same_root(struct task_struct *task1, struct task_struct *task2) +{ + int rc = 0; + struct hardchroot_info *entry; + struct dentry *dentry1 = NULL; + struct dentry *dentry2 = NULL; + + rcu_read_lock(); + list_for_each_entry_rcu(entry, &hardchroot_infos, node) { + if (entry->invalid) + continue; + if (entry->task == task1) + dentry1 = entry->dentry; + if (entry->task == task2) + dentry2 = entry->dentry; + } + if (dentry1 && (dentry1 == dentry2)) { + rc = 1; + pr_info("HCRT: pids %d and %d have the same root\n", + task_pid_nr(task1), task_pid_nr(task2)); + } + rcu_read_unlock(); + return rc; +} + +/** + * is_inside_chroot - check if dentry and mount + * are inside the current process fs root + * @u_dentry: dentry to be checked + * @u_mnt: mnt to be checked + * + * Returns 1 if dentry and mount are under fs root. + */ +int is_inside_chroot(const struct dentry *u_dentry, + const struct vfsmount *u_mnt) +{ + struct path path; + struct path currentroot; + int ret = 0; + + path.dentry = (struct dentry *)u_dentry; + path.mnt = (struct vfsmount *)u_mnt; + get_fs_root(current->fs, ¤troot); + if (path_is_under(&path, ¤troot)) + ret = 1; + else + pr_info("HCRT: dentry %lu is outside current task %d root\n", + d_backing_inode(u_dentry)->i_ino, + task_pid_nr(current)); + path_put(¤troot); + return ret; +} + +/** + * hardchroot_path_chroot - validate chroot entry + * @path contains the path structure. + * + * Returns 0 if chroot is allowed, -ve on error. + */ +static int hardchroot_path_chroot(const struct path *path) +{ + int rc = 0; + struct task_struct *myself = current; + + pr_info("HCRT, chroot: inode %lu and pid %d\n", + d_backing_inode(path->dentry)->i_ino, + task_pid_nr(myself)); + + get_task_struct(myself); + if (is_process_chrooted(myself) && + !is_inside_chroot(path->dentry, path->mnt)) { + put_task_struct(myself); + pr_info("HCRT, chroot denied: for inode %lu and pid %d\n", + d_backing_inode(path->dentry)->i_ino, + task_pid_nr(myself)); + return -EACCES; + } + + if (task_pid_nr(myself) > 1 && + path->dentry != init_task.fs->root.dentry && + path->dentry != myself->nsproxy->mnt_ns->root->mnt.mnt_root) { + /* task is attempting to chroot, add it to the list */ + rc = hardchroot_info_add(myself, path->dentry); + pr_info("HCRT, chroot: adding %d to chrooted task list\n", + task_pid_nr(myself)); + } + + /* set the current working directory of all newly-chrooted + * processes to the the root directory of the chroot + */ + set_fs_pwd(myself->fs, path); + put_task_struct(myself); + + return rc; +} + +/** + * hardchroot_task_unshare - check if process is + * allowed to unshare its namespaces + * @unshare_flags flags + * @new_fs contains the new fs_struct if created. + * @new_fd contains the new files_struct if created. + * @new_creds contains the new cred if created. + * @new_nsproxy contains the new nsproxy if created. + * + * Returns 0 if unshare is allowed, -ve on error. + */ +static int hardchroot_task_unshare(unsigned long unshare_flags, + const struct fs_struct *new_fs, + const struct files_struct *new_fd, + const struct cred *new_cred, + const struct nsproxy *new_nsproxy) +{ + int rc = 0; + struct task_struct *myself = current; + const struct nsproxy *tnsproxy = new_nsproxy; + + pr_info("HCRT, unshare: unshare_flags %lu and pid %d\n", + unshare_flags, task_pid_nr(myself)); + if (new_fs) + pr_info("HCRT, unshare: new_fs->root.dentry inode%lu\n", + d_backing_inode(new_fs->root.dentry)->i_ino); + + if (!tnsproxy) + tnsproxy = myself->nsproxy; + + if (new_fs && task_pid_nr(myself) > 1 && + new_fs->root.dentry != init_task.fs->root.dentry && + new_fs->root.dentry != tnsproxy->mnt_ns->root->mnt.mnt_root) { + rc = hardchroot_info_add(myself, new_fs->root.dentry); + pr_info("HCRT, unshare: adding %d to chrooted task list\n", + task_pid_nr(myself)); + } + + return rc; +} + +/** + * hardchroot_sb_unsharefs - check if process is + * allowed to unshare fs_struct + * @path contains the path for the new root structure. + * + * Returns 0 if unsharefs is allowed, -ve on error. + */ +static int hardchroot_sb_unsharefs(const struct path *path) +{ + int rc = 0; + struct task_struct *myself = current; + + pr_info("HCRT, unsharefs: inode %lu and pid %d\n", + d_backing_inode(path->dentry)->i_ino, + task_pid_nr(myself)); + + if (task_pid_nr(myself) > 1 && + path->dentry != init_task.fs->root.dentry && + path->dentry != myself->nsproxy->mnt_ns->root->mnt.mnt_root) { + rc = hardchroot_info_add(myself, path->dentry); + pr_info("HCRT, unsharefs: adding %d to chrooted task list\n", + task_pid_nr(myself)); + } + + return rc; +} + +/** + * hardchroot_path_chmod - validate if chmod is allowed + * @mnt contains the vfsmnt structure. + * @mode contains DAC's mode + * + * Returns 0 if allowed, -ve on error. + */ +static int hardchroot_path_chmod(const struct path *path, umode_t mode) +{ + int rc = 0; + struct task_struct *myself = current; + + pr_info("HCRT, chmod: inode %lu, pid %d\n", + d_backing_inode(path->dentry)->i_ino, + task_pid_nr(myself)); + + /* allow chmod +s on directories, but not files */ + if (!S_ISDIR(path->dentry->d_inode->i_mode) && ((mode & S_ISUID) || + ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP))) && + is_process_chrooted(myself)) { + pr_info("HCRT, chmod denied: inode %lu, pid %d\n", + d_backing_inode(path->dentry)->i_ino, + task_pid_nr(myself)); + return -EACCES; + } + + return rc; + +} + +/** + * hardchroot_path_fchdir - validate if fchdir is allowed + * @path: contains the path structure + * + * Returns 0 if allowed, -ve on error. + */ +static int hardchroot_path_fchdir(const struct path *path) +{ + int rc = 0; + struct task_struct *myself = current; + + pr_info("HCRT, fchdir: pid %d, path %lu", + task_pid_nr(myself), + d_backing_inode(path->dentry)->i_ino); + + if (!is_process_chrooted(myself)) + return rc; + if (!is_inside_chroot(path->dentry, path->mnt)) { + pr_info("HCRT, fchdir denied: pid %d, path %lu", + task_pid_nr(myself), + d_backing_inode(path->dentry)->i_ino); + return -EACCES; + } + + return rc; +} + +/** + * hardchroot_path_fhandle - validate if converting + * handle to path is allowed + * @path: contains the path structure + * + * Returns 0 if allowed, -ve on error. + */ +static int hardchroot_path_fhandle(const struct path *path) +{ + int rc = 0; + struct task_struct *myself = current; + + pr_info("HCRT, fhandle: pid %d, path %lu", + task_pid_nr(myself), + d_backing_inode(path->dentry)->i_ino); + + if (is_process_chrooted(myself)) { + pr_info("HCRT, fhandle denied: pid %d, path %lu", + task_pid_nr(myself), + d_backing_inode(path->dentry)->i_ino); + return -EACCES; + } + + return rc; +} + +/** + * hardchroot_task_setnice - check if setting nice is allowed + * @task contains the task_struct of process. + * @nice contains the new nice value. + * + * Return 0 if allowed, -ve on error. + */ +static int hardchroot_task_setnice(struct task_struct *task, int nice) +{ + int rc = 0; + struct task_struct *myself = current; + + pr_info("HCRT, setnice: current %d, nice %d, for pid %d and current nice %d\n", + task_pid_nr(myself), nice, + task_pid_nr(task), task_nice(task)); + + if (is_process_chrooted(myself) && (nice < task_nice(task))) { + pr_info("HCRT, setnice denied: current %d, nice %d, for pid %d and current nice %d\n", + task_pid_nr(myself), nice, + task_pid_nr(task), task_nice(task)); + return -EACCES; + } + return rc; +} + +/** + * hardchroot_path_mknod - check if mknod is allowed + * @dir contains the path structure of parent of the new file. + * @dentry contains the dentry structure of the new file. + * @mode contains the mode of the new file. + * @dev contains the undecoded device number. Use new_decode_dev() to get + * the decoded device number. + * + * Return 0 if allowed, -ve on error. + */ +static int hardchroot_path_mknod(const struct path *dir, struct dentry *dentry, + umode_t mode, unsigned int dev) +{ + int rc = 0; + struct task_struct *myself = current; + + pr_info("HCRT, mknod: dir %lu, mode %d pid %d\n", + d_backing_inode(dir->dentry)->i_ino, + mode, task_pid_nr(myself)); + + if (!S_ISFIFO(mode) && !S_ISREG(mode) && is_process_chrooted(myself)) { + pr_info("HCRT, mknod denied: dir %lu, mode %d pid %d\n", + d_backing_inode(dir->dentry)->i_ino, + mode, task_pid_nr(myself)); + return -EACCES; + } + return rc; +} + +/** + * hardchroot_sb_mount - check if mount is allowed + * @dev_name contains the name for object being mounted. + * @path contains the path for mount point object. + * @type contains the filesystem type. + * @flags contains the mount flags. + * @data contains the filesystem-specific data. + * + * Return 0 if allowed, -ve on error. + */ +static int hardchroot_sb_mount(const char *dev_name, const struct path *path, + const char *type, unsigned long flags, void *data) +{ + int rc = 0; + struct task_struct *myself = current; + + pr_info("HCRT, mount: dev %s, inode %lu, flags %lu pid %d\n", + dev_name, d_backing_inode(path->dentry)->i_ino, + flags, task_pid_nr(myself)); + + if (is_process_chrooted(myself)) { + pr_info("HCRT, mount denied: dev %s, inode %lu, flags %lu, pid %d\n", + dev_name, d_backing_inode(path->dentry)->i_ino, + flags, task_pid_nr(myself)); + return -EACCES; + } + return rc; +} + +/** + * hardchroot_sb_pivotroot - check if pivotroot is allowed + * @old_path contains the path for the new location of the + * current root (put_old). + * @new_path contains the path for the new root (new_root). + * + * Return 0 if allowed, -ve on error. + */ +static int hardchroot_sb_pivotroot(const struct path *old_path, + const struct path *new_path) +{ + int rc = 0; + struct task_struct *myself = current; + + pr_info("HCRT, pivotroot: old %lu, new %lu, pid %d\n", + d_backing_inode(old_path->dentry)->i_ino, + d_backing_inode(new_path->dentry)->i_ino, + task_pid_nr(myself)); + + if (is_process_chrooted(myself)) { + pr_info("HCRT, pivotroot denied: old %lu, new %lu, pid %d\n", + d_backing_inode(old_path->dentry)->i_ino, + d_backing_inode(new_path->dentry)->i_ino, + task_pid_nr(myself)); + return -EACCES; + } + return rc; +} + +/** + * hardchroot_shm_shmat - check if shmat is allowed + * @shp contains the shared memory structure to be modified. + * @shmaddr contains the address to attach memory region to. + * @shmflg contains the operational flags. + * + * Return 0 if allowed, -ve on error. + */ +int hardchroot_shm_shmat(struct shmid_kernel *shp, char __user *shmaddr, + int shmflg) +{ + int rc = 0; + struct task_struct *p; + struct task_struct *myself = current; + u64 st; + time_t ct; + + pr_info("HCRT, shmat: shp %d, shmflg %d, pid %d\n", + shp->shm_perm.id, shmflg, + task_pid_nr(myself)); + + if (likely(!is_process_chrooted(myself))) + return rc; + + rcu_read_lock(); + read_lock(&tasklist_lock); + + p = find_task_by_vpid(shp->shm_cprid); + if (p) { + st = p->start_time; + ct = shp->shm_ctim; + if (time_before_eq((unsigned long)st, (unsigned long)ct)) { + if (is_same_root(myself, p)) + goto allow; + else { + read_unlock(&tasklist_lock); + rcu_read_unlock(); + pr_info("HCRT, shmat denied: shp %d, shmflg %d, pid %d\n", + shp->shm_perm.id, shmflg, + task_pid_nr(myself)); + return -EACCES; + } + } + /* creator exited, pid reuse, fall through to next check */ + } + p = find_task_by_vpid(shp->shm_lprid); + if (p) { + if (unlikely(!is_same_root(myself, p))) { + read_unlock(&tasklist_lock); + rcu_read_unlock(); + pr_info("HCRT, shmat denied: shp %d, shmflg %d, pid %d\n", + shp->shm_perm.id, shmflg, + task_pid_nr(myself)); + return -EACCES; + } + } + +allow: + read_unlock(&tasklist_lock); + rcu_read_unlock(); + + return rc; + + +} + +static struct security_hook_list hardchroot_hooks[] = { + LSM_HOOK_INIT(path_chroot, hardchroot_path_chroot), + LSM_HOOK_INIT(path_chmod, hardchroot_path_chmod), + LSM_HOOK_INIT(path_mknod, hardchroot_path_mknod), + LSM_HOOK_INIT(path_fchdir, hardchroot_path_fchdir), + LSM_HOOK_INIT(path_fhandle, hardchroot_path_fhandle), + LSM_HOOK_INIT(sb_mount, hardchroot_sb_mount), + LSM_HOOK_INIT(sb_pivotroot, hardchroot_sb_pivotroot), + LSM_HOOK_INIT(sb_unsharefs, hardchroot_sb_unsharefs), + LSM_HOOK_INIT(task_setnice, hardchroot_task_setnice), + LSM_HOOK_INIT(task_free, hardchroot_task_free), + LSM_HOOK_INIT(task_unshare, hardchroot_task_unshare), + LSM_HOOK_INIT(shm_shmat, hardchroot_shm_shmat), +}; + +static inline void hardchroot_init_sysctl(void) { } + +void __init hardchroot_add_hooks(void) +{ + pr_info("Hardchroot: Getting stronger.\n"); + security_add_hooks(hardchroot_hooks, ARRAY_SIZE(hardchroot_hooks)); + hardchroot_init_sysctl(); +} diff --git a/security/security.c b/security/security.c index 95487b9..ff65f06 100644 --- a/security/security.c +++ b/security/security.c @@ -61,6 +61,7 @@ int __init security_init(void) capability_add_hooks(); yama_add_hooks(); loadpin_add_hooks(); + hardchroot_add_hooks(); /* * Load all the remaining security modules.
This adds a new Hardchroot LSM that is intended to make classical chroot more secure. It is based on GRKERNSEC_CHROOT feature with necessary changes needed to make it fit inside LSM. Currently not all GRKERNSEC_CHROOT features are supported, but support is planned to be added on granular basis. The credits for feature itself should go to the original authors of GRKERNSEC_CHROOT. Since there is no way to share security metadata between LSMs yet, the Hardchroot info task management is done based on Yama LSM. When support is added, the required info can be stored as part of task struct and it can drastically simplify the internal management. Currently supported features from GRKERNSEC_CHROOT are - GRKERNSEC_CHROOT_MOUNT: prevents process inside chroot from mounting and unmounting filesystems - GRKERNSEC_CHROOT_DOUBLE: prevents process inside chroot from chrooting again outside of current chroot location - GRKERNSEC_CHROOT_PIVOT: prevents process inside chroot from calling pivot_root() system call. - GRKERNSEC_CHROOT_CHDIR: make sure the current working dir of processes inside chroot is set to chroot location. - GRKERNSEC_CHROOT_CHMOD: prevents process inside chroot from executing chmod or fchmod on files to make them have suid or sgid bits. - GRKERNSEC_CHROOT_FCHDIR: prevent process inside chroot from fchdir to a file outside of chroot location. Also prevents opening a file by a file descriptor located outside of chroot. - GRKERNSEC_CHROOT_MKNOD: prevents process inside chroot from making new device nodes. - GRKERNSEC_CHROOT_SHMAT: prevents process inside chroot from attaching to shared memory segments that were created outside of chroot. - GRKERNSEC_CHROOT_NICE: prevents process inside chroot from raising priority of processes inside or outside of chroot. Note: this feature currently prevents rtkit daemon from normal operation. Also original GRKERNSEC_CHROOT_NICE feature did not allowed processes inside chroot to make any priority changes to processed outside chroot. The same behavior can be enforced also, but it breaks rtkit daemon even more. Signed-off-by: Elena Reshetova <elena.reshetova@intel.com> --- include/linux/lsm_hooks.h | 5 + security/Kconfig | 1 + security/Makefile | 2 + security/hardchroot/Kconfig | 10 + security/hardchroot/Makefile | 3 + security/hardchroot/hardchroot_lsm.c | 654 +++++++++++++++++++++++++++++++++++ security/security.c | 1 + 7 files changed, 676 insertions(+) create mode 100644 security/hardchroot/Kconfig create mode 100644 security/hardchroot/Makefile create mode 100644 security/hardchroot/hardchroot_lsm.c