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

[master] Fix the SELinux context for Salt Minion service #66780

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

meaksh
Copy link
Contributor

@meaksh meaksh commented Aug 6, 2024

What does this PR do?

IMPORTANT: This error is only seen when salt-minion is a service started by systemd, as it gets unconfined_service_t context. It doesn't happen if salt-minion is executed manually or salt-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 an unconfined_t domain, i.a. when running cmd.run env 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)

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":

systemd[1]: [/usr/lib/systemd/system/firewalld.service:17] Unknown lvalue 'SELinuxContext' in section 'Service'

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

minion # su -c env nobody
[... all expected environment variables ...]

master # salt MINION cmd.run env runas=nobody
[... NOT all expected environment variables ..]

An ERROR message is produced in the Salt logs and denied AVC message on /var/log/audit.log.

New Behavior

minion # su -c env nobody
[... all expected environment variables ...]

master # salt MINION cmd.run env runas=nobody
[... all expected environment variables ...]

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.

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.
Copy link
Contributor

@dmurphy18 dmurphy18 left a 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.

@meaksh
Copy link
Contributor Author

meaksh commented Aug 7, 2024

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 3006.x branch.. did the PR policy change here? Should we take care of creating the backports to the respective release branches? Or, in case we create a single PR to 3006.x, are you taking care of forwarding this to master branch?

Thanks for clarifying!

@dmurphy18
Copy link
Contributor

@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.
Yes, we are doing merge-forwards from 3006.x to 3007.x and master.

Will review your updates later today and recommend using 3006.x

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

Successfully merging this pull request may close these issues.

[BUG] Triggering "cmd.run" with "runas" on SELinux enabled minion produces ERROR and denied message
2 participants