Re: [PATCH v19 22/23] LSM: Add /proc attr entry for full LSM context
From: Casey Schaufler <casey@schaufler-ca.com>
Date: 2020-07-30 20:44:36
Also in:
linux-security-module, selinux
On 7/30/2020 3:03 AM, John Johansen wrote:
On 7/24/20 1:32 PM, Casey Schaufler wrote:quoted
Add an entry /proc/.../attr/context which displays the full process security "context" in compound format: lsm1\0value\0lsm2\0value\0... This entry is not writable. A security module may decide that its policy does not allow this information to be displayed. In this case none of the information will be displayed. Reviewed-by: Kees Cook <redacted> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> Cc: linux-api@vger.kernel.org --- Documentation/security/lsm.rst | 28 +++++++++++ fs/proc/base.c | 1 + include/linux/lsm_hooks.h | 6 +++ security/apparmor/include/procattr.h | 2 +- security/apparmor/lsm.c | 8 +++- security/apparmor/procattr.c | 22 +++++---- security/security.c | 70 ++++++++++++++++++++++++++++ security/selinux/hooks.c | 2 +- security/smack/smack_lsm.c | 2 +- 9 files changed, 126 insertions(+), 15 deletions(-)
<snip>
quoted
/**diff --git a/security/security.c b/security/security.c index d35e578fa45b..bce6be720401 100644 --- a/security/security.c +++ b/security/security.c@@ -754,6 +754,48 @@ static void __init lsm_early_task(struct task_struct *task) panic("%s: Early task alloc failed.\n", __func__); } +/** + * append_ctx - append a lsm/context pair to a compound context + * @ctx: the existing compound context + * @ctxlen: size of the old context, including terminating nul byte + * @lsm: new lsm name, nul terminated + * @new: new context, possibly nul terminated + * @newlen: maximum size of @new + * + * replace @ctx with a new compound context, appending @newlsm and @new + * to @ctx. On exit the new data replaces the old, which is freed. + * @ctxlen is set to the new size, which includes a trailing nul byte. + * + * Returns 0 on success, -ENOMEM if no memory is available. + */ +static int append_ctx(char **ctx, int *ctxlen, const char *lsm, char *new, + int newlen) +{ + char *final; + size_t llen; + + llen = strlen(lsm) + 1; + /* + * A security module may or may not provide a trailing nul on + * when returning a security context. There is no definition + * of which it should be, and there are modules that do it + * each way. + */ + newlen = strnlen(new, newlen) + 1; + + final = kzalloc(*ctxlen + llen + newlen, GFP_KERNEL); + if (final == NULL) + return -ENOMEM; + if (*ctxlen) + memcpy(final, *ctx, *ctxlen); + memcpy(final + *ctxlen, lsm, llen); + memcpy(final + *ctxlen + llen, new, newlen);if @new doesn't have a newline appended at its end this will read 1 byte passed the end of the @new buffer. Nor will the result have a trailing \0 as expected unless we get lucky.
@new will never have a newline at the end. The trailing nul comes from the allocation being done with kzalloc(). This function has to be considered in the context of its caller.
quoted
+ kfree(*ctx); + *ctx = final; + *ctxlen = *ctxlen + llen + newlen; + return 0; +} + /* * The default value of the LSM hook is defined in linux/lsm_hook_defs.h and * can be accessed with:@@ -2124,6 +2166,10 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name, char **value) { struct security_hook_list *hp; + char *final = NULL; + char *cp; + int rc = 0; + int finallen = 0;these are only used by context so they could be moved under its if, this is really just a style comment and I'll leave it up to you
Old coding habits die hard. Unless there's value to gain, I'll leave it as is.
quoted
int display = lsm_task_display(current); int slot = 0;@@ -2151,6 +2197,30 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name, return -ENOMEM; } + if (!strcmp(name, "context")) { + hlist_for_each_entry(hp, &security_hook_heads.getprocattr, + list) { + rc = hp->hook.getprocattr(p, "context", &cp); + if (rc == -EINVAL) + continue; + if (rc < 0) { + kfree(final); + return rc; + } + rc = append_ctx(&final, &finallen, hp->lsmid->lsm, + cp, rc); + kfree(cp); + if (rc < 0) { + kfree(final); + return rc; + } + } + if (final == NULL) + return -EINVAL; + *value = final; + return finallen; + } + hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) { if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm)) continue;diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index c13c207c5da1..43d5c09b9a9e 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c@@ -6288,7 +6288,7 @@ static int selinux_getprocattr(struct task_struct *p, goto bad; } - if (!strcmp(name, "current")) + if (!strcmp(name, "current") || !strcmp(name, "context")) sid = __tsec->sid; else if (!strcmp(name, "prev")) sid = __tsec->osid;diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 6f0cdb40addc..d7bb6442f192 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c@@ -3463,7 +3463,7 @@ static int smack_getprocattr(struct task_struct *p, char *name, char **value) char *cp; int slen; - if (strcmp(name, "current") != 0) + if (strcmp(name, "current") != 0 && strcmp(name, "context") != 0) return -EINVAL; cp = kstrdup(skp->smk_known, GFP_KERNEL);