Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should DevicePolicy affect execution of processes privileged using +? #26035

Closed
helmutg opened this issue Jan 12, 2023 · 3 comments · Fixed by #26065
Closed

Should DevicePolicy affect execution of processes privileged using +? #26035

helmutg opened this issue Jan 12, 2023 · 3 comments · Fixed by #26065
Assignees
Milestone

Comments

@helmutg
Copy link
Contributor

helmutg commented Jan 12, 2023

systemd version the issue has been seen with

252.4

Used distribution

Debian unstable

Linux kernel version used

No response

CPU architectures issue was seen on

None

Component

systemd-run

Expected behaviour you didn't see

No response

Unexpected behaviour you saw

No response

Steps to reproduce the problem

Consider the following invocations:

1$ systemd-run -p DevicePolicy=closed -p "ExecStartPre=/bin/sh -c ':>/dev/autofs'" true
2$ systemd-run -p "ExecStartPre=/bin/sh -c ':>/dev/autofs'" true
3$ systemd-run -p DevicePolicy=closed -p "ExecStartPre=+/bin/sh -c ':>/dev/autofs'" true

Expectation vs reality:

  1. I expect this to fail the shell invocation, because the default device policy denies the device in question. It actually gets -EPERM on the open call. This works as expected.
  2. I expect that his invocation works, because no device policy prevents the opening of /dev/autofs. It works as expected.
  3. My and @bluca's intuition is that the + should disable sandboxing, so the DevicePolicy should not affect the shell. We actually see the same -EPERM as in the first case.

That raises the question of whether private devices should affect processes that have been privileged using the + prefix. The documentation is not clear about this aspect. It generally says that + causes the process to be executed "with full privileges" (in systemd.service). A number of directives (but not DevicePolicy or DeviceAllow) explain how + is affected by them. That leaves the intended semantics a bit unclear. If this behavior is intended, I suggest extending systemd.resource-control with a note explaining its interaction with +.

Additional program output to the terminal or log subsystem illustrating the issue

No response

@helmutg helmutg added the bug 🐛 Programming errors, that need preferential fixing label Jan 12, 2023
@github-actions github-actions bot added the run label Jan 12, 2023
@bluca bluca self-assigned this Jan 12, 2023
@bluca
Copy link
Member

bluca commented Jan 15, 2023

So as it turns out, we never consider + for cgroup-based sandboxing options (devices, network). This is probably an oversight. It might be acceptable to fix it, given in practice it would relax the policy - ie nothing should stop working, as the implementation is stricter than it should be and we'd make it more open

@bluca bluca added pid1 and removed run labels Jan 15, 2023
@bluca
Copy link
Member

bluca commented Jan 15, 2023

Actually thinking about it, these are cgroup-based options, so disabling them would affect every process in the cgroup. The rest are process based, so each Exec* can have different settings. I'll clarify the docs.

bluca added a commit to bluca/systemd that referenced this issue Jan 15, 2023
DeviceAllow= and others are applied to the whole cgroup via bpf, so
using '+' on an Exec line will not bypass them. Explain this in the
manpage.

Fixes systemd#26035
bluca added a commit to bluca/systemd that referenced this issue Jan 15, 2023
DeviceAllow= and others are applied to the whole cgroup via bpf, so
using '+' on an Exec line will not bypass them. Explain this in the
manpage.

Fixes systemd#26035
@poettering
Copy link
Member

So, we really can't fix this, since this restrictions are cgroup-bound, not process bound.

We should just document this (both on the DeviceAllow= documentation and on the Exec* documentation) and accept it.

@poettering poettering added documentation and removed bug 🐛 Programming errors, that need preferential fixing labels Jan 16, 2023
@poettering poettering added this to the v253 milestone Jan 16, 2023
bluca added a commit to bluca/systemd that referenced this issue Jan 18, 2023
DeviceAllow= and others are applied to the whole cgroup via bpf, so
using '+' on an Exec line will not bypass them. Explain this in the
manpage.

Fixes systemd#26035
bluca added a commit to bluca/systemd that referenced this issue Jan 18, 2023
DeviceAllow= and others are applied to the whole cgroup via bpf, so
using '+' on an Exec line will not bypass them. Explain this in the
manpage.

Fixes systemd#26035
bluca added a commit that referenced this issue Jan 18, 2023
DeviceAllow= and others are applied to the whole cgroup via bpf, so
using '+' on an Exec line will not bypass them. Explain this in the
manpage.

Fixes #26035
d-hatayama pushed a commit to d-hatayama/systemd that referenced this issue Feb 15, 2023
DeviceAllow= and others are applied to the whole cgroup via bpf, so
using '+' on an Exec line will not bypass them. Explain this in the
manpage.

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

Successfully merging a pull request may close this issue.

3 participants