On Wed, Jan 6, 2021 at 7:03 PM Paul Moore [off-list ref] wrote:

On Wed, Nov 11, 2020 at 8:54 PM Lokesh Gidra [off-list ref] wrote:

quoted

From: Daniel Colascione <redacted>

This change uses the anon_inodes and LSM infrastructure introduced in
the previous patches to give SELinux the ability to control
anonymous-inode files that are created using the new
anon_inode_getfd_secure() function.

A SELinux policy author detects and controls these anonymous inodes by
adding a name-based type_transition rule that assigns a new security
type to anonymous-inode files created in some domain. The name used
for the name-based transition is the name associated with the
anonymous inode for file listings --- e.g., "[userfaultfd]" or
"[perf_event]".

Example:

type uffd_t;
type_transition sysadm_t sysadm_t : anon_inode uffd_t "[userfaultfd]";
allow sysadm_t uffd_t:anon_inode { create };

(The next patch in this series is necessary for making userfaultfd
support this new interface.  The example above is just
for exposition.)

Signed-off-by: Daniel Colascione <redacted>
Signed-off-by: Lokesh Gidra <redacted>
---
 security/selinux/hooks.c            | 56 +++++++++++++++++++++++++++++
 security/selinux/include/classmap.h |  2 ++
 2 files changed, 58 insertions(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 6b1826fc3658..d092aa512868 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c

@@ -2927,6 +2927,61 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
        return 0;
 }

+static int selinux_inode_init_security_anon(struct inode *inode,
+                                           const struct qstr *name,
+                                           const struct inode *context_inode)
+{
+       const struct task_security_struct *tsec = selinux_cred(current_cred());
+       struct common_audit_data ad;
+       struct inode_security_struct *isec;
+       int rc;
+
+       if (unlikely(!selinux_initialized(&selinux_state)))
+               return 0;
+
+       isec = selinux_inode(inode);
+
+       /*
+        * We only get here once per ephemeral inode.  The inode has
+        * been initialized via inode_alloc_security but is otherwise
+        * untouched.
+        */
+
+       if (context_inode) {
+               struct inode_security_struct *context_isec =
+                       selinux_inode(context_inode);
+               if (context_isec->initialized != LABEL_INITIALIZED)
+                       return -EACCES;
+
+               isec->sclass = context_isec->sclass;

Taking the object class directly from the context_inode is
interesting, and I suspect problematic.  In the case below where no
context_inode is supplied the object class is set to
SECCLASS_ANON_INODE, which is correct, but when a context_inode is
supplied there is no guarantee that the object class will be set to
SECCLASS_ANON_INODE.  This could both pose a problem for policy
writers (how do you distinguish the anon inode from other normal file
inodes in this case?) as well as an outright fault later in this
function when we try to check the ANON_INODE__CREATE on an object
other than a SECCLASS_ANON_INODE object.

Thanks for catching this. I'll initialize 'sclass' unconditionally to
SECCLASS_ANON_INODE in the next version. Also, do you think I should
add a check that context_inode's sclass must be SECCLASS_ANON_INODE to
confirm that we never receive a regular inode as context_inode?

It works in the userfaultfd case because the context_inode is
originally created with this function so the object class is correctly
set to SECCLASS_ANON_INODE, but can we always guarantee that to be the
case?  Do we ever need or want to support using a context_inode that
is not SECCLASS_ANON_INODE?

I don't think there is any requirement of supporting context_inode
which isn't anon-inode. And even if there is, as you described
earlier, for ANON_INODE__CREATE to work the sclass has to be
SECCLASS_ANON_INODE. I'll appreciate comments on this from others,
particularly Daniel and Stephen who originally discussed and
implemented this patch.

quoted

+               isec->sid = context_isec->sid;
+       } else {
+               isec->sclass = SECCLASS_ANON_INODE;
+               rc = security_transition_sid(
+                       &selinux_state, tsec->sid, tsec->sid,
+                       isec->sclass, name, &isec->sid);
+               if (rc)
+                       return rc;
+       }
+
+       isec->initialized = LABEL_INITIALIZED;
+
+       /*
+        * Now that we've initialized security, check whether we're
+        * allowed to actually create this type of anonymous inode.
+        */
+
+       ad.type = LSM_AUDIT_DATA_INODE;
+       ad.u.inode = inode;
+
+       return avc_has_perm(&selinux_state,
+                           tsec->sid,
+                           isec->sid,
+                           isec->sclass,
+                           ANON_INODE__CREATE,
+                           &ad);
+}

--
paul moore
www.paul-moore.com