Skip to content

Commit

Permalink
core: bypass dynamic user lookups from dbus-daemon
Browse files Browse the repository at this point in the history
dbus-daemon does NSS name look-ups in order to enforce its bus policy. This
might dead-lock if an NSS module use wants to use D-Bus for the look-up itself,
like our nss-systemd does. Let's work around this by bypassing bus
communication in the NSS module if we run inside of dbus-daemon. To make this
work we keep a bit of extra state in /run/systemd/dynamic-uid/ so that we don't
have to consult the bus, but can still resolve the names.

Note that the normal codepath continues to be via the bus, so that resolving
works from all mount namespaces and is subject to authentication, as before.

This is a bit dirty, but not too dirty, as dbus daemon is kinda special anyway
for PID 1.
  • Loading branch information
poettering committed Aug 18, 2016
1 parent 8a38484 commit fd63e71
Show file tree
Hide file tree
Showing 3 changed files with 235 additions and 93 deletions.
51 changes: 45 additions & 6 deletions src/core/dynamic-user.c
Expand Up @@ -150,6 +150,42 @@ int dynamic_user_acquire(Manager *m, const char *name, DynamicUser** ret) {
return 1;
}

static int make_uid_symlinks(uid_t uid, const char *name, bool b) {

char path1[strlen("/run/systemd/dynamic-uid/direct:") + DECIMAL_STR_MAX(uid_t) + 1];
const char *path2;
int r = 0;

/* Add direct additional symlinks for direct lookups of dynamic UIDs and their names by userspace code. The
* only reason we have this is because dbus-daemon cannot use D-Bus for resolving users and groups (since it
* would be its own client then). We hence keep these world-readable symlinks in place, so that the
* unprivileged dbus user can read the mappings when it needs them via these symlinks instead of having to go
* via the bus. Ideally, we'd use the lock files we keep for this anyway, but we can't since we use BSD locks
* on them and as those may be taken by any user with read access we can't make them world-readable. */

xsprintf(path1, "/run/systemd/dynamic-uid/direct:" UID_FMT, uid);
if (unlink(path1) < 0) {
if (errno != ENOENT)
r = -errno;
}
if (b) {
if (symlink(name, path1) < 0)
r = -errno;
}

path2 = strjoina("/run/systemd/dynamic-uid/direct:", name);
if (unlink(path2) < 0) {
if (errno != ENOENT)
r = -errno;
}
if (b) {
if (symlink(path1 + strlen("/run/systemd/dynamic-uid/direct:"), path2) < 0)
r = -errno;
}

return r;
}

static int pick_uid(const char *name, uid_t *ret_uid) {

static const uint8_t hash_key[] = {
Expand Down Expand Up @@ -223,6 +259,7 @@ static int pick_uid(const char *name, uid_t *ret_uid) {
}

(void) ftruncate(lock_fd, l);
(void) make_uid_symlinks(candidate, name, true); /* also add direct lookup symlinks */

*ret_uid = candidate;
r = lock_fd;
Expand Down Expand Up @@ -324,14 +361,16 @@ static int dynamic_user_push(DynamicUser *d, uid_t uid, int lock_fd) {
return 0;
}

static void unlink_uid_lock(int lock_fd, uid_t uid) {
static void unlink_uid_lock(int lock_fd, uid_t uid, const char *name) {
char lock_path[strlen("/run/systemd/dynamic-uid/") + DECIMAL_STR_MAX(uid_t) + 1];

if (lock_fd < 0)
return;

xsprintf(lock_path, "/run/systemd/dynamic-uid/" UID_FMT, uid);
(void) unlink_noerrno(lock_path);
(void) unlink(lock_path);

(void) make_uid_symlinks(uid, name, false); /* remove direct lookup symlinks */
}

int dynamic_user_realize(DynamicUser *d, uid_t *ret) {
Expand Down Expand Up @@ -399,15 +438,15 @@ int dynamic_user_realize(DynamicUser *d, uid_t *ret) {

/* So, we found a working UID/lock combination. Let's see if we actually still need it. */
if (lockf(d->storage_socket[0], F_LOCK, 0) < 0) {
unlink_uid_lock(uid_lock_fd, uid);
unlink_uid_lock(uid_lock_fd, uid, d->name);
return -errno;
}

r = dynamic_user_pop(d, &new_uid, &new_uid_lock_fd);
if (r < 0) {
if (r != -EAGAIN) {
/* OK, something bad happened, let's get rid of the bits we acquired. */
unlink_uid_lock(uid_lock_fd, uid);
unlink_uid_lock(uid_lock_fd, uid, d->name);
goto finish;
}

Expand All @@ -416,7 +455,7 @@ int dynamic_user_realize(DynamicUser *d, uid_t *ret) {
/* Hmm, so as it appears there's now something stored in the storage socket. Throw away what we
* acquired, and use what's stored now. */

unlink_uid_lock(uid_lock_fd, uid);
unlink_uid_lock(uid_lock_fd, uid, d->name);
safe_close(uid_lock_fd);

uid = new_uid;
Expand Down Expand Up @@ -513,7 +552,7 @@ static int dynamic_user_close(DynamicUser *d) {
goto finish;

/* This dynamic user was realized and dynamically allocated. In this case, let's remove the lock file. */
unlink_uid_lock(lock_fd, uid);
unlink_uid_lock(lock_fd, uid, d->name);
r = 1;

finish:
Expand Down
15 changes: 14 additions & 1 deletion src/core/execute.c
Expand Up @@ -91,6 +91,7 @@
#include "selinux-util.h"
#include "signal-util.h"
#include "smack-util.h"
#include "special.h"
#include "string-table.h"
#include "string-util.h"
#include "strv.h"
Expand Down Expand Up @@ -1384,6 +1385,7 @@ static void do_idle_pipe_dance(int idle_pipe[4]) {
}

static int build_environment(
Unit *u,
const ExecContext *c,
const ExecParameters *p,
unsigned n_fds,
Expand All @@ -1401,7 +1403,7 @@ static int build_environment(
assert(c);
assert(ret);

our_env = new0(char*, 12);
our_env = new0(char*, 13);
if (!our_env)
return -ENOMEM;

Expand Down Expand Up @@ -1436,6 +1438,16 @@ static int build_environment(
our_env[n_env++] = x;
}

/* If this is D-Bus, tell the nss-systemd module, since it relies on being able to use D-Bus look up dynamic
* users via PID 1, possibly dead-locking the dbus daemon. This way it will not use D-Bus to resolve names, but
* check the database directly. */
if (unit_has_name(u, SPECIAL_DBUS_SERVICE)) {
x = strdup("SYSTEMD_NSS_BYPASS_BUS=1");
if (!x)
return -ENOMEM;
our_env[n_env++] = x;
}

if (home) {
x = strappend("HOME=", home);
if (!x)
Expand Down Expand Up @@ -2100,6 +2112,7 @@ static int exec_child(
}

r = build_environment(
unit,
context,
params,
n_fds,
Expand Down

0 comments on commit fd63e71

Please sign in to comment.