From 16fcb1918a0e4514b1d0d78259d4ffd646b5da59 Mon Sep 17 00:00:00 2001 From: Tobias Kaufmann Date: Fri, 31 Jul 2020 10:57:15 +0200 Subject: [PATCH 1/2] core: fix comments on ambient capabilities The comments on the code for ambient capabilities was wrong/outdated. --- src/core/execute.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index 2d0db6d3a6..a04c8801c9 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -4255,8 +4255,16 @@ static int exec_child( } } - /* This is done before enforce_user, but ambient set - * does not survive over setresuid() if keep_caps is not set. */ + /* Ambient capabilities are cleared during setresuid() (in enforce_user()) even with + * keep-caps set. + * To be able to raise the ambient capabilities after setresuid() they have to be + * added to the inherited set and keep caps has to be set (done in enforce_user()). + * After setresuid() the ambient capabilities can be raised as they are present in + * the permitted and inhertiable set. However it is possible that someone wants to + * set ambient capabilities without changing the user, so we also set the ambient + * capabilities here. + * The requested ambient capabilities are raised in the inheritable set if the + * second argument is true. */ if (!needs_ambient_hack) { r = capability_ambient_set_apply(context->capability_ambient_set, true); if (r < 0) { @@ -4282,7 +4290,7 @@ static int exec_child( if (!needs_ambient_hack && context->capability_ambient_set != 0) { - /* Fix the ambient capabilities after user change. */ + /* Raise the ambient capabilities after user change. */ r = capability_ambient_set_apply(context->capability_ambient_set, false); if (r < 0) { *exit_status = EXIT_CAPABILITIES; From 198dc1784514b800c3e69153839084a55ebf2e84 Mon Sep 17 00:00:00 2001 From: Tobias Kaufmann Date: Fri, 31 Jul 2020 10:57:39 +0200 Subject: [PATCH 2/2] core: fix set keep caps for ambient capabilities The securebit keep-caps retains the capabilities in the permitted set over an UID change (ambient capabilities are cleared though). Setting the keep-caps securebit after the uid change and before execve doesn't make sense as it is cleared during execve and there is no additional user ID change after this point. Altough the documentation (man 7 capabilities) is ambigious, keep-caps is reset during execve although keep-caps-locked is set. After execve only keep-caps-locked is set and keep-caps is cleared. --- src/core/execute.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index a04c8801c9..694033e82c 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -4296,15 +4296,6 @@ static int exec_child( *exit_status = EXIT_CAPABILITIES; return log_unit_error_errno(unit, r, "Failed to apply ambient capabilities (after UID change): %m"); } - - /* If we were asked to change user and ambient capabilities - * were requested, we had to add keep-caps to the securebits - * so that we would maintain the inherited capability set - * through the setresuid(). Make sure that the bit is added - * also to the context secure_bits so that we don't try to - * drop the bit away next. */ - - secure_bits |= 1<