Re: [PATCH v4 3/3] LSM: Add context interface for proc attrs

From: Paul Moore
Date: Wed Jun 29 2016 - 13:04:10 EST

Next message: H. Peter Anvin: "Re: [patch] crypto: sha256-mb - cleanup a || vs | typo"
Previous message: Paul Moore: "Re: [PATCH v5 2/3] LSM: module hierarchy in /proc/.../attr"
In reply to: Casey Schaufler: "[PATCH v5 3/3] LSM: Add context interface for proc attrs"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Fri, Jun 24, 2016 at 7:29 PM, Casey Schaufler <casey@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxx>
>
> ---
> 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@xxxxxxxxxxxxxx>

> 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);
>

--
paul moore
www.paul-moore.com

Next message: H. Peter Anvin: "Re: [patch] crypto: sha256-mb - cleanup a || vs | typo"
Previous message: Paul Moore: "Re: [PATCH v5 2/3] LSM: module hierarchy in /proc/.../attr"
In reply to: Casey Schaufler: "[PATCH v5 3/3] LSM: Add context interface for proc attrs"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]