| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5777F90A.6000609@canonical.com>
Date: Sat, 2 Jul 2016 10:25:30 -0700
From: John Johansen <john.johansen@...onical.com>
To: Paul Moore <paul@...l-moore.com>,
Casey Schaufler <casey@...aufler-ca.com>
Cc: Kees Cook <keescook@...omium.org>,
LSM <linux-security-module@...r.kernel.org>,
James Morris <jmorris@...ei.org>,
Stephen Smalley <sds@...ho.nsa.gov>,
Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
LKLM <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 3/3] LSM: Add context interface for proc attrs
On 06/29/2016 10:04 AM, Paul Moore wrote:
> On Fri, Jun 24, 2016 at 7:29 PM, Casey Schaufler <casey@...aufler-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@...aufler-ca.com>
Acked-by: John Johansen <john.johansen@...onical.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@...l-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);
>>
>
>
>
Powered by blists - more mailing lists