From 40dd2a1c24b886d9e635fef8f7b5336700df03c3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 3 Aug 2024 14:19:35 +0900 Subject: [PATCH 1/3] core: make ImportCredentialEx= DBus property support without renaming Note that the conf parser for ImportCredential= checks in the same way. Follow-up for 831f208783aeac443e6f2fc2efc3119535a032ef. --- src/core/dbus-execute.c | 2 +- test/units/TEST-54-CREDS.sh | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index feafa15e43..c7c43ce7d3 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -2225,7 +2225,7 @@ int bus_exec_context_set_transient_property( if (!credential_glob_valid(glob)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Credential name or glob is invalid: %s", glob); - if (rename && !credential_name_valid(rename)) + if (!isempty(rename) && !credential_name_valid(rename)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Credential name is invalid: %s", rename); empty = false; diff --git a/test/units/TEST-54-CREDS.sh b/test/units/TEST-54-CREDS.sh index 9f7f4b1160..aa0a1329e0 100755 --- a/test/units/TEST-54-CREDS.sh +++ b/test/units/TEST-54-CREDS.sh @@ -279,6 +279,8 @@ echo -n ignored >/run/credstore/test.creds.second mkdir -p /etc/credstore echo -n b >/etc/credstore/test.creds.second echo -n c >/etc/credstore/test.creds.third + +# Check that all valid credentials are imported. systemd-run -p "ImportCredential=test.creds.*" \ --unit=test-54-ImportCredential.service \ -p DynamicUser=1 \ @@ -289,6 +291,17 @@ systemd-run -p "ImportCredential=test.creds.*" \ '${CREDENTIALS_DIRECTORY}/test.creds.third' >/tmp/ts54-concat cmp /tmp/ts54-concat <(echo -n abc) +# Check that ImportCredentialEx= works without renaming. +systemd-run -p "ImportCredentialEx=test.creds.*" \ + --unit=test-54-ImportCredential.service \ + -p DynamicUser=1 \ + --wait \ + --pipe \ + cat '${CREDENTIALS_DIRECTORY}/test.creds.first' \ + '${CREDENTIALS_DIRECTORY}/test.creds.second' \ + '${CREDENTIALS_DIRECTORY}/test.creds.third' >/tmp/ts54-concat +cmp /tmp/ts54-concat <(echo -n abc) + # Check that renaming with globs works as expected. systemd-run -p "ImportCredentialEx=test.creds.*:renamed.creds." \ --unit=test-54-ImportCredential.service \ From b376dbc83dd7243f85ffe876068418cdd8d097bf Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 3 Aug 2024 14:22:37 +0900 Subject: [PATCH 2/3] test: a credential can be imported multiple times with different names This is supported since 831f208783aeac443e6f2fc2efc3119535a032ef. Let's explicitly test the functionality. --- test/units/TEST-54-CREDS.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/units/TEST-54-CREDS.sh b/test/units/TEST-54-CREDS.sh index aa0a1329e0..f0e6932825 100755 --- a/test/units/TEST-54-CREDS.sh +++ b/test/units/TEST-54-CREDS.sh @@ -332,6 +332,19 @@ systemd-run -p "ImportCredentialEx=test.creds.first:renamed.creds.first" \ cat '${CREDENTIALS_DIRECTORY}/renamed.creds.first' >/tmp/ts54-concat cmp /tmp/ts54-concat <(echo -n a) +# Test that a credential can be imported multiple times with different names. +systemd-run -p "ImportCredentialEx=test.creds.first" \ + -p "ImportCredentialEx=test.creds.first:renamed.creds.first" \ + -p "ImportCredentialEx=test.creds.first:renamed.creds.second" \ + --unit=test-54-ImportCredential.service \ + -p DynamicUser=1 \ + --wait \ + --pipe \ + cat '${CREDENTIALS_DIRECTORY}/test.creds.first' \ + '${CREDENTIALS_DIRECTORY}/renamed.creds.first' \ + '${CREDENTIALS_DIRECTORY}/renamed.creds.second' >/tmp/ts54-concat +cmp /tmp/ts54-concat <(echo -n aaa) + # Now test encrypted credentials (only supported when built with OpenSSL though) if systemctl --version | grep -q -- +OPENSSL ; then echo -n $RANDOM >/tmp/test-54-plaintext From 74d1ee03737e285b15bfe95a623ff67603ab34f3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 3 Aug 2024 14:25:16 +0900 Subject: [PATCH 3/3] core: refuse credentials with invalid names matching with glob Even if the glob pattern is valid, the pattern may match credentials with invalid names. So, we need to check the names of the found credentials. Follow-up for 947c4d3952e30604b97f657dca08f93a0a8f4bae. --- src/core/exec-credential.c | 5 +++++ test/units/TEST-54-CREDS.sh | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/core/exec-credential.c b/src/core/exec-credential.c index e1b09f6718..45cb7f5ce2 100644 --- a/src/core/exec-credential.c +++ b/src/core/exec-credential.c @@ -502,6 +502,11 @@ static int load_credential_glob( free_and_replace(fn, renamed); } + if (!credential_name_valid(fn)) { + log_debug("Skipping credential with invalid name: %s", fn); + continue; + } + if (faccessat(write_dfd, fn, F_OK, AT_SYMLINK_NOFOLLOW) >= 0) { log_debug("Skipping credential with duplicated ID %s at %s", fn, *p); continue; diff --git a/test/units/TEST-54-CREDS.sh b/test/units/TEST-54-CREDS.sh index f0e6932825..29b789d361 100755 --- a/test/units/TEST-54-CREDS.sh +++ b/test/units/TEST-54-CREDS.sh @@ -279,6 +279,24 @@ echo -n ignored >/run/credstore/test.creds.second mkdir -p /etc/credstore echo -n b >/etc/credstore/test.creds.second echo -n c >/etc/credstore/test.creds.third +# Credential name cannot contain ':' +echo -n hoge >/etc/credstore/test.creds.hoge:invalid + +# Check if credentials with invalid names are not imported. +systemd-run -p "ImportCredential=test.creds.*" \ + --unit=test-54-ImportCredential.service \ + -p DynamicUser=1 \ + --wait \ + --pipe \ + test ! -e '${CREDENTIALS_DIRECTORY}/test.creds.hoge:invalid' + +# Check if credentials with invalid names are not imported (with renaming). +systemd-run -p "ImportCredentialEx=test.creds.*:renamed.creds." \ + --unit=test-54-ImportCredential.service \ + -p DynamicUser=1 \ + --wait \ + --pipe \ + test ! -e '${CREDENTIALS_DIRECTORY}/renamed.creds.hoge:invalid' # Check that all valid credentials are imported. systemd-run -p "ImportCredential=test.creds.*" \