[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 08/12] Add an SELinux access control driver





On 05/02/2012 07:44 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange redhat com>

---
  po/POTFILES.in                      |    1 +
  src/Makefile.am                     |    3 +-
  src/access/viraccessdriverselinux.c |  388 +++++++++++++++++++++++++++++++++++
  src/access/viraccessdriverselinux.h |   28 +++
  src/access/viraccessmanager.c       |    2 +
  5 files changed, 421 insertions(+), 1 deletion(-)
  create mode 100644 src/access/viraccessdriverselinux.c
  create mode 100644 src/access/viraccessdriverselinux.h

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 0c18fa0..7ab6dba 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -7,6 +7,7 @@ daemon/stream.c
  gnulib/lib/gai_strerror.c
  gnulib/lib/regcomp.c
  src/access/viraccessdriverpolkit.c
+src/access/viraccessdriverselinux.c
  src/access/viraccessmanager.c
  src/conf/cpu_conf.c
  src/conf/domain_conf.c
diff --git a/src/Makefile.am b/src/Makefile.am
index f9972ac..64398f0 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -537,7 +537,8 @@ ACCESS_DRIVER_SOURCES = \
  		access/viraccessdriver.h \
  		access/viraccessdrivernop.h access/viraccessdrivernop.c \
  		access/viraccessdriverstack.h access/viraccessdriverstack.c \
-		access/viraccessdriverpolkit.h access/viraccessdriverpolkit.c
+		access/viraccessdriverpolkit.h access/viraccessdriverpolkit.c \
+		access/viraccessdriverselinux.h access/viraccessdriverselinux.c

  ACCESS_DRIVER_POLKIT_POLICY = \
  		access/org.libvirt.domain.policy
