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.

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