On Fri, Jun 24, 2016 at 7:29 PM, Casey Schaufler <ca...@schaufler-ca.com> wrote: > Subject: [PATCH v5 2/3] LSM: module hierarchy in /proc/.../attr > > Back in 2007 I made what turned out to be a rather serious > mistake in the implementation of the Smack security module. > The SELinux module used an interface in /proc to manipulate > the security context on processes. Rather than use a similar > interface, I used the same interface. The AppArmor team did > likewise. Now /proc/.../attr/current will tell you the > security "context" of the process, but it will be different > depending on the security module you're using. That hasn't > been a problem to date, as you can only have one module > that supports process attributes at a time. We are coming > up on a change to that, where multiple modules with process > attributes can be supported. (Not included here) > > This patch provides a subdirectory in /proc/.../attr for > each of the security modules that use the LSM hooks > getprocattr() and setprocattr(). Each of the interfaces > used by a module are presented in the subdirectory. The > old interfaces remain and work the same as before. > User space code can begin migrating to the subdirectory > interfaces in anticipation of the time when what comes > from /proc/self/attr/current might not be what a runtime > wants. > > The original implementation is by Kees Cook. The code > has been changed a bit to reflect changes in the direction > of the multiple concurrent module work, to be independent > of it, and to bring it up to date with the current tree. > > Signed-off-by: Casey Schaufler <ca...@schaufler-ca.com> > > --- > Documentation/security/LSM.txt | 19 ++++++--- > fs/proc/base.c | 91 > +++++++++++++++++++++++++++++++++++++----- > fs/proc/internal.h | 1 + > include/linux/security.h | 15 ++++--- > security/security.c | 31 ++++++++++++-- > 5 files changed, 133 insertions(+), 24 deletions(-)
Acked-by: Paul Moore <p...@paul-moore.com> > diff --git a/Documentation/security/LSM.txt b/Documentation/security/LSM.txt > index c2683f2..125c489 100644 > --- a/Documentation/security/LSM.txt > +++ b/Documentation/security/LSM.txt > @@ -16,11 +16,10 @@ MAC extensions, other extensions can be built using the > LSM to provide > specific changes to system operation when these tweaks are not available > in the core functionality of Linux itself. > > -Without a specific LSM built into the kernel, the default LSM will be the > -Linux capabilities system. Most LSMs choose to extend the capabilities > -system, building their checks on top of the defined capability hooks. > -For more details on capabilities, see capabilities(7) in the Linux > -man-pages project. > +The Linux capabilities modules will always be included. For more details > +on capabilities, see capabilities(7) in the Linux man-pages project. > +This may be followed by any number of "minor" modules and at most one > +"major" module. > > A list of the active security modules can be found by reading > /sys/kernel/security/lsm. This is a comma separated list, and > @@ -29,6 +28,14 @@ order in which checks are made. The capability module will > always > be first, followed by any "minor" modules (e.g. Yama) and then > the one "major" module (e.g. SELinux) if there is one configured. > > +Process attributes associated with "major" security modules should > +be accessed and maintained using the special files in the module > +specific subdirectories in /proc/.../attr. The attributes related > +to Smack would be found in /proc/.../attr/smack while the attributes > +for SELinux would be in /proc/.../attr/selinux. Using the files > +found directly in /proc/.../attr (e.g. current) should be avoided. > +These files remain as legacy interfaces. > + > Based on https://lkml.org/lkml/2007/10/26/215, > a new LSM is accepted into the kernel when its intent (a description of > what it tries to protect against and in what cases one would expect to > @@ -38,4 +45,4 @@ that end users and distros can make a more informed > decision about which > LSMs suit their requirements. > > For extensive documentation on the available LSM hook interfaces, please > -see include/linux/security.h. > +see include/linux/lsm_hooks.h. > diff --git a/fs/proc/base.c b/fs/proc/base.c > index a11eb71..182bc28 100644 > --- a/fs/proc/base.c > +++ b/fs/proc/base.c > @@ -131,9 +131,13 @@ struct pid_entry { > #define REG(NAME, MODE, fops) \ > NOD(NAME, (S_IFREG|(MODE)), NULL, &fops, {}) > #define ONE(NAME, MODE, show) \ > - NOD(NAME, (S_IFREG|(MODE)), \ > + NOD(NAME, (S_IFREG|(MODE)), \ > NULL, &proc_single_file_operations, \ > { .proc_show = show } ) > +#define ATTR(LSM, NAME, MODE) \ > + NOD(NAME, (S_IFREG|(MODE)), \ > + NULL, &proc_pid_attr_operations, \ > + { .lsm = LSM }) > > /* > * Count the number of hardlinks for the pid_entry table, excluding the . > @@ -2433,7 +2437,7 @@ static ssize_t proc_pid_attr_read(struct file * file, > char __user * buf, > if (!task) > return -ESRCH; > > - length = security_getprocattr(task, > + length = security_getprocattr(task, PROC_I(inode)->op.lsm, > (char*)file->f_path.dentry->d_name.name, > &p); > put_task_struct(task); > @@ -2473,7 +2477,7 @@ static ssize_t proc_pid_attr_write(struct file * file, > const char __user * buf, > if (length < 0) > goto out_free; > > - length = security_setprocattr(task, > + length = security_setprocattr(task, PROC_I(inode)->op.lsm, > (char*)file->f_path.dentry->d_name.name, > page, count); > mutex_unlock(&task->signal->cred_guard_mutex); > @@ -2491,13 +2495,82 @@ static const struct file_operations > proc_pid_attr_operations = { > .llseek = generic_file_llseek, > }; > > +#define LSM_DIR_OPS(LSM) \ > +static int proc_##LSM##_attr_dir_iterate(struct file *filp, \ > + struct dir_context *ctx) \ > +{ \ > + return proc_pident_readdir(filp, ctx, \ > + LSM##_attr_dir_stuff, \ > + ARRAY_SIZE(LSM##_attr_dir_stuff)); \ > +} \ > +\ > +static const struct file_operations proc_##LSM##_attr_dir_ops = { \ > + .read = generic_read_dir, \ > + .iterate = proc_##LSM##_attr_dir_iterate, \ > + .llseek = default_llseek, \ > +}; \ > +\ > +static struct dentry *proc_##LSM##_attr_dir_lookup(struct inode *dir, \ > + struct dentry *dentry, unsigned int flags) \ > +{ \ > + return proc_pident_lookup(dir, dentry, \ > + LSM##_attr_dir_stuff, \ > + ARRAY_SIZE(LSM##_attr_dir_stuff)); \ > +} \ > +\ > +static const struct inode_operations proc_##LSM##_attr_dir_inode_ops = { \ > + .lookup = proc_##LSM##_attr_dir_lookup, \ > + .getattr = pid_getattr, \ > + .setattr = proc_setattr, \ > +} > + > +#ifdef CONFIG_SECURITY_SELINUX > +static const struct pid_entry selinux_attr_dir_stuff[] = { > + ATTR("selinux", "current", S_IRUGO|S_IWUGO), > + ATTR("selinux", "prev", S_IRUGO), > + ATTR("selinux", "exec", S_IRUGO|S_IWUGO), > + ATTR("selinux", "fscreate", S_IRUGO|S_IWUGO), > + ATTR("selinux", "keycreate", S_IRUGO|S_IWUGO), > + ATTR("selinux", "sockcreate", S_IRUGO|S_IWUGO), > +}; > +LSM_DIR_OPS(selinux); > +#endif > + > +#ifdef CONFIG_SECURITY_SMACK > +static const struct pid_entry smack_attr_dir_stuff[] = { > + ATTR("smack", "current", S_IRUGO|S_IWUGO), > +}; > +LSM_DIR_OPS(smack); > +#endif > + > +#ifdef CONFIG_SECURITY_APPARMOR > +static const struct pid_entry apparmor_attr_dir_stuff[] = { > + ATTR("apparmor", "current", S_IRUGO|S_IWUGO), > + ATTR("apparmor", "prev", S_IRUGO), > + ATTR("apparmor", "exec", S_IRUGO|S_IWUGO), > +}; > +LSM_DIR_OPS(apparmor); > +#endif > + > static const struct pid_entry attr_dir_stuff[] = { > - REG("current", S_IRUGO|S_IWUGO, proc_pid_attr_operations), > - REG("prev", S_IRUGO, proc_pid_attr_operations), > - REG("exec", S_IRUGO|S_IWUGO, proc_pid_attr_operations), > - REG("fscreate", S_IRUGO|S_IWUGO, proc_pid_attr_operations), > - REG("keycreate", S_IRUGO|S_IWUGO, proc_pid_attr_operations), > - REG("sockcreate", S_IRUGO|S_IWUGO, proc_pid_attr_operations), > + ATTR(NULL, "current", S_IRUGO|S_IWUGO), > + ATTR(NULL, "prev", S_IRUGO), > + ATTR(NULL, "exec", S_IRUGO|S_IWUGO), > + ATTR(NULL, "fscreate", S_IRUGO|S_IWUGO), > + ATTR(NULL, "keycreate", S_IRUGO|S_IWUGO), > + ATTR(NULL, "sockcreate", S_IRUGO|S_IWUGO), > +#ifdef CONFIG_SECURITY_SELINUX > + DIR("selinux", S_IRUGO|S_IXUGO, > + proc_selinux_attr_dir_inode_ops, proc_selinux_attr_dir_ops), > +#endif > +#ifdef CONFIG_SECURITY_SMACK > + DIR("smack", S_IRUGO|S_IXUGO, > + proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops), > +#endif > +#ifdef CONFIG_SECURITY_APPARMOR > + DIR("apparmor", S_IRUGO|S_IXUGO, > + proc_apparmor_attr_dir_inode_ops, proc_apparmor_attr_dir_ops), > +#endif > }; > > static int proc_attr_dir_readdir(struct file *file, struct dir_context *ctx) > diff --git a/fs/proc/internal.h b/fs/proc/internal.h > index aa27810..b607cd5 100644 > --- a/fs/proc/internal.h > +++ b/fs/proc/internal.h > @@ -56,6 +56,7 @@ union proc_op { > int (*proc_show)(struct seq_file *m, > struct pid_namespace *ns, struct pid *pid, > struct task_struct *task); > + const char *lsm; > }; > > struct proc_inode { > diff --git a/include/linux/security.h b/include/linux/security.h > index 14df373..383fcb0 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -355,8 +355,10 @@ int security_sem_semctl(struct sem_array *sma, int cmd); > int security_sem_semop(struct sem_array *sma, struct sembuf *sops, > unsigned nsops, int alter); > void security_d_instantiate(struct dentry *dentry, struct inode *inode); > -int security_getprocattr(struct task_struct *p, char *name, char **value); > -int security_setprocattr(struct task_struct *p, char *name, void *value, > size_t size); > +int security_getprocattr(struct task_struct *p, const char *lsm, char *name, > + char **value); > +int security_setprocattr(struct task_struct *p, const char *lsm, char *name, > + void *value, size_t size); > int security_netlink_send(struct sock *sk, struct sk_buff *skb); > int security_ismaclabel(const char *name); > int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen); > @@ -1075,15 +1077,18 @@ static inline int security_sem_semop(struct sem_array > *sma, > return 0; > } > > -static inline void security_d_instantiate(struct dentry *dentry, struct > inode *inode) > +static inline void security_d_instantiate(struct dentry *dentry, > + struct inode *inode) > { } > > -static inline int security_getprocattr(struct task_struct *p, char *name, > char **value) > +static inline int security_getprocattr(struct task_struct *p, const char > *lsm, > + char *name, char **value) > { > return -EINVAL; > } > > -static inline int security_setprocattr(struct task_struct *p, char *name, > void *value, size_t size) > +static inline int security_setprocattr(struct task_struct *p, const char > *lsm, > + char *name, void *value, size_t size) > { > return -EINVAL; > } > diff --git a/security/security.c b/security/security.c > index 1093f73..1e9cb55 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -1182,14 +1182,37 @@ void security_d_instantiate(struct dentry *dentry, > struct inode *inode) > } > EXPORT_SYMBOL(security_d_instantiate); > > -int security_getprocattr(struct task_struct *p, char *name, char **value) > +int security_getprocattr(struct task_struct *p, const char *lsm, char *name, > + char **value) > { > - return call_int_hook(getprocattr, -EINVAL, p, name, value); > + struct security_hook_list *hp; > + int rc = -EINVAL; > + > + > + list_for_each_entry(hp, &security_hook_heads.getprocattr, list) { > + if (lsm != NULL && strcmp(lsm, hp->lsm)) > + continue; > + rc = hp->hook.getprocattr(p, name, value); > + if (rc != -ENOENT) > + return rc; > + } > + return -EINVAL; > } > > -int security_setprocattr(struct task_struct *p, char *name, void *value, > size_t size) > +int security_setprocattr(struct task_struct *p, const char *lsm, char *name, > + void *value, size_t size) > { > - return call_int_hook(setprocattr, -EINVAL, p, name, value, size); > + struct security_hook_list *hp; > + int rc = -EINVAL; > + > + list_for_each_entry(hp, &security_hook_heads.setprocattr, list) { > + if (lsm != NULL && strcmp(lsm, hp->lsm)) > + continue; > + rc = hp->hook.setprocattr(p, name, value, size); > + if (rc != -ENOENT) > + break; > + } > + return rc; > } > > int security_netlink_send(struct sock *sk, struct sk_buff *skb) > -- paul moore www.paul-moore.com
