summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLennart Poettering <lennart@poettering.net>2016-12-02 15:05:55 +0100
committerLennart Poettering <lennart@poettering.net>2016-12-13 20:59:36 +0100
commitb3415f5daef49642be3d5f417b8880c078420ff7 (patch)
treef2b72bc429090f3fac47e9d6131bb696efc43659
parentcore: run each system service with a fresh session keyring (diff)
downloadsystemd-b3415f5daef49642be3d5f417b8880c078420ff7.tar.xz
systemd-b3415f5daef49642be3d5f417b8880c078420ff7.zip
core: store the invocation ID in the per-service keyring
Let's store the invocation ID in the per-service keyring as a root-owned key, with strict access rights. This has the advantage over the environment-based ID passing that it also works from SUID binaries (as they key cannot be overidden by unprivileged code starting them), in contrast to the secure_getenv() based mode. The invocation ID is now passed in three different ways to a service: - As environment variable $INVOCATION_ID. This is easy to use, but may be overriden by unprivileged code (which might be a bad or a good thing), which means it's incompatible with SUID code (see above). - As extended attribute on the service cgroup. This cannot be overriden by unprivileged code, and may be queried safely from "outside" of a service. However, it is incompatible with containers right now, as unprivileged containers generally cannot set xattrs on cgroupfs. - As "invocation_id" key in the kernel keyring. This has the benefit that the key cannot be changed by unprivileged service code, and thus is safe to access from SUID code (see above). But do note that service code can replace the session keyring with a fresh one that lacks the key. However in that case the key will not be owned by root, which is easily detectable. The keyring is also incompatible with containers right now, as it is not properly namespace aware (but this is being worked on), and thus most container managers mask the keyring-related system calls. Ideally we'd only have one way to pass the invocation ID, but the different ways all have limitations. The invocation ID hookup in journald is currently only available on the host but not in containers, due to the mentioned limitations. How to verify the new invocation ID in the keyring: # systemd-run -t /bin/sh Running as unit: run-rd917366c04f847b480d486017f7239d6.service Press ^] three times within 1s to disconnect TTY. # keyctl show Session Keyring 680208392 --alswrv 0 0 keyring: _ses 250926536 ----s-rv 0 0 \_ user: invocation_id # keyctl request user invocation_id 250926536 # keyctl read 250926536 16 bytes of data in key: 9c96317c ac64495a a42b9cd7 4f3ff96b # echo $INVOCATION_ID 9c96317cac64495aa42b9cd74f3ff96b # ^D This creates a new transient service runnint a shell. Then verifies the contents of the keyring, requests the invocation ID key, and reads its payload. For comparison the invocation ID as passed via the environment variable is also displayed.
-rw-r--r--src/core/execute.c15
-rw-r--r--src/libsystemd/sd-id128/sd-id128.c128
-rw-r--r--src/test/test-id128.c8
3 files changed, 146 insertions, 5 deletions
diff --git a/src/core/execute.c b/src/core/execute.c
index 5ac270aa12..4262f9433b 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -2226,6 +2226,21 @@ static int setup_keyring(Unit *u, const ExecParameters *p, uid_t uid, gid_t gid)
return 0;
}
+ /* Populate they keyring with the invocation ID by default. */
+ if (!sd_id128_is_null(u->invocation_id)) {
+ key_serial_t key;
+
+ key = add_key("user", "invocation_id", &u->invocation_id, sizeof(u->invocation_id), KEY_SPEC_SESSION_KEYRING);
+ if (key == -1)
+ log_debug_errno(errno, "Failed to add invocation ID to keyring, ignoring: %m");
+ else {
+ if (keyctl(KEYCTL_SETPERM, key,
+ KEY_POS_VIEW|KEY_POS_READ|KEY_POS_SEARCH|
+ KEY_USR_VIEW|KEY_USR_READ|KEY_USR_SEARCH, 0, 0) < 0)
+ return log_error_errno(errno, "Failed to restrict invocation ID permission: %m");
+ }
+ }
+
/* And now, make the keyring owned by the service's user */
if (uid_is_valid(uid) || gid_is_valid(gid))
if (keyctl(KEYCTL_CHOWN, keyring, uid, gid, 0) < 0)
diff --git a/src/libsystemd/sd-id128/sd-id128.c b/src/libsystemd/sd-id128/sd-id128.c
index 0d673ba655..cc89f2de2e 100644
--- a/src/libsystemd/sd-id128/sd-id128.c
+++ b/src/libsystemd/sd-id128/sd-id128.c
@@ -23,13 +23,16 @@
#include "sd-id128.h"
+#include "alloc-util.h"
#include "fd-util.h"
#include "hexdecoct.h"
#include "id128-util.h"
#include "io-util.h"
#include "khash.h"
#include "macro.h"
+#include "missing.h"
#include "random-util.h"
+#include "user-util.h"
#include "util.h"
_public_ char *sd_id128_to_string(sd_id128_t id, char s[SD_ID128_STRING_MAX]) {
@@ -130,6 +133,105 @@ _public_ int sd_id128_get_boot(sd_id128_t *ret) {
return 0;
}
+static int get_invocation_from_keyring(sd_id128_t *ret) {
+
+ _cleanup_free_ char *description = NULL;
+ char *d, *p, *g, *u, *e;
+ unsigned long perms;
+ key_serial_t key;
+ size_t sz = 256;
+ uid_t uid;
+ gid_t gid;
+ int r, c;
+
+#define MAX_PERMS ((unsigned long) (KEY_POS_VIEW|KEY_POS_READ|KEY_POS_SEARCH| \
+ KEY_USR_VIEW|KEY_USR_READ|KEY_USR_SEARCH))
+
+ assert(ret);
+
+ key = request_key("user", "invocation_id", NULL, 0);
+ if (key == -1) {
+ /* Keyring support not available? No invocation key stored? */
+ if (IN_SET(errno, ENOSYS, ENOKEY))
+ return 0;
+
+ return -errno;
+ }
+
+ for (;;) {
+ description = new(char, sz);
+ if (!description)
+ return -ENOMEM;
+
+ c = keyctl(KEYCTL_DESCRIBE, key, (unsigned long) description, sz, 0);
+ if (c < 0)
+ return -errno;
+
+ if ((size_t) c <= sz)
+ break;
+
+ sz = c;
+ free(description);
+ }
+
+ /* The kernel returns a final NUL in the string, verify that. */
+ assert(description[c-1] == 0);
+
+ /* Chop off the final description string */
+ d = strrchr(description, ';');
+ if (!d)
+ return -EIO;
+ *d = 0;
+
+ /* Look for the permissions */
+ p = strrchr(description, ';');
+ if (!p)
+ return -EIO;
+
+ errno = 0;
+ perms = strtoul(p + 1, &e, 16);
+ if (errno > 0)
+ return -errno;
+ if (e == p + 1) /* Read at least one character */
+ return -EIO;
+ if (e != d) /* Must reached the end */
+ return -EIO;
+
+ if ((perms & ~MAX_PERMS) != 0)
+ return -EPERM;
+
+ *p = 0;
+
+ /* Look for the group ID */
+ g = strrchr(description, ';');
+ if (!g)
+ return -EIO;
+ r = parse_gid(g + 1, &gid);
+ if (r < 0)
+ return r;
+ if (gid != 0)
+ return -EPERM;
+ *g = 0;
+
+ /* Look for the user ID */
+ u = strrchr(description, ';');
+ if (!u)
+ return -EIO;
+ r = parse_uid(u + 1, &uid);
+ if (r < 0)
+ return r;
+ if (uid != 0)
+ return -EPERM;
+
+ c = keyctl(KEYCTL_READ, key, (unsigned long) ret, sizeof(sd_id128_t), 0);
+ if (c < 0)
+ return -errno;
+ if (c != sizeof(sd_id128_t))
+ return -EIO;
+
+ return 1;
+}
+
_public_ int sd_id128_get_invocation(sd_id128_t *ret) {
static thread_local sd_id128_t saved_invocation_id = {};
int r;
@@ -137,15 +239,31 @@ _public_ int sd_id128_get_invocation(sd_id128_t *ret) {
assert_return(ret, -EINVAL);
if (sd_id128_is_null(saved_invocation_id)) {
- const char *e;
- e = secure_getenv("INVOCATION_ID");
- if (!e)
- return -ENXIO;
+ /* We first try to read the invocation ID from the kernel keyring. This has the benefit that it is not
+ * fakeable by unprivileged code. If the information is not available in the keyring, we use
+ * $INVOCATION_ID but ignore the data if our process was called by less privileged code
+ * (i.e. secure_getenv() instead of getenv()).
+ *
+ * The kernel keyring is only relevant for system services (as for user services we don't store the
+ * invocation ID in the keyring, as there'd be no trust benefit in that). The environment variable is
+ * primarily relevant for user services, and sufficiently safe as no privilege boundary is involved. */
- r = sd_id128_from_string(e, &saved_invocation_id);
+ r = get_invocation_from_keyring(&saved_invocation_id);
if (r < 0)
return r;
+
+ if (r == 0) {
+ const char *e;
+
+ e = secure_getenv("INVOCATION_ID");
+ if (!e)
+ return -ENXIO;
+
+ r = sd_id128_from_string(e, &saved_invocation_id);
+ if (r < 0)
+ return r;
+ }
}
*ret = saved_invocation_id;
diff --git a/src/test/test-id128.c b/src/test/test-id128.c
index ab5a111ba9..e8c4c3e550 100644
--- a/src/test/test-id128.c
+++ b/src/test/test-id128.c
@@ -39,6 +39,7 @@ int main(int argc, char *argv[]) {
char t[33], q[37];
_cleanup_free_ char *b = NULL;
_cleanup_close_ int fd = -1;
+ int r;
assert_se(sd_id128_randomize(&id) == 0);
printf("random: %s\n", sd_id128_to_string(id, t));
@@ -159,5 +160,12 @@ int main(int argc, char *argv[]) {
assert_se(sd_id128_get_machine_app_specific(SD_ID128_MAKE(51,df,0b,4b,c3,b0,4c,97,80,e2,99,b9,8c,a3,73,b8), &id2) >= 0);
assert_se(!sd_id128_equal(id, id2));
+ /* Query the invocation ID */
+ r = sd_id128_get_invocation(&id);
+ if (r < 0)
+ log_warning_errno(r, "Failed to get invocation ID, ignoring: %m");
+ else
+ log_info("Invocation ID: " SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(id));
+
return 0;
}