shared/unit-file: add a function to validate unit alias symlinks

It turns out most possible symlinks are invalid, because the type has to match,
and template units can only be linked to template units.

I'm not sure if the existing code made the same checks consistently. At least
I don't see the same rules expressed in a single place.

diff --git a/src/shared/unit-file.c b/src/shared/unit-file.c
index deed7dcf097a908c3a0bb0d9b07f02dbc58a4d99..cde38c472b8c859aeb36d941e38c46f8438a7f5a 100644 (file)
--- a/src/shared/unit-file.c
+++ b/src/shared/unit-file.c
@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: LGPL-2.1+ */
 
 #include "macro.h"
+#include "string-util.h"
 #include "unit-file.h"
 
 bool unit_type_may_alias(UnitType type) {
@@ -21,3 +22,75 @@ bool unit_type_may_template(UnitType type) {
                       UNIT_TIMER,
                       UNIT_PATH);
 }
+
+int unit_validate_alias_symlink_and_warn(const char *filename, const char *target) {
+        const char *src, *dst;
+        _cleanup_free_ char *src_instance = NULL, *dst_instance = NULL;
+        UnitType src_unit_type, dst_unit_type;
+        int src_name_type, dst_name_type;
+
+        /* Check if the *alias* symlink is valid. This applies to symlinks like
+         * /etc/systemd/system/dbus.service → dbus-broker.service, but not to .wants or .requires symlinks
+         * and such. Neither does this apply to symlinks which *link* units, i.e. symlinks to outside of the
+         * unit lookup path.
+         *
+         * -EINVAL is returned if the something is wrong with the source filename or the source unit type is
+         *         not allowed to symlink,
+         * -EXDEV if the target filename is not a valid unit name or doesn't match the source.
+         */
+
+        src = basename(filename);
+        dst = basename(target);
+
+        /* src checks */
+
+        src_name_type = unit_name_to_instance(src, &src_instance);
+        if (src_name_type < 0)
+                return log_notice_errno(src_name_type,
+                                        "%s: not a valid unit name \"%s\": %m", filename, src);
+
+        src_unit_type = unit_name_to_type(src);
+        assert(src_unit_type >= 0); /* unit_name_to_instance() checked the suffix already */
+
+        if (!unit_type_may_alias(src_unit_type))
+                return log_notice_errno(SYNTHETIC_ERRNO(EINVAL),
+                                        "%s: symlinks are not allowed for units of this type, rejecting.",
+                                        filename);
+
+        if (src_name_type != UNIT_NAME_PLAIN &&
+            !unit_type_may_template(src_unit_type))
+                return log_notice_errno(SYNTHETIC_ERRNO(EINVAL),
+                                        "%s: templates not allowed for %s units, rejecting.",
+                                        filename, unit_type_to_string(src_unit_type));
+
+        /* dst checks */
+
+        dst_name_type = unit_name_to_instance(dst, &dst_instance);
+        if (dst_name_type < 0)
+                return log_notice_errno(dst_name_type == -EINVAL ? SYNTHETIC_ERRNO(EXDEV) : dst_name_type,
+                                        "%s points to \"%s\" which is not a valid unit name: %m",
+                                        filename, dst);
+
+        if (!(dst_name_type == src_name_type ||
+              (src_name_type == UNIT_NAME_INSTANCE && dst_name_type == UNIT_NAME_TEMPLATE)))
+                return log_notice_errno(SYNTHETIC_ERRNO(EXDEV),
+                                        "%s: symlink target name type \"%s\" does not match source, rejecting.",
+                                        filename, dst);
+
+        if (dst_name_type == UNIT_NAME_INSTANCE) {
+                assert(src_instance);
+                assert(dst_instance);
+                if (!streq(src_instance, dst_instance))
+                        return log_notice_errno(SYNTHETIC_ERRNO(EXDEV),
+                                                "%s: unit symlink target \"%s\" instance name doesn't match, rejecting.",
+                                                filename, dst);
+        }
+
+        dst_unit_type = unit_name_to_type(dst);
+        if (dst_unit_type != src_unit_type)
+                return log_notice_errno(SYNTHETIC_ERRNO(EXDEV),
+                                        "%s: symlink target \"%s\" has incompatible suffix, rejecting.",
+                                        filename, dst);
+
+        return 0;
+}

