selinux: rework selinux access check logic

a) Instead of parsing the bus messages inside of selinux-access.c
simply pass everything pre-parsed in the functions

b) implement the access checking with a macro that resolves to nothing
on non-selinux builds

c) split out the selinux checks into their own sources
selinux-util.[ch]

d) this unifies the job creation code behind the D-Bus calls
Manager.StartUnit() and Unit.Start().

1 files changed, 163 insertions, 354 deletions

diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c
index 732fcbfd2c..3a244ad9ff 100644
--- a/src/core/selinux-access.c
+++ b/src/core/selinux-access.c
@@ -31,6 +31,7 @@
 #include "bus-errors.h"
 #include "dbus-common.h"
 #include "audit.h"
+#include "selinux-util.h"
 
 #include <stdio.h>
 #include <string.h>
@@ -42,11 +43,8 @@
 #endif
 #include <limits.h>
 
-/* FD to send audit messages to */
+static bool initialized = false;
 static int audit_fd = -1;
-static int selinux_enabled = -1;
-static int first_time = 1;
-static int selinux_enforcing = 0;
 
 struct auditstruct {
         const char *path;
@@ -56,106 +54,13 @@ struct auditstruct {
         gid_t gid;
 };
 
-/*
-   Define a mapping between the systemd method calls and the SELinux access to check.
-   We define two tables, one for access checks on unit files, and one for
-   access checks for the system in general.
-
-   If we do not find a match in either table, then the "undefined" system
-   check will be called.
-*/
-
-static const char unit_methods[] =
-        "DisableUnitFiles\0"       "disable\0"
-        "EnableUnitFiles\0"        "enable\0"
-        "GetUnit\0"                "status\0"
-        "GetUnitFileState\0"       "status\0"
-        "Kill\0"                   "stop\0"
-        "KillUnit\0"               "stop\0"
-        "LinkUnitFiles\0"          "enable\0"
-        "MaskUnitFiles\0"          "disable\0"
-        "PresetUnitFiles\0"        "enable\0"
-        "ReenableUnitFiles\0"      "enable\0"
-        "ReloadOrRestart\0"        "start\0"
-        "ReloadOrRestartUnit\0"    "start\0"
-        "ReloadOrTryRestart\0"     "start\0"
-        "ReloadOrTryRestartUnit\0" "start\0"
-        "Reload\0"                 "reload\0"
-        "ReloadUnit\0"             "reload\0"
-        "ResetFailedUnit\0"        "stop\0"
-        "Restart\0"                "start\0"
-        "RestartUnit\0"            "start\0"
-        "Start\0"                  "start\0"
-        "StartUnit\0"              "start\0"
-        "StartUnitReplace\0"       "start\0"
-        "Stop\0"                   "stop\0"
-        "StopUnit\0"               "stop\0"
-        "TryRestart\0"             "start\0"
-        "TryRestartUnit\0"         "start\0"
-        "UnmaskUnitFiles\0"        "enable\0";
-
-static const char system_methods[] =
-        "ClearJobs\0"              "reboot\0"
-        "CreateSnapshot\0"         "status\0"
-        "Dump\0"                   "status\0"
-        "Exit\0"                   "halt\0"
-        "FlushDevices\0"           "halt\0"
-        "Get\0"                    "status\0"
-        "GetAll\0"                 "status\0"
-        "GetJob\0"                 "status\0"
-        "GetSeat\0"                "status\0"
-        "GetSession\0"             "status\0"
-        "GetSessionByPID\0"        "status\0"
-        "GetUnitByPID\0"           "status\0"
-        "GetUser\0"                "status\0"
-        "Halt\0"                   "halt\0"
-        "Introspect\0"             "status\0"
-        "KExec\0"                  "reboot\0"
-        "KillSession\0"            "halt\0"
-        "KillUser\0"               "halt\0"
-        "LoadUnit\0"               "reload\0"
-        "ListJobs\0"               "status\0"
-        "ListSeats\0"              "status\0"
-        "ListSessions\0"           "status\0"
-        "ListUnits\0"              "status\0"
-        "ListUnitFiles\0"          "status\0"
-        "ListUsers\0"              "status\0"
-        "LockSession\0"            "halt\0"
-        "PowerOff\0"               "halt\0"
-        "Reboot\0"                 "reboot\0"
-        "Reload\0"                 "reload\0"
-        "Reexecute\0"              "reload\0"
-        "ResetFailed\0"            "reload\0"
-        "Subscribe\0"              "status\0"
-        "SwithcRoot\0"             "reboot\0"
-        "SetEnvironment\0"         "status\0"
-        "SetUserLinger\0"          "halt\0"
-        "TerminateSeat\0"          "halt\0"
-        "TerminateSession\0"       "halt\0"
-        "TerminateUser\0"          "halt\0"
-        "Unsubscribe\0"            "status\0"
-        "UnsetEnvironment\0"       "status\0"
-        "UnsetAndSetEnvironment\0" "status\0";
-
-/*
-   If the admin toggles the selinux enforcment mode this callback
-   will get called before the next access check
-*/
-static int setenforce_callback(int enforcing)
-{
-        selinux_enforcing = enforcing;
-        return 0;
-}
-
-/* This mimics dbus_bus_get_unix_user() */
 static int bus_get_selinux_security_context(
                 DBusConnection *connection,
                 const char *name,
                 char **scon,
                 DBusError *error) {
 
-        DBusMessage *m = NULL, *reply = NULL;
-        int r;
+        _cleanup_dbus_message_unref_ DBusMessage *m = NULL, *reply = NULL;
 
         m = dbus_message_new_method_call(
                         DBUS_SERVICE_DBUS,
@@ -163,54 +68,34 @@ static int bus_get_selinux_security_context(
                         DBUS_INTERFACE_DBUS,
                         "GetConnectionSELinuxSecurityContext");
         if (!m) {
-                r = -errno;
                 dbus_set_error_const(error, DBUS_ERROR_NO_MEMORY, NULL);
-                goto finish;
+                return -ENOMEM;
         }
 
-        r = dbus_message_append_args(
-                m,
-                DBUS_TYPE_STRING, &name,
-                DBUS_TYPE_INVALID);
-        if (!r) {
-                r = -errno;
+        if (!dbus_message_append_args(
+                            m,
+                            DBUS_TYPE_STRING, &name,
+                            DBUS_TYPE_INVALID)) {
                 dbus_set_error_const(error, DBUS_ERROR_NO_MEMORY, NULL);
-                goto finish;
+                return -ENOMEM;
         }
 
         reply = dbus_connection_send_with_reply_and_block(connection, m, -1, error);
-        if (!reply) {
-                r = -errno;
-                goto finish;
-        }
-
-        r = dbus_set_error_from_message(error, reply);
-        if (!r) {
-                r = -errno;
-                goto finish;
-        }
-
-        r = dbus_message_get_args(
-                reply, error,
-                DBUS_TYPE_STRING, scon,
-                DBUS_TYPE_INVALID);
-        if (!r) {
-                r = -errno;
-                goto finish;
-        }
+        if (!reply)
+                return -EIO;
 
-        r = 0;
-finish:
-        if (m)
-                dbus_message_unref(m);
+        if (dbus_set_error_from_message(error, reply))
+                return -EIO;
 
-        if (reply)
-                dbus_message_unref(reply);
+        if (!dbus_message_get_args(
+                            reply, error,
+                            DBUS_TYPE_STRING, scon,
+                            DBUS_TYPE_INVALID))
+                return -EIO;
 
-        return r;
+        return 0;
 }
 
-/* This mimics dbus_bus_get_unix_user() */
 static int bus_get_audit_data(
                 DBusConnection *connection,
                 const char *name,
@@ -222,7 +107,7 @@ static int bus_get_audit_data(
 
         pid = bus_get_unix_process_id(connection, name, error);
         if (pid <= 0)
-                return -EINVAL;
+                return -EIO;
 
         r = audit_loginuid_from_pid(pid, &audit->loginuid);
         if (r < 0)
@@ -247,26 +132,27 @@ static int bus_get_audit_data(
    Any time an access gets denied this callback will be called
    with the aduit data.  We then need to just copy the audit data into the msgbuf.
 */
-static int audit_callback(void *auditdata, security_class_t cls,
-                          char *msgbuf, size_t msgbufsize)
-{
+static int audit_callback(
+                void *auditdata,
+                security_class_t cls,
+                char *msgbuf,
+                size_t msgbufsize) {
+
         struct auditstruct *audit = (struct auditstruct *) auditdata;
+
         snprintf(msgbuf, msgbufsize,
-                 "auid=%d uid=%d gid=%d",
+                 "auid=%d uid=%d gid=%d%s%s%s%s%s%s",
                  audit->loginuid,
-                 audit->uid, audit->gid);
-
-        if (audit->path) {
-		strncat(msgbuf," path=\"", msgbufsize);
-		strncat(msgbuf, audit->path, msgbufsize);
-		strncat(msgbuf,"\"", msgbufsize);
-        }
+                 audit->uid,
+                 audit->gid,
+                 (audit->path ? " path=\"" : ""),
+                 strempty(audit->path),
+                 (audit->path ? "\"" : ""),
+                 (audit->cmdline ? " cmdline=\"" : ""),
+                 strempty(audit->cmdline),
+                 (audit->cmdline ? "\"" : ""));
 
-        if (audit->cmdline) {
-		strncat(msgbuf," cmdline=\"", msgbufsize);
-		strncat(msgbuf, audit->cmdline, msgbufsize);
-		strncat(msgbuf,"\"", msgbufsize);
-        }
+        msgbuf[msgbufsize-1] = 0;
 
         return 0;
 }
@@ -277,24 +163,25 @@ static int audit_callback(void *auditdata, security_class_t cls,
    user_avc's into the /var/log/audit/audit.log, otherwise they will be
    sent to syslog.
 */
-static int log_callback(int type, const char *fmt, ...)
-{
+static int log_callback(int type, const char *fmt, ...) {
         va_list ap;
 
         va_start(ap, fmt);
+
 #ifdef HAVE_AUDIT
         if (audit_fd >= 0) {
-                char buf[LINE_MAX*2];
+                char buf[LINE_MAX];
 
                 vsnprintf(buf, sizeof(buf), fmt, ap);
-                audit_log_user_avc_message(audit_fd, AUDIT_USER_AVC,
-                                           buf, NULL, NULL, NULL, 0);
+                audit_log_user_avc_message(audit_fd, AUDIT_USER_AVC, buf, NULL, NULL, NULL, 0);
                 va_end(ap);
+
                 return 0;
         }
 #endif
         log_metav(LOG_USER | LOG_INFO, __FILE__, __LINE__, __FUNCTION__, fmt, ap);
         va_end(ap);
+
         return 0;
 }
 
@@ -304,42 +191,35 @@ static int log_callback(int type, const char *fmt, ...)
    If you want to cleanup memory you should need to call selinux_access_finish.
 */
 static int access_init(void) {
-
-        int r = -1;
+        int r;
 
         if (avc_open(NULL, 0)) {
-                log_error("avc_open failed: %m");
+                log_error("avc_open() failed: %m");
                 return -errno;
         }
 
-        selinux_set_callback(SELINUX_CB_AUDIT, (union selinux_callback) &audit_callback);
-        selinux_set_callback(SELINUX_CB_LOG, (union selinux_callback) &log_callback);
-        selinux_set_callback(SELINUX_CB_SETENFORCE, (union selinux_callback) &setenforce_callback);
+        selinux_set_callback(SELINUX_CB_AUDIT, (union selinux_callback) audit_callback);
+        selinux_set_callback(SELINUX_CB_LOG, (union selinux_callback) log_callback);
 
-        if ((r = security_getenforce()) >= 0) {
-                setenforce_callback(r);
+        if (security_getenforce() >= 0)
                 return 0;
-        }
+
         r = -errno;
         avc_destroy();
+
         return r;
 }
 
 static int selinux_init(Manager *m, DBusError *error) {
-
         int r;
 
 #ifdef HAVE_AUDIT
         audit_fd = m->audit_fd;
 #endif
-        if (!first_time)
+        if (initialized)
                 return 0;
 
-        if (selinux_enabled < 0)
-                selinux_enabled = is_selinux_enabled() == 1;
-
-        if (selinux_enabled) {
-                /* if not first time is not set, then initialize access */
+        if (use_selinux()) {
                 r = access_init();
                 if (r < 0) {
                         dbus_set_error(error, DBUS_ERROR_ACCESS_DENIED, "Failed to initialize SELinux.");
@@ -347,49 +227,46 @@ static int selinux_init(Manager *m, DBusError *error) {
                 }
         }
 
-        first_time = 0;
+        initialized = true;
         return 0;
 }
 
 static int get_audit_data(
-        DBusConnection *connection,
-        DBusMessage *message,
-        struct auditstruct *audit,
-        DBusError *error) {
+                DBusConnection *connection,
+                DBusMessage *message,
+                struct auditstruct *audit,
+                DBusError *error) {
 
         const char *sender;
-        int r;
+        int r, fd;
+        struct ucred ucred;
+        socklen_t len;
 
         sender = dbus_message_get_sender(message);
         if (sender)
                 return bus_get_audit_data(connection, sender, audit, error);
-        else {
-                int fd;
-                struct ucred ucred;
-                socklen_t len;
 
-                if (!dbus_connection_get_unix_fd(connection, &fd))
-                        return -EINVAL;
+        if (!dbus_connection_get_unix_fd(connection, &fd))
+                return -EINVAL;
 
-                r = getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &ucred, &len);
-                if (r < 0) {
-                        log_error("Failed to determine peer credentials: %m");
-                        return -errno;
-                }
+        r = getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &ucred, &len);
+        if (r < 0) {
+                log_error("Failed to determine peer credentials: %m");
+                return -errno;
+        }
 
-                audit->uid = ucred.uid;
-                audit->gid = ucred.gid;
+        audit->uid = ucred.uid;
+        audit->gid = ucred.gid;
 
-                r = audit_loginuid_from_pid(ucred.pid, &audit->loginuid);
-                if (r < 0)
-                        return r;
+        r = audit_loginuid_from_pid(ucred.pid, &audit->loginuid);
+        if (r < 0)
+                return r;
 
-                r = get_process_cmdline(ucred.pid, LINE_MAX, true, &audit->cmdline);
-                if (r < 0)
-                        return r;
+        r = get_process_cmdline(ucred.pid, LINE_MAX, true, &audit->cmdline);
+        if (r < 0)
+                return r;
 
-                return 0;
-        }
+        return 0;
 }
 
 /*
@@ -397,10 +274,10 @@ static int get_audit_data(
    connections.  Whether it is on the bus or a local connection.
 */
 static int get_calling_context(
-        DBusConnection *connection,
-        DBusMessage *message,
-        security_context_t *scon,
-        DBusError *error) {
+                DBusConnection *connection,
+                DBusMessage *message,
+                security_context_t *scon,
+                DBusError *error) {
 
         const char *sender;
         int r;
@@ -414,14 +291,13 @@ static int get_calling_context(
         sender = dbus_message_get_sender(message);
         if (sender) {
                 r = bus_get_selinux_security_context(connection, sender, scon, error);
-                if (r == 0)
-                        return 0;
+                if (r >= 0)
+                        return r;
 
                 log_debug("bus_get_selinux_security_context failed %m");
         }
 
-        r = dbus_connection_get_unix_fd(connection, &fd);
-        if (! r) {
+        if (!dbus_connection_get_unix_fd(connection, &fd)) {
                 log_error("bus_connection_get_unix_fd failed %m");
                 return -EINVAL;
         }
@@ -436,53 +312,50 @@ static int get_calling_context(
 }
 
 /*
-   This function returns the SELinux permission to check and whether or not the
-   check requires a unit file.
-*/
-static void selinux_perm_lookup(const char *method, const char **perm, bool *require_unit) {
-        const char *m, *p;
-
-        NULSTR_FOREACH_PAIR(m, p, unit_methods)
-                if (streq(method, m)) {
-                        *perm = p;
-                        *require_unit = true;
-                        return;
-                }
-
-        NULSTR_FOREACH_PAIR(m, p, system_methods)
-                if (streq(method, m)) {
-                        *perm = p;
-                        *require_unit = false;
-                        return;
-                }
-
-        *require_unit = false;
-        *perm = "undefined";
-}
-
-/*
    This function communicates with the kernel to check whether or not it should
    allow the access.
    If the machine is in permissive mode it will return ok.  Audit messages will
    still be generated if the access would be denied in enforcing mode.
 */
-static int selinux_access_check(DBusConnection *connection, DBusMessage *message, Manager *m, DBusError *error, const char *perm, const char *path) {
-        security_context_t scon = NULL;
-        security_context_t fcon = NULL;
+static int selinux_access_check(
+                Manager *m,
+                DBusConnection *connection,
+                DBusMessage *message,
+                const char *path,
+                const char *permission,
+                DBusError *error) {
+
+        security_context_t scon = NULL, fcon = NULL;
         int r = 0;
         const char *tclass = NULL;
         struct auditstruct audit;
 
+        assert(m);
+        assert(connection);
+        assert(message);
+        assert(permission);
+        assert(error);
+
+        r = selinux_init(m, error);
+        if (r < 0)
+                return r;
+
+        if (!use_selinux())
+                return 0;
+
+        log_debug("SELinux access check for path=%s permission=%s", strna(path), permission);
+
         audit.uid = audit.loginuid = (uid_t) -1;
         audit.gid = (gid_t) -1;
         audit.cmdline = NULL;
         audit.path = path;
 
         r = get_calling_context(connection, message, &scon, error);
-        if (r != 0) {
+        if (r < 0) {
                 log_error("Failed to get caller's security context on: %m");
                 goto finish;
         }
+
         if (path) {
                 tclass = "service";
                 /* get the file context of the unit file */
@@ -490,7 +363,7 @@ static int selinux_access_check(DBusConnection *connection, DBusMessage *message
                 if (r < 0) {
                         dbus_set_error(error, DBUS_ERROR_ACCESS_DENIED, "Failed to get file context on %s.", path);
                         r = -errno;
-                        log_error("Failed to get security context on: %s %m",path);
+                        log_error("Failed to get security context on %s: %m",path);
                         goto finish;
                 }
 
@@ -507,55 +380,22 @@ static int selinux_access_check(DBusConnection *connection, DBusMessage *message
 
         (void) get_audit_data(connection, message, &audit, error);
 
-        errno= 0;
-        r = selinux_check_access(scon, fcon, tclass, perm, &audit);
+        errno = 0;
+        r = selinux_check_access(scon, fcon, tclass, permission, &audit);
         if (r < 0) {
+                dbus_set_error(error, DBUS_ERROR_ACCESS_DENIED, "SELinux policy denies access.");
                 r = -errno;
                 log_error("SELinux policy denies access.");
-                dbus_set_error(error, DBUS_ERROR_ACCESS_DENIED, "SELinux policy denies access.");
         }
 
-        log_debug("SELinux checkaccess scon %s tcon %s tclass %s perm %s path %s cmdline %s: %d", scon, fcon, tclass, perm, path, audit.cmdline, r);
+        log_debug("SELinux access check scon=%s tcon=%s tclass=%s perm=%s path=%s cmdline=%s: %i", scon, fcon, tclass, permission, path, audit.cmdline, r);
+
 finish:
         free(audit.cmdline);
         freecon(scon);
         freecon(fcon);
 
-        return r;
-}
-
-/*
-  Clean up memory allocated in selinux_avc_init
-*/
-void selinux_access_finish(void) {
-        if (!first_time)
-                avc_destroy();
-        first_time = 1;
-}
-
-int selinux_unit_access_check(DBusConnection *connection, DBusMessage *message, Manager *m, const char *path, DBusError *error) {
-        const char *perm;
-        bool require_unit;
-        const char *member;
-        int r;
-
-        log_debug("SELinux unit access check %s\n", path);
-        r = selinux_init(m, error);
-        if (r < 0)
-                return r;
-
-        if (! selinux_enabled)
-                return 0;
-
-        member = dbus_message_get_member(message);
-
-        selinux_perm_lookup(member, &perm, &require_unit);
-        log_debug("SELinux dbus-unit Look %s up perm %s require_unit %d", member, perm, require_unit);
-
-        r = selinux_access_check(connection, message, m, error, perm, path);
-
-        /* if SELinux is in permissive mode return 0 */
-        if (r && (security_getenforce() != 1 )) {
+        if (r && security_getenforce() != 1) {
                 dbus_error_init(error);
                 r = 0;
         }
@@ -563,100 +403,69 @@ int selinux_unit_access_check(DBusConnection *connection, DBusMessage *message,
         return r;
 }
 
-int selinux_manager_access_check(DBusConnection *connection, DBusMessage *message, Manager *m, DBusError *error) {
-        int r = -1;
-        const char *member;
-        bool require_unit;
-        const char *perm;
-        char *path = NULL;
-
-        log_debug("SELinux manager access check\n");
-        r = selinux_init(m, error);
-        if (r < 0)
-                return r;
-
-        if (! selinux_enabled)
-                return 0;
+int selinux_unit_access_check(
+                Unit *u,
+                DBusConnection *connection,
+                DBusMessage *message,
+                const char *permission,
+                DBusError *error) {
 
-        member = dbus_message_get_member(message);
+        assert(u);
+        assert(connection);
+        assert(message);
+        assert(permission);
+        assert(error);
 
-        selinux_perm_lookup(member, &perm, &require_unit);
-        log_debug("SELinux dbus-manager Lookup %s perm %s require_unit %d", member, perm, require_unit);
+        return selinux_access_check(u->manager, connection, message, u->source_path ? u->source_path : u->fragment_path, permission, error);
+}
 
-        if (require_unit) {
-                const char *name, *smode, *old_name = NULL;
-                Unit *u;
+int selinux_manager_access_check(
+                Manager *m,
+                DBusConnection *connection,
+                DBusMessage *message,
+                const char *permission,
+                DBusError *error) {
 
-                if (! dbus_message_get_args(
-                            message,
-                            error,
-                            DBUS_TYPE_STRING, &old_name,
-                            DBUS_TYPE_STRING, &name,
-                            DBUS_TYPE_STRING, &smode,
-                            DBUS_TYPE_INVALID)) {
-                        dbus_error_init(error);
-                        if (!dbus_message_get_args(
-                                    message,
-                                    error,
-                                    DBUS_TYPE_STRING, &name,
-                                    DBUS_TYPE_STRING, &smode,
-                                    DBUS_TYPE_INVALID)) {
-                                dbus_error_init(error);
-                                if (!dbus_message_get_args(
-                                            message,
-                                            error,
-                                            DBUS_TYPE_STRING, &name,
-                                            DBUS_TYPE_INVALID)) {
-                                        r = -EINVAL;
-                                        /* This is broken for now, if I can not get a name
-                                           return success.
-                                        */
-                                        log_error("SELinux dbus-manager failed to find unit %m");
-                                        r = 0;
-                                        goto finish;
-                                }
-                        }
-                }
+        assert(m);
+        assert(connection);
+        assert(message);
+        assert(permission);
+        assert(error);
 
-                log_debug("SELinux dbus-manager load unit %s", name);
-                r = manager_load_unit(m, name, NULL, error, &u);
-                if (r < 0) {
-                        log_error("Unit %s is not loaded.", name);
-                        /* This is broken for now, if I can not load a unit
-                           return success.
-                         */
-                        dbus_error_init(error);
-                        r = 0;
-                        goto finish;
-                }
+        return selinux_access_check(m, connection, message, NULL, permission, error);
+}
 
-                path = u->source_path ? u->source_path : u->fragment_path;
-                if (!path) {
-//                      r = -1;
-                        log_error("Unit %s does not have path.", name);
-                        goto finish;
-                }
-        }
-        r = selinux_access_check(connection, message, m, error, perm, path);
+void selinux_access_finish(void) {
+        if (!initialized)
+                return;
 
-finish:
-        /* if SELinux is in permissive mode return 0 */
-        if (r && (security_getenforce() != 1 )) {
-                dbus_error_init(error);
-                r = 0;
-        }
-        return r;
+        avc_destroy();
+        initialized = false;
 }
 
 #else
-int selinux_unit_access_check(DBusConnection *connection, DBusMessage *message, Manager *m, const char *path, DBusError *error) {
+
+int selinux_unit_access_check(
+                Unit *u,
+                DBusConnection *connection,
+                DBusMessage *message,
+                const char *permission,
+                DBusError *error) {
+
         return 0;
 }
 
-int selinux_manager_access_check(DBusConnection *connection, DBusMessage *message, Manager *m, DBusError *error) {
+int selinux_manager_access_check(
+                Manager *m,
+                DBusConnection *connection,
+                DBusMessage *message,
+                const char *permission,
+                DBusError *error) {
+
         return 0;
 }
 
 void selinux_access_finish(void) {
 }
+
 #endif