systemctl link fails since 223: Unit name is not valid.

[martinpitt]

Until systemd 222 this worked:

$ echo '[Unit'] | sudo tee /tmp/foo.service
$ sudo systemctl link /tmp/foo.service
Created symlink from /etc/systemd/system/foo.service to /tmp/foo.service.

With 223 (and 224) this got broken:

$ sudo systemctl link /tmp/foo.service
Failed to execute operation: Unit name /tmp/foo.service is not valid.

[martinpitt]

This got broken by 4938696 ("selinux: fix missing SELinux unit access check"). Reverting this commit on top of current master fixes this again. @d-hatayama , any idea?

[martinpitt]

This is an utter hack and not a final solution, but it may illustrate where the problem is:

--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1650,9 +1650,11 @@ static int method_enable_unit_files_generic(
         if (r < 0)
                 return r;

-        r = mac_selinux_unit_access_check_strv(l, message, m, verb, error);
-        if (r < 0)
-                return r;
+        if (call != unit_file_link) {
+                r = mac_selinux_unit_access_check_strv(l, message, m, verb, error);
+                if (r < 0)
+                        return r;
+        }

         r = bus_verify_manage_unit_files_async(m, message, error);
         if (r < 0)

I just don't have any clue about SELinux or what this access check does, so I don't know how to fix that properly, I'm afraid..

[chmorgan]

Seeing something similar here under 219 that just popped up after a Fedora 22 update:

Failed to execute operation: Unit name /home/cmorgan/projects/x/build/rootfs/lib/systemd/system/xx.service is not valid.

This has been working for us daily for some time now. Not sure if there was a micro bump in systemd revision or if a separate F22 change broke things but it looks like the same issue from the output. If it seems F22 related apologies for the noise.

[hat180]

This is due to a overlook of the case that mac_selinux_unit_access_check_strv() could pass a path via l to the 2nd argument of manager_load_unit(). As a result, the following check in manager_load_unit_prepare() invalidates absolute paths.

if (t == _UNIT_TYPE_INVALID || !unit_name_is_valid(name, UNIT_NAME_PLAIN|UNIT_NAME_INSTANCE))
    return sd_bus_error_setf(e, SD_BUS_ERROR_INVALID_ARGS, "Unit name %s is not valid.", name);

The fix I come up with is:

--- a/src/core/selinux-access.c
+++ b/src/core/selinux-access.c
@@ -302,7 +302,10 @@ int mac_selinux_unit_access_check_strv(
         int r;

         STRV_FOREACH(i, units) {
-                r = manager_load_unit(m, *i, NULL, error, &u);
+                if (is_path(*i))
+                        r = manager_load_unit(m, NULL, *i, error, &u);
+                else
+                        r = manager_load_unit(m, *i, NULL, error, &u);
                 if (r < 0)
                         return r;
                 r = mac_selinux_unit_access_check(u, message, permission, error);

[d-hatayama]

I created the fix as a pull request. Please see #1044.

[martinpitt]

Thanks @d-hatayama ! Merged.