Skip to content

Conversation

steelman
Copy link
Contributor

Neither ambient capabilities should be passed implicitly to user
services. Dropping them does not affect the permitted and effective sets
which are important for the manager itself to operate.

@steelman
Copy link
Contributor Author

steelman commented Jul 12, 2022

The inheritable set is also left intact (at least for now). Clearing it renders file capabilities partially useless. Although filecapped binaries can gain permitted and effective capabilities, without inheriting the inheritable set from systemd they won't be able to pass them down (as long as they are capability-dumb i.e. they don't manipulate their capabilities with libcap) to their children, which as of now is possible. Of course, setting AmbientCapabilities in appropriate service files solves the problem, but on working systems using file capabilities migration may be a bit of a pain at least.

https://lists.freedesktop.org/archives/systemd-devel/2022-July/048070.html

@yuwata yuwata added the pid1 label Jul 13, 2022
@steelman
Copy link
Contributor Author

@yuwata does pid1 label mean only the system manager or does it cover systemd --user too? Because this patch is meant for the user manager.

@yuwata
Copy link
Member

yuwata commented Jul 13, 2022

@yuwata does pid1 label mean only the system manager or does it cover systemd --user too? Because this patch is meant for the user manager.

Nice catch.

src/core/main.c Outdated
@@ -2816,6 +2816,10 @@ int main(int argc, char *argv[]) {
/* clear the kernel timestamp, because we are not PID 1 */
kernel_timestamp = DUAL_TIMESTAMP_NULL;

/* Clear ambient capabilities, so services do not inherit
them implicitly. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please add the comment you added to the commit msg here, i.e. that dropping the ambient caps doesn't affect the caps effective for the service manager itself, but only what it passes further down. this is important info for the reader of the code.

(btw, we line break at 109chars these days)

@poettering poettering added the reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks label Jul 15, 2022
@steelman steelman force-pushed the drop-ambient-caps branch 2 times, most recently from 42f75af to cfab788 Compare July 15, 2022 07:27
@poettering poettering changed the title RFC: core: drop ambient capabilities in user manager core: drop ambient capabilities in user manager Jul 15, 2022
@poettering poettering added good-to-merge/with-minor-suggestions and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Jul 15, 2022
@poettering
Copy link
Member

can you drop the "RFC" from the commit msg?

@poettering
Copy link
Member

lgtm, otherwise

Ambient capabilities should not be passed implicitly to user
services. Dropping them does not affect the permitted and effective sets
which are important for the manager itself to operate.
@steelman steelman force-pushed the drop-ambient-caps branch from cfab788 to f56dd99 Compare July 15, 2022 08:21
@poettering poettering merged commit 963b6b9 into systemd:main Jul 15, 2022
@steelman steelman deleted the drop-ambient-caps branch July 18, 2022 20:40
@daiaji
Copy link

daiaji commented Nov 3, 2023

So actually, I can't use AmbientCapabilities in the user service, which sounds unfortunate, I do want to allow some programs in the service to use some privileges when running as an unprivileged user, specifically podman.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

6 participants