condition: make ConditionCredential= also search the credential stores

Credentials can reach a system through the credential stores. E.g.
`systemd-imds --import` writes cloud provided credentials to
`/run/credstore/` in the initrd. `ImportCredential=` already finds them
there, but `ConditionCredential=` and `AssertCredential=` only checked
the credentials passed into the service manager itself. Hence, a
credential in the store could configure a service, but not decide
whether a unit runs.

Make the credential condition search the same credential store
directories as `ImportCredential=`. Only the existence of a credential
is checked, encrypted ones are not decrypted.

This is a behaviour change, see NEWS. As before, non-root callers which
can't access a credential store treat these as absent, otherwise this
would break running e.g. `systemd-analyze` or `sysetmd-sysupdate` as
user.

Signed-off-by: Martin Pitt <martin@amutable.com>

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

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.

man/systemd.unit.xml

@@ -1580,6 +1580,18 @@
           <filename>unit@.service</filename> service instance belonging to the user. The argument must be a
           valid credential name.</para>
 
+          <para>If no such credential was passed into the service manager, the credential stores are searched
+          too, i.e. the same directories that <varname>ImportCredential=</varname> searches (see
+          <citerefentry><refentrytitle>systemd.exec</refentrytitle><manvolnum>5</manvolnum></citerefentry>),
+          such as <filename>/etc/credstore/</filename>, <filename>/run/credstore/</filename>,
+          <filename>/usr/lib/credstore/</filename> and their <filename>.encrypted</filename> counterparts for
+          the system service manager, and <filename>$XDG_CONFIG_HOME/credstore/</filename>,
+          <filename>$XDG_RUNTIME_DIR/credstore/</filename>, <filename>$HOME/.local/lib/credstore/</filename>
+          and their <filename>.encrypted</filename> counterparts for the per-user service manager. Hence any
+          credential that <varname>ImportCredential=</varname> 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.</para>
+
           <xi:include href="version-info.xml" xpointer="v252"/></listitem>
         </varlistentry>
 

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) {

src/test/test-condition.c

@@ -3,6 +3,7 @@
 #include <gnu/libc-version.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <sys/stat.h>
 #include <sys/utsname.h>
 #include <unistd.h>
 
@@ -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;
 

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 \