From 5c338de6761aa3b3285a3f507f2517dd7dd3e9a6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Apr 2015 00:58:08 +0200 Subject: [PATCH] sd-bus: when augmenting creds, remember which ones were augmented Also, when we do permissions checks using creds, verify that we don't do so based on augmented creds, as extra safety check. --- src/libelogind/libelogind.sym.m4 | 1 + src/libelogind/sd-bus/bus-convenience.c | 15 +++++++++++ src/libelogind/sd-bus/bus-creds.c | 36 +++++++++++++++---------- src/libelogind/sd-bus/bus-creds.h | 2 ++ src/libelogind/sd-bus/bus-util.c | 3 +++ src/systemd/sd-bus.h | 1 + 6 files changed, 44 insertions(+), 14 deletions(-) diff --git a/src/libelogind/libelogind.sym.m4 b/src/libelogind/libelogind.sym.m4 index 81f112269..1b50486cf 100644 --- a/src/libelogind/libelogind.sym.m4 +++ b/src/libelogind/libelogind.sym.m4 @@ -322,6 +322,7 @@ global: sd_bus_creds_ref; sd_bus_creds_unref; sd_bus_creds_get_mask; + sd_bus_creds_get_augmented_mask; sd_bus_creds_get_uid; sd_bus_creds_get_gid; sd_bus_creds_get_pid; diff --git a/src/libelogind/sd-bus/bus-convenience.c b/src/libelogind/sd-bus/bus-convenience.c index 71ce757f7..28bc8d281 100644 --- a/src/libelogind/sd-bus/bus-convenience.c +++ b/src/libelogind/sd-bus/bus-convenience.c @@ -499,10 +499,18 @@ _public_ int sd_bus_query_sender_privilege(sd_bus_message *call, int capability) return -ENOTCONN; if (capability >= 0) { + r = sd_bus_query_sender_creds(call, SD_BUS_CREDS_UID|SD_BUS_CREDS_EUID|SD_BUS_CREDS_EFFECTIVE_CAPS, &creds); if (r < 0) return r; + /* We cannot use augmented caps for authorization, + * since then data is acquired raceful from + * /proc. This can never actually happen, but let's + * better be safe than sorry, and do an extra check + * here. */ + assert_return((sd_bus_creds_get_augmented_mask(creds) & SD_BUS_CREDS_EFFECTIVE_CAPS) == 0, -EPERM); + /* Note that not even on kdbus we might have the caps * field, due to faked identities, or namespace * translation issues. */ @@ -523,6 +531,13 @@ _public_ int sd_bus_query_sender_privilege(sd_bus_message *call, int capability) if (our_uid != 0 || !know_caps || capability < 0) { uid_t sender_uid; + /* We cannot use augmented uid/euid for authorization, + * since then data is acquired raceful from + * /proc. This can never actually happen, but let's + * better be safe than sorry, and do an extra check + * here. */ + assert_return((sd_bus_creds_get_augmented_mask(creds) & (SD_BUS_CREDS_UID|SD_BUS_CREDS_EUID)) == 0, -EPERM); + /* Try to use the EUID, if we have it. */ r = sd_bus_creds_get_euid(creds, &sender_uid); if (r < 0) diff --git a/src/libelogind/sd-bus/bus-creds.c b/src/libelogind/sd-bus/bus-creds.c index 0f13d6650..f86bd27ff 100644 --- a/src/libelogind/sd-bus/bus-creds.c +++ b/src/libelogind/sd-bus/bus-creds.c @@ -130,6 +130,12 @@ _public_ uint64_t sd_bus_creds_get_mask(const sd_bus_creds *c) { return c->mask; } +_public_ uint64_t sd_bus_creds_get_augmented_mask(const sd_bus_creds *c) { + assert_return(c, 0); + + return c->augmented; +} + sd_bus_creds* bus_creds_new(void) { sd_bus_creds *c; @@ -663,25 +669,25 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) { if (!(mask & SD_BUS_CREDS_AUGMENT)) return 0; - missing = mask & ~c->mask; - if (missing == 0) - return 0; - /* Try to retrieve PID from creds if it wasn't passed to us */ if (pid <= 0 && (c->mask & SD_BUS_CREDS_PID)) pid = c->pid; - if (tid <= 0 && (c->mask & SD_BUS_CREDS_TID)) - tid = c->pid; - /* Without pid we cannot do much... */ if (pid <= 0) return 0; - if (pid > 0) { - c->pid = pid; - c->mask |= SD_BUS_CREDS_PID; - } + /* Try to retrieve TID from creds if it wasn't passed to us */ + if (tid <= 0 && (c->mask & SD_BUS_CREDS_TID)) + tid = c->tid; + + /* Calculate what we shall and can add */ + missing = mask & ~(c->mask|SD_BUS_CREDS_PID|SD_BUS_CREDS_TID|SD_BUS_CREDS_UNIQUE_NAME|SD_BUS_CREDS_WELL_KNOWN_NAMES|SD_BUS_CREDS_DESCRIPTION|SD_BUS_CREDS_AUGMENT); + if (missing == 0) + return 0; + + c->pid = pid; + c->mask |= SD_BUS_CREDS_PID; if (tid > 0) { c->tid = tid; @@ -939,6 +945,8 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) { c->mask |= SD_BUS_CREDS_AUDIT_LOGIN_UID; } + c->augmented = missing & c->mask; + return 0; } @@ -1113,11 +1121,11 @@ int bus_creds_extend_by_pid(sd_bus_creds *c, uint64_t mask, sd_bus_creds **ret) n->mask |= SD_BUS_CREDS_DESCRIPTION; } + n->augmented = c->augmented & n->mask; + /* Get more data */ - r = bus_creds_add_more(n, mask, - c->mask & SD_BUS_CREDS_PID ? c->pid : 0, - c->mask & SD_BUS_CREDS_TID ? c->tid : 0); + r = bus_creds_add_more(n, mask, 0, 0); if (r < 0) return r; diff --git a/src/libelogind/sd-bus/bus-creds.h b/src/libelogind/sd-bus/bus-creds.h index 5430e535f..74062a583 100644 --- a/src/libelogind/sd-bus/bus-creds.h +++ b/src/libelogind/sd-bus/bus-creds.h @@ -28,7 +28,9 @@ struct sd_bus_creds { bool allocated; unsigned n_ref; + uint64_t mask; + uint64_t augmented; uid_t uid; uid_t euid; diff --git a/src/libelogind/sd-bus/bus-util.c b/src/libelogind/sd-bus/bus-util.c index 5bd7885e4..a84d3381c 100644 --- a/src/libelogind/sd-bus/bus-util.c +++ b/src/libelogind/sd-bus/bus-util.c @@ -206,6 +206,9 @@ static int check_good_user(sd_bus_message *m, uid_t good_user) { if (r < 0) return r; + /* Don't trust augmented credentials for authorization */ + assert_return((sd_bus_creds_get_augmented_mask(creds) & SD_BUS_CREDS_EUID) == 0, -EPERM); + r = sd_bus_creds_get_euid(creds, &sender_uid); if (r < 0) return r; diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h index f6262a3cc..f24cc08bd 100644 --- a/src/systemd/sd-bus.h +++ b/src/systemd/sd-bus.h @@ -329,6 +329,7 @@ int sd_bus_creds_new_from_pid(sd_bus_creds **ret, pid_t pid, uint64_t creds_mask sd_bus_creds *sd_bus_creds_ref(sd_bus_creds *c); sd_bus_creds *sd_bus_creds_unref(sd_bus_creds *c); uint64_t sd_bus_creds_get_mask(const sd_bus_creds *c); +uint64_t sd_bus_creds_get_augmented_mask(const sd_bus_creds *c); int sd_bus_creds_get_pid(sd_bus_creds *c, pid_t *pid); int sd_bus_creds_get_tid(sd_bus_creds *c, pid_t *tid); -- 2.30.2