Messages in this thread | | | Subject | Re: [PATCH v4 3/3] LSM: Add context interface for proc attrs | From | John Johansen <> | Date | Sat, 2 Jul 2016 10:25:30 -0700 |
| |
On 06/29/2016 10:04 AM, Paul Moore wrote: > On Fri, Jun 24, 2016 at 7:29 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: >> Subject: [PATCH v4 3/3] LSM: Add context interface for proc attrs >> >> The /proc/.../attr/current interface is used by all three >> Linux security modules (SELinux, Smack and AppArmor) to >> report and modify the process security attribute. This is >> all fine when there is exactly one of these modules active >> and the userspace code knows which it module it is. >> It would require a major change to the "current" interface >> to provide information about more than one set of process >> security attributes. Instead, a "context" attribute is >> added, which identifies the security module that the >> information applies to. The format is: >> >> lsmname='context-value' >> >> When multiple concurrent modules are supported the >> /proc/.../attr/context interface will include the data >> for all of the active modules. >> >> lsmname1='context-value1',lsmname2='context-value2' >> >> The module specific subdirectories under attr contain context >> entries that report the information for that specific module >> in the same format. >> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Acked-by: John Johansen <john.johansen@canonical.com>
>> >> --- >> Documentation/security/LSM.txt | 8 +++ >> fs/proc/base.c | 4 ++ >> security/apparmor/lsm.c | 35 +++++++++++-- >> security/security.c | 108 +++++++++++++++++++++++++++++++++++++++++ >> security/selinux/hooks.c | 20 +++++++- >> security/smack/smack_lsm.c | 20 ++++---- >> 6 files changed, 180 insertions(+), 15 deletions(-) > > Acked-by: Paul Moore <paul@paul-moore.com> > >> diff --git a/Documentation/security/LSM.txt b/Documentation/security/LSM.txt >> index 125c489..af3eb11 100644 >> --- a/Documentation/security/LSM.txt >> +++ b/Documentation/security/LSM.txt >> @@ -36,6 +36,14 @@ 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. >> >> +The files named "context" in the attr directories contain the >> +same information as the "current" files, but formatted to >> +identify the module it comes from. >> + >> +if selinux is the active security module: >> + /proc/self/attr/context could contain selinux='unconfined_t' >> + /proc/self/attr/selinux/context could contain selinux='unconfined_t' >> + >> 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 >> diff --git a/fs/proc/base.c b/fs/proc/base.c >> index 182bc28..df94f26 100644 >> --- a/fs/proc/base.c >> +++ b/fs/proc/base.c >> @@ -2532,6 +2532,7 @@ static const struct pid_entry selinux_attr_dir_stuff[] = { >> ATTR("selinux", "fscreate", S_IRUGO|S_IWUGO), >> ATTR("selinux", "keycreate", S_IRUGO|S_IWUGO), >> ATTR("selinux", "sockcreate", S_IRUGO|S_IWUGO), >> + ATTR("selinux", "context", S_IRUGO|S_IWUGO), >> }; >> LSM_DIR_OPS(selinux); >> #endif >> @@ -2539,6 +2540,7 @@ LSM_DIR_OPS(selinux); >> #ifdef CONFIG_SECURITY_SMACK >> static const struct pid_entry smack_attr_dir_stuff[] = { >> ATTR("smack", "current", S_IRUGO|S_IWUGO), >> + ATTR("smack", "context", S_IRUGO|S_IWUGO), >> }; >> LSM_DIR_OPS(smack); >> #endif >> @@ -2548,6 +2550,7 @@ 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), >> + ATTR("apparmor", "context", S_IRUGO|S_IWUGO), >> }; >> LSM_DIR_OPS(apparmor); >> #endif >> @@ -2559,6 +2562,7 @@ static const struct pid_entry attr_dir_stuff[] = { >> ATTR(NULL, "fscreate", S_IRUGO|S_IWUGO), >> ATTR(NULL, "keycreate", S_IRUGO|S_IWUGO), >> ATTR(NULL, "sockcreate", S_IRUGO|S_IWUGO), >> + ATTR(NULL, "context", 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), >> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c >> index fb0fb03..5cac15f 100644 >> --- a/security/apparmor/lsm.c >> +++ b/security/apparmor/lsm.c >> @@ -476,9 +476,13 @@ static int apparmor_getprocattr(struct task_struct *task, char *name, >> const struct cred *cred = get_task_cred(task); >> struct aa_task_cxt *cxt = cred_cxt(cred); >> struct aa_profile *profile = NULL; >> + char *vp; >> + char *np; >> >> if (strcmp(name, "current") == 0) >> profile = aa_get_newest_profile(cxt->profile); >> + else if (strcmp(name, "context") == 0) >> + profile = aa_get_newest_profile(cxt->profile); >> else if (strcmp(name, "prev") == 0 && cxt->previous) >> profile = aa_get_newest_profile(cxt->previous); >> else if (strcmp(name, "exec") == 0 && cxt->onexec) >> @@ -486,9 +490,29 @@ static int apparmor_getprocattr(struct task_struct *task, char *name, >> else >> error = -EINVAL; >> >> - if (profile) >> - error = aa_getprocattr(profile, value); >> + if (profile == NULL) >> + goto put_out; >> + >> + error = aa_getprocattr(profile, &vp); >> + if (error < 0) >> + goto put_out; >> + >> + if (strcmp(name, "context") == 0) { >> + *value = kasprintf(GFP_KERNEL, "apparmor='%s'", vp); >> + if (*value == NULL) { >> + error = -ENOMEM; >> + goto put_out; >> + } >> + np = strchr(*value, '\n'); >> + if (np != NULL) { >> + np[0] = '\''; >> + np[1] = '\0'; >> + } >> + error = strlen(*value); >> + } else >> + *value = vp; >> >> +put_out: >> aa_put_profile(profile); >> put_cred(cred); >> >> @@ -530,7 +554,7 @@ static int apparmor_setprocattr(struct task_struct *task, char *name, >> return -EINVAL; >> >> arg_size = size - (args - (char *) value); >> - if (strcmp(name, "current") == 0) { >> + if (strcmp(name, "current") == 0 || strcmp(name, "context") == 0) { >> if (strcmp(command, "changehat") == 0) { >> error = aa_setprocattr_changehat(args, arg_size, >> !AA_DO_TEST); >> @@ -552,7 +576,10 @@ static int apparmor_setprocattr(struct task_struct *task, char *name, >> else >> goto fail; >> } else >> - /* only support the "current" and "exec" process attributes */ >> + /* >> + * only support the "current", context and "exec" >> + * process attributes >> + */ >> return -EINVAL; >> >> if (!error) >> diff --git a/security/security.c b/security/security.c >> index 1e9cb55..f97f0d9 100644 >> --- a/security/security.c >> +++ b/security/security.c >> @@ -1186,8 +1186,47 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name, >> char **value) >> { >> struct security_hook_list *hp; >> + char *vp; >> + char *cp = NULL; >> int rc = -EINVAL; >> + int trc; >> >> + /* >> + * "context" requires work here in addition to what >> + * the modules provide. >> + */ >> + if (strcmp(name, "context") == 0) { >> + *value = NULL; >> + list_for_each_entry(hp, >> + &security_hook_heads.getprocattr, list) { >> + if (lsm != NULL && strcmp(lsm, hp->lsm)) >> + continue; >> + trc = hp->hook.getprocattr(p, "context", &vp); >> + if (trc == -ENOENT) >> + continue; >> + if (trc <= 0) { >> + kfree(*value); >> + return trc; >> + } >> + rc = trc; >> + if (*value == NULL) { >> + *value = vp; >> + } else { >> + cp = kasprintf(GFP_KERNEL, "%s,%s", *value, vp); >> + if (cp == NULL) { >> + kfree(*value); >> + kfree(vp); >> + return -ENOMEM; >> + } >> + kfree(*value); >> + kfree(vp); >> + *value = cp; >> + } >> + } >> + if (rc > 0) >> + return strlen(*value); >> + return rc; >> + } >> >> list_for_each_entry(hp, &security_hook_heads.getprocattr, list) { >> if (lsm != NULL && strcmp(lsm, hp->lsm)) >> @@ -1204,7 +1243,76 @@ int security_setprocattr(struct task_struct *p, const char *lsm, char *name, >> { >> struct security_hook_list *hp; >> int rc = -EINVAL; >> + char *local; >> + char *cp; >> + int slen; >> + int failed = 0; >> + >> + /* >> + * If lsm is NULL look at all the modules to find one >> + * that processes name. If lsm is not NULL only look at >> + * that module. >> + * >> + * "context" is handled directly here. >> + */ >> + if (strcmp(name, "context") == 0) { >> + /* >> + * First verify that the input is acceptable. >> + * lsm1='v1'lsm2='v2'lsm3='v3' >> + * >> + * A note on the use of strncmp() below. >> + * The check is for the substring at the beginning of cp. >> + * The kzalloc of size + 1 ensures a terminated string. >> + */ >> + local = kzalloc(size + 1, GFP_KERNEL); >> + memcpy(local, value, size); >> + cp = local; >> + list_for_each_entry(hp, &security_hook_heads.setprocattr, >> + list) { >> + if (lsm != NULL && strcmp(lsm, hp->lsm)) >> + continue; >> + if (cp[0] == ',') { >> + if (cp == local) >> + goto free_out; >> + cp++; >> + } >> + slen = strlen(hp->lsm); >> + if (strncmp(cp, hp->lsm, slen)) >> + goto free_out; >> + cp += slen; >> + if (cp[0] != '=' || cp[1] != '\'' || cp[2] == '\'') >> + goto free_out; >> + for (cp += 2; cp[0] != '\''; cp++) >> + if (cp[0] == '\0') >> + goto free_out; >> + cp++; >> + } >> >> + cp = local; >> + list_for_each_entry(hp, &security_hook_heads.setprocattr, >> + list) { >> + if (lsm != NULL && strcmp(lsm, hp->lsm)) >> + continue; >> + if (cp[0] == ',') >> + cp++; >> + cp += strlen(hp->lsm) + 2; >> + for (slen = 0; cp[slen] != '\''; slen++) >> + ; >> + cp[slen] = '\0'; >> + >> + rc = hp->hook.setprocattr(p, "context", cp, slen); >> + if (rc < 0) >> + failed = rc; >> + cp += slen + 1; >> + } >> + if (failed != 0) >> + rc = failed; >> + else >> + rc = size; >> +free_out: >> + kfree(local); >> + return rc; >> + } >> list_for_each_entry(hp, &security_hook_heads.setprocattr, list) { >> if (lsm != NULL && strcmp(lsm, hp->lsm)) >> continue; >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >> index ed3a757..6397721 100644 >> --- a/security/selinux/hooks.c >> +++ b/security/selinux/hooks.c >> @@ -5711,6 +5711,8 @@ static int selinux_getprocattr(struct task_struct *p, >> >> if (!strcmp(name, "current")) >> sid = __tsec->sid; >> + else if (!strcmp(name, "context")) >> + sid = __tsec->sid; >> else if (!strcmp(name, "prev")) >> sid = __tsec->osid; >> else if (!strcmp(name, "exec")) >> @@ -5728,7 +5730,19 @@ static int selinux_getprocattr(struct task_struct *p, >> if (!sid) >> return 0; >> >> - error = security_sid_to_context(sid, value, &len); >> + if (strcmp(name, "context")) { >> + error = security_sid_to_context(sid, value, &len); >> + } else { >> + char *vp; >> + >> + error = security_sid_to_context(sid, &vp, &len); >> + if (!error) { >> + *value = kasprintf(GFP_KERNEL, "selinux='%s'", vp); >> + if (*value == NULL) >> + error = -ENOMEM; >> + } >> + } >> + >> if (error) >> return error; >> return len; >> @@ -5768,6 +5782,8 @@ static int selinux_setprocattr(struct task_struct *p, >> error = current_has_perm(p, PROCESS__SETSOCKCREATE); >> else if (!strcmp(name, "current")) >> error = current_has_perm(p, PROCESS__SETCURRENT); >> + else if (!strcmp(name, "context")) >> + error = current_has_perm(p, PROCESS__SETCURRENT); >> else >> error = -EINVAL; >> if (error) >> @@ -5827,7 +5843,7 @@ static int selinux_setprocattr(struct task_struct *p, >> tsec->keycreate_sid = sid; >> } else if (!strcmp(name, "sockcreate")) { >> tsec->sockcreate_sid = sid; >> - } else if (!strcmp(name, "current")) { >> + } else if (!strcmp(name, "current") || !strcmp(name, "context")) { >> error = -EINVAL; >> if (sid == 0) >> goto abort_change; >> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c >> index 3577009..92e66f8 100644 >> --- a/security/smack/smack_lsm.c >> +++ b/security/smack/smack_lsm.c >> @@ -3574,18 +3574,20 @@ static int smack_getprocattr(struct task_struct *p, char *name, char **value) >> { >> struct smack_known *skp = smk_of_task_struct(p); >> char *cp; >> - int slen; >> >> - if (strcmp(name, "current") != 0) >> + if (strcmp(name, "current") == 0) { >> + cp = kstrdup(skp->smk_known, GFP_KERNEL); >> + if (cp == NULL) >> + return -ENOMEM; >> + } else if (strcmp(name, "context") == 0) { >> + cp = kasprintf(GFP_KERNEL, "smack='%s'", skp->smk_known); >> + if (cp == NULL) >> + return -ENOMEM; >> + } else >> return -EINVAL; >> >> - cp = kstrdup(skp->smk_known, GFP_KERNEL); >> - if (cp == NULL) >> - return -ENOMEM; >> - >> - slen = strlen(cp); >> *value = cp; >> - return slen; >> + return strlen(cp); >> } >> >> /** >> @@ -3622,7 +3624,7 @@ static int smack_setprocattr(struct task_struct *p, char *name, >> if (value == NULL || size == 0 || size >= SMK_LONGLABEL) >> return -EINVAL; >> >> - if (strcmp(name, "current") != 0) >> + if (strcmp(name, "current") != 0 && strcmp(name, "context") != 0) >> return -EINVAL; >> >> skp = smk_import_entry(value, size); >> > > >
| |