Commit 60cde1bd authored by Dimitri John Ledkov's avatar Dimitri John Ledkov
Browse files

core: gracefully bail out keyring operations when chown fails

LP: #1691096
parent b236503b
From: Dimitri John Ledkov <xnox@ubuntu.com>
Date: Tue, 26 Sep 2017 10:23:09 -0400
Subject: core: unlink the invocation id key, if cannot change keyring owner
KEYCTL_CHOWN fails under unpriviledged usernamespace containers that drop
CAP_SYS_ADMIN (eg. LXD, OpenVZ, etc). Because kernel checks the capability in
the initial namespace, rather than in the user namespace. Thus if KEYCTL_CHOWN
operation is required, but will be impossible to perform, unlink the key and
thus skip the keyring setup.
Fixes #6281
(cherry picked from commit e4945f3a577ac9233c0e71349b6c139899e742fc)
---
src/basic/missing.h | 8 ++++++++
src/core/execute.c | 15 +++++++++++----
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/src/basic/missing.h b/src/basic/missing.h
index 7830a4f..9694792 100644
--- a/src/basic/missing.h
+++ b/src/basic/missing.h
@@ -1102,6 +1102,14 @@ typedef int32_t key_serial_t;
#define KEYCTL_DESCRIBE 6
#endif
+#ifndef KEYCTL_LINK
+#define KEYCTL_LINK 8
+#endif
+
+#ifndef KEYCTL_UNLINK
+#define KEYCTL_UNLINK 9
+#endif
+
#ifndef KEYCTL_READ
#define KEYCTL_READ 11
#endif
diff --git a/src/core/execute.c b/src/core/execute.c
index d72e5bf..4b02f5a 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -2071,10 +2071,14 @@ static int apply_working_directory(
static int setup_keyring(Unit *u, const ExecParameters *p, uid_t uid, gid_t gid) {
key_serial_t keyring;
+ key_serial_t key;
+ int r;
assert(u);
assert(p);
+ key = -1;
+
/* Let's set up a new per-service "session" kernel keyring for each system service. This has the benefit that
* each service runs with its own keyring shared among all processes of the service, but with no hook-up beyond
* that scope, and in particular no link to the per-UID keyring. If we don't do this the keyring will be
@@ -2101,8 +2105,6 @@ static int setup_keyring(Unit *u, const ExecParameters *p, uid_t uid, gid_t gid)
/* 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");
@@ -2116,8 +2118,13 @@ static int setup_keyring(Unit *u, const ExecParameters *p, uid_t uid, gid_t gid)
/* 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)
- return log_error_errno(errno, "Failed to change ownership of session keyring: %m");
+ if (keyctl(KEYCTL_CHOWN, keyring, uid, gid, 0) < 0) {
+ log_error_errno(errno, "Failed to change ownership of session keyring: %m");
+ /* well, the kernel didn't - cause the kernel is borked */
+ if (keyctl(KEYCTL_UNLINK, key, keyring, 0, 0) < 0)
+ log_debug_errno(errno, "Failed to unlink (clean-up) key, after failing to change ownership: %m");
+ return 0;
+ }
return 0;
}
......@@ -4,6 +4,7 @@ seccomp-arm64-x32-do-not-have-_sysctl.patch
seccomp-arm64-does-not-have-mmap2.patch
test-seccomp-arm64-does-not-have-access-and-poll.patch
networkd-change-UseMTU-default-to-true.-6837.patch
core-unlink-the-invocation-id-key-if-cannot-change-keyrin.patch
debian/Use-Debian-specific-config-files.patch
debian/don-t-try-to-start-autovt-units-when-not-running-wit.patch
debian/Make-logind-hostnamed-localed-timedated-D-Bus-activa.patch
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment