core: cache unit file selinux label, and make decisions based on that

Do not go back to disk on each selinux access, but instead cache the
label off the inode we are actually reading. That way unit file contents
and unit file label we use for access checks are always in sync.

Based on discussions here:

https://github.com/systemd/systemd/pull/10023#issuecomment-1179835586

Replaces:

https://github.com/systemd/systemd/pull/23910

This changes behaviour a bit, because we'll reach and cache the label at
the moment of loading the unit (i.e. usually on boot and reload), but
not after relabelling. Thus, users must refresh the cache explicitly via
a "systemctl daemon-reload" if they relabelled things.

This makes the SELinux story a bit more debuggable, as it adds an
AccessSELinuxContext bus property to units that will report the label we are
using for a unit (or the empty string if not known).

This also drops using the "source" path of a unit as label source. if
there's value in it, then generators should manually copy the selinux
label from the source files onto the generated unit files, so that the
rule that "access labels are read when we read the definition files" is
upheld. But I am not convinced this is really a necessary, good idea.

1 files changed, 17 insertions, 29 deletions

diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c
index d77901caa5..878dea13f1 100644
--- a/src/core/selinux-access.c
+++ b/src/core/selinux-access.c
@@ -179,13 +179,14 @@ static int access_init(sd_bus_error *error) {
 */
 int mac_selinux_access_check_internal(
                 sd_bus_message *message,
-                const char *path,
+                const char *unit_path,
+                const char *unit_context,
                 const char *permission,
                 const char *function,
                 sd_bus_error *error) {
 
         _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL;
-        const char *tclass, *scon;
+        const char *tclass, *scon, *acon;
         _cleanup_free_ char *cl = NULL;
         _cleanup_freecon_ char *fcon = NULL;
         char **cmdline = NULL;
@@ -214,37 +215,22 @@ int mac_selinux_access_check_internal(
         if (r < 0)
                 return r;
 
-        /* The SELinux context is something we really should have
-         * gotten directly from the message or sender, and not be an
-         * augmented field. If it was augmented we cannot use it for
-         * authorization, since this is racy and vulnerable. Let's add
-         * an extra check, just in case, even though this really
-         * shouldn't be possible. */
+        /* The SELinux context is something we really should have gotten directly from the message or sender,
+         * and not be an augmented field. If it was augmented we cannot use it for authorization, since this
+         * is racy and vulnerable. Let's add an extra check, just in case, even though this really shouldn't
+         * be possible. */
         assert_return((sd_bus_creds_get_augmented_mask(creds) & SD_BUS_CREDS_SELINUX_CONTEXT) == 0, -EPERM);
 
         r = sd_bus_creds_get_selinux_context(creds, &scon);
         if (r < 0)
                 return r;
 
-        if (path) {
-                /* Get the file context of the unit file */
-
-                if (getfilecon_raw(path, &fcon) < 0) {
-                        r = -errno;
-
-                        log_warning_errno(r, "SELinux getfilecon_raw() on '%s' failed%s (perm=%s): %m",
-                                          path,
-                                          enforce ? "" : ", ignoring",
-                                          permission);
-                        if (!enforce)
-                                return 0;
-
-                        return sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Failed to get file context on %s.", path);
-                }
-
+        if (unit_context) {
+                /* Nice! The unit comes with a SELinux context read from the unit file */
+                acon = unit_context;
                 tclass = "service";
-
         } else {
+                /* If no unit context is known, use our own */
                 if (getcon_raw(&fcon) < 0) {
                         r = -errno;
 
@@ -257,6 +243,7 @@ int mac_selinux_access_check_internal(
                         return sd_bus_error_set(error, SD_BUS_ERROR_ACCESS_DENIED, "Failed to get current context.");
                 }
 
+                acon = fcon;
                 tclass = "system";
         }
 
@@ -265,12 +252,12 @@ int mac_selinux_access_check_internal(
 
         struct audit_info audit_info = {
                 .creds = creds,
-                .path = path,
+                .path = unit_path,
                 .cmdline = cl,
                 .function = function,
         };
 
-        r = selinux_check_access(scon, fcon, tclass, permission, &audit_info);
+        r = selinux_check_access(scon, acon, tclass, permission, &audit_info);
         if (r < 0) {
                 r = errno_or_else(EPERM);
 
@@ -280,7 +267,7 @@ int mac_selinux_access_check_internal(
 
         log_full_errno_zerook(LOG_DEBUG, r,
                               "SELinux access check scon=%s tcon=%s tclass=%s perm=%s state=%s function=%s path=%s cmdline=%s: %m",
-                              scon, fcon, tclass, permission, enforce ? "enforcing" : "permissive", function, strna(path), isempty(cl) ? "n/a" : cl);
+                              scon, acon, tclass, permission, enforce ? "enforcing" : "permissive", function, strna(unit_path), strna(empty_to_null(cl)));
         return enforce ? r : 0;
 }
 
@@ -288,7 +275,8 @@ int mac_selinux_access_check_internal(
 
 int mac_selinux_access_check_internal(
                 sd_bus_message *message,
-                const char *path,
+                const char *unit_path,
+                const char *unit_label,
                 const char *permission,
                 const char *function,
                 sd_bus_error *error) {