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

systemd (initrc_t) does not domain transition when executing shell_exec_t #778

Closed
cgwalters opened this issue Jun 14, 2021 · 13 comments
Closed

Comments

@cgwalters
Copy link
Contributor

See https://bugzilla.redhat.com/show_bug.cgi?id=1956836#c35

[root@cosa-devsh ~]# cat /etc/systemd/system/bash-id.service 
[Service]
Type=oneshot
RemainAfterExit=yes
ExecStart=/bin/bash -c 'id'
[root@cosa-devsh ~]# cat /etc/systemd/system/direct-id.service
[Service]
Type=oneshot
RemainAfterExit=yes
ExecStart=id
[root@cosa-devsh ~]# journalctl -u bash-id -u direct-id
-- Logs begin at Mon 2021-06-14 21:00:52 UTC, end at Mon 2021-06-14 21:10:02 UTC. --
Jun 14 21:09:53 cosa-devsh systemd[1]: Starting bash-id.service...
Jun 14 21:09:53 cosa-devsh bash[4569]: uid=0(root) gid=0(root) groups=0(root) context=system_u:system_r:initrc_t:s0
Jun 14 21:09:53 cosa-devsh systemd[1]: Finished bash-id.service.
Jun 14 21:10:02 cosa-devsh systemd[1]: Starting direct-id.service...
Jun 14 21:10:02 cosa-devsh id[4575]: uid=0(root) gid=0(root) groups=0(root) context=system_u:system_r:unconfined_service_t:s0
Jun 14 21:10:02 cosa-devsh systemd[1]: Finished direct-id.service.
[root@cosa-devsh ~]# 

And this is true on FCOS too, it has nothing to do with the label of /usr/local/sbin.

Invoking things via bash is suppressing the domain transition. This seems like a general selinux-policy issue, though I am
not sure if it's an intentional design decision or not. I find this very surprising and it's going to be a recurring source of bugs.

Is this intentional?

@cgwalters
Copy link
Contributor Author

I also triple checked this by doing cp /usr/bin/bash /usr/local/bin/myshell (where myshell is bin_t not shell_exec_t) and as expected ExecStart=/usr/local/bin/myshell -c id is unconfined_service_t.

@zpytela
Copy link
Contributor

zpytela commented Jun 15, 2021

See https://bugzilla.redhat.com/show_bug.cgi?id=1956836#c35

[root@cosa-devsh ~]# cat /etc/systemd/system/bash-id.service 
[Service]
Type=oneshot
RemainAfterExit=yes
ExecStart=/bin/bash -c 'id'
[root@cosa-devsh ~]# cat /etc/systemd/system/direct-id.service
[Service]
Type=oneshot
RemainAfterExit=yes
ExecStart=id
[root@cosa-devsh ~]# journalctl -u bash-id -u direct-id
-- Logs begin at Mon 2021-06-14 21:00:52 UTC, end at Mon 2021-06-14 21:10:02 UTC. --
Jun 14 21:09:53 cosa-devsh systemd[1]: Starting bash-id.service...
Jun 14 21:09:53 cosa-devsh bash[4569]: uid=0(root) gid=0(root) groups=0(root) context=system_u:system_r:initrc_t:s0
Jun 14 21:09:53 cosa-devsh systemd[1]: Finished bash-id.service.
Jun 14 21:10:02 cosa-devsh systemd[1]: Starting direct-id.service...
Jun 14 21:10:02 cosa-devsh id[4575]: uid=0(root) gid=0(root) groups=0(root) context=system_u:system_r:unconfined_service_t:s0
Jun 14 21:10:02 cosa-devsh systemd[1]: Finished direct-id.service.
[root@cosa-devsh ~]# 

And this is true on FCOS too, it has nothing to do with the label of /usr/local/sbin.

Invoking things via bash is suppressing the domain transition. This seems like a general selinux-policy issue, though I am
not sure if it's an intentional design decision or not. I find this very surprising and it's going to be a recurring source of bugs.

Is this intentional?

I don't know if it was made intentionally, but I know it is used as it is now. Any change like the suggested transition needs to be considered very carefully.

@cgwalters
Copy link
Contributor Author

I don't know if it was made intentionally, but I know it is used as it is now. Any change like the suggested transition needs to be considered very carefully.

Agree. I think what we may need to do in the short term is add some sort of easy opt-in way to do this. I guess today one can explicitly:

[Service]
Type=oneshot
RemainAfterExit=yes
ExecStart=/bin/bash -c 'id'
SELinuxContext=system_u:system_r:unconfined_service_t:s0

But I think that's hardcoding too much of the policy in the unit. Hmm. Maybe something like SELinuxContext=/usr/bin to basically do a domain transition as if we were executing a file of the target type (i.e. bin_t)?

