From 508fa02d6f112c323b0ed595da85cc5bcdd2d122 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 26 Aug 2020 10:42:27 +0200 Subject: [PATCH 1/2] man: shorten description of recursive credential passing in nspawn The text suggested that either nspawn or systemd can make use of credentials themselves. In fact they only pass them to children. --- man/systemd-nspawn.xml | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/man/systemd-nspawn.xml b/man/systemd-nspawn.xml index e1fec3d7a8..1e7e6a82d5 100644 --- a/man/systemd-nspawn.xml +++ b/man/systemd-nspawn.xml @@ -1412,33 +1412,22 @@ ID:PATH ID:VALUE - Pass a credential to the container. These two options correspond to the + Pass a credential to the container. These two options correspond to the LoadCredential= and SetCredential= settings in unit files. See systemd.exec5 for details about these concepts, as well as the syntax of the option's arguments. - Note: + Note: when systemd-nspawn runs as systemd system service it can propagate + the credentials it received via LoadCredential=/SetCredential= + to the container payload. A systemd service manager running as PID 1 in the container can further + propagate them to the services it itself starts. It is thus possible to easily propagate credentials + from a parent service manager to a container manager service and from there into its payload. This + can even be done recursively. - - When systemd-nspawn runs as systemd system service it can make - use and propagate credentials it received via - LoadCredential=/SetCredential= to the container - payload. - - A systemd service manager running as PID 1 in the container can make use of - credentials passed in this way, and propagate them further to services it itself - runs. - - - Thus it is possible to easily propagate credentials from a host service manager to a - systemd-nspawn service and from there into its payload and services running within - it. - - In order to embed binary data into - the credential data for use C-style escaping - (i.e. \n to embed a newline, or \x00 to embed a NUL byte. Note - that the invoking shell might already apply unescaping once, hence this might require double - escaping!). + In order to embed binary data into the credential data for + use C-style escaping (i.e. \n to embed a newline, or \x00 to + embed a NUL byte. Note that the invoking shell might already apply unescaping + once, hence this might require double escaping!). From 567aeb5801e3df568ac336f5d7da945964912c32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 26 Aug 2020 10:59:32 +0200 Subject: [PATCH 2/2] shared/acl-util: convert rd,wr,ex to a bitmask I find this version much more readable. Add replacement defines so that when acl/libacl.h is not available, the ACL_{READ,WRITE,EXECUTE} constants are also defined. Those constants were declared in the kernel headers already in 1da177e4c3f41524e886b7f1b8a0c1f, so they should be the same pretty much everywhere. --- src/core/execute.c | 12 ++---------- src/coredump/coredump.c | 4 ++-- src/journal/journald-server.c | 7 +++---- src/shared/acl-util.c | 20 ++++++++++++++------ src/shared/acl-util.h | 14 ++++++++++++-- src/test/test-acl-util.c | 6 +++--- 6 files changed, 36 insertions(+), 27 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index aede50c5fe..ddff3a2fb2 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -2429,11 +2429,7 @@ static int write_credential( return -errno; if (uid_is_valid(uid) && uid != getuid()) { -#if HAVE_ACL - r = fd_add_uid_acl_permission(fd, uid, /* read = */ true, /* write = */ false, /* execute = */ false); -#else - r = -EOPNOTSUPP; -#endif + r = fd_add_uid_acl_permission(fd, uid, ACL_READ); if (r < 0) { if (!ERRNO_IS_NOT_SUPPORTED(r) && !ERRNO_IS_PRIVILEGE(r)) return r; @@ -2549,11 +2545,7 @@ static int acquire_credentials( * accessible */ if (uid_is_valid(uid) && uid != getuid()) { -#if HAVE_ACL - r = fd_add_uid_acl_permission(dfd, uid, /* read = */ true, /* write = */ false, /* execute = */ true); -#else - r = -EOPNOTSUPP; -#endif + r = fd_add_uid_acl_permission(dfd, uid, ACL_READ | ACL_EXECUTE); if (r < 0) { if (!ERRNO_IS_NOT_SUPPORTED(r) && !ERRNO_IS_PRIVILEGE(r)) return r; diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 9b7811ae54..ab6a840ebd 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -186,9 +186,9 @@ static int fix_acl(int fd, uid_t uid) { return 0; /* Make sure normal users can read (but not write or delete) their own coredumps */ - r = fd_add_uid_acl_permission(fd, uid, /* read = */ true, /* write = */ false, /* execute = */ false); + r = fd_add_uid_acl_permission(fd, uid, ACL_READ); if (r < 0) - return log_error_errno(r, "Failed to adjust ACL of coredump: %m"); + return log_error_errno(r, "Failed to adjust ACL of the coredump: %m"); #endif return 0; diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index 8a8c41b7a5..6df43085ec 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -247,16 +247,15 @@ static bool uid_for_system_journal(uid_t uid) { } static void server_add_acls(JournalFile *f, uid_t uid) { -#if HAVE_ACL - int r; -#endif assert(f); #if HAVE_ACL + int r; + if (uid_for_system_journal(uid)) return; - r = fd_add_uid_acl_permission(f->fd, uid, /* read = */ true, /* write = */ false, /* execute = */ false); + r = fd_add_uid_acl_permission(f->fd, uid, ACL_READ); if (r < 0) log_warning_errno(r, "Failed to set ACL on %s, ignoring: %m", f->path); #endif diff --git a/src/shared/acl-util.c b/src/shared/acl-util.c index 02c94f9358..7a2767c37b 100644 --- a/src/shared/acl-util.c +++ b/src/shared/acl-util.c @@ -378,12 +378,20 @@ int acls_for_file(const char *path, acl_type_t type, acl_t new, acl_t *acl) { return 0; } +/* POSIX says that ACL_{READ,WRITE,EXECUTE} don't have to be bitmasks. But that is a natural thing to do and + * all extant implementations do it. Let's make sure that we fail verbosely in the (imho unlikely) scenario + * that we get a new implementation that does not satisfy this. */ +assert_cc(!(ACL_READ & ACL_WRITE)); +assert_cc(!(ACL_WRITE & ACL_EXECUTE)); +assert_cc(!(ACL_EXECUTE & ACL_READ)); +assert_cc((unsigned) ACL_READ == ACL_READ); +assert_cc((unsigned) ACL_WRITE == ACL_WRITE); +assert_cc((unsigned) ACL_EXECUTE == ACL_EXECUTE); + int fd_add_uid_acl_permission( int fd, uid_t uid, - bool rd, - bool wr, - bool ex) { + unsigned mask) { _cleanup_(acl_freep) acl_t acl = NULL; acl_permset_t permset; @@ -411,11 +419,11 @@ int fd_add_uid_acl_permission( if (acl_get_permset(entry, &permset) < 0) return -errno; - if (rd && acl_add_perm(permset, ACL_READ) < 0) + if ((mask & ACL_READ) && acl_add_perm(permset, ACL_READ) < 0) return -errno; - if (wr && acl_add_perm(permset, ACL_WRITE) < 0) + if ((mask & ACL_WRITE) && acl_add_perm(permset, ACL_WRITE) < 0) return -errno; - if (ex && acl_add_perm(permset, ACL_EXECUTE) < 0) + if ((mask & ACL_EXECUTE) && acl_add_perm(permset, ACL_EXECUTE) < 0) return -errno; r = calc_acl_mask_if_needed(&acl); diff --git a/src/shared/acl-util.h b/src/shared/acl-util.h index ace0fe0955..b6a6f480f8 100644 --- a/src/shared/acl-util.h +++ b/src/shared/acl-util.h @@ -1,8 +1,10 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ #pragma once -#if HAVE_ACL +#include +#include +#if HAVE_ACL #include #include #include @@ -15,7 +17,7 @@ int add_base_acls_if_needed(acl_t *acl_p, const char *path); int acl_search_groups(const char* path, char ***ret_groups); int parse_acl(const char *text, acl_t *acl_access, acl_t *acl_default, bool want_mask); int acls_for_file(const char *path, acl_type_t type, acl_t new, acl_t *acl); -int fd_add_uid_acl_permission(int fd, uid_t uid, bool rd, bool wr, bool ex); +int fd_add_uid_acl_permission(int fd, uid_t uid, unsigned mask); /* acl_free takes multiple argument types. * Multiple cleanup functions are necessary. */ @@ -27,4 +29,12 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(uid_t*, acl_free_uid_tp); #define acl_free_gid_tp acl_free DEFINE_TRIVIAL_CLEANUP_FUNC(gid_t*, acl_free_gid_tp); +#else +#define ACL_READ 0x04 +#define ACL_WRITE 0x02 +#define ACL_EXECUTE 0x01 + +static inline int fd_add_uid_acl_permission(int fd, uid_t uid, unsigned mask) { + return -EOPNOTSUPP; +} #endif diff --git a/src/test/test-acl-util.c b/src/test/test-acl-util.c index 9a3db3c8e3..00c482efed 100644 --- a/src/test/test-acl-util.c +++ b/src/test/test-acl-util.c @@ -41,8 +41,8 @@ static void test_add_acls_for_user(void) { } else uid = getuid(); - r = fd_add_uid_acl_permission(fd, uid, true, false, false); - log_info_errno(r, "fd_add_uid_acl_permission(%i, "UID_FMT", true, false, false): %m", fd, uid); + r = fd_add_uid_acl_permission(fd, uid, ACL_READ); + log_info_errno(r, "fd_add_uid_acl_permission(%i, "UID_FMT", ACL_READ): %m", fd, uid); assert_se(r >= 0); cmd = strjoina("ls -l ", fn); @@ -53,7 +53,7 @@ static void test_add_acls_for_user(void) { /* set the acls again */ - r = fd_add_uid_acl_permission(fd, uid, true, false, false); + r = fd_add_uid_acl_permission(fd, uid, ACL_READ); assert_se(r >= 0); cmd = strjoina("ls -l ", fn);