core: restrict ImportCredential= globbing

Let's restrict how we apply credential globbing in ImportCredential=, so
that we have some flexibility in automatically extending the glob
expression with per-instance data eventually without getting into
conflict with the globbing parts.

In our current uses we only allow globbing at the end of the expression,
and this is a new, unreleased feature hence let's be restrictive on this
initially. We can still relax this later if we feel the need to after
all.

Fixes: #28022

diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
index 84eda5c58471c573810f3aff5dc54bfbf3693195..4752e0e0f390e9bb5fd99a03454a30aca903af84 100644 (file)
--- a/man/systemd.exec.xml
+++ b/man/systemd.exec.xml
@@ -3320,6 +3320,12 @@ StandardInputData=V2XigLJyZSBubyBzdHJhbmdlcnMgdG8gbG92ZQpZb3Uga25vdyB0aGUgcnVsZX
         <filename>/usr/lib/credstore.encrypted/</filename> in that order. When multiple credentials of the
         same name are found, the first one found is used.</para>
 
+        <para>The globbing expression implements a restrictive subset of <citerefentry
+        project='man-pages'><refentrytitle>glob</refentrytitle><manvolnum>7</manvolnum></citerefentry>: only
+        a single trailing <literal>*</literal> wildcard may be specified. Both <literal>?</literal> and
+        <literal>[]</literal> wildcards are not permitted, nor are <literal>*</literal> wildcards anywhere
+        except at the end of the glob expression.</para>
+
         <para>When multiple credentials of the same name are found, credentials found by
         <varname>LoadCredential=</varname> and <varname>LoadCredentialEncrypted=</varname> take priority over
         credentials found by <varname>ImportCredential=</varname>.</para></listitem>

diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c
index 80a035ab90f8b9ab1e3667809c391fe3dd14cbfb..c3255398c6fdb4eb43ace1a5cb1ab88f3f04178a 100644 (file)
--- a/src/core/dbus-execute.c
+++ b/src/core/dbus-execute.c
@@ -2355,8 +2355,8 @@ int bus_exec_context_set_transient_property(
                         if (r == 0)
                                 break;
 
-                        if (!filename_is_valid(s))
-                                return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Credential name is invalid: %s", s);
+                        if (!credential_glob_valid(s))
+                                return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Credential name or glob is invalid: %s", s);
 
                         isempty = false;
 

diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 6d129af7b2aec4f1139970d8fcac87c2179be9ce..f0f60cf2ab63dfc99c0db5d2502451f6f1ccb510 100644 (file)
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -4985,8 +4985,8 @@ int config_parse_import_credential(
                 log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to resolve unit specifiers in \"%s\", ignoring: %m", s);
                 return 0;
         }