@WOnder93
Copy link
Member

Unless I'm mistaken, initrc_t is not the domain of systemd (that's init_t), but rather a generic domain for what used to be the "rc" scripts (i.e. the stuff in /etc/rc.d and friends). So I think it makes sense for this transition (type_transition init_t shell_exec_t:process initrc_t;) to exist, even if it's kind of a legacy artifact from the pre-systemd age. Or at least it's not as bad as you make it out to be (i.e. that there is no transition at all and the script runs with the same context as systemd).

(Note: The above is based on assumptions and may not be accurate - my UNIX/SELinux beard is not that long and I don't know the full history behind all this :)

If you need to enforce a specific transition for your shell-based service, you should simply put the commands into an executable file with a shell shebang, label that file with the appropriate type, and execute that file directly from the systemd unit (ExecStart=/path/to/script.sh, but not ExecStart=bash /path/to/script.sh).

@cgwalters
Copy link
Contributor Author

If you need to enforce a specific transition for your shell-based service, you should simply put the commands into an executable file with a shell shebang, label that file with the appropriate type, and execute that file directly from the systemd unit (ExecStart=/path/to/script.sh, but not ExecStart=bash /path/to/script.sh).

Yes...the tricky case is more that it's often convenient to inline ExecStart=bash -c "some code here" and doing what you're saying requires creating a new file separate from the unit. (Which, is a best practice but gets annoying for "one liners")

cgwalters added a commit to cgwalters/machine-config-operator that referenced this issue Jun 16, 2021
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1956836

Unfortunately right now, executing `bash` skips a domain transition
(see fedora-selinux/selinux-policy#778)
so the way we're sourcing the script means we stay in `initrc_t`
and end up triggering a SELinux policy denial.

(BTW this denial turns out to just delay the successful exit
 of the script, which will then end up just delaying kubelet start.
 it's otherwise harmless, but we also don't want SELinux policy denials
 in our product by default)

Fix this in two ways:

- First, just move the thing to `/usr/local/bin` to avoid issues
  with labeling of `/usr/local/sbin` that were fixed in openshift/os#551
- Second, rework it to be executed directly

While we're here:

- Clean the confusing+outdated comment about being a NM dispatcher
- Drop the `logger` bit which was only necessary as a NM dispatcher;
  since we're *always* running under systemd, this makes `journalctl -u node-valid-hostname`
  actually show the script output.
- Make it crystal clear that the "truncate hostname" is only run in GCP.
- Fix various typos
- Use the more precise term "non-localhost" in various places instead of the more ambiguous
  terms "real"/"valid"
@zpytela
Copy link
Contributor

zpytela commented Jun 18, 2021

Ondrej is correct. Frankly, I don't know how we can help you further. For instant testing, you can use transient services:

# systemd-run id; systemd-run bash -c id
Running as unit: run-r4f5e533a1a534fda8176c8d719cfab86.service
Running as unit: run-r07a08c61062140fb989e311b445a48d0.service

# journalctl -u run-r4f5e533a1a534fda8176c8d719cfab86.service -u run-r07a08c61062140fb989e311b445a48d0.service
-- Logs begin at Tue 2021-02-16 19:24:29 CET, end at Fri 2021-06-18 17:36:44 CEST. --
Jun 18 17:36:44 luggage systemd[1]: Started /usr/bin/id.
Jun 18 17:36:44 luggage id[75690]: uid=0(root) gid=0(root) skupiny=0(root) kontext=system_u:system_r:unconfined_service_t:s0
Jun 18 17:36:44 luggage systemd[1]: run-r4f5e533a1a534fda8176c8d719cfab86.service: Succeeded.
Jun 18 17:36:44 luggage systemd[1]: Started /usr/bin/bash -c id.
Jun 18 17:36:44 luggage bash[75692]: uid=0(root) gid=0(root) skupiny=0(root) kontext=system_u:system_r:initrc_t:s0
Jun 18 17:36:44 luggage systemd[1]: run-r07a08c61062140fb989e311b445a48d0.service: Succeeded.

@zpytela
Copy link
Contributor

zpytela commented Nov 3, 2021

@cgwalters Currently I don't have anything else to comment on. If you don't mind, I'll close this issue.

@cgwalters
Copy link
Contributor Author

Well, I think this comment still stands - it'd be nice to have some easy way to force the domain transition for "one liners".

Or perhaps even better, try to eventually flip the default and force anyone who wants the old behavior to opt-in to it.

Something short and declarative in unit file syntax perhaps like:

SELinuxTransition=default|legacy

And linting tools could start emitting a warning for any systemd units which ExecStart=/bin/(ba)sh ... In a quick grep in this shell,

$ grep -rE 'ExecStart=.*/bin/(ba)?sh' /usr/lib/systemd/system/
/usr/lib/systemd/system/nfs-server.service:ExecStart=-/bin/sh -c 'if systemctl -q is-active gssproxy; then systemctl reload gssproxy ; fi'
/usr/lib/systemd/system/iscsi-init.service:ExecStart=/usr/bin/sh -c 'echo "InitiatorName=`/usr/sbin/iscsi-iname`" > /etc/iscsi/initiatorname.iscsi'
/usr/lib/systemd/system/man-db-cache-update.service:ExecStart=/bin/sh -c '[ "$SERVICE" != "no" ] && /usr/bin/mandb $OPTS || true'
/usr/lib/systemd/system/debug-shell.service:ExecStart=/bin/sh
$

Which admittedly isn't much. But this issue sure confused the heck out of me when working on some systemd units in OpenShift.

@zpytela
Copy link
Contributor

zpytela commented Nov 5, 2021

I think my notes are still valid, too.
For transient services or one-liners, systemd-run or also runcon can be used:

# runcon -r system_r -t unconfined_service_t bash -c id
uid=0(root) gid=0(root) groups=0(root) context=unconfined_u:system_r:unconfined_service_t:s0-s0:c0.c1023

If you are not satisfied with using tools like these, let's restart and describe what is the actual goal.

@cgwalters
Copy link
Contributor Author

For the case I hit in OpenShift we wanted to define a static service, not a dynamic one via systemd-run.

I think the point I'm trying to make here is the current behavior did probably make sense in the early days of the SysV ➡️ systemd transition, but now seems like a relic. In other words, I'd say only pid 1 should be running as initrc_t and we should think of this "bash special casing" as legacy and move to get things away from it.

We shouldn't hardcode runcon -t unconfined_service_t in upstream systemd units, which also applies to the systemd SELinuxContext= configuration option.

We could try to push everything to execute code via a separate binary. But it's annoying for "one liners".

Hence my argument for something simple that allows opting-in.

@cgwalters
Copy link
Contributor Author

I guess taking my argument out, even the very naming is a legacy now, i.e. it should be systemd_t - and if we think of that way, then not having bash be systemd_t seems like a good thing to me.

@ghost
Copy link

ghost commented Nov 5, 2021 via email

@zpytela
Copy link
Contributor

zpytela commented Jan 7, 2022

Closing this issue as no new information has appeared since 2 months ago.

@zpytela zpytela closed this as completed Jan 7, 2022
cgwalters added a commit to cgwalters/assisted-service that referenced this issue Mar 21, 2023
This will reportedly fix a SELinux issue that happens with the RHEL 9 rebase for OCP 4.13, where the dispatcher policy is a bit tighter for scripts by default.

See also fedora-selinux/selinux-policy#778 which is highly related to this - the policy is stricter for bash versus executables.
openshift-merge-robot pushed a commit to openshift/assisted-service that referenced this issue Mar 21, 2023
This will reportedly fix a SELinux issue that happens with the RHEL 9 rebase for OCP 4.13, where the dispatcher policy is a bit tighter for scripts by default.

See also fedora-selinux/selinux-policy#778 which is highly related to this - the policy is stricter for bash versus executables.
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/assisted-service that referenced this issue Mar 21, 2023
This will reportedly fix a SELinux issue that happens with the RHEL 9 rebase for OCP 4.13, where the dispatcher policy is a bit tighter for scripts by default.

See also fedora-selinux/selinux-policy#778 which is highly related to this - the policy is stricter for bash versus executables.
openshift-merge-robot pushed a commit to openshift/assisted-service that referenced this issue Mar 21, 2023
This will reportedly fix a SELinux issue that happens with the RHEL 9 rebase for OCP 4.13, where the dispatcher policy is a bit tighter for scripts by default.

See also fedora-selinux/selinux-policy#778 which is highly related to this - the policy is stricter for bash versus executables.

Co-authored-by: Colin Walters <walters@verbum.org>
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/assisted-service that referenced this issue Mar 28, 2023
This will reportedly fix a SELinux issue that happens with the RHEL 9 rebase for OCP 4.13, where the dispatcher policy is a bit tighter for scripts by default.

See also fedora-selinux/selinux-policy#778 which is highly related to this - the policy is stricter for bash versus executables.
openshift-merge-robot pushed a commit to openshift/assisted-service that referenced this issue Mar 28, 2023
This will reportedly fix a SELinux issue that happens with the RHEL 9 rebase for OCP 4.13, where the dispatcher policy is a bit tighter for scripts by default.

See also fedora-selinux/selinux-policy#778 which is highly related to this - the policy is stricter for bash versus executables.

Co-authored-by: Colin Walters <walters@verbum.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants