Skip to content

Commit

Permalink
sd-bus: when augmenting creds, remember which ones were augmented
Browse files Browse the repository at this point in the history
Also, when we do permissions checks using creds, verify that we don't do
so based on augmented creds, as extra safety check.
  • Loading branch information
poettering committed Apr 20, 2015
1 parent 822d9b6 commit 0f51442
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 14 deletions.
8 changes: 8 additions & 0 deletions src/core/selinux-access.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/libsystemd/libsystemd.sym.m4
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
15 changes: 15 additions & 0 deletions src/libsystemd/sd-bus/bus-convenience.c
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -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)
Expand Down
36 changes: 22 additions & 14 deletions src/libsystemd/sd-bus/bus-creds.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;

Expand Down
2 changes: 2 additions & 0 deletions src/libsystemd/sd-bus/bus-creds.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions src/libsystemd/sd-bus/bus-util.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/systemd/sd-bus.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 0f51442

Please sign in to comment.