-        if (!filename_is_valid(s)) {
-                log_syntax(unit, LOG_WARNING, filename, line, 0, "Credential name \"%s\" not valid, ignoring.", s);
+        if (!credential_glob_valid(s)) {
+                log_syntax(unit, LOG_WARNING, filename, line, 0, "Credential name or glob \"%s\" not valid, ignoring.", s);
                 return 0;
         }
 

diff --git a/src/shared/creds-util.c b/src/shared/creds-util.c
index efc36e2d6d629e385b87f7c67a81ad640083a027..83a4c84aa9a4f8a0b12f5ed5166074de5039f881 100644 (file)
--- a/src/shared/creds-util.c
+++ b/src/shared/creds-util.c
@@ -37,6 +37,44 @@ bool credential_name_valid(const char *s) {
         return filename_is_valid(s) && fdname_is_valid(s);
 }
 
+bool credential_glob_valid(const char *s) {
+        const char *e, *a;
+        size_t n;
+
+        /* Checks if a credential glob expression is valid. Note that this is more restrictive than
+         * fnmatch()! We only allow trailing asterisk matches for now (simply because we want some freedom
+         * with automatically extending the pattern in a systematic way to cover for unit instances getting
+         * per-instance credentials or similar. Moreover, credential globbing expressions are also more
+         * restrictive then credential names: we don't allow *, ?, [, ] in them (except for the asterisk
+         * match at the end of the string), simply to not allow ambiguity. After all, we want the flexibility
+         * to one day add full globbing should the need arise.  */
+
+        if (isempty(s))
+                return false;
+
+        /* Find first glob (or NUL byte) */
+        n = strcspn(s, "*?[]");
+        e = s + n;
+
+        /* For now, only allow asterisk wildcards, and only at the end of the string. If it's anything else, refuse. */
+        if (isempty(e))
+                return credential_name_valid(s);
+
+        if (!streq(e, "*")) /* only allow trailing "*", no other globs */
+                return false;
+
+        if (n == 0) /* Explicitly allow the complete wildcard. */
+                return true;
+
+        if (n > NAME_MAX + strlen(e)) /* before we make a copy on the stack, let's check this is not overly large */
+                return false;
+
+        /* Make a copy of the string without the '*' suffix */
+        a = strndupa(s, n);
+
+        return credential_name_valid(a);
+}
+
 static int get_credentials_dir_internal(const char *envvar, const char **ret) {
         const char *e;
 

diff --git a/src/shared/creds-util.h b/src/shared/creds-util.h
index 05d8b746348973cdf9f37c6ab2a596985bd0e747..1742678cb95c3149d31137729d74973974e9c4e5 100644 (file)
--- a/src/shared/creds-util.h
+++ b/src/shared/creds-util.h
@@ -25,6 +25,7 @@
 #define CREDENTIAL_ENCRYPTED_SIZE_MAX (CREDENTIAL_SIZE_MAX + 128U*1024U)
 
 bool credential_name_valid(const char *s);
+bool credential_glob_valid(const char *s);
 
 /* Where creds have been passed to the local execution context */
 int get_credentials_dir(const char **ret);

diff --git a/src/test/test-creds.c b/src/test/test-creds.c
index 25b0c34a5976526a5faf70f5f7d72bfa80743a19..acb198c1c1d946eea9b0c727b1d03e7712ffef13 100644 (file)
--- a/src/test/test-creds.c
+++ b/src/test/test-creds.c
@@ -71,4 +71,51 @@ TEST(read_credential_strings) {
                 assert_se(unsetenv("CREDENTIALS_DIRECTORY") >= 0);
 }
 
+TEST(credential_name_valid) {
+        char buf[NAME_MAX+2];
+
+        assert_se(!credential_name_valid(NULL));
+        assert_se(!credential_name_valid(""));
+        assert_se(!credential_name_valid("."));
+        assert_se(!credential_name_valid(".."));
+        assert_se(!credential_name_valid("foo/bar"));
+        assert_se(credential_name_valid("foo"));
+
+        memset(buf, 'x', sizeof(buf)-1);
+        buf[sizeof(buf)-1] = 0;
+        assert_se(!credential_name_valid(buf));
+
+        buf[sizeof(buf)-2] = 0;
+        assert_se(credential_name_valid(buf));
+}
+
+TEST(credential_glob_valid) {
+        char buf[NAME_MAX+2];
+
+        assert_se(!credential_glob_valid(NULL));
+        assert_se(!credential_glob_valid(""));
+        assert_se(!credential_glob_valid("."));
+        assert_se(!credential_glob_valid(".."));
+        assert_se(!credential_glob_valid("foo/bar"));
+        assert_se(credential_glob_valid("foo"));
+        assert_se(credential_glob_valid("foo*"));
+        assert_se(credential_glob_valid("x*"));
+        assert_se(credential_glob_valid("*"));
+        assert_se(!credential_glob_valid("?"));
+        assert_se(!credential_glob_valid("*a"));
+        assert_se(!credential_glob_valid("a?"));
+        assert_se(!credential_glob_valid("a[abc]"));
+        assert_se(!credential_glob_valid("a[abc]"));
+
+        memset(buf, 'x', sizeof(buf)-1);
+        buf[sizeof(buf)-1] = 0;
+        assert_se(!credential_glob_valid(buf));
+
+        buf[sizeof(buf)-2] = 0;
+        assert_se(credential_glob_valid(buf));
+
+        buf[sizeof(buf)-2] = '*';
+        assert_se(credential_glob_valid(buf));
+}
+
 DEFINE_TEST_MAIN(LOG_INFO);