| Message ID | 20190618230551.7475-16-casey@schaufler-ca.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | LSM: Module stacking for AppArmor | expand |
On Tue, Jun 18, 2019 at 04:05:41PM -0700, Casey Schaufler wrote: > Create a new entry "display" in /proc/.../attr for controlling > which LSM security information is displayed for a process. > The name of an active LSM that supplies hooks for human readable > data may be written to "display" to set the value. The name of > the LSM currently in use can be read from "display". > At this point there can only be one LSM capable of display > active. > > This affects /proc/.../attr/current and SO_PEERSEC. What happened to creating /proc/$pid/lsm/$lsm_name/current for "modern" LSM libraries to start using (instead of possibly fighting over the /proc/$pid/attr/display)? (Obviously "display" is needed for "old" libraries, and I'm fine with it.) Similarly, is there something that can be done for SO_PEERSEC that doesn't require using "display" for "modern" libraries? -Kees > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > --- > fs/proc/base.c | 1 + > security/security.c | 108 +++++++++++++++++++++++++++++++++++--------- > 2 files changed, 88 insertions(+), 21 deletions(-) > > diff --git a/fs/proc/base.c b/fs/proc/base.c > index ddef482f1334..7bf70e041315 100644 > --- a/fs/proc/base.c > +++ b/fs/proc/base.c > @@ -2618,6 +2618,7 @@ static const struct pid_entry attr_dir_stuff[] = { > ATTR(NULL, "fscreate", 0666), > ATTR(NULL, "keycreate", 0666), > ATTR(NULL, "sockcreate", 0666), > + ATTR(NULL, "display", 0666), > #ifdef CONFIG_SECURITY_SMACK > DIR("smack", 0555, > proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops), > diff --git a/security/security.c b/security/security.c > index 46f6cf21d33c..9cfdc664239e 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -46,7 +46,9 @@ static struct kmem_cache *lsm_file_cache; > static struct kmem_cache *lsm_inode_cache; > > char *lsm_names; > -static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init; > +static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = { > + .lbs_task = sizeof(int), > +}; > > /* Boot-time LSM user choice */ > static __initdata const char *chosen_lsm_order; > @@ -578,6 +580,8 @@ int lsm_inode_alloc(struct inode *inode) > */ > static int lsm_task_alloc(struct task_struct *task) > { > + int *display; > + > if (blob_sizes.lbs_task == 0) { > task->security = NULL; > return 0; > @@ -586,6 +590,10 @@ static int lsm_task_alloc(struct task_struct *task) > task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL); > if (task->security == NULL) > return -ENOMEM; > + > + display = task->security; > + *display = LSMDATA_INVALID; > + > return 0; > } > > @@ -1574,14 +1582,27 @@ int security_file_open(struct file *file) > > int security_task_alloc(struct task_struct *task, unsigned long clone_flags) > { > + int *odisplay = current->security; > + int *ndisplay; > int rc = lsm_task_alloc(task); > > - if (rc) > + if (unlikely(rc)) > return rc; > + > rc = call_int_hook(task_alloc, 0, task, clone_flags); > - if (unlikely(rc)) > + if (unlikely(rc)) { > security_task_free(task); > - return rc; > + return rc; > + } > + > + ndisplay = task->security; > + if (ndisplay == NULL) > + return 0; > + > + if (odisplay != NULL) > + *ndisplay = *odisplay; > + > + return 0; > } > > void security_task_free(struct task_struct *task) > @@ -1967,10 +1988,28 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name, > char **value) > { > struct security_hook_list *hp; > + int *display = current->security; > + > + if (!strcmp(name, "display")) { > + hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, > + list) { > + if (*display == LSMDATA_INVALID || > + hp->slot == *display) { > + *value = kstrdup(hp->lsm, GFP_KERNEL); > + if (*value) > + return strlen(hp->lsm); > + return -ENOMEM; > + } > + } > + return -EINVAL; > + } > > hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) { > if (lsm != NULL && strcmp(lsm, hp->lsm)) > continue; > + if (lsm == NULL && *display != LSMDATA_INVALID && > + *display != hp->slot) > + continue; > return hp->hook.getprocattr(p, name, value); > } > return -EINVAL; > @@ -1980,10 +2019,27 @@ int security_setprocattr(const char *lsm, const char *name, void *value, > size_t size) > { > struct security_hook_list *hp; > + int *display = current->security; > + int len; > + > + if (!strcmp(name, "display")) { > + hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, > + list) { > + len = strlen(hp->lsm); > + if (size >= len && !strncmp(value, hp->lsm, len)) { > + *display = hp->slot; > + return size; > + } > + } > + return -EINVAL; > + } > > hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) { > if (lsm != NULL && strcmp(lsm, hp->lsm)) > continue; > + if (lsm == NULL && *display != LSMDATA_INVALID && > + *display != hp->slot) > + continue; > return hp->hook.setprocattr(name, value, size); > } > return -EINVAL; > @@ -2002,38 +2058,41 @@ EXPORT_SYMBOL(security_ismaclabel); > > int security_secid_to_secctx(struct lsmblob *l, char **secdata, u32 *seclen) > { > + int *display = current->security; > struct security_hook_list *hp; > - int rc; > > - hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) { > - rc = hp->hook.secid_to_secctx(l->secid[hp->slot], > - secdata, seclen); > - if (rc != 0) > - return rc; > - } > + hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) > + if (*display == LSMDATA_INVALID || *display == hp->slot) > + return hp->hook.secid_to_secctx(l->secid[hp->slot], > + secdata, seclen); > return 0; > } > EXPORT_SYMBOL(security_secid_to_secctx); > > int security_secctx_to_secid(const char *secdata, u32 seclen, struct lsmblob *l) > { > + int *display = current->security; > struct security_hook_list *hp; > - int rc; > > lsmblob_init(l, 0); > - hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) { > - rc = hp->hook.secctx_to_secid(secdata, seclen, > - &l->secid[hp->slot]); > - if (rc != 0) > - return rc; > - } > + hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) > + if (*display == LSMDATA_INVALID || *display == hp->slot) > + return hp->hook.secctx_to_secid(secdata, seclen, > + &l->secid[hp->slot]); > return 0; > } > EXPORT_SYMBOL(security_secctx_to_secid); > > void security_release_secctx(char *secdata, u32 seclen) > { > - call_void_hook(release_secctx, secdata, seclen); > + int *display = current->security; > + struct security_hook_list *hp; > + > + hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list) > + if (*display == LSMDATA_INVALID || *display == hp->slot) { > + hp->hook.release_secctx(secdata, seclen); > + return; > + } > } > EXPORT_SYMBOL(security_release_secctx); > > @@ -2158,8 +2217,15 @@ EXPORT_SYMBOL(security_sock_rcv_skb); > int security_socket_getpeersec_stream(struct socket *sock, char __user *optval, > int __user *optlen, unsigned len) > { > - return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock, > - optval, optlen, len); > + int *display = current->security; > + struct security_hook_list *hp; > + > + hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream, > + list) > + if (*display == LSMDATA_INVALID || *display == hp->slot) > + return hp->hook.socket_getpeersec_stream(sock, optval, > + optlen, len); > + return -ENOPROTOOPT; > } > > int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, > -- > 2.20.1 >
On Tue, Jun 18, 2019 at 04:05:41PM -0700, Casey Schaufler wrote: > Create a new entry "display" in /proc/.../attr for controlling > which LSM security information is displayed for a process. > The name of an active LSM that supplies hooks for human readable > data may be written to "display" to set the value. The name of > the LSM currently in use can be read from "display". > At this point there can only be one LSM capable of display > active. > > This affects /proc/.../attr/current and SO_PEERSEC. > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > --- > fs/proc/base.c | 1 + > security/security.c | 108 +++++++++++++++++++++++++++++++++++--------- > 2 files changed, 88 insertions(+), 21 deletions(-) > > diff --git a/fs/proc/base.c b/fs/proc/base.c > index ddef482f1334..7bf70e041315 100644 > --- a/fs/proc/base.c > +++ b/fs/proc/base.c > @@ -2618,6 +2618,7 @@ static const struct pid_entry attr_dir_stuff[] = { > ATTR(NULL, "fscreate", 0666), > ATTR(NULL, "keycreate", 0666), > ATTR(NULL, "sockcreate", 0666), > + ATTR(NULL, "display", 0666), > #ifdef CONFIG_SECURITY_SMACK > DIR("smack", 0555, > proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops), > diff --git a/security/security.c b/security/security.c > index 46f6cf21d33c..9cfdc664239e 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -46,7 +46,9 @@ static struct kmem_cache *lsm_file_cache; > static struct kmem_cache *lsm_inode_cache; > > char *lsm_names; > -static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init; > +static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = { > + .lbs_task = sizeof(int), > +}; This needs some comments. I know what's happening here only because I understand very well how the blob sizing works. :) Perhaps: .lbs_task = sizeof(int), /* storage for selected "display" LSM */ > > /* Boot-time LSM user choice */ > static __initdata const char *chosen_lsm_order; > @@ -578,6 +580,8 @@ int lsm_inode_alloc(struct inode *inode) > */ > static int lsm_task_alloc(struct task_struct *task) > { > + int *display; > + > if (blob_sizes.lbs_task == 0) { > task->security = NULL; > return 0; > @@ -586,6 +590,10 @@ static int lsm_task_alloc(struct task_struct *task) > task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL); > if (task->security == NULL) > return -ENOMEM; > + > + display = task->security; > + *display = LSMDATA_INVALID; Similarly I think a comment here would be nice. "Initialize currently selected "display" LSM to unselected" or something. Also: "int" is okay here for now, but if the LSM infrastructure wants to do more like this we'll want to convert to a struct of some kind at the start of the lbs_task. > + > return 0; > } > > @@ -1574,14 +1582,27 @@ int security_file_open(struct file *file) > > int security_task_alloc(struct task_struct *task, unsigned long clone_flags) > { > + int *odisplay = current->security; > + int *ndisplay; > int rc = lsm_task_alloc(task); > > - if (rc) > + if (unlikely(rc)) > return rc; > + > rc = call_int_hook(task_alloc, 0, task, clone_flags); > - if (unlikely(rc)) > + if (unlikely(rc)) { > security_task_free(task); > - return rc; > + return rc; > + } > + > + ndisplay = task->security; > + if (ndisplay == NULL) > + return 0; > + > + if (odisplay != NULL) Perhaps merge these into "if (ndisplay && odisplay)" to drop the early return 0 check? > + *ndisplay = *odisplay; > + > + return 0; > } > > void security_task_free(struct task_struct *task) > @@ -1967,10 +1988,28 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name, > char **value) > { > struct security_hook_list *hp; > + int *display = current->security; > + > + if (!strcmp(name, "display")) { > + hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, > + list) { > + if (*display == LSMDATA_INVALID || > + hp->slot == *display) { > + *value = kstrdup(hp->lsm, GFP_KERNEL); > + if (*value) > + return strlen(hp->lsm); > + return -ENOMEM; > + } > + } > + return -EINVAL; > + } > > hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) { > if (lsm != NULL && strcmp(lsm, hp->lsm)) > continue; > + if (lsm == NULL && *display != LSMDATA_INVALID && > + *display != hp->slot) > + continue; > return hp->hook.getprocattr(p, name, value); > } > return -EINVAL; > @@ -1980,10 +2019,27 @@ int security_setprocattr(const char *lsm, const char *name, void *value, > size_t size) > { > struct security_hook_list *hp; > + int *display = current->security; > + int len; > + > + if (!strcmp(name, "display")) { > + hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, > + list) { > + len = strlen(hp->lsm); > + if (size >= len && !strncmp(value, hp->lsm, len)) { > + *display = hp->slot; > + return size; > + } > + } > + return -EINVAL; > + } > > hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) { > if (lsm != NULL && strcmp(lsm, hp->lsm)) > continue; > + if (lsm == NULL && *display != LSMDATA_INVALID && > + *display != hp->slot) > + continue; > return hp->hook.setprocattr(name, value, size); > } > return -EINVAL; > @@ -2002,38 +2058,41 @@ EXPORT_SYMBOL(security_ismaclabel); > > int security_secid_to_secctx(struct lsmblob *l, char **secdata, u32 *seclen) > { > + int *display = current->security; > struct security_hook_list *hp; > - int rc; > > - hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) { > - rc = hp->hook.secid_to_secctx(l->secid[hp->slot], > - secdata, seclen); > - if (rc != 0) > - return rc; > - } > + hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) > + if (*display == LSMDATA_INVALID || *display == hp->slot) > + return hp->hook.secid_to_secctx(l->secid[hp->slot], > + secdata, seclen); > return 0; > } > EXPORT_SYMBOL(security_secid_to_secctx); > > int security_secctx_to_secid(const char *secdata, u32 seclen, struct lsmblob *l) > { > + int *display = current->security; > struct security_hook_list *hp; > - int rc; > > lsmblob_init(l, 0); > - hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) { > - rc = hp->hook.secctx_to_secid(secdata, seclen, > - &l->secid[hp->slot]); > - if (rc != 0) > - return rc; > - } > + hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) > + if (*display == LSMDATA_INVALID || *display == hp->slot) > + return hp->hook.secctx_to_secid(secdata, seclen, > + &l->secid[hp->slot]); > return 0; > } > EXPORT_SYMBOL(security_secctx_to_secid); > > void security_release_secctx(char *secdata, u32 seclen) > { > - call_void_hook(release_secctx, secdata, seclen); > + int *display = current->security; > + struct security_hook_list *hp; > + > + hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list) > + if (*display == LSMDATA_INVALID || *display == hp->slot) { > + hp->hook.release_secctx(secdata, seclen); > + return; > + } > } > EXPORT_SYMBOL(security_release_secctx); > > @@ -2158,8 +2217,15 @@ EXPORT_SYMBOL(security_sock_rcv_skb); > int security_socket_getpeersec_stream(struct socket *sock, char __user *optval, > int __user *optlen, unsigned len) > { > - return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock, > - optval, optlen, len); > + int *display = current->security; > + struct security_hook_list *hp; > + > + hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream, > + list) > + if (*display == LSMDATA_INVALID || *display == hp->slot) > + return hp->hook.socket_getpeersec_stream(sock, optval, > + optlen, len); > + return -ENOPROTOOPT; > } > > int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, > -- > 2.20.1 >
On 6/18/2019 9:33 PM, Kees Cook wrote: > On Tue, Jun 18, 2019 at 04:05:41PM -0700, Casey Schaufler wrote: >> Create a new entry "display" in /proc/.../attr for controlling >> which LSM security information is displayed for a process. >> The name of an active LSM that supplies hooks for human readable >> data may be written to "display" to set the value. The name of >> the LSM currently in use can be read from "display". >> At this point there can only be one LSM capable of display >> active. >> >> This affects /proc/.../attr/current and SO_PEERSEC. > What happened to creating /proc/$pid/lsm/$lsm_name/current for "modern" > LSM libraries to start using (instead of possibly fighting over the > /proc/$pid/attr/display)? Smack already has it and the mechanics are available for anyone who wants to use it. John says that AppArmor is moving away from using the attr interfaces. Stephen and Paul have been silent on the topic, but my assumption is that the SELinux attitude is "I had them first, it's not my problem". When we get around to creating liblsm, with the "real" LSM user space APIs it will probably drive the addition of more /proc/$pid/lsm/$lsm_name directories. > (Obviously "display" is needed for "old" > libraries, and I'm fine with it.) Yes, and we can expect some distos to cling to the old libraries for a looooong time. > Similarly, is there something that can be done for SO_PEERSEC that > doesn't require using "display" for "modern" libraries? Yes, and I made sure the implementation could accommodate that. It would be easy to add a "display" that would use the much discussed $lsm1="a",$lsm2="b" format. I didn't include it because without a liblsm to use it it's just clutter. > > -Kees > >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >> --- >> fs/proc/base.c | 1 + >> security/security.c | 108 +++++++++++++++++++++++++++++++++++--------- >> 2 files changed, 88 insertions(+), 21 deletions(-) >> >> diff --git a/fs/proc/base.c b/fs/proc/base.c >> index ddef482f1334..7bf70e041315 100644 >> --- a/fs/proc/base.c >> +++ b/fs/proc/base.c >> @@ -2618,6 +2618,7 @@ static const struct pid_entry attr_dir_stuff[] = { >> ATTR(NULL, "fscreate", 0666), >> ATTR(NULL, "keycreate", 0666), >> ATTR(NULL, "sockcreate", 0666), >> + ATTR(NULL, "display", 0666), >> #ifdef CONFIG_SECURITY_SMACK >> DIR("smack", 0555, >> proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops), >> diff --git a/security/security.c b/security/security.c >> index 46f6cf21d33c..9cfdc664239e 100644 >> --- a/security/security.c >> +++ b/security/security.c >> @@ -46,7 +46,9 @@ static struct kmem_cache *lsm_file_cache; >> static struct kmem_cache *lsm_inode_cache; >> >> char *lsm_names; >> -static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init; >> +static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = { >> + .lbs_task = sizeof(int), >> +}; >> >> /* Boot-time LSM user choice */ >> static __initdata const char *chosen_lsm_order; >> @@ -578,6 +580,8 @@ int lsm_inode_alloc(struct inode *inode) >> */ >> static int lsm_task_alloc(struct task_struct *task) >> { >> + int *display; >> + >> if (blob_sizes.lbs_task == 0) { >> task->security = NULL; >> return 0; >> @@ -586,6 +590,10 @@ static int lsm_task_alloc(struct task_struct *task) >> task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL); >> if (task->security == NULL) >> return -ENOMEM; >> + >> + display = task->security; >> + *display = LSMDATA_INVALID; >> + >> return 0; >> } >> >> @@ -1574,14 +1582,27 @@ int security_file_open(struct file *file) >> >> int security_task_alloc(struct task_struct *task, unsigned long clone_flags) >> { >> + int *odisplay = current->security; >> + int *ndisplay; >> int rc = lsm_task_alloc(task); >> >> - if (rc) >> + if (unlikely(rc)) >> return rc; >> + >> rc = call_int_hook(task_alloc, 0, task, clone_flags); >> - if (unlikely(rc)) >> + if (unlikely(rc)) { >> security_task_free(task); >> - return rc; >> + return rc; >> + } >> + >> + ndisplay = task->security; >> + if (ndisplay == NULL) >> + return 0; >> + >> + if (odisplay != NULL) >> + *ndisplay = *odisplay; >> + >> + return 0; >> } >> >> void security_task_free(struct task_struct *task) >> @@ -1967,10 +1988,28 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name, >> char **value) >> { >> struct security_hook_list *hp; >> + int *display = current->security; >> + >> + if (!strcmp(name, "display")) { >> + hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, >> + list) { >> + if (*display == LSMDATA_INVALID || >> + hp->slot == *display) { >> + *value = kstrdup(hp->lsm, GFP_KERNEL); >> + if (*value) >> + return strlen(hp->lsm); >> + return -ENOMEM; >> + } >> + } >> + return -EINVAL; >> + } >> >> hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) { >> if (lsm != NULL && strcmp(lsm, hp->lsm)) >> continue; >> + if (lsm == NULL && *display != LSMDATA_INVALID && >> + *display != hp->slot) >> + continue; >> return hp->hook.getprocattr(p, name, value); >> } >> return -EINVAL; >> @@ -1980,10 +2019,27 @@ int security_setprocattr(const char *lsm, const char *name, void *value, >> size_t size) >> { >> struct security_hook_list *hp; >> + int *display = current->security; >> + int len; >> + >> + if (!strcmp(name, "display")) { >> + hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, >> + list) { >> + len = strlen(hp->lsm); >> + if (size >= len && !strncmp(value, hp->lsm, len)) { >> + *display = hp->slot; >> + return size; >> + } >> + } >> + return -EINVAL; >> + } >> >> hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) { >> if (lsm != NULL && strcmp(lsm, hp->lsm)) >> continue; >> + if (lsm == NULL && *display != LSMDATA_INVALID && >> + *display != hp->slot) >> + continue; >> return hp->hook.setprocattr(name, value, size); >> } >> return -EINVAL; >> @@ -2002,38 +2058,41 @@ EXPORT_SYMBOL(security_ismaclabel); >> >> int security_secid_to_secctx(struct lsmblob *l, char **secdata, u32 *seclen) >> { >> + int *display = current->security; >> struct security_hook_list *hp; >> - int rc; >> >> - hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) { >> - rc = hp->hook.secid_to_secctx(l->secid[hp->slot], >> - secdata, seclen); >> - if (rc != 0) >> - return rc; >> - } >> + hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) >> + if (*display == LSMDATA_INVALID || *display == hp->slot) >> + return hp->hook.secid_to_secctx(l->secid[hp->slot], >> + secdata, seclen); >> return 0; >> } >> EXPORT_SYMBOL(security_secid_to_secctx); >> >> int security_secctx_to_secid(const char *secdata, u32 seclen, struct lsmblob *l) >> { >> + int *display = current->security; >> struct security_hook_list *hp; >> - int rc; >> >> lsmblob_init(l, 0); >> - hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) { >> - rc = hp->hook.secctx_to_secid(secdata, seclen, >> - &l->secid[hp->slot]); >> - if (rc != 0) >> - return rc; >> - } >> + hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) >> + if (*display == LSMDATA_INVALID || *display == hp->slot) >> + return hp->hook.secctx_to_secid(secdata, seclen, >> + &l->secid[hp->slot]); >> return 0; >> } >> EXPORT_SYMBOL(security_secctx_to_secid); >> >> void security_release_secctx(char *secdata, u32 seclen) >> { >> - call_void_hook(release_secctx, secdata, seclen); >> + int *display = current->security; >> + struct security_hook_list *hp; >> + >> + hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list) >> + if (*display == LSMDATA_INVALID || *display == hp->slot) { >> + hp->hook.release_secctx(secdata, seclen); >> + return; >> + } >> } >> EXPORT_SYMBOL(security_release_secctx); >> >> @@ -2158,8 +2217,15 @@ EXPORT_SYMBOL(security_sock_rcv_skb); >> int security_socket_getpeersec_stream(struct socket *sock, char __user *optval, >> int __user *optlen, unsigned len) >> { >> - return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock, >> - optval, optlen, len); >> + int *display = current->security; >> + struct security_hook_list *hp; >> + >> + hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream, >> + list) >> + if (*display == LSMDATA_INVALID || *display == hp->slot) >> + return hp->hook.socket_getpeersec_stream(sock, optval, >> + optlen, len); >> + return -ENOPROTOOPT; >> } >> >> int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, >> -- >> 2.20.1 >>
On 6/18/2019 10:28 PM, Kees Cook wrote: > On Tue, Jun 18, 2019 at 04:05:41PM -0700, Casey Schaufler wrote: >> Create a new entry "display" in /proc/.../attr for controlling >> which LSM security information is displayed for a process. >> The name of an active LSM that supplies hooks for human readable >> data may be written to "display" to set the value. The name of >> the LSM currently in use can be read from "display". >> At this point there can only be one LSM capable of display >> active. >> >> This affects /proc/.../attr/current and SO_PEERSEC. >> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >> --- >> fs/proc/base.c | 1 + >> security/security.c | 108 +++++++++++++++++++++++++++++++++++--------- >> 2 files changed, 88 insertions(+), 21 deletions(-) >> >> diff --git a/fs/proc/base.c b/fs/proc/base.c >> index ddef482f1334..7bf70e041315 100644 >> --- a/fs/proc/base.c >> +++ b/fs/proc/base.c >> @@ -2618,6 +2618,7 @@ static const struct pid_entry attr_dir_stuff[] = { >> ATTR(NULL, "fscreate", 0666), >> ATTR(NULL, "keycreate", 0666), >> ATTR(NULL, "sockcreate", 0666), >> + ATTR(NULL, "display", 0666), >> #ifdef CONFIG_SECURITY_SMACK >> DIR("smack", 0555, >> proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops), >> diff --git a/security/security.c b/security/security.c >> index 46f6cf21d33c..9cfdc664239e 100644 >> --- a/security/security.c >> +++ b/security/security.c >> @@ -46,7 +46,9 @@ static struct kmem_cache *lsm_file_cache; >> static struct kmem_cache *lsm_inode_cache; >> >> char *lsm_names; >> -static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init; >> +static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = { >> + .lbs_task = sizeof(int), >> +}; > This needs some comments. I know what's happening here only because I > understand very well how the blob sizing works. :) Perhaps: > > .lbs_task = sizeof(int), /* storage for selected "display" LSM */ Point. >> >> /* Boot-time LSM user choice */ >> static __initdata const char *chosen_lsm_order; >> @@ -578,6 +580,8 @@ int lsm_inode_alloc(struct inode *inode) >> */ >> static int lsm_task_alloc(struct task_struct *task) >> { >> + int *display; >> + >> if (blob_sizes.lbs_task == 0) { >> task->security = NULL; >> return 0; >> @@ -586,6 +590,10 @@ static int lsm_task_alloc(struct task_struct *task) >> task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL); >> if (task->security == NULL) >> return -ENOMEM; >> + >> + display = task->security; >> + *display = LSMDATA_INVALID; > Similarly I think a comment here would be nice. "Initialize currently > selected "display" LSM to unselected" or something. Point. > Also: "int" is okay here for now, but if the LSM infrastructure wants to > do more like this we'll want to convert to a struct of some kind at the > start of the lbs_task. We could go whole hog and include a lsm_info pointer (once the slot number moves there) instead of an int, but I think it best to leave it as is for now. I don't see a case where more information would be required, and it would not be a hard change to make later. >> + >> return 0; >> } >> >> @@ -1574,14 +1582,27 @@ int security_file_open(struct file *file) >> >> int security_task_alloc(struct task_struct *task, unsigned long clone_flags) >> { >> + int *odisplay = current->security; >> + int *ndisplay; >> int rc = lsm_task_alloc(task); >> >> - if (rc) >> + if (unlikely(rc)) >> return rc; >> + >> rc = call_int_hook(task_alloc, 0, task, clone_flags); >> - if (unlikely(rc)) >> + if (unlikely(rc)) { >> security_task_free(task); >> - return rc; >> + return rc; >> + } >> + >> + ndisplay = task->security; >> + if (ndisplay == NULL) >> + return 0; >> + >> + if (odisplay != NULL) > Perhaps merge these into "if (ndisplay && odisplay)" to drop the early > return 0 check? Sure. The logic took a few iterations before it got to what you see here. >> + *ndisplay = *odisplay; >> + >> + return 0; >> } >> >> void security_task_free(struct task_struct *task) >> @@ -1967,10 +1988,28 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name, >> char **value) >> { >> struct security_hook_list *hp; >> + int *display = current->security; >> + >> + if (!strcmp(name, "display")) { >> + hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, >> + list) { >> + if (*display == LSMDATA_INVALID || >> + hp->slot == *display) { >> + *value = kstrdup(hp->lsm, GFP_KERNEL); >> + if (*value) >> + return strlen(hp->lsm); >> + return -ENOMEM; >> + } >> + } >> + return -EINVAL; >> + } >> >> hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) { >> if (lsm != NULL && strcmp(lsm, hp->lsm)) >> continue; >> + if (lsm == NULL && *display != LSMDATA_INVALID && >> + *display != hp->slot) >> + continue; >> return hp->hook.getprocattr(p, name, value); >> } >> return -EINVAL; >> @@ -1980,10 +2019,27 @@ int security_setprocattr(const char *lsm, const char *name, void *value, >> size_t size) >> { >> struct security_hook_list *hp; >> + int *display = current->security; >> + int len; >> + >> + if (!strcmp(name, "display")) { >> + hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, >> + list) { >> + len = strlen(hp->lsm); >> + if (size >= len && !strncmp(value, hp->lsm, len)) { >> + *display = hp->slot; >> + return size; >> + } >> + } >> + return -EINVAL; >> + } >> >> hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) { >> if (lsm != NULL && strcmp(lsm, hp->lsm)) >> continue; >> + if (lsm == NULL && *display != LSMDATA_INVALID && >> + *display != hp->slot) >> + continue; >> return hp->hook.setprocattr(name, value, size); >> } >> return -EINVAL; >> @@ -2002,38 +2058,41 @@ EXPORT_SYMBOL(security_ismaclabel); >> >> int security_secid_to_secctx(struct lsmblob *l, char **secdata, u32 *seclen) >> { >> + int *display = current->security; >> struct security_hook_list *hp; >> - int rc; >> >> - hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) { >> - rc = hp->hook.secid_to_secctx(l->secid[hp->slot], >> - secdata, seclen); >> - if (rc != 0) >> - return rc; >> - } >> + hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) >> + if (*display == LSMDATA_INVALID || *display == hp->slot) >> + return hp->hook.secid_to_secctx(l->secid[hp->slot], >> + secdata, seclen); >> return 0; >> } >> EXPORT_SYMBOL(security_secid_to_secctx); >> >> int security_secctx_to_secid(const char *secdata, u32 seclen, struct lsmblob *l) >> { >> + int *display = current->security; >> struct security_hook_list *hp; >> - int rc; >> >> lsmblob_init(l, 0); >> - hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) { >> - rc = hp->hook.secctx_to_secid(secdata, seclen, >> - &l->secid[hp->slot]); >> - if (rc != 0) >> - return rc; >> - } >> + hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) >> + if (*display == LSMDATA_INVALID || *display == hp->slot) >> + return hp->hook.secctx_to_secid(secdata, seclen, >> + &l->secid[hp->slot]); >> return 0; >> } >> EXPORT_SYMBOL(security_secctx_to_secid); >> >> void security_release_secctx(char *secdata, u32 seclen) >> { >> - call_void_hook(release_secctx, secdata, seclen); >> + int *display = current->security; >> + struct security_hook_list *hp; >> + >> + hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list) >> + if (*display == LSMDATA_INVALID || *display == hp->slot) { >> + hp->hook.release_secctx(secdata, seclen); >> + return; >> + } >> } >> EXPORT_SYMBOL(security_release_secctx); >> >> @@ -2158,8 +2217,15 @@ EXPORT_SYMBOL(security_sock_rcv_skb); >> int security_socket_getpeersec_stream(struct socket *sock, char __user *optval, >> int __user *optlen, unsigned len) >> { >> - return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock, >> - optval, optlen, len); >> + int *display = current->security; >> + struct security_hook_list *hp; >> + >> + hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream, >> + list) >> + if (*display == LSMDATA_INVALID || *display == hp->slot) >> + return hp->hook.socket_getpeersec_stream(sock, optval, >> + optlen, len); >> + return -ENOPROTOOPT; >> } >> >> int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, >> -- >> 2.20.1 >>
diff --git a/fs/proc/base.c b/fs/proc/base.c index ddef482f1334..7bf70e041315 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -2618,6 +2618,7 @@ static const struct pid_entry attr_dir_stuff[] = { ATTR(NULL, "fscreate", 0666), ATTR(NULL, "keycreate", 0666), ATTR(NULL, "sockcreate", 0666), + ATTR(NULL, "display", 0666), #ifdef CONFIG_SECURITY_SMACK DIR("smack", 0555, proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops), diff --git a/security/security.c b/security/security.c index 46f6cf21d33c..9cfdc664239e 100644 --- a/security/security.c +++ b/security/security.c @@ -46,7 +46,9 @@ static struct kmem_cache *lsm_file_cache; static struct kmem_cache *lsm_inode_cache; char *lsm_names; -static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init; +static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = { + .lbs_task = sizeof(int), +}; /* Boot-time LSM user choice */ static __initdata const char *chosen_lsm_order; @@ -578,6 +580,8 @@ int lsm_inode_alloc(struct inode *inode) */ static int lsm_task_alloc(struct task_struct *task) { + int *display; + if (blob_sizes.lbs_task == 0) { task->security = NULL; return 0; @@ -586,6 +590,10 @@ static int lsm_task_alloc(struct task_struct *task) task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL); if (task->security == NULL) return -ENOMEM; + + display = task->security; + *display = LSMDATA_INVALID; + return 0; } @@ -1574,14 +1582,27 @@ int security_file_open(struct file *file) int security_task_alloc(struct task_struct *task, unsigned long clone_flags) { + int *odisplay = current->security; + int *ndisplay; int rc = lsm_task_alloc(task); - if (rc) + if (unlikely(rc)) return rc; + rc = call_int_hook(task_alloc, 0, task, clone_flags); - if (unlikely(rc)) + if (unlikely(rc)) { security_task_free(task); - return rc; + return rc; + } + + ndisplay = task->security; + if (ndisplay == NULL) + return 0; + + if (odisplay != NULL) + *ndisplay = *odisplay; + + return 0; } void security_task_free(struct task_struct *task) @@ -1967,10 +1988,28 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name, char **value) { struct security_hook_list *hp; + int *display = current->security; + + if (!strcmp(name, "display")) { + hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, + list) { + if (*display == LSMDATA_INVALID || + hp->slot == *display) { + *value = kstrdup(hp->lsm, GFP_KERNEL); + if (*value) + return strlen(hp->lsm); + return -ENOMEM; + } + } + return -EINVAL; + } hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) { if (lsm != NULL && strcmp(lsm, hp->lsm)) continue; + if (lsm == NULL && *display != LSMDATA_INVALID && + *display != hp->slot) + continue; return hp->hook.getprocattr(p, name, value); } return -EINVAL; @@ -1980,10 +2019,27 @@ int security_setprocattr(const char *lsm, const char *name, void *value, size_t size) { struct security_hook_list *hp; + int *display = current->security; + int len; + + if (!strcmp(name, "display")) { + hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, + list) { + len = strlen(hp->lsm); + if (size >= len && !strncmp(value, hp->lsm, len)) { + *display = hp->slot; + return size; + } + } + return -EINVAL; + } hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) { if (lsm != NULL && strcmp(lsm, hp->lsm)) continue; + if (lsm == NULL && *display != LSMDATA_INVALID && + *display != hp->slot) + continue; return hp->hook.setprocattr(name, value, size); } return -EINVAL; @@ -2002,38 +2058,41 @@ EXPORT_SYMBOL(security_ismaclabel); int security_secid_to_secctx(struct lsmblob *l, char **secdata, u32 *seclen) { + int *display = current->security; struct security_hook_list *hp; - int rc; - hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) { - rc = hp->hook.secid_to_secctx(l->secid[hp->slot], - secdata, seclen); - if (rc != 0) - return rc; - } + hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) + if (*display == LSMDATA_INVALID || *display == hp->slot) + return hp->hook.secid_to_secctx(l->secid[hp->slot], + secdata, seclen); return 0; } EXPORT_SYMBOL(security_secid_to_secctx); int security_secctx_to_secid(const char *secdata, u32 seclen, struct lsmblob *l) { + int *display = current->security; struct security_hook_list *hp; - int rc; lsmblob_init(l, 0); - hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) { - rc = hp->hook.secctx_to_secid(secdata, seclen, - &l->secid[hp->slot]); - if (rc != 0) - return rc; - } + hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) + if (*display == LSMDATA_INVALID || *display == hp->slot) + return hp->hook.secctx_to_secid(secdata, seclen, + &l->secid[hp->slot]); return 0; } EXPORT_SYMBOL(security_secctx_to_secid); void security_release_secctx(char *secdata, u32 seclen) { - call_void_hook(release_secctx, secdata, seclen); + int *display = current->security; + struct security_hook_list *hp; + + hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list) + if (*display == LSMDATA_INVALID || *display == hp->slot) { + hp->hook.release_secctx(secdata, seclen); + return; + } } EXPORT_SYMBOL(security_release_secctx); @@ -2158,8 +2217,15 @@ EXPORT_SYMBOL(security_sock_rcv_skb); int security_socket_getpeersec_stream(struct socket *sock, char __user *optval, int __user *optlen, unsigned len) { - return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock, - optval, optlen, len); + int *display = current->security; + struct security_hook_list *hp; + + hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream, + list) + if (*display == LSMDATA_INVALID || *display == hp->slot) + return hp->hook.socket_getpeersec_stream(sock, optval, + optlen, len); + return -ENOPROTOOPT; } int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
Create a new entry "display" in /proc/.../attr for controlling which LSM security information is displayed for a process. The name of an active LSM that supplies hooks for human readable data may be written to "display" to set the value. The name of the LSM currently in use can be read from "display". At this point there can only be one LSM capable of display active. This affects /proc/.../attr/current and SO_PEERSEC. Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> --- fs/proc/base.c | 1 + security/security.c | 108 +++++++++++++++++++++++++++++++++++--------- 2 files changed, 88 insertions(+), 21 deletions(-)