-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
[master] Fix the SELinux context for Salt Minion service #66780
base: master
Are you sure you want to change the base?
[master] Fix the SELinux context for Salt Minion service #66780
Conversation
Currently there are no SELinux policies for Salt. By default, the Salt Minion service runs as 'unconfined_service_t' when SELinux is enabled. This works fine in most cases but generates a problem then trying to transition to an 'unconfined_t', i.a. when running "cmd.run .... runas=nobody". Then we see this denied in audit logs: type=AVC msg=audit(1722870119.142:718): avc: denied { transition } for pid=3421 comm="su" path="/usr/bin/bash" dev="vda3" ino=28565 scontext=system_u:system_r:unconfined_service_t:s0 tcontext=unconfined_u:unconfined_r:unconfined_t:s0 tclass=process permissive=0 (This happens for cmd.run at the time of trying to invoke a shell as a different user to gather the environment variables from this particular user) Fixing the SELinuxContext for the Salt Minion systemd service to a general 'unconfined_t' workarounds this situation. SELinuxContext attribute was added on systemd version 209.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good, but you have checked the tick-box that tests were written and I see none in the 'Files Changed' ?. Need tests to exercise the changes.
FYI - RHEL 7 is EOL and no longer supported.
Also, the changes are for the master branch, if you want to see them quicker, I would suggest using the 3006.x branch.
Thanks for the review @dmurphy18 and sorry about the checkbox, that was a mistake! I've just added a test now. BTW, since you mention to target Thanks for clarifying! |
@meaksh Recommend 3006.x branch since the master branch is where all the salt-extensions work is going to be done, hence it will be a while before that work gets released, hence if you wanted it sooner, 3006.x is the better place. Will review your updates later today and recommend using 3006.x |
What does this PR do?
IMPORTANT: This error is only seen when
salt-minion
is a service started by systemd, as it getsunconfined_service_t
context. It doesn't happen ifsalt-minion
is executed manually orsalt-call
is used.NOTE: Currently there are no SELinux policies for Salt. Until we have a dedicated SELinux domain for Salt Minion, this PR implements a workaround to this issue.
By default, the Salt Minion systemd service runs as
unconfined_service_t
when SELinux is enabled. This works fine in most cases but generates a problem then trying to transition to anunconfined_t
domain, i.a. when runningcmd.run env runas=nobody
.Then we see this denied in audit logs:
(This happens for "cmd.run" at the time of trying to invoke a shell as a different user to gather the environment variables from this particular user)
The issue produces an ERROR message in the Salt logs but the command execution usually works fine (as long as your command is not relying on any of the missing environment variables, as they are not really in place at the time of executing the command)
Fixing the SELinuxContext for the Salt Minion systemd service to a general
unconfined_t
workarounds this situation.SELinuxContext attribute was added on systemd version 209. If SELinux is disabled or not installed, then this directive is just ignored: https://www.freedesktop.org/software/systemd/man/latest/systemd.exec.html#SELinuxContext=
When systemd version is lower than 209, like in an old CentOS 7.0, having a service with
SELinuxContext
directive will produces this message in the journal at "daemon-reload":But it does not produce any error and the service is able to start.
What issues does this PR fix or reference?
Fixes #66779
Previous Behavior
An ERROR message is produced in the Salt logs and denied AVC message on
/var/log/audit.log
.New Behavior
No denied message or error seen.
Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Commits signed with GPG?
Yes
Please review Salt's Contributing Guide for best practices, including the
PR Guidelines.
See GitHub's page on GPG signing for more information about signing commits with GPG.