summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLennart Poettering <lennart@poettering.net>2015-04-21 00:58:08 +0200
committerLennart Poettering <lennart@poettering.net>2015-04-21 00:58:56 +0200
commit0f51442056157cfec2efc52ddbff7392b0ff674a (patch)
treed806661843f62a79fea36f50657728b185146d05
parent822d9b6e4c2f0dc1ebc606006dc52257f06850c5 (diff)
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.
-rw-r--r--src/core/selinux-access.c8
-rw-r--r--src/libsystemd/libsystemd.sym.m41
-rw-r--r--src/libsystemd/sd-bus/bus-convenience.c15
-rw-r--r--src/libsystemd/sd-bus/bus-creds.c36
-rw-r--r--src/libsystemd/sd-bus/bus-creds.h2
-rw-r--r--src/libsystemd/sd-bus/bus-util.c3
-rw-r--r--src/systemd/sd-bus.h1
7 files changed, 52 insertions, 14 deletions
diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c
index 7058b7802d..5e9a4a5e02 100644
--- a/src/core/selinux-access.c
+++ b/src/core/selinux-access.c
@@ -222,6 +222,14 @@ int mac_selinux_generic_access_check(
if (r < 0)
goto finish;
+ /* The SELinux context is something we really should have
+ * gotten directly from the message or sender, and not be an
+ * augmented field. If it was augmented we cannot use it for
+ * authorization, since this is racy and vulnerable. Let's add
+ * an extra check, just in case, even though this really
+ * shouldn't be possible. */
+ assert_return((sd_bus_creds_get_augmented_mask(creds) & SD_BUS_CREDS_SELINUX_CONTEXT) == 0, -EPERM);
+
r = sd_bus_creds_get_selinux_context(creds, &scon);
if (r < 0)
goto finish;
diff --git a/src/libsystemd/libsystemd.sym.m4 b/src/libsystemd/libsystemd.sym.m4
index 81f1122697..1b50486cfa 100644
--- a/src/libsystemd/libsystemd.sym.m4
+++ b/src/libsystemd/libsystemd.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/libsystemd/sd-bus/bus-convenience.c b/src/libsystemd/sd-bus/bus-convenience.c
index 71ce757f70..28bc8d2818 100644
--- a/src/libsystemd/sd-bus/bus-convenience.c
+++ b/src/libsystemd/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/libsystemd/sd-bus/bus-creds.c b/src/libsystemd/sd-bus/bus-creds.c
index 1183bbe6a1..b00b5308a8 100644
--- a/src/libsystemd/sd-bus/bus-creds.c
+++ b/src/libsystemd/sd-bus/bus-creds.c
@@ -131,6 +131,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;
@@ -697,25 +703,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;
@@ -973,6 +979,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;
}
@@ -1147,11 +1155,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/libsystemd/sd-bus/bus-creds.h b/src/libsystemd/sd-bus/bus-creds.h
index 5430e535f0..74062a583d 100644
--- a/src/libsystemd/sd-bus/bus-creds.h
+++ b/src/libsystemd/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/libsystemd/sd-bus/bus-util.c b/src/libsystemd/sd-bus/bus-util.c
index ed366145d9..840878931f 100644
--- a/src/libsystemd/sd-bus/bus-util.c
+++ b/src/libsystemd/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 f6262a3ccf..f24cc08bd8 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);