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
This commit is contained in:
Lennart Poettering
2023-06-22 16:07:09 +02:00
parent 54c0ba3409
commit 947c4d3952
6 changed files with 96 additions and 4 deletions
+6
View File
@@ -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>
+2 -2
View File
@@ -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;
+2 -2
View File
@@ -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;
}
+38
View File
@@ -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;
+1
View File
@@ -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);
+47
View File
@@ -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);