Merge pull request #14400 from keszybz/alias-check

Alias check rework
This commit is contained in:
Lennart Poettering 2020-01-13 18:03:13 +01:00 committed by GitHub
commit 765d88698f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 321 additions and 27 deletions

View File

@ -136,11 +136,25 @@
has the alias <filename>dbus-org.freedesktop.network1.service</filename>, created during installation as
a symlink, so when <command>systemd</command> is asked through D-Bus to load
<filename>dbus-org.freedesktop.network1.service</filename>, it'll load
<filename>systemd-networkd.service</filename>. Alias names may be used in commands like
<command>disable</command>, <command>start</command>, <command>stop</command>, <command>status</command>,
and similar, and in all unit dependency directives, including <varname>Wants=</varname>,
<varname>Requires=</varname>, <varname>Before=</varname>, <varname>After=</varname>. Aliases cannot be
used with the <command>preset</command> command.</para>
<filename>systemd-networkd.service</filename>. As another example, <filename>default.target</filename>
the default system target started at boot — is commonly symlinked (aliased) to either
<filename>multi-user.target</filename> or <filename>graphical.target</filename> to select what is started
by default. Alias names may be used in commands like <command>disable</command>,
<command>start</command>, <command>stop</command>, <command>status</command>, and similar, and in all
unit dependency directives, including <varname>Wants=</varname>, <varname>Requires=</varname>,
<varname>Before=</varname>, <varname>After=</varname>. Aliases cannot be used with the
<command>preset</command> command.</para>
<para>Aliases obey the following restrictions: a unit of a certain type (<literal>.service</literal>,
<literal>.socket</literal>, …) can only be aliased by a name with the same type suffix. A plain unit (not
a template or an instance), may only be aliased by a plain name. A template instance may only be aliased
by another template instance, and the instance part must be identical. A template may be aliased by
another template (in which case the alias applies to all instances of the template). As a special case, a
template instance (e.g. <literal>alias@inst.service</literal>) may be a symlink to different template
(e.g. <literal>template@inst.service</literal>). In that case, just this specific instance is aliased,
while other instances of the template (e.g. <literal>alias@foo.service</literal>,
<literal>alias@bar.service</literal>) are not aliased. Those rule preserve the requirement that the
instance (if any) is always uniquely defined for a given unit and all its aliases.</para>
<para>Unit files may specify aliases through the <varname>Alias=</varname> directive in the [Install]
section. When the unit is enabled, symlinks will be created for those names, and removed when the unit is
@ -184,6 +198,16 @@
i.e. <filename>foo-bar-.service.d/10-override.conf</filename> overrides
<filename>foo-.service.d/10-override.conf</filename>.</para>
<para>In cases of unit aliases (described above), dropins for the aliased name and all aliases are
loaded. In the example of <filename>default.target</filename> aliasing
<filename>graphical.target</filename>, <filename>default.target.d/</filename>,
<filename>default.target.wants/</filename>, <filename>default.target.requires/</filename>,
<filename>graphical.target.d/</filename>, <filename>graphical.target.wants/</filename>,
<filename>graphical.target.requires/</filename> would all be read. For templates, dropins for the
template, any template aliases, the template instance, and all alias instances are read. When just a
specific template instance is aliased, then the dropins for the target template, the target template
instance, and the alias template instance are read.</para>
<para>In addition to <filename>/etc/systemd/system</filename>, the drop-in <literal>.d/</literal>
directories for system services can be placed in <filename>/usr/lib/systemd/system</filename> or
<filename>/run/systemd/system</filename> directories. Drop-in files in <filename>/etc</filename>

View File

@ -13,6 +13,7 @@ typedef enum UnitNameFlags {
UNIT_NAME_TEMPLATE = 1 << 1, /* Allow foo@.service */
UNIT_NAME_INSTANCE = 1 << 2, /* Allow foo@bar.service */
UNIT_NAME_ANY = UNIT_NAME_PLAIN|UNIT_NAME_TEMPLATE|UNIT_NAME_INSTANCE,
_UNIT_NAME_INVALID = -1,
} UnitNameFlags;
bool unit_name_is_valid(const char *n, UnitNameFlags flags) _pure_;

View File

