shaba/systemd
1
0
Fork 0
mirror of https://github.com/systemd/systemd.git synced 2025-03-19 22:50:17 +03:00
Code

Merge pull request #23157 from poettering/execute-refactor-fix

Browse Source 
execute: refactor credential passing code, and fix two bugs
This commit is contained in:
Lennart Poettering 2022-04-22 15:51:41 +02:00 committed by GitHub
commit 0f2ac643d8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 106 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

160
src/core/execute.c
View File

@ -2598,7 +2598,9 @@ static int write_credential(
static int load_credential(
const ExecContext *context,
const ExecParameters *params,
ExecLoadCredential *lc,
const char *id,
const char *path,
bool encrypted,
const char *unit,
int read_dfd,
int write_dfd,
@ -2606,12 +2608,6 @@ static int load_credential(
bool ownership_ok,
uint64_t *left) {
assert(context);
assert(lc);
assert(unit);
assert(write_dfd >= 0);
assert(left);
ReadFullFileFlags flags = READ_FULL_FILE_SECURE|READ_FULL_FILE_FAIL_WHEN_LARGER;
_cleanup_(erase_and_freep) char *data = NULL;
_cleanup_free_ char *j = NULL, *bindname = NULL;
@ -2620,14 +2616,23 @@ static int load_credential(
size_t size, add;
int r;
if (path_is_absolute(lc->path) || read_dfd >= 0) {
/* If this is an absolute path, read the data directly from it, and support AF_UNIX sockets */
source = lc->path;
assert(context);
assert(params);
assert(id);
assert(path);
assert(unit);
assert(write_dfd >= 0);
assert(left);
if (path_is_absolute(path) || read_dfd >= 0) {
/* If this is an absolute path (or a directory fd is specifier relative which to read), read
* the data directly from it, and support AF_UNIX sockets */
source = path;
flags |= READ_FULL_FILE_CONNECT_SOCKET;
/* Pass some minimal info about the unit and the credential name we are looking to acquire
* via the source socket address in case we read off an AF_UNIX socket. */
if (asprintf(&bindname, "@%" PRIx64"/unit/%s/%s", random_u64(), unit, lc->id) < 0)
if (asprintf(&bindname, "@%" PRIx64"/unit/%s/%s", random_u64(), unit, id) < 0)
return -ENOMEM;
missing_ok = false;
@ -2636,7 +2641,7 @@ static int load_credential(
/* If this is a relative path, take it relative to the credentials we received
* ourselves. We don't support the AF_UNIX stuff in this mode, since we are operating
* on a credential store, i.e. this is guaranteed to be regular files. */
j = path_join(params->received_credentials, lc->path);
j = path_join(params->received_credentials, path);
if (!j)
return -ENOMEM;
@ -2648,14 +2653,14 @@ static int load_credential(
r = read_full_file_full(
read_dfd, source,
UINT64_MAX,
lc->encrypted ? CREDENTIAL_ENCRYPTED_SIZE_MAX : CREDENTIAL_SIZE_MAX,
flags | (lc->encrypted ? READ_FULL_FILE_UNBASE64 : 0),
encrypted ? CREDENTIAL_ENCRYPTED_SIZE_MAX : CREDENTIAL_SIZE_MAX,
flags | (encrypted ? READ_FULL_FILE_UNBASE64 : 0),
bindname,
&data, &size);
else
r = -ENOENT;
if (r == -ENOENT && (missing_ok || hashmap_contains(context->set_credentials, lc->id))) {
if (r == -ENOENT && (missing_ok || hashmap_contains(context->set_credentials, id))) {
/* Make a missing inherited credential non-fatal, let's just continue. After all apps
* will get clear errors if we don't pass such a missing credential on as they
* themselves will get ENOENT when trying to read them, which should not be much
@ -2663,17 +2668,17 @@ static int load_credential(
*
* Also, if the source file doesn't exist, but a fallback is set via SetCredentials=
* we are fine, too. */
log_debug_errno(r, "Couldn't read inherited credential '%s', skipping: %m", lc->path);
log_debug_errno(r, "Couldn't read inherited credential '%s', skipping: %m", path);
return 0;
}
if (r < 0)
return log_debug_errno(r, "Failed to read credential '%s': %m", lc->path);
return log_debug_errno(r, "Failed to read credential '%s': %m", path);
if (lc->encrypted) {
if (encrypted) {
_cleanup_free_ void *plaintext = NULL;
size_t plaintext_size = 0;
r = decrypt_credential_and_warn(lc->id, now(CLOCK_REALTIME), NULL, data, size, &plaintext, &plaintext_size);
r = decrypt_credential_and_warn(id, now(CLOCK_REALTIME), NULL, data, size, &plaintext, &plaintext_size);
if (r < 0)
return r;
@ -2681,24 +2686,22 @@ static int load_credential(
size = plaintext_size;
}
add = strlen(lc->id) + size;
add = strlen(id) + size;
if (add > *left)
return -E2BIG;
r = write_credential(write_dfd, lc->id, data, size, uid, ownership_ok);
r = write_credential(write_dfd, id, data, size, uid, ownership_ok);
if (r < 0)
return r;
return log_debug_errno(r, "Failed to write credential '%s': %m", id);
*left -= add;
return 0;
}
struct load_cred_args {
Set *seen_creds;
const ExecContext *context;
const ExecParameters *params;
ExecLoadCredential *parent_local_credential;
bool encrypted;
const char *unit;
int dfd;
uid_t uid;
@ -2715,8 +2718,8 @@ static int load_cred_recurse_dir_cb(
const struct statx *sx,
void *userdata) {
_cleanup_free_ char *credname = NULL, *sub_id = NULL;
struct load_cred_args *args = userdata;
struct load_cred_args *args = ASSERT_PTR(userdata);
_cleanup_free_ char *sub_id = NULL;
int r;
if (event != RECURSE_DIR_ENTRY)
@ -2725,32 +2728,32 @@ static int load_cred_recurse_dir_cb(
if (!IN_SET(de->d_type, DT_REG, DT_SOCK))
return RECURSE_DIR_CONTINUE;
credname = strreplace(path, "/", "_");
if (!credname)
return -ENOMEM;
sub_id = strjoin(args->parent_local_credential->id, "_", credname);
sub_id = strreplace(path, "/", "_");
if (!sub_id)
return -ENOMEM;
if (!credential_name_valid(sub_id))
return -EINVAL;
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Credential would get ID %s, which is not valid, refusing", sub_id);
if (set_contains(args->seen_creds, sub_id)) {
if (faccessat(args->dfd, sub_id, F_OK, AT_SYMLINK_NOFOLLOW) >= 0) {
log_debug("Skipping credential with duplicated ID %s at %s", sub_id, path);
return RECURSE_DIR_CONTINUE;
}
if (errno != ENOENT)
return log_debug_errno(errno, "Failed to test if credential %s exists: %m", sub_id);
r = set_put_strdup(&args->seen_creds, sub_id);
if (r < 0)
return r;
r = load_credential(args->context, args->params,
&(ExecLoadCredential) {
.id = sub_id,
.path = (char *) de->d_name,
.encrypted = args->parent_local_credential->encrypted,
}, args->unit, dir_fd, args->dfd, args->uid, args->ownership_ok, args->left);
r = load_credential(
args->context,
args->params,
sub_id,
de->d_name,
args->encrypted,
args->unit,
dir_fd,
args->dfd,
args->uid,
args->ownership_ok,
args->left);
if (r < 0)
return r;
@ -2767,7 +2770,6 @@ static int acquire_credentials(
uint64_t left = CREDENTIALS_TOTAL_SIZE_MAX;
_cleanup_close_ int dfd = -1;
_cleanup_set_free_ Set *seen_creds = NULL;
ExecLoadCredential *lc;
ExecSetCredential *sc;
int r;
@ -2779,62 +2781,71 @@ static int acquire_credentials(
if (dfd < 0)
return -errno;
seen_creds = set_new(&string_hash_ops_free);
if (!seen_creds)
return -ENOMEM;
/* First, load credentials off disk (or acquire via AF_UNIX socket) */
HASHMAP_FOREACH(lc, context->load_credentials) {
_cleanup_close_ int sub_fd = -1;
/* Skip over credentials with unspecified paths. These are received by the
* service manager via the $CREDENTIALS_DIRECTORY environment variable. */
if (!is_path(lc->path) && streq(lc->id, lc->path))
continue;
/* If this is an absolute path, then try to open it as a directory. If that works, then we'll
* recurse into it. If it is an absolute path but it isn't a directory, then we'll open it as
* a regular file. Finally, if it's a relative path we will use it as a credential name to
* propagate a credential passed to us from further up. */
sub_fd = open(lc->path, O_DIRECTORY|O_CLOEXEC|O_RDONLY);
if (sub_fd < 0 && errno != ENOTDIR)
return -errno;
if (path_is_absolute(lc->path)) {
sub_fd = open(lc->path, O_DIRECTORY|O_CLOEXEC|O_RDONLY);
if (sub_fd < 0 && !IN_SET(errno,
ENOTDIR, /* Not a directory */
ENOENT)) /* Doesn't exist? */
return log_debug_errno(errno, "Failed to open '%s': %m", lc->path);
}
if (sub_fd < 0) {
r = set_put_strdup(&seen_creds, lc->id);
if (r < 0)
return r;
r = load_credential(context, params, lc, unit, -1, dfd, uid, ownership_ok, &left);
if (r < 0)
return r;
} else {
if (sub_fd < 0)
/* Regular file (incl. a credential passed in from higher up) */
r = load_credential(
context,
params,
lc->id,
lc->path,
lc->encrypted,
unit,
-1,
dfd,
uid,
ownership_ok,
&left);
else
/* Directory */
r = recurse_dir(
sub_fd,
/* path= */ "",
/* path= */ lc->id, /* recurse_dir() will suffix the subdir paths from here to the top-level id */
/* statx_mask= */ 0,
/* n_depth_max= */ UINT_MAX,
RECURSE_DIR_IGNORE_DOT|RECURSE_DIR_ENSURE_TYPE,
RECURSE_DIR_SORT|RECURSE_DIR_IGNORE_DOT|RECURSE_DIR_ENSURE_TYPE,
load_cred_recurse_dir_cb,
&(struct load_cred_args) {
.seen_creds = seen_creds,
.context = context,
.params = params,
.parent_local_credential = lc,
.encrypted = lc->encrypted,
.unit = unit,
.dfd = dfd,
.uid = uid,
.ownership_ok = ownership_ok,
.left = &left,
});
if (r < 0)
return r;
}
if (r < 0)
return r;
}
/* First we use the literally specified credentials. Note that they might be overridden again below,
* and thus act as a "default" if the same credential is specified multiple times */
/* Second, we add in literally specified credentials. If the credentials already exist, we'll not add
* them, so that they can act as a "default" if the same credential is specified multiple times. */
HASHMAP_FOREACH(sc, context->set_credentials) {
_cleanup_(erase_and_freep) void *plaintext = NULL;
const char *data;
size_t size, add;
/* Note that we check ahead of time here instead of relying on O_EXCL|O_CREAT later to return
* EEXIST if the credential already exists. That's because the TPM2-based decryption is kinda
* slow and involved, hence it's nice to be able to skip that if the credential already
* exists anyway. */
if (faccessat(dfd, sc->id, F_OK, AT_SYMLINK_NOFOLLOW) >= 0)
continue;
if (errno != ENOENT)
@ -2859,7 +2870,6 @@ static int acquire_credentials(
if (r < 0)
return r;
left -= add;
}

1
test/TEST-54-CREDS/test.sh
View File

@ -3,6 +3,7 @@
set -e
TEST_DESCRIPTION="test credentials"
NSPAWN_ARGUMENTS="--set-credential=mynspawncredential:strangevalue"
# shellcheck source=test/test-functions
. "${TEST_BASE_DIR:?}/test-functions"

20
test/units/testsuite-54.sh
View File

@ -16,6 +16,26 @@ systemd-run -p LoadCredential=passwd:/etc/passwd \
( cat /etc/passwd /etc/shadow && echo -n wuff ) | cmp /tmp/ts54-concat
rm /tmp/ts54-concat
# Test that SetCredential= acts as fallback for LoadCredential=
echo piff > /tmp/ts54-fallback
[ "$(systemd-run -p LoadCredential=paff:/tmp/ts54-fallback -p SetCredential=paff:poff --pipe --wait systemd-creds cat paff)" = "piff" ]
rm /tmp/ts54-fallback
[ "$(systemd-run -p LoadCredential=paff:/tmp/ts54-fallback -p SetCredential=paff:poff --pipe --wait systemd-creds cat paff)" = "poff" ]
if systemd-detect-virt -q -c ; then
# If this test is run in nspawn a credential should have been passed to us. See test/TEST-54-CREDS/test.sh
[ "$(systemd-creds --system cat mynspawncredential)" = "strangevalue" ]
# Test that propagation from system credential to service credential works
[ "$(systemd-run -p LoadCredential=mynspawncredential --pipe --wait systemd-creds cat mynspawncredential)" = "strangevalue" ]
# Check it also works, if we rename it while propagating it
[ "$(systemd-run -p LoadCredential=miau:mynspawncredential --pipe --wait systemd-creds cat miau)" = "strangevalue" ]
# Combine it with a fallback (which should have no effect, given the cred should be passed down)
[ "$(systemd-run -p LoadCredential=mynspawncredential -p SetCredential=mynspawncredential:zzz --pipe --wait systemd-creds cat mynspawncredential)" = "strangevalue" ]
fi
# Verify that the creds are immutable
systemd-run -p LoadCredential=passwd:/etc/passwd \
-p DynamicUser=1 \