diff --git a/src/access/viraccessdriverselinux.c b/src/access/viraccessdriverselinux.c
new file mode 100644
index 0000000..2c64aff
--- /dev/null
+++ b/src/access/viraccessdriverselinux.c
@@ -0,0 +1,388 @@
+/*
+ * viraccessdriverselinux.c: selinuxed access control driver
+ *
+ * Copyright (C) 2012 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ */
+
+#include <config.h>
+
+#include "access/viraccessdriverselinux.h"
+#include "memory.h"
+#include "command.h"
+#include "logging.h"
+#include "threads.h"
+#include "virterror_internal.h"
+
+#include <selinux/selinux.h>
+#include <selinux/avc.h>
+#include <selinux/av_permissions.h>
+#include <selinux/flask.h>
+
+
+#define VIR_FROM_THIS VIR_FROM_ACCESS
+#define virAccessError(code, ...)                                       \
+    virReportErrorHelper(VIR_FROM_THIS, code, __FILE__,                 \
+                         __FUNCTION__, __LINE__, __VA_ARGS__)
+
+static void virAccessDriverSELinuxAVCLog(const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(1, 2);
+static void virAccessDriverSELinuxAVCLogAudit(void *data, security_class_t class, char *buf, size_t bufleft);
+static void *virAccessDriverSELinuxAVCCreateThread(void (*run) (void));
+static void virAccessDriverSELinuxAVCStopThread(void *thread);
+static void *virAccessDriverSELinuxAVCAllocLock(void);
+static void virAccessDriverSELinuxAVCGetLock(void *lock);
+static void virAccessDriverSELinuxAVCReleaseLock(void *lock);
+static void virAccessDriverSELinuxAVCFreeLock(void *lock);
+
+
+/* AVC callback structures for use in avc_init.  */
+static const struct avc_memory_callback virAccessDriverSELinuxAVCMemCallbacks =
+{
+    .func_malloc = malloc,
+    .func_free = free,
+};
+static const struct avc_log_callback virAccessDriverSELinuxAVCLogCallbacks =
+{
+    .func_log = virAccessDriverSELinuxAVCLog,
+    .func_audit = virAccessDriverSELinuxAVCLogAudit,
+};
+static const struct avc_thread_callback virAccessDriverSELinuxAVCThreadCallbacks =
+{
+    .func_create_thread = virAccessDriverSELinuxAVCCreateThread,
+    .func_stop_thread = virAccessDriverSELinuxAVCStopThread,
+};
+static const struct avc_lock_callback virAccessDriverSELinuxAVCLockCallbacks =
+{
+    .func_alloc_lock = virAccessDriverSELinuxAVCAllocLock,
+    .func_get_lock = virAccessDriverSELinuxAVCGetLock,
+    .func_release_lock = virAccessDriverSELinuxAVCReleaseLock,
+    .func_free_lock = virAccessDriverSELinuxAVCFreeLock,
+};
+
+
+typedef struct _virAccessDriverSELinuxPrivate virAccessDriverSELinuxPrivate;
+typedef virAccessDriverSELinuxPrivate *virAccessDriverSELinuxPrivatePtr;
+
+struct _virAccessDriverSELinuxPrivate {
+    bool enabled;
+
+    /* Cache for AVCs */
+    struct avc_entry_ref aeref;
+
+    /* SID of the daemon */
+    security_id_t localSid;
+};
+
+
+static int virAccessDriverSELinuxSetup(virAccessManagerPtr manager)
+{
+    virAccessDriverSELinuxPrivatePtr priv = virAccessManagerGetPrivateData(manager);
+    int r;
+    security_context_t localCon;
+    int ret = -1;
+
+    if ((r = is_selinux_enabled()) < 0) {
+        virReportSystemError(errno, "%s",
+                             _("Unable to determine if SELinux is enabled"));
+        return -1;
+    }
+    priv->enabled = r != 0;
+    priv->localSid = SECSID_WILD;
+
+    avc_entry_ref_init(&priv->aeref);
+
+    if (avc_init("avc",
+                 &virAccessDriverSELinuxAVCMemCallbacks,
+                 &virAccessDriverSELinuxAVCLogCallbacks,
+                 &virAccessDriverSELinuxAVCThreadCallbacks,
+                 &virAccessDriverSELinuxAVCLockCallbacks) < 0) {

It looks like avc_init is deprecated.

+        virReportSystemError(errno, "%s",
+                             _("Unable to initialize AVC system"));
+        goto cleanup;
+    }
+
+    if (getcon(&localCon) < 0) {
+        virReportSystemError(errno, "%s",
+                             _("Unable to get context of daemon"));
+        goto cleanup;
+    }
+
+    if (avc_context_to_sid(localCon, &priv->localSid) < 0) {
+        virReportSystemError(errno,
+                             _("Unable to convert context %s to SID"),
+                             (char*)localCon);
+        goto cleanup;
+    }
+    VIR_FREE(localCon);
+
+    ret = 0;
+cleanup:
+    if (ret < 0)
+        priv->localSid = SECSID_WILD;
+    freecon(localCon);
+    return ret;
+}
+
+
+static void virAccessDriverSELinuxCleanup(virAccessManagerPtr manager)
+{
+    virAccessDriverSELinuxPrivatePtr priv = virAccessManagerGetPrivateData(manager);
+
+    priv->localSid = SECSID_WILD;
+}
+
+
+static security_id_t
+virAccessDriverSELinuxGetClientSID(void)
+{
+    virIdentityPtr identity = virAccessManagerGetEffectiveIdentity();
+    const char *seccon = NULL;
+    security_id_t sid = SECSID_WILD;
+
+    if (!identity) {
+        virAccessError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("No identity available"));
+        return NULL;
+    }
+    if (virIdentityGetAttr(identity, VIR_IDENTITY_ATTR_SECURITY_CONTEXT, &seccon) < 0)
+        goto cleanup;
+
+    if (!seccon) {
+        virAccessError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("No security context available"));
+        goto cleanup;
+    }
+
+    if (avc_context_to_sid((security_context_t)seccon, &sid) < 0) {
+        virReportSystemError(errno,
+                             _("Unable to convert context %s to SID"),
+                             seccon);
+        sid = SECSID_WILD;
+        goto cleanup;
+    }
+
+cleanup:
+    virIdentityFree(identity);
+    return sid;
+}
+
+
+static security_class_t
+virAccessDriverSELinuxGetObjectClass(const char *typename)
+{
+    security_class_t ret;
+
+    if ((ret = string_to_security_class(typename)) == 0) {
+        virReportSystemError(errno,
+                             _("Unable to find security class '%s'"), typename);
+        return 0;
+    }
+
+    return ret;
+}
+
+
+static access_vector_t
+virAccessDriverSELinuxGetObjectPerm(const char *avname, const char *typename, security_class_t objectClass)
+{
+    access_vector_t ret;
+
+    if (objectClass == 0)
+        return 0;
+
+    if ((ret = string_to_av_perm(objectClass, avname)) == 0) {
+        virReportSystemError(errno,
+                             _("Unable to find access vector '%s' for '%s'"), avname, typename);
+        return 0;
+    }
+
+    return ret;
+}
+
+static bool
+virAccessDriverSELinuxCheck(virAccessManagerPtr manager,
+                            security_id_t objectSid,
+                            const char *typename,
+                            const char *avname)
+{
+    virAccessDriverSELinuxPrivatePtr priv = virAccessManagerGetPrivateData(manager);
+    security_id_t clientSid = virAccessDriverSELinuxGetClientSID();
+    security_class_t objectClass = virAccessDriverSELinuxGetObjectClass(typename);
+    access_vector_t objectVector = virAccessDriverSELinuxGetObjectPerm(avname, typename, objectClass);
+    int ret = false;
+
+    if (clientSid == SECSID_WILD ||
+        objectClass == 0 ||
+        objectVector == 0) {

So if the client has the wildcard sid it gets access.  That makes sense.

What about objectClass == 0 and objectVector == 0? I'm thinking these cover the case where the MAC admin has modified the enforcing policy and removed the corresponding class/perm definitions. It just makes me feel a little uncomfortable that we could accidentally fall through and grant access here, perhaps due to a coding error on a virAccessManagerCheckConnect or virAccessManagerCheckDomain call.

+        if (security_deny_unknown() == 0) {
+            VIR_WARN("Allow access, because policy does not deny unknown objects");
+            ret = true;
+        }
+        goto cleanup;
+    }
+
+    if (avc_has_perm(clientSid, objectSid,
+                     objectClass, objectVector,
+                     &priv->aeref, NULL) < 0) {
+        int save_errno = errno;
+        if (security_getenforce() == 0) {
+            char ebuf[1024];
+            VIR_WARN("Ignoring denial in non-enforcing mode: %s",
+                     virStrerror(save_errno, ebuf, sizeof(ebuf)));
+            ret = true;
+            goto cleanup;
+        }
+        switch (save_errno) {
+        case EACCES:
+            virAccessError(VIR_ERR_ACCESS_DENIED, "%s",
+                           _("SELinux denying due to security policy"));
+            break;
+        case EINVAL:
+            virAccessError(VIR_ERR_ACCESS_DENIED, "%s",
+                           _("SELinux denying due to invalid security context"));
+            break;
+        default:
+            virReportSystemError(errno, "%s",
+                                 _("SELinux denying"));
+            break;
+        }
+        goto cleanup;
+    }
+
+    ret = true;
+
+cleanup:
+    return ret;
+}
+
+
+static bool
+virAccessDriverSELinuxCheckConnect(virAccessManagerPtr manager,
+                                   virAccessPermConnect av)
+{
+    virAccessDriverSELinuxPrivatePtr priv = virAccessManagerGetPrivateData(manager);
+
+    /* There's no object to use for targetSid here, so we
+     * instead use the daemon's context as the targetSid */
+    return virAccessDriverSELinuxCheck(manager,
+                                       priv->localSid,
+                                       "connect",
+                                       virAccessPermConnectTypeToString(av));
+}
+
+
+static bool
+virAccessDriverSELinuxCheckDomain(virAccessManagerPtr manager,
+                                  virDomainDefPtr def ATTRIBUTE_UNUSED,
+                                  virAccessPermDomain av)
+{
+    security_id_t objectSid = 0; /* XXX get from 'def' */
+
+    return virAccessDriverSELinuxCheck(manager,
+                                       objectSid,
+                                       "domain",
+                                       virAccessPermDomainTypeToString(av));
+}
+
+
+
+static void virAccessDriverSELinuxAVCLog(const char *fmt, ...)
+{
+    va_list ap;
+    va_start(ap, fmt);
+    virLogVMessage("avc", VIR_LOG_WARN,__func__, __LINE__, 0, fmt, ap);
+    va_end(ap);
+}
+
+
+static void virAccessDriverSELinuxAVCLogAudit(void *data ATTRIBUTE_UNUSED,
+                                              security_class_t class ATTRIBUTE_UNUSED,
+                                              char *buf ATTRIBUTE_UNUSED,
+                                              size_t bufleft ATTRIBUTE_UNUSED)
+{
+}
+
+
+static void *virAccessDriverSELinuxAVCCreateThread(void (*run) (void))
+{
+    virThreadPtr thread;
+
+    if (VIR_ALLOC(thread) < 0) {
+        virReportOOMError();
+        return NULL;
+    }
+
+    if (virThreadCreate(thread, false, (virThreadFunc)run, NULL) < 0) {
+        virReportSystemError(errno, "%s",
+                             _("Unable to create thread"));
+        VIR_FREE(thread);
+    }
+
+    return thread;
+}
+
+
+static void virAccessDriverSELinuxAVCStopThread(void *thread)
+{
+    virThreadCancel(thread);
+    VIR_FREE(thread);
+}
+
+
+static void *virAccessDriverSELinuxAVCAllocLock(void)
+{
+    virMutexPtr lock;
+    if (VIR_ALLOC(lock) < 0) {
+        virReportOOMError();
+        return NULL;
+    }
+    if (virMutexInit(lock) < 0) {
+        virReportSystemError(errno, "%s",
+                             _("Unable to initialize mutex"));
+        VIR_FREE(lock);
+        return NULL;
+    }
+    return lock;
+}
+
+
+static void virAccessDriverSELinuxAVCGetLock(void *lock)
+{
+    virMutexLock(lock);
+}
+
+
+static void virAccessDriverSELinuxAVCReleaseLock(void *lock)
+{
+    virMutexUnlock(lock);
+}
+
+
+static void virAccessDriverSELinuxAVCFreeLock(void *lock)
+{
+    virMutexDestroy(lock);
+    VIR_FREE(lock);
+}
+
+
+virAccessDriver accessDriverSELinux = {
+    .privateDataLen = sizeof(virAccessDriverSELinuxPrivate),
+    .name = "selinux",
+    .setup = virAccessDriverSELinuxSetup,
+    .cleanup = virAccessDriverSELinuxCleanup,
+    .checkConnect = virAccessDriverSELinuxCheckConnect,
+    .checkDomain = virAccessDriverSELinuxCheckDomain,
+};
diff --git a/src/access/viraccessdriverselinux.h b/src/access/viraccessdriverselinux.h
new file mode 100644
index 0000000..3a53686
--- /dev/null
+++ b/src/access/viraccessdriverselinux.h
@@ -0,0 +1,28 @@
+/*
+ * viraccessdriverselinux.h: selinuxed access control driver
+ *
+ * Copyright (C) 2012 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ */
+
+#ifndef __VIR_ACCESS_DRIVER_SELINUX_H__
+# define __VIR_ACCESS_DRIVER_SELINUX_H__
+
+# include "access/viraccessdriver.h"
+
+extern virAccessDriver accessDriverSELinux;
+
+#endif /* __VIR_ACCESS_DRIVER_SELINUX_H__ */
diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c
index e7444d5..4b215c4 100644
--- a/src/access/viraccessmanager.c
+++ b/src/access/viraccessmanager.c
@@ -30,6 +30,7 @@
  #include "access/viraccessdrivernop.h"
  #include "access/viraccessdriverstack.h"
  #include "access/viraccessdriverpolkit.h"
+#include "access/viraccessdriverselinux.h"
  #include "logging.h"

  #define VIR_FROM_THIS VIR_FROM_ACCESS
@@ -218,6 +219,7 @@ static virAccessManagerPtr virAccessManagerNewDriver(virAccessDriverPtr drv)
  static virAccessDriverPtr accessDrivers[] = {
      &accessDriverNop,
      &accessDriverPolkit,
+    &accessDriverSELinux,
  };



--
Regards,
Corey



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]