Merge pull request #16841 from keszybz/acl-util-bitmask

Use a bitmask in fd_add_uid_acl_permission()
This commit is contained in:
Lennart Poettering 2020-08-31 16:45:13 +02:00 committed by GitHub
commit b519529104
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 47 additions and 49 deletions

View File

@ -1412,33 +1412,22 @@
<term><option>--load-credential=</option><replaceable>ID</replaceable>:<replaceable>PATH</replaceable></term>
<term><option>--set-credential=</option><replaceable>ID</replaceable>:<replaceable>VALUE</replaceable></term>
<para>Pass a credential to the container. These two options correspond to the
<listitem><para>Pass a credential to the container. These two options correspond to the
<varname>LoadCredential=</varname> and <varname>SetCredential=</varname> settings in unit files. See
<citerefentry><refentrytitle>systemd.exec</refentrytitle><manvolnum>5</manvolnum></citerefentry> for
details about these concepts, as well as the syntax of the option's arguments.</para>
<para>Note:</para>
<para>Note: when <command>systemd-nspawn</command> runs as systemd system service it can propagate
the credentials it received via <varname>LoadCredential=</varname>/<varname>SetCredential=</varname>
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.</para>
<orderedlist>
<listitem><para>When <command>systemd-nspawn</command> runs as systemd system service it can make
use and propagate credentials it received via
<varname>LoadCredential=</varname>/<varname>SetCredential=</varname> to the container
payload.</para></listitem>
<listitem><para>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.</para></listitem>
</orderedlist>
<para>Thus it is possible to easily propagate credentials from a host service manager to a
<command>systemd-nspawn</command> service and from there into its payload and services running within
it.</para>
<para>In order to embed binary data into
the credential data for <option>--set-credential=</option> use C-style escaping
(i.e. <literal>\n</literal> to embed a newline, or <literal>\x00</literal> to embed a NUL byte. Note
that the invoking shell might already apply unescaping once, hence this might require double
escaping!).</para>
<para>In order to embed binary data into the credential data for <option>--set-credential=</option>
use C-style escaping (i.e. <literal>\n</literal> to embed a newline, or <literal>\x00</literal> to
embed a <constant>NUL</constant> byte. Note that the invoking shell might already apply unescaping
once, hence this might require double escaping!).</para></listitem>
</varlistentry>
</variablelist>

View File

@ -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;

View File

@ -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;

View File

@ -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

View File

@ -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);

View File

@ -1,8 +1,10 @@
/* SPDX-License-Identifier: LGPL-2.1+ */
#pragma once
#if HAVE_ACL
#include <errno.h>
#include <unistd.h>
#if HAVE_ACL
#include <acl/libacl.h>
#include <stdbool.h>
#include <sys/acl.h>
@ -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

View File

@ -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);