@ -11,24 +11,6 @@
#include "unit-name.h"
#include "unit.h"
static int unit_name_compatible(const char *a, const char *b) {
_cleanup_free_ char *template = NULL;
int r;
/* The straightforward case: the symlink name matches the target */
if (streq(a, b))
return 1;
r = unit_name_template(a, &template);
if (r == -EINVAL)
return 0; /* Not a template */
if (r < 0)
return r; /* OOM, or some other failure. Just skip the warning. */
/* An instance name points to a target that is just the template name */
return streq(template, b);
}
static int process_deps(Unit *u, UnitDependency dependency, const char *dir_suffix) {
_cleanup_strv_free_ char **paths = NULL;
char **p;
@ -83,9 +65,10 @@ static int process_deps(Unit *u, UnitDependency dependency, const char *dir_suff
/* We don't treat this as an error, especially because we didn't check this for a
* long time. Nevertheless, we warn, because such mismatch can be mighty confusing. */
r = unit_name_compatible(entry, basename(target));
r = unit_symlink_name_compatible(entry, basename(target), u->instance);
if (r < 0) {
log_unit_warning_errno(u, r, "Can't check if names %s and %s are compatible, ignoring: %m", entry, basename(target));
log_unit_warning_errno(u, r, "Can't check if names %s and %s are compatible, ignoring: %m",
entry, basename(target));
continue;
}
if (r == 0)

View File

@ -1703,6 +1703,82 @@ static int install_info_discover_and_check(
return install_info_may_process(ret ? *ret : NULL, paths, changes, n_changes);
}
int unit_file_verify_alias(const UnitFileInstallInfo *i, const char *dst, char **ret_dst) {
_cleanup_free_ char *dst_updated = NULL;
int r;
/* Verify that dst is a valid either a valid alias or a valid .wants/.requires symlink for the target
* unit *i. Return negative on error or if not compatible, zero on success.
*
* ret_dst is set in cases where "instance propagation" happens, i.e. when the instance part is
* inserted into dst. It is not normally set, even on success, so that the caller can easily
* distinguish the case where instance propagation occured.
*/
const char *path_alias = strrchr(dst, '/');
if (path_alias) {
/* This branch covers legacy Alias= function of creating .wants and .requires symlinks. */
_cleanup_free_ char *dir = NULL;
char *p;
path_alias ++; /* skip over slash */
dir = dirname_malloc(dst);
if (!dir)
return log_oom();
p = endswith(dir, ".wants");
if (!p)
p = endswith(dir, ".requires");
if (!p)
return log_warning_errno(SYNTHETIC_ERRNO(EXDEV),
"Invalid path \"%s\" in alias.", dir);
*p = '\0'; /* dir should now be a unit name */
r = unit_name_classify(dir);
if (r < 0)
return log_warning_errno(SYNTHETIC_ERRNO(EXDEV),
"Invalid unit name component \"%s\" in alias.", dir);
const bool instance_propagation = r == UNIT_NAME_TEMPLATE;
/* That's the name we want to use for verification. */
r = unit_symlink_name_compatible(path_alias, i->name, instance_propagation);
if (r < 0)
return log_error_errno(r, "Failed to verify alias validity: %m");
if (r == 0)
return log_warning_errno(SYNTHETIC_ERRNO(EXDEV),
"Invalid unit %s symlink %s.",
i->name, dst);
} else {
/* If the symlink target has an instance set and the symlink source doesn't, we "propagate
* the instance", i.e. instantiate the symlink source with the target instance. */
if (unit_name_is_valid(dst, UNIT_NAME_TEMPLATE)) {
_cleanup_free_ char *inst = NULL;
r = unit_name_to_instance(i->name, &inst);
if (r < 0)
return log_error_errno(r, "Failed to extract instance name from %s: %m", i->name);
if (r == UNIT_NAME_INSTANCE) {
r = unit_name_replace_instance(dst, inst, &dst_updated);
if (r < 0)
return log_error_errno(r, "Failed to build unit name from %s+%s: %m",
dst, inst);
}
}
r = unit_validate_alias_symlink_and_warn(dst_updated ?: dst, i->name);
if (r < 0)
return r;
}
*ret_dst = TAKE_PTR(dst_updated);
return 0;
}
static int install_info_symlink_alias(
UnitFileInstallInfo *i,
const LookupPaths *paths,
@ -1719,13 +1795,17 @@ static int install_info_symlink_alias(
assert(config_path);
STRV_FOREACH(s, i->aliases) {
_cleanup_free_ char *alias_path = NULL, *dst = NULL;
_cleanup_free_ char *alias_path = NULL, *dst = NULL, *dst_updated = NULL;
q = install_full_printf(i, *s, &dst);
if (q < 0)
return q;
alias_path = path_make_absolute(dst, config_path);
q = unit_file_verify_alias(i, dst, &dst_updated);
if (q < 0)
continue;
alias_path = path_make_absolute(dst_updated ?: dst, config_path);
if (!alias_path)
return -ENOMEM;

View File

@ -187,6 +187,8 @@ int unit_file_changes_add(UnitFileChange **changes, size_t *n_changes, UnitFileC
void unit_file_changes_free(UnitFileChange *changes, size_t n_changes);
void unit_file_dump_changes(int r, const char *verb, const UnitFileChange *changes, size_t n_changes, bool quiet);
int unit_file_verify_alias(const UnitFileInstallInfo *i, const char *dst, char **ret_dst);
int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char *name);
const char *unit_file_state_to_string(UnitFileState s) _const_;

View File

@ -31,6 +31,41 @@ bool unit_type_may_template(UnitType type) {
UNIT_PATH);
}
int unit_symlink_name_compatible(const char *symlink, const char *target, bool instance_propagation) {
_cleanup_free_ char *template = NULL;
int r, un_type1, un_type2;
un_type1 = unit_name_classify(symlink);
/* The straightforward case: the symlink name matches the target and we have a valid unit */
if (streq(symlink, target) &&
(un_type1 & (UNIT_NAME_PLAIN | UNIT_NAME_INSTANCE)))
return 1;
r = unit_name_template(symlink, &template);
if (r == -EINVAL)
return 0; /* Not a template */
if (r < 0)
return r;
un_type2 = unit_name_classify(target);
/* An instance name points to a target that is just the template name */
if (un_type1 == UNIT_NAME_INSTANCE &&
un_type2 == UNIT_NAME_TEMPLATE &&
streq(template, target))
return 1;
/* foo@.target.requires/bar@.service: instance will be propagated */
if (instance_propagation &&
un_type1 == UNIT_NAME_TEMPLATE &&
un_type2 == UNIT_NAME_TEMPLATE &&
streq(template, target))
return 1;
return 0;
}
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;

View File

@ -39,6 +39,7 @@ enum UnitFileScope {
bool unit_type_may_alias(UnitType type) _const_;
bool unit_type_may_template(UnitType type) _const_;
int unit_symlink_name_compatible(const char *symlink, const char *target, bool instance_propagation);
int unit_validate_alias_symlink_and_warn(const char *filename, const char *target);
int unit_file_build_name_map(

View File

@ -1043,6 +1043,168 @@ static void test_preset_multiple_instances(const char *root) {
unit_file_changes_free(changes, n_changes);
}
static void verify_one(
const UnitFileInstallInfo *i,
const char *alias,
int expected,
const char *updated_name) {
int r;
static const UnitFileInstallInfo *last_info = NULL;
_cleanup_free_ char *alias2 = NULL;
if (i != last_info)
log_info("-- %s --", (last_info = i)->name);
r = unit_file_verify_alias(i, alias, &alias2);
log_info_errno(r, "alias %s ← %s: %d/%m (expected %d)%s%s%s",
i->name, alias, r, expected,
alias2 ? " [" : "", alias2 ?: "", alias2 ? "]" : "");
assert(r == expected);
/* This is is test for "instance propagation". This propagation matters mostly for WantedBy= and
* RequiredBy= settings, and less so for Alias=. The only case where it should happen is when we have
* an Alias=alias@.service an instantiated template template@instance. In that case the instance name
* should be propagated into the alias as alias@instance. */
assert(streq_ptr(alias2, updated_name));
}
static void test_verify_alias(void) {
const UnitFileInstallInfo
plain_service = { .name = (char*) "plain.service" },
bare_template = { .name = (char*) "template1@.service" },
di_template = { .name = (char*) "template2@.service",
.default_instance = (char*) "di" },
inst_template = { .name = (char*) "template3@inst.service" },
di_inst_template = { .name = (char*) "template4@inst.service",
.default_instance = (char*) "di" };
verify_one(&plain_service, "alias.service", 0, NULL);
verify_one(&plain_service, "alias.socket", -EXDEV, NULL);
verify_one(&plain_service, "alias@.service", -EXDEV, NULL);
verify_one(&plain_service, "alias@inst.service", -EXDEV, NULL);
verify_one(&plain_service, "foo.target.wants/plain.service", 0, NULL);
verify_one(&plain_service, "foo.target.wants/plain.socket", -EXDEV, NULL);
verify_one(&plain_service, "foo.target.wants/plain@.service", -EXDEV, NULL);
verify_one(&plain_service, "foo.target.wants/service", -EXDEV, NULL);
verify_one(&plain_service, "foo.target.requires/plain.service", 0, NULL);
verify_one(&plain_service, "foo.target.requires/plain.socket", -EXDEV, NULL);
verify_one(&plain_service, "foo.target.requires/plain@.service", -EXDEV, NULL);
verify_one(&plain_service, "foo.target.requires/service", -EXDEV, NULL);
verify_one(&plain_service, "foo.target.conf/plain.service", -EXDEV, NULL);
verify_one(&plain_service, "foo.service/plain.service", -EXDEV, NULL); /* missing dir suffix */
verify_one(&plain_service, "asdf.requires/plain.service", -EXDEV, NULL); /* invalid unit name component */
verify_one(&bare_template, "alias.service", -EXDEV, NULL);
verify_one(&bare_template, "alias.socket", -EXDEV, NULL);
verify_one(&bare_template, "alias@.socket", -EXDEV, NULL);
verify_one(&bare_template, "alias@inst.socket", -EXDEV, NULL);
/* A general alias alias@.service → template1@.service. */
verify_one(&bare_template, "alias@.service", 0, NULL);
/* Only a specific instance is aliased, see the discussion in https://github.com/systemd/systemd/pull/13119. */
verify_one(&bare_template, "alias@inst.service", 0, NULL);
verify_one(&bare_template, "foo.target.wants/plain.service", -EXDEV, NULL);
verify_one(&bare_template, "foo.target.wants/plain.socket", -EXDEV, NULL);
verify_one(&bare_template, "foo.target.wants/plain@.service", -EXDEV, NULL);
/* Name mistmatch: we cannot allow this, because plain@foo.service would be pulled in by foo.taget,
* but would not be resolvable on its own, since systemd doesn't know how to load the fragment. */
verify_one(&bare_template, "foo.target.wants/plain@foo.service", -EXDEV, NULL);
verify_one(&bare_template, "foo.target.wants/template1@foo.service", 0, NULL);
verify_one(&bare_template, "foo.target.wants/service", -EXDEV, NULL);
verify_one(&bare_template, "foo.target.requires/plain.service", -EXDEV, NULL);
verify_one(&bare_template, "foo.target.requires/plain.socket", -EXDEV, NULL);
verify_one(&bare_template, "foo.target.requires/plain@.service", -EXDEV, NULL); /* instance missing */
verify_one(&bare_template, "foo.target.requires/template1@inst.service", 0, NULL);
verify_one(&bare_template, "foo.target.requires/service", -EXDEV, NULL);
verify_one(&bare_template, "foo.target.conf/plain.service", -EXDEV, NULL);
verify_one(&bare_template, "FOO@.target.requires/plain@.service", -EXDEV, NULL); /* template name mistatch */
verify_one(&bare_template, "FOO@inst.target.requires/plain@.service", -EXDEV, NULL);
verify_one(&bare_template, "FOO@inst.target.requires/plain@inst.service", -EXDEV, NULL);
verify_one(&bare_template, "FOO@.target.requires/template1@.service", 0, NULL); /* instance propagated */
verify_one(&bare_template, "FOO@inst.target.requires/template1@.service", -EXDEV, NULL); /* instance missing */
verify_one(&bare_template, "FOO@inst.target.requires/template1@inst.service", 0, NULL); /* instance provided */
verify_one(&di_template, "alias.service", -EXDEV, NULL);
verify_one(&di_template, "alias.socket", -EXDEV, NULL);
verify_one(&di_template, "alias@.socket", -EXDEV, NULL);
verify_one(&di_template, "alias@inst.socket", -EXDEV, NULL);
verify_one(&di_template, "alias@inst.service", 0, NULL);
verify_one(&di_template, "alias@.service", 0, NULL);
verify_one(&di_template, "alias@di.service", 0, NULL);
verify_one(&di_template, "foo.target.wants/plain.service", -EXDEV, NULL);
verify_one(&di_template, "foo.target.wants/plain.socket", -EXDEV, NULL);
verify_one(&di_template, "foo.target.wants/plain@.service", -EXDEV, NULL);
verify_one(&di_template, "foo.target.wants/plain@di.service", -EXDEV, NULL);
verify_one(&di_template, "foo.target.wants/template2@di.service", 0, NULL);
verify_one(&di_template, "foo.target.wants/service", -EXDEV, NULL);
verify_one(&di_template, "foo.target.requires/plain.service", -EXDEV, NULL);
verify_one(&di_template, "foo.target.requires/plain.socket", -EXDEV, NULL);
verify_one(&di_template, "foo.target.requires/plain@.service", -EXDEV, NULL);
verify_one(&di_template, "foo.target.requires/plain@di.service", -EXDEV, NULL);
verify_one(&di_template, "foo.target.requires/plain@foo.service", -EXDEV, NULL);
verify_one(&di_template, "foo.target.requires/template2@.service", -EXDEV, NULL); /* instance missing */
verify_one(&di_template, "foo.target.requires/template2@di.service", 0, NULL);
verify_one(&di_template, "foo.target.requires/service", -EXDEV, NULL);
verify_one(&di_template, "foo.target.conf/plain.service", -EXDEV, NULL);
verify_one(&inst_template, "alias.service", -EXDEV, NULL);
verify_one(&inst_template, "alias.socket", -EXDEV, NULL);
verify_one(&inst_template, "alias@.socket", -EXDEV, NULL);
verify_one(&inst_template, "alias@inst.socket", -EXDEV, NULL);
verify_one(&inst_template, "alias@inst.service", 0, NULL);
verify_one(&inst_template, "alias@.service", 0, "alias@inst.service");
verify_one(&inst_template, "alias@di.service", -EXDEV, NULL);
verify_one(&inst_template, "bar.target.wants/plain.service", -EXDEV, NULL);
verify_one(&inst_template, "bar.target.wants/plain.socket", -EXDEV, NULL);
verify_one(&inst_template, "bar.target.wants/plain@.service", -EXDEV, NULL);
verify_one(&inst_template, "bar.target.wants/plain@di.service", -EXDEV, NULL);
verify_one(&inst_template, "bar.target.wants/plain@inst.service", -EXDEV, NULL);
verify_one(&inst_template, "bar.target.wants/template3@foo.service", -EXDEV, NULL);
verify_one(&inst_template, "bar.target.wants/template3@inst.service", 0, NULL);
verify_one(&inst_template, "bar.target.wants/service", -EXDEV, NULL);
verify_one(&inst_template, "bar.target.requires/plain.service", -EXDEV, NULL);
verify_one(&inst_template, "bar.target.requires/plain.socket", -EXDEV, NULL);
verify_one(&inst_template, "bar.target.requires/plain@.service", -EXDEV, NULL);
verify_one(&inst_template, "bar.target.requires/plain@di.service", -EXDEV, NULL);
verify_one(&inst_template, "bar.target.requires/plain@inst.service", -EXDEV, NULL);
verify_one(&inst_template, "bar.target.requires/template3@foo.service", -EXDEV, NULL);
verify_one(&inst_template, "bar.target.requires/template3@inst.service", 0, NULL);
verify_one(&inst_template, "bar.target.requires/service", -EXDEV, NULL);
verify_one(&inst_template, "bar.target.conf/plain.service", -EXDEV, NULL);
verify_one(&inst_template, "BAR@.target.requires/plain@.service", -EXDEV, NULL); /* template name mistatch */
verify_one(&inst_template, "BAR@inst.target.requires/plain@.service", -EXDEV, NULL);
verify_one(&inst_template, "BAR@inst.target.requires/plain@inst.service", -EXDEV, NULL);
verify_one(&inst_template, "BAR@.target.requires/template3@.service", -EXDEV, NULL); /* instance missing */
verify_one(&inst_template, "BAR@inst.target.requires/template3@.service", -EXDEV, NULL); /* instance missing */
verify_one(&inst_template, "BAR@inst.target.requires/template3@inst.service", 0, NULL); /* instance provided */
verify_one(&inst_template, "BAR@inst.target.requires/template3@ins2.service", -EXDEV, NULL); /* instance mismatch */
/* explicit alias overrides DefaultInstance */
verify_one(&di_inst_template, "alias.service", -EXDEV, NULL);
verify_one(&di_inst_template, "alias.socket", -EXDEV, NULL);
verify_one(&di_inst_template, "alias@.socket", -EXDEV, NULL);
verify_one(&di_inst_template, "alias@inst.socket", -EXDEV, NULL);
verify_one(&di_inst_template, "alias@inst.service", 0, NULL);
verify_one(&di_inst_template, "alias@.service", 0, "alias@inst.service");
verify_one(&di_inst_template, "alias@di.service", -EXDEV, NULL);
verify_one(&di_inst_template, "goo.target.wants/plain.service", -EXDEV, NULL);
verify_one(&di_inst_template, "goo.target.wants/plain.socket", -EXDEV, NULL);
verify_one(&di_inst_template, "goo.target.wants/plain@.service", -EXDEV, NULL);
verify_one(&di_inst_template, "goo.target.wants/plain@di.service", -EXDEV, NULL);
verify_one(&di_inst_template, "goo.target.wants/template4@foo.service", -EXDEV, NULL);
verify_one(&di_inst_template, "goo.target.wants/template4@inst.service", 0, NULL);
verify_one(&di_inst_template, "goo.target.wants/template4@di.service", -EXDEV, NULL);
verify_one(&di_inst_template, "goo.target.wants/service", -EXDEV, NULL);
verify_one(&di_inst_template, "goo.target.requires/plain.service", -EXDEV, NULL);
verify_one(&di_inst_template, "goo.target.requires/plain.socket", -EXDEV, NULL);
verify_one(&di_inst_template, "goo.target.requires/plain@.service", -EXDEV, NULL);
verify_one(&di_inst_template, "goo.target.requires/plain@di.service", -EXDEV, NULL);
verify_one(&di_inst_template, "goo.target.requires/plain@inst.service", -EXDEV, NULL);
verify_one(&di_inst_template, "goo.target.requires/template4@foo.service", -EXDEV, NULL);
verify_one(&di_inst_template, "goo.target.requires/template4@inst.service", 0, NULL);
verify_one(&di_inst_template, "goo.target.requires/service", -EXDEV, NULL);
verify_one(&di_inst_template, "goo.target.conf/plain.service", -EXDEV, NULL);
}
int main(int argc, char *argv[]) {
char root[] = "/tmp/rootXXXXXX";
const char *p;
@ -1080,5 +1242,7 @@ int main(int argc, char *argv[]) {
assert_se(rm_rf(root, REMOVE_ROOT|REMOVE_PHYSICAL) >= 0);
test_verify_alias();
return 0;
}

View File

@ -78,6 +78,10 @@ static void test_unit_name_is_valid(void) {
test_unit_name_is_valid_one("foo@%%i.service", UNIT_NAME_INSTANCE, false);
test_unit_name_is_valid_one("foo@%%i%f.service", UNIT_NAME_INSTANCE, false);
test_unit_name_is_valid_one("foo@%F.service", UNIT_NAME_INSTANCE, false);
test_unit_name_is_valid_one("foo.target.wants/plain.service", UNIT_NAME_ANY, false);
test_unit_name_is_valid_one("foo.target.conf/foo.conf", UNIT_NAME_ANY, false);
test_unit_name_is_valid_one("foo.target.requires/plain.socket", UNIT_NAME_ANY, false);
}
static void test_unit_name_replace_instance_one(const char *pattern, const char *repl, const char *expected, int ret) {