On 3/29/2023 6:13 PM, Paul Moore wrote:

On Wed, Mar 15, 2023 at 6:52 PM Casey Schaufler [off-list ref] wrote:

quoted

Add hooks for setselfattr and getselfattr. These hooks are not very
different from their setprocattr and getprocattr equivalents, and
much of the code is shared.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Cc: selinux@vger.kernel.org
Cc: Paul Moore <paul@paul-moore.com>
---
 security/selinux/hooks.c | 147 +++++++++++++++++++++++++++++++--------
 1 file changed, 117 insertions(+), 30 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 9403aee75981..8896edf80aa9 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c

@@ -6348,8 +6348,7 @@ static void selinux_d_instantiate(struct dentry *dentry, struct inode *inode)
                inode_doinit_with_dentry(inode, dentry);
 }

-static int selinux_getprocattr(struct task_struct *p,
-                              const char *name, char **value)
+static int do_getattr(unsigned int attr, struct task_struct *p, char **value)

Are you ready for more naming nitpicks? ;)

I would expect nothing less. :)

Let's call this 'selinux_lsm_getattr()', and the matching setter
should be 'selinux_lsm_setattr()'.

As you wish. It's your LSM.

quoted

 {
        const struct task_security_struct *__tsec;
        u32 sid;

@@ -6367,20 +6366,27 @@ static int selinux_getprocattr(struct task_struct *p,
                        goto bad;
        }

-       if (!strcmp(name, "current"))
+       switch (attr) {
+       case LSM_ATTR_CURRENT:
                sid = __tsec->sid;
-       else if (!strcmp(name, "prev"))
+               break;
+       case LSM_ATTR_PREV:
                sid = __tsec->osid;
-       else if (!strcmp(name, "exec"))
+               break;
+       case LSM_ATTR_EXEC:
                sid = __tsec->exec_sid;
-       else if (!strcmp(name, "fscreate"))
+               break;
+       case LSM_ATTR_FSCREATE:
                sid = __tsec->create_sid;
-       else if (!strcmp(name, "keycreate"))
+               break;
+       case LSM_ATTR_KEYCREATE:
                sid = __tsec->keycreate_sid;
-       else if (!strcmp(name, "sockcreate"))
+               break;
+       case LSM_ATTR_SOCKCREATE:
                sid = __tsec->sockcreate_sid;
-       else {
-               error = -EINVAL;
+               break;
+       default:
+               error = -EOPNOTSUPP;

The error should probably be -EINVAL.

It's possible that we may add an attribute that SELinux doesn't
support, say LSM_ATTR_CRYPTO_KEY, that another LSM does. This is
the same behavior the other LSMs exhibit in the face of a request
for attributes such as LSM_ATTR_KEYCREATE that they don't support.

quoted

                goto bad;
        }
        rcu_read_unlock();

@@ -6398,7 +6404,7 @@ static int selinux_getprocattr(struct task_struct *p,
        return error;
 }

-static int selinux_setprocattr(const char *name, void *value, size_t size)
+static int do_setattr(u64 attr, void *value, size_t size)
 {
        struct task_security_struct *tsec;
        struct cred *new;

@@ -6409,28 +6415,36 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
        /*
         * Basic control over ability to set these attributes at all.
         */
-       if (!strcmp(name, "exec"))
+       switch (attr) {
+       case LSM_ATTR_CURRENT:
+               error = avc_has_perm(&selinux_state,
+                                    mysid, mysid, SECCLASS_PROCESS,
+                                    PROCESS__SETCURRENT, NULL);
+               break;
+       case LSM_ATTR_EXEC:
                error = avc_has_perm(&selinux_state,
                                     mysid, mysid, SECCLASS_PROCESS,
                                     PROCESS__SETEXEC, NULL);
-       else if (!strcmp(name, "fscreate"))
+               break;
+       case LSM_ATTR_FSCREATE:
                error = avc_has_perm(&selinux_state,
                                     mysid, mysid, SECCLASS_PROCESS,
                                     PROCESS__SETFSCREATE, NULL);
-       else if (!strcmp(name, "keycreate"))
+               break;
+       case LSM_ATTR_KEYCREATE:
                error = avc_has_perm(&selinux_state,
                                     mysid, mysid, SECCLASS_PROCESS,
                                     PROCESS__SETKEYCREATE, NULL);
-       else if (!strcmp(name, "sockcreate"))
+               break;
+       case LSM_ATTR_SOCKCREATE:
                error = avc_has_perm(&selinux_state,
                                     mysid, mysid, SECCLASS_PROCESS,
                                     PROCESS__SETSOCKCREATE, NULL);
-       else if (!strcmp(name, "current"))
-               error = avc_has_perm(&selinux_state,
-                                    mysid, mysid, SECCLASS_PROCESS,
-                                    PROCESS__SETCURRENT, NULL);
-       else
-               error = -EINVAL;
+               break;
+       default:
+               error = -EOPNOTSUPP;

Same as above, should be -EINVAL.

Same as above, there may be attributes SELinux doesn't support.

quoted

+               break;
+       }
        if (error)
                return error;

@@ -6442,13 +6456,14 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
                }
                error = security_context_to_sid(&selinux_state, value, size,
                                                &sid, GFP_KERNEL);
-               if (error == -EINVAL && !strcmp(name, "fscreate")) {
+               if (error == -EINVAL && attr == LSM_ATTR_FSCREATE) {
                        if (!has_cap_mac_admin(true)) {
                                struct audit_buffer *ab;
                                size_t audit_size;

-                               /* We strip a nul only if it is at the end, otherwise the
-                                * context contains a nul and we should audit that */
+                               /* We strip a nul only if it is at the end,
+                                * otherwise the context contains a nul and
+                                * we should audit that */

You *do* get gold stars for fixing line lengths in close proximity ;)

I guess I'm the Last User of the 80 character terminal.

quoted

                                if (str[size - 1] == '\0')
                                        audit_size = size - 1;
                                else

@@ -6459,7 +6474,8 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
                                if (!ab)
                                        return error;
                                audit_log_format(ab, "op=fscreate invalid_context=");
-                               audit_log_n_untrustedstring(ab, value, audit_size);
+                               audit_log_n_untrustedstring(ab, value,
+                                                           audit_size);
                                audit_log_end(ab);

                                return error;

@@ -6483,11 +6499,11 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
           checks and may_create for the file creation checks. The
           operation will then fail if the context is not permitted. */
        tsec = selinux_cred(new);
-       if (!strcmp(name, "exec")) {
+       if (attr == LSM_ATTR_EXEC) {
                tsec->exec_sid = sid;
-       } else if (!strcmp(name, "fscreate")) {
+       } else if (attr == LSM_ATTR_FSCREATE) {
                tsec->create_sid = sid;
-       } else if (!strcmp(name, "keycreate")) {
+       } else if (attr == LSM_ATTR_KEYCREATE) {
                if (sid) {
                        error = avc_has_perm(&selinux_state, mysid, sid,
                                             SECCLASS_KEY, KEY__CREATE, NULL);

@@ -6495,9 +6511,9 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
                                goto abort_change;
                }
                tsec->keycreate_sid = sid;
-       } else if (!strcmp(name, "sockcreate")) {
+       } else if (attr == LSM_ATTR_SOCKCREATE) {
                tsec->sockcreate_sid = sid;
-       } else if (!strcmp(name, "current")) {
+       } else if (attr == LSM_ATTR_CURRENT) {
                error = -EINVAL;
                if (sid == 0)
                        goto abort_change;

@@ -6542,6 +6558,75 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
        return error;
 }

+static int selinux_getselfattr(unsigned int __user attr,
+                              struct lsm_ctx __user *ctx, size_t *size,
+                              u32 __user flags)
+{
+       char *value;
+       size_t total_len;
+       int len;
+       int rc = 0;
+
+       len = do_getattr(attr, current, &value);
+       if (len < 0)
+               return len;
+
+       total_len = len + sizeof(*ctx);
+
+       if (total_len > *size)
+               rc = -E2BIG;
+       else
+               lsm_fill_user_ctx(ctx, value, len, LSM_ID_SELINUX, 0);
+
+       *size = total_len;
+       return rc;
+}
+
+static int selinux_setselfattr(unsigned int __user attr,
+                              struct lsm_ctx __user *ctx, size_t __user size,
+                              u32 __user flags)
+{
+       struct lsm_ctx *lctx;
+       void *context;
+       int rc;
+
+       context = kmalloc(size, GFP_KERNEL);
+       if (context == NULL)
+               return -ENOMEM;
+
+       lctx = (struct lsm_ctx *)context;
+       if (copy_from_user(context, ctx, size))
+               rc = -EFAULT;
+       else if (lctx->ctx_len > size)
+               rc = -EINVAL;
+       else
+               rc = do_setattr(attr, lctx + 1, lctx->ctx_len);
+
+       kfree(context);
+       if (rc > 0)
+               return 0;
+       return rc;
+}
+
+static int selinux_getprocattr(struct task_struct *p,
+                              const char *name, char **value)
+{
+       unsigned int attr = lsm_name_to_attr(name);
+
+       if (attr)
+               return do_getattr(attr, p, value);
+       return -EINVAL;
+}
+
+static int selinux_setprocattr(const char *name, void *value, size_t size)
+{
+       int attr = lsm_name_to_attr(name);
+
+       if (attr)
+               return do_setattr(attr, value, size);
+       return -EINVAL;
+}
+
 static int selinux_ismaclabel(const char *name)
 {
        return (strcmp(name, XATTR_SELINUX_SUFFIX) == 0);

@@ -7183,6 +7268,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {

        LSM_HOOK_INIT(d_instantiate, selinux_d_instantiate),

+       LSM_HOOK_INIT(getselfattr, selinux_getselfattr),
+       LSM_HOOK_INIT(setselfattr, selinux_setselfattr),
        LSM_HOOK_INIT(getprocattr, selinux_getprocattr),
        LSM_HOOK_INIT(setprocattr, selinux_setprocattr),

--

2.39.2

--
paul-moore.com