From: Lennart Poettering Date: Wed, 21 Jun 2023 08:53:24 +0000 (+0200) Subject: core: restrict ImportCredential= globbing X-Git-Tag: v254-rc1~144 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=947c4d3952e30604b97f657dca08f93a0a8f4bae;p=thirdparty%2Fsystemd.git 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 84eda5c5847..4752e0e0f39 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -3320,6 +3320,12 @@ StandardInputData=V2XigLJyZSBubyBzdHJhbmdlcnMgdG8gbG92ZQpZb3Uga25vdyB0aGUgcnVsZX /usr/lib/credstore.encrypted/ in that order. When multiple credentials of the same name are found, the first one found is used. + The globbing expression implements a restrictive subset of glob7: only + a single trailing * wildcard may be specified. Both ? and + [] wildcards are not permitted, nor are * wildcards anywhere + except at the end of the glob expression. + When multiple credentials of the same name are found, credentials found by LoadCredential= and LoadCredentialEncrypted= take priority over credentials found by ImportCredential=. diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 80a035ab90f..c3255398c6f 100644 --- 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 6d129af7b2a..f0f60cf2ab6 100644 --- 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 efc36e2d6d6..83a4c84aa9a 100644 --- 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 05d8b746348..1742678cb95 100644 --- 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 25b0c34a597..acb198c1c1d 100644 --- 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);