diff --git a/src/shared/unit-file.h b/src/shared/unit-file.h
index 2b9df655ee2126db16cb7ec427cf9577d488b779..e57f472f4f7aaf3d067c8dc659e8e6c49b686806 100644 (file)
--- a/src/shared/unit-file.h
+++ b/src/shared/unit-file.h
@@ -35,3 +35,5 @@ enum UnitFileScope {
 
 bool unit_type_may_alias(UnitType type) _const_;
 bool unit_type_may_template(UnitType type) _const_;
+
+int unit_validate_alias_symlink_and_warn(const char *filename, const char *target);

diff --git a/src/test/meson.build b/src/test/meson.build
index ddc04dda65ebfeb14a3ecd5418cf264d56eff187..c25ecf62fcf392f6c6e87914954e16f047d2a57c 100644 (file)
--- a/src/test/meson.build
+++ b/src/test/meson.build
@@ -137,6 +137,10 @@ tests += [
          [],
          'ENABLE_EFI'],
 
+        [['src/test/test-unit-file.c'],
+         [],
+         []],
+
         [['src/test/test-unit-name.c',
           'src/test/test-helper.c'],
          [libcore,

diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c
new file mode 100644 (file)
index 0000000..5e281b2
--- /dev/null
+++ b/src/test/test-unit-file.c
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: LGPL-2.1+ */
+
+#include "path-lookup.h"
+#include "strv.h"
+#include "tests.h"
+#include "unit-file.h"
+
+static void test_unit_validate_alias_symlink_and_warn(void) {
+        log_info("/* %s */", __func__);
+
+        assert_se(unit_validate_alias_symlink_and_warn("/path/a.service", "/other/b.service") == 0);
+        assert_se(unit_validate_alias_symlink_and_warn("/path/a.service", "/other/b.socket") == -EXDEV);
+        assert_se(unit_validate_alias_symlink_and_warn("/path/a.service", "/other/b.foobar") == -EXDEV);
+        assert_se(unit_validate_alias_symlink_and_warn("/path/a@.service", "/other/b@.service") == 0);
+        assert_se(unit_validate_alias_symlink_and_warn("/path/a@.service", "/other/b@.socket") == -EXDEV);
+        assert_se(unit_validate_alias_symlink_and_warn("/path/a@XXX.service", "/other/b@YYY.service") == -EXDEV);
+        assert_se(unit_validate_alias_symlink_and_warn("/path/a@XXX.service", "/other/b@YYY.socket") == -EXDEV);
+        assert_se(unit_validate_alias_symlink_and_warn("/path/a@.service", "/other/b@YYY.service") == -EXDEV);
+        assert_se(unit_validate_alias_symlink_and_warn("/path/a@XXX.service", "/other/b@XXX.service") == 0);
+        assert_se(unit_validate_alias_symlink_and_warn("/path/a@XXX.service", "/other/b@.service") == 0);
+        assert_se(unit_validate_alias_symlink_and_warn("/path/a@.service", "/other/b.service") == -EXDEV);
+        assert_se(unit_validate_alias_symlink_and_warn("/path/a.service", "/other/b@.service") == -EXDEV);
+        assert_se(unit_validate_alias_symlink_and_warn("/path/a@.slice", "/other/b.slice") == -EINVAL);
+        assert_se(unit_validate_alias_symlink_and_warn("/path/a.slice", "/other/b.slice") == -EINVAL);
+}
+
+int main(int argc, char **argv) {
+        test_setup_logging(LOG_DEBUG);
+
+        test_unit_validate_alias_symlink_and_warn();
+
+        return 0;
+}