[systemd-commits] 8 commits - CODING_STYLE TODO src/core src/libsystemd src/libsystemd-network src/systemd src/tmpfiles
Lennart Poettering
lennart at kemper.freedesktop.org
Mon Apr 20 16:13:24 PDT 2015
CODING_STYLE | 16 ++++
TODO | 8 ++
src/core/selinux-access.c | 8 ++
src/libsystemd-network/sd-dhcp6-client.c | 3
src/libsystemd/libsystemd.sym.m4 | 1
src/libsystemd/sd-bus/bus-convenience.c | 15 ++++
src/libsystemd/sd-bus/bus-creds.c | 107 +++++++++++++++++++------------
src/libsystemd/sd-bus/bus-creds.h | 2
src/libsystemd/sd-bus/bus-util.c | 3
src/systemd/sd-bus.h | 1
src/tmpfiles/tmpfiles.c | 2
11 files changed, 122 insertions(+), 44 deletions(-)
New commits:
commit aa5f6817bcb4cfea8708edf765358f4f507d5923
Author: Lennart Poettering <lennart at poettering.net>
Date: Tue Apr 21 01:10:19 2015 +0200
tmpfiles: consider an argument of "-" as non-specified
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index b7dd37c..8cbeb67 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -1760,7 +1760,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) {
return -EIO;
}
- if (!isempty(buffer)) {
+ if (!isempty(buffer) && !streq(buffer, "-")) {
i.argument = strdup(buffer);
if (!i.argument)
return log_oom();
commit 0f51442056157cfec2efc52ddbff7392b0ff674a
Author: Lennart Poettering <lennart at poettering.net>
Date: Tue Apr 21 00:58:08 2015 +0200
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.
diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c
index 7058b78..5e9a4a5 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 81f1122..1b50486 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 71ce757..28bc8d2 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 1183bbe..b00b530 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 5430e53..74062a5 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 ed36614..8408789 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 f6262a3..f24cc08 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);
commit 822d9b6e4c2f0dc1ebc606006dc52257f06850c5
Author: Lennart Poettering <lennart at poettering.net>
Date: Tue Apr 21 00:53:43 2015 +0200
sd-bus: augmenting cgroups-based creds when we have the cgroup path already is free
diff --git a/src/libsystemd/sd-bus/bus-creds.c b/src/libsystemd/sd-bus/bus-creds.c
index ea805f7..1183bbe 100644
--- a/src/libsystemd/sd-bus/bus-creds.c
+++ b/src/libsystemd/sd-bus/bus-creds.c
@@ -937,17 +937,22 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) {
if (missing & (SD_BUS_CREDS_CGROUP|SD_BUS_CREDS_UNIT|SD_BUS_CREDS_USER_UNIT|SD_BUS_CREDS_SLICE|SD_BUS_CREDS_SESSION|SD_BUS_CREDS_OWNER_UID)) {
- r = cg_pid_get_path(NULL, pid, &c->cgroup);
- if (r < 0) {
- if (r != -EPERM && r != -EACCES)
- return r;
- } else {
+ if (!c->cgroup) {
+ r = cg_pid_get_path(NULL, pid, &c->cgroup);
+ if (r < 0) {
+ if (r != -EPERM && r != -EACCES)
+ return r;
+ }
+ }
+
+ if (!c->cgroup_root) {
r = cg_get_root_path(&c->cgroup_root);
if (r < 0)
return r;
+ }
+ if (c->cgroup)
c->mask |= missing & (SD_BUS_CREDS_CGROUP|SD_BUS_CREDS_UNIT|SD_BUS_CREDS_USER_UNIT|SD_BUS_CREDS_SLICE|SD_BUS_CREDS_SESSION|SD_BUS_CREDS_OWNER_UID);
- }
}
if (missing & SD_BUS_CREDS_AUDIT_SESSION_ID) {
commit f887fa73eef0f505bd1e2d077b8eadbfd33a9c77
Author: Lennart Poettering <lennart at poettering.net>
Date: Tue Apr 21 00:52:24 2015 +0200
sd-bus: when augmenting creds, don't override any creds in any case
Let's better be safe than sorry.
diff --git a/src/libsystemd/sd-bus/bus-creds.c b/src/libsystemd/sd-bus/bus-creds.c
index 3ef69b0..ea805f7 100644
--- a/src/libsystemd/sd-bus/bus-creds.c
+++ b/src/libsystemd/sd-bus/bus-creds.c
@@ -754,10 +754,15 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) {
if (sscanf(p, "%lu %lu %lu %lu", &uid, &euid, &suid, &fsuid) != 4)
return -EIO;
- c->uid = (uid_t) uid;
- c->euid = (uid_t) euid;
- c->suid = (uid_t) suid;
- c->fsuid = (uid_t) fsuid;
+ if (missing & SD_BUS_CREDS_UID)
+ c->uid = (uid_t) uid;
+ if (missing & SD_BUS_CREDS_EUID)
+ c->euid = (uid_t) euid;
+ if (missing & SD_BUS_CREDS_SUID)
+ c->suid = (uid_t) suid;
+ if (missing & SD_BUS_CREDS_FSUID)
+ c->fsuid = (uid_t) fsuid;
+
c->mask |= missing & (SD_BUS_CREDS_UID|SD_BUS_CREDS_EUID|SD_BUS_CREDS_SUID|SD_BUS_CREDS_FSUID);
continue;
}
@@ -772,10 +777,15 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) {
if (sscanf(p, "%lu %lu %lu %lu", &gid, &egid, &sgid, &fsgid) != 4)
return -EIO;
- c->gid = (gid_t) gid;
- c->egid = (gid_t) egid;
- c->sgid = (gid_t) sgid;
- c->fsgid = (gid_t) fsgid;
+ if (missing & SD_BUS_CREDS_GID)
+ c->gid = (gid_t) gid;
+ if (missing & SD_BUS_CREDS_EGID)
+ c->egid = (gid_t) egid;
+ if (missing & SD_BUS_CREDS_SGID)
+ c->sgid = (gid_t) sgid;
+ if (missing & SD_BUS_CREDS_FSGID)
+ c->fsgid = (gid_t) fsgid;
+
c->mask |= missing & (SD_BUS_CREDS_GID|SD_BUS_CREDS_EGID|SD_BUS_CREDS_SGID|SD_BUS_CREDS_FSGID);
continue;
}
commit da634f97ebe1ce83a5bb47f8f78fd85471c5a8db
Author: Lennart Poettering <lennart at poettering.net>
Date: Tue Apr 21 00:50:43 2015 +0200
sd-bus: when copying creds objects, make sure we copy even the implicit well known names
diff --git a/src/libsystemd/sd-bus/bus-creds.c b/src/libsystemd/sd-bus/bus-creds.c
index a5d3574..3ef69b0 100644
--- a/src/libsystemd/sd-bus/bus-creds.c
+++ b/src/libsystemd/sd-bus/bus-creds.c
@@ -231,7 +231,6 @@ _public_ int sd_bus_creds_get_gid(sd_bus_creds *c, gid_t *gid) {
return 0;
}
-
_public_ int sd_bus_creds_get_egid(sd_bus_creds *c, gid_t *egid) {
assert_return(c, -EINVAL);
assert_return(egid, -EINVAL);
@@ -597,10 +596,11 @@ static int has_cap(sd_bus_creds *c, unsigned offset, int capability) {
assert(capability >= 0);
assert(c->capability);
- sz = DIV_ROUND_UP(cap_last_cap(), 32U);
- if ((unsigned)capability > cap_last_cap())
+ if ((unsigned) capability > cap_last_cap())
return 0;
+ sz = DIV_ROUND_UP(cap_last_cap(), 32U);
+
return !!(c->capability[offset * sz + CAP_TO_INDEX(capability)] & CAP_TO_MASK(capability));
}
@@ -982,6 +982,16 @@ int bus_creds_extend_by_pid(sd_bus_creds *c, uint64_t mask, sd_bus_creds **ret)
/* Copy the original data over */
+ if (c->mask & mask & SD_BUS_CREDS_PID) {
+ n->pid = c->pid;
+ n->mask |= SD_BUS_CREDS_PID;
+ }
+
+ if (c->mask & mask & SD_BUS_CREDS_TID) {
+ n->tid = c->tid;
+ n->mask |= SD_BUS_CREDS_TID;
+ }
+
if (c->mask & mask & SD_BUS_CREDS_UID) {
n->uid = c->uid;
n->mask |= SD_BUS_CREDS_UID;
@@ -1030,16 +1040,6 @@ int bus_creds_extend_by_pid(sd_bus_creds *c, uint64_t mask, sd_bus_creds **ret)
n->mask |= SD_BUS_CREDS_SUPPLEMENTARY_GIDS;
}
- if (c->mask & mask & SD_BUS_CREDS_PID) {
- n->pid = c->pid;
- n->mask |= SD_BUS_CREDS_PID;
- }
-
- if (c->mask & mask & SD_BUS_CREDS_TID) {
- n->tid = c->tid;
- n->mask |= SD_BUS_CREDS_TID;
- }
-
if (c->mask & mask & SD_BUS_CREDS_COMM) {
n->comm = strdup(c->comm);
if (!n->comm)
@@ -1120,6 +1120,8 @@ int bus_creds_extend_by_pid(sd_bus_creds *c, uint64_t mask, sd_bus_creds **ret)
n->well_known_names = strv_copy(c->well_known_names);
if (!n->well_known_names)
return -ENOMEM;
+ n->well_known_names_driver = c->well_known_names_driver;
+ n->well_known_names_local = c->well_known_names_local;
n->mask |= SD_BUS_CREDS_WELL_KNOWN_NAMES;
}
commit 62e3d1aed512d68cab1fc9b509e813a1fa2b3790
Author: Lennart Poettering <lennart at poettering.net>
Date: Mon Apr 20 20:57:04 2015 +0200
dhcp6: remove unnecessary if check
diff --git a/src/libsystemd-network/sd-dhcp6-client.c b/src/libsystemd-network/sd-dhcp6-client.c
index cd33237..85162dc 100644
--- a/src/libsystemd-network/sd-dhcp6-client.c
+++ b/src/libsystemd-network/sd-dhcp6-client.c
@@ -1205,8 +1205,7 @@ sd_dhcp6_client *sd_dhcp6_client_unref(sd_dhcp6_client *client) {
client_reset(client);
sd_dhcp6_client_detach_event(client);
- if (client->lease)
- sd_dhcp6_lease_unref(client->lease);
+ sd_dhcp6_lease_unref(client->lease);
free(client->req_opts);
free(client);
commit 17978b17d9f5d3591f1c644938efc9c27aa60485
Author: Lennart Poettering <lennart at poettering.net>
Date: Mon Apr 20 20:56:44 2015 +0200
update TODO
diff --git a/TODO b/TODO
index 2e70362..b0e7ea6 100644
--- a/TODO
+++ b/TODO
@@ -48,6 +48,14 @@ Before 220:
Features:
+* rework C11 utf8.[ch] to use char32_t instead of uint32_t when referring
+ to unicode chars, to make things more expressive.
+
+* networkd: MTU= switch for .network units needs documentation in systemd.network(5)
+
+* "machinectl migrate" or similar to copy a container from or to a
+ difference host, via ssh
+
* tmpfiles: creating new directories/subvolumes/fifos/device nodes
should not follow symlinks. None of the other adjustment or creation
calls follow symlinks.
commit ba780c116fc919c58fad07f45f4e800a062af63e
Author: Lennart Poettering <lennart at poettering.net>
Date: Mon Apr 20 20:56:17 2015 +0200
CODING_STYLE: document how destructors should work
diff --git a/CODING_STYLE b/CODING_STYLE
index feb1a9d..a295ca7 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -239,3 +239,19 @@
2, i.e. stdin, stdout, stderr, should those fds be closed. Given the
special semantics of those fds, it's probably a good idea to avoid
them. F_DUPFD_CLOEXEC with "3" as parameter avoids them.
+
+- When you define a destructor or unref() call for an object, please
+ accept a NULL object and simply treat this as NOP. This is similar
+ to how libc free() works, which accepts NULL pointers and becomes a
+ NOP for them. By following this scheme a lot of if checks can be
+ removed before invoking your destructor, which makes the code
+ substantially more readable and robust.
+
+- Related to this: when you define a destructor or unref() call for an
+ object, please make it return the same type it takes and always
+ return NULL from it. This allows writing code like this:
+
+ p = foobar_unref(p);
+
+ which will always work regardless if p is initialized or not, and
+ guarantees that p is NULL afterwards, all in just one line.
More information about the systemd-commits
mailing list