diff --git a/NEWS b/NEWS index 26526a58196..0a159dacb85 100644 --- a/NEWS +++ b/NEWS @@ -85,6 +85,19 @@ CHANGES WITH 263 in spe: images, or set $SYSTEMD_DISSECT_VALIDATEFS=0 when using them to restore the previous behaviour. + * ConditionCredential= and AssertCredential= now also search the + credential store directories (/etc/credstore/, /run/credstore/, + /usr/lib/credstore/, their .encrypted counterparts, and the XDG + counterparts for the per-user service manager), the same way + ImportCredential= does. Previously they only checked the credentials + passed into the service manager itself. This makes credentials + imported by e.g. "systemd-imds --import" usable for conditionalizing + units. Units with such conditions may now start (or, with "!", be + skipped) because of credential store entries that were not intended + as per-boot switches. The same applies to SuggestOnCredential= in + systemd-sysupdate. The Credential= match in .network, .netdev, and + .link files is unchanged. + Changes in systemd-udevd: * systemd-udevd now uses fanotify rather than inotify to watch device diff --git a/docs/CREDENTIALS.md b/docs/CREDENTIALS.md index d2303f67717..faab8d2b74e 100644 --- a/docs/CREDENTIALS.md +++ b/docs/CREDENTIALS.md @@ -588,3 +588,5 @@ These directories are hence a great place to store credentials to load on the sy Sometimes it makes sense to conditionalize system services and invoke them only if the right system credential is passed to the system. Use the `ConditionCredential=` and `AssertCredential=` unit file settings for that. +Like `ImportCredential=`, they also search the credential store directories described above, +so a credential placed in e.g. `/run/credstore/` by an initrd or cloud provisioning tool also counts. diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index a6b00a0d1c5..145d814222b 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -1580,6 +1580,18 @@ unit@.service service instance belonging to the user. The argument must be a valid credential name. + If no such credential was passed into the service manager, the credential stores are searched + too, i.e. the same directories that ImportCredential= searches (see + systemd.exec5), + such as /etc/credstore/, /run/credstore/, + /usr/lib/credstore/ and their .encrypted counterparts for + the system service manager, and $XDG_CONFIG_HOME/credstore/, + $XDG_RUNTIME_DIR/credstore/, $HOME/.local/lib/credstore/ + and their .encrypted counterparts for the per-user service manager. Hence any + credential that ImportCredential= would pass to a service also satisfies this + condition. For encrypted credentials only their existence is checked, they are not decrypted. Before + version 263, only the credentials passed into the service manager were checked. + diff --git a/src/shared/condition.c b/src/shared/condition.c index 97bc370b3ab..05da956d9a9 100644 --- a/src/shared/condition.c +++ b/src/shared/condition.c @@ -48,6 +48,7 @@ #include "nulstr-util.h" #include "os-util.h" #include "parse-util.h" +#include "path-lookup.h" #include "path-util.h" #include "percent-util.h" #include "pidref.h" @@ -146,6 +147,7 @@ static int condition_test_kernel_command_line(Condition *c, char **env, RuntimeS } static int condition_test_credential(Condition *c, char **env, RuntimeScope scope) { + _cleanup_strv_free_ char **dirs = NULL; int r; assert(c); @@ -161,7 +163,6 @@ static int condition_test_credential(Condition *c, char **env, RuntimeScope scop int (*gd)(const char **ret); FOREACH_ARGUMENT(gd, get_credentials_dir, get_encrypted_credentials_dir) { - _cleanup_free_ char *j = NULL; const char *cd; r = gd(&cd); @@ -170,20 +171,59 @@ static int condition_test_credential(Condition *c, char **env, RuntimeScope scop if (r < 0) return r; - j = path_join(cd, c->parameter); + r = strv_extend(&dirs, cd); + if (r < 0) + return r; + } + size_t n_own_dirs = strv_length(dirs); + + /* Service managers also search the credential stores, the same as ImportCredential= does */ + if (IN_SET(scope, RUNTIME_SCOPE_SYSTEM, RUNTIME_SCOPE_USER)) { + _cleanup_strv_free_ char **plain = NULL, **encrypted = NULL; + + r = credential_store_path(scope, &plain); + if (r < 0) + return r; + + r = credential_store_path_encrypted(scope, &encrypted); + if (r < 0) + return r; + + r = strv_extend_strv_consume(&dirs, TAKE_PTR(plain), /* filter_duplicates= */ false); + if (r < 0) + return r; + + r = strv_extend_strv_consume(&dirs, TAKE_PTR(encrypted), /* filter_duplicates= */ false); + if (r < 0) + return r; + } + + /* A broken directory must not hide a match in a later one, so only report errors if the credential + * was found nowhere. */ + int ret = 0; + STRV_FOREACH(d, dirs) { + _cleanup_free_ char *j = NULL; + + j = path_join(*d, c->parameter); if (!j) return -ENOMEM; r = access_nofollow(j, F_OK); if (r >= 0) return true; /* yay! */ - if (r != -ENOENT) - return r; + if (r == -ENOENT) + continue; /* not found in this dir */ - /* not found in this dir */ + /* Credential stores may be inaccessible to the caller, in particular the system ones are only + * accessible to root. Unprivileged callers (e.g. systemd-sysupdate or systemd-analyze run as + * user) just cannot see credentials in there, which is not an error. */ + if ((size_t) (d - dirs) >= n_own_dirs && ERRNO_IS_NEG_PRIVILEGE(r)) + continue; + + RET_GATHER(ret, r); } - return false; + return ret; /* error, or 0 for "not found" */ } static int condition_test_version_cmp(const char *condition, const char *ver) { diff --git a/src/test/test-condition.c b/src/test/test-condition.c index cecfb3e7832..6f02c7624e8 100644 --- a/src/test/test-condition.c +++ b/src/test/test-condition.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -27,6 +28,7 @@ #include "libaudit-util.h" #include "limits-util.h" #include "log.h" +#include "mkdir.h" #include "nulstr-util.h" #include "os-util.h" #include "path-util.h" @@ -883,13 +885,16 @@ TEST(condition_test_credential) { ASSERT_OK_ERRNO(unsetenv("CREDENTIALS_DIRECTORY")); ASSERT_OK_ERRNO(unsetenv("ENCRYPTED_CREDENTIALS_DIRECTORY")); + /* Without a scope, only the credential directories from the environment are checked, which keeps + * this independent of the host's credential stores. Those are covered by the next test. */ + ASSERT_NOT_NULL((condition = condition_new(CONDITION_CREDENTIAL, "definitelymissing", /* trigger= */ false, /* negate= */ false))); - ASSERT_OK_ZERO(condition_test(condition, environ, RUNTIME_SCOPE_SYSTEM)); + ASSERT_OK_ZERO(condition_test(condition, environ, _RUNTIME_SCOPE_INVALID)); condition_free(condition); /* invalid */ ASSERT_NOT_NULL((condition = condition_new(CONDITION_CREDENTIAL, "..", /* trigger= */ false, /* negate= */ false))); - ASSERT_OK_ZERO(condition_test(condition, environ, RUNTIME_SCOPE_SYSTEM)); + ASSERT_OK_ZERO(condition_test(condition, environ, _RUNTIME_SCOPE_INVALID)); condition_free(condition); ASSERT_OK(mkdtemp_malloc(NULL, &n1)); @@ -899,26 +904,140 @@ TEST(condition_test_credential) { ASSERT_OK_ERRNO(setenv("ENCRYPTED_CREDENTIALS_DIRECTORY", n2, /* overwrite= */ true)); ASSERT_NOT_NULL((condition = condition_new(CONDITION_CREDENTIAL, "stillmissing", /* trigger= */ false, /* negate= */ false))); - ASSERT_OK_ZERO(condition_test(condition, environ, RUNTIME_SCOPE_SYSTEM)); + ASSERT_OK_ZERO(condition_test(condition, environ, _RUNTIME_SCOPE_INVALID)); condition_free(condition); ASSERT_NOT_NULL((j = path_join(n1, "existing"))); ASSERT_OK(touch(j)); ASSERT_NOT_NULL((condition = condition_new(CONDITION_CREDENTIAL, "existing", /* trigger= */ false, /* negate= */ false))); - ASSERT_OK_POSITIVE(condition_test(condition, environ, RUNTIME_SCOPE_SYSTEM)); + ASSERT_OK_POSITIVE(condition_test(condition, environ, _RUNTIME_SCOPE_INVALID)); condition_free(condition); free(j); ASSERT_NOT_NULL((j = path_join(n2, "existing-encrypted"))); ASSERT_OK(touch(j)); ASSERT_NOT_NULL((condition = condition_new(CONDITION_CREDENTIAL, "existing-encrypted", /* trigger= */ false, /* negate= */ false))); - ASSERT_OK_POSITIVE(condition_test(condition, environ, RUNTIME_SCOPE_SYSTEM)); + ASSERT_OK_POSITIVE(condition_test(condition, environ, _RUNTIME_SCOPE_INVALID)); condition_free(condition); ASSERT_OK(set_unset_env("CREDENTIALS_DIRECTORY", d1, /* overwrite= */ true)); ASSERT_OK(set_unset_env("ENCRYPTED_CREDENTIALS_DIRECTORY", d2, /* overwrite= */ true)); } +TEST(condition_test_credential_store) { + _cleanup_(rm_rf_physical_and_freep) char *t = NULL; + _cleanup_free_ char *saved = NULL, *j = NULL; + Condition *condition; + + /* The user scope credential stores are below $XDG_CONFIG_HOME, which we can redirect */ + ASSERT_OK(free_and_strdup(&saved, getenv("XDG_CONFIG_HOME"))); + ASSERT_OK(mkdtemp_malloc(NULL, &t)); + ASSERT_OK_ERRNO(setenv("XDG_CONFIG_HOME", t, /* overwrite= */ true)); + + ASSERT_NOT_NULL((j = path_join(t, "credstore", "instore"))); + ASSERT_OK(mkdir_parents(j, 0700)); + ASSERT_OK(touch(j)); + j = mfree(j); + + ASSERT_NOT_NULL((j = path_join(t, "credstore.encrypted", "instore-encrypted"))); + ASSERT_OK(mkdir_parents(j, 0700)); + ASSERT_OK(touch(j)); + + FOREACH_STRING(name, "instore", "instore-encrypted") { + ASSERT_NOT_NULL((condition = condition_new(CONDITION_CREDENTIAL, name, /* trigger= */ false, /* negate= */ false))); + ASSERT_OK_POSITIVE(condition_test(condition, environ, RUNTIME_SCOPE_USER)); + /* without a service manager scope, the credential stores are not searched */ + ASSERT_OK_ZERO(condition_test(condition, environ, _RUNTIME_SCOPE_INVALID)); + condition_free(condition); + } + + ASSERT_NOT_NULL((condition = condition_new(CONDITION_CREDENTIAL, "notinstore", /* trigger= */ false, /* negate= */ false))); + ASSERT_OK_ZERO(condition_test(condition, environ, RUNTIME_SCOPE_USER)); + condition_free(condition); + + ASSERT_OK(set_unset_env("XDG_CONFIG_HOME", saved, /* overwrite= */ true)); +} + +TEST(condition_test_credential_store_error) { + _cleanup_(rm_rf_physical_and_freep) char *t = NULL; + _cleanup_free_ char *saved_config = NULL, *saved_runtime = NULL, *config = NULL, *j = NULL; + Condition *condition; + + /* A broken credential store must not hide a match in a later one. $XDG_CONFIG_HOME/credstore is + * searched before $XDG_RUNTIME_DIR/credstore; make the former a file, so that it fails with ENOTDIR + * (unlike EACCES, this also works as root). */ + ASSERT_OK(free_and_strdup(&saved_config, getenv("XDG_CONFIG_HOME"))); + ASSERT_OK(free_and_strdup(&saved_runtime, getenv("XDG_RUNTIME_DIR"))); + ASSERT_OK(mkdtemp_malloc(NULL, &t)); + ASSERT_NOT_NULL((config = path_join(t, "config"))); + ASSERT_OK_ERRNO(setenv("XDG_CONFIG_HOME", config, /* overwrite= */ true)); + ASSERT_OK_ERRNO(setenv("XDG_RUNTIME_DIR", t, /* overwrite= */ true)); + + ASSERT_NOT_NULL((j = path_join(config, "credstore"))); + ASSERT_OK(mkdir_parents(j, 0700)); + ASSERT_OK(touch(j)); + j = mfree(j); + + ASSERT_NOT_NULL((j = path_join(t, "credstore", "inruntime"))); + ASSERT_OK(mkdir_parents(j, 0700)); + ASSERT_OK(touch(j)); + + ASSERT_NOT_NULL((condition = condition_new(CONDITION_CREDENTIAL, "inruntime", /* trigger= */ false, /* negate= */ false))); + ASSERT_OK_POSITIVE(condition_test(condition, environ, RUNTIME_SCOPE_USER)); + condition_free(condition); + + /* if it is not found anywhere, the error is reported */ + ASSERT_NOT_NULL((condition = condition_new(CONDITION_CREDENTIAL, "nowhere", /* trigger= */ false, /* negate= */ false))); + ASSERT_ERROR(condition_test(condition, environ, RUNTIME_SCOPE_USER), ENOTDIR); + condition_free(condition); + + ASSERT_OK(set_unset_env("XDG_CONFIG_HOME", saved_config, /* overwrite= */ true)); + ASSERT_OK(set_unset_env("XDG_RUNTIME_DIR", saved_runtime, /* overwrite= */ true)); +} + +TEST(condition_test_credential_store_inaccessible) { + _cleanup_(rm_rf_physical_and_freep) char *t = NULL; + _cleanup_free_ char *saved_config = NULL, *saved_creds = NULL, *store = NULL, *own = NULL, *j = NULL; + Condition *condition; + + if (geteuid() == 0) + return (void) log_tests_skipped("root can access any directory"); + + /* Credentials in a credential store that the caller cannot access are just invisible, while an + * inaccessible own credentials directory is still an error. */ + ASSERT_OK(free_and_strdup(&saved_config, getenv("XDG_CONFIG_HOME"))); + ASSERT_OK(free_and_strdup(&saved_creds, getenv("CREDENTIALS_DIRECTORY"))); + ASSERT_OK(mkdtemp_malloc(NULL, &t)); + ASSERT_OK_ERRNO(setenv("XDG_CONFIG_HOME", t, /* overwrite= */ true)); + ASSERT_OK_ERRNO(unsetenv("CREDENTIALS_DIRECTORY")); + + ASSERT_NOT_NULL((store = path_join(t, "credstore"))); + ASSERT_NOT_NULL((j = path_join(store, "hidden"))); + ASSERT_OK(mkdir_parents(j, 0700)); + ASSERT_OK(touch(j)); + ASSERT_OK_ERRNO(chmod(store, 0000)); + + FOREACH_STRING(name, "hidden", "nowhere") { + ASSERT_NOT_NULL((condition = condition_new(CONDITION_CREDENTIAL, name, /* trigger= */ false, /* negate= */ false))); + ASSERT_OK_ZERO(condition_test(condition, environ, RUNTIME_SCOPE_USER)); + condition_free(condition); + } + + ASSERT_NOT_NULL((own = path_join(t, "own"))); + ASSERT_OK_ERRNO(mkdir(own, 0000)); + ASSERT_OK_ERRNO(setenv("CREDENTIALS_DIRECTORY", own, /* overwrite= */ true)); + ASSERT_NOT_NULL((condition = condition_new(CONDITION_CREDENTIAL, "nowhere", /* trigger= */ false, /* negate= */ false))); + ASSERT_ERROR(condition_test(condition, environ, RUNTIME_SCOPE_USER), EACCES); + condition_free(condition); + + /* let the cleanup remove everything */ + ASSERT_OK_ERRNO(chmod(store, 0700)); + ASSERT_OK_ERRNO(chmod(own, 0700)); + + ASSERT_OK(set_unset_env("XDG_CONFIG_HOME", saved_config, /* overwrite= */ true)); + ASSERT_OK(set_unset_env("CREDENTIALS_DIRECTORY", saved_creds, /* overwrite= */ true)); +} + TEST(condition_test_cpufeature) { Condition *condition; diff --git a/test/units/TEST-54-CREDS.sh b/test/units/TEST-54-CREDS.sh index 75d1c41253d..a57bd04b669 100755 --- a/test/units/TEST-54-CREDS.sh +++ b/test/units/TEST-54-CREDS.sh @@ -369,6 +369,12 @@ echo -n c >/etc/credstore/test.creds.third # Credential name cannot contain ':' echo -n hoge >/etc/credstore/test.creds.hoge:invalid +# Conditions and asserts consider the credential stores, too +systemd-run -p AssertCredential=test.creds.first -p Type=oneshot true +systemd-run -p AssertCredential=test.creds.third -p Type=oneshot true +(! systemd-run -p AssertCredential=!test.creds.first -p Type=oneshot true) +(! systemd-run -p AssertCredential=test.creds.undefined -p Type=oneshot true) + # Check if credentials with invalid names are not imported. systemd-run -p "ImportCredential=test.creds.*" \ --unit=test-54-ImportCredential.service \