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

selinux: update kernel boot params when disabling/re-enabling SELinux #142

Merged

Conversation

WOnder93
Copy link
Contributor

SUMMARY

The ability to disable SELinux from userspace based on the configuration
file is being deprecated in favor of the selinux=0 kernel boot
parameter. (Note that this affects only the "full" disable; switching
to/from permissive mode will work the same as before.)

Therefore, enhance the selinux module to try to set/unset the kernel
command-line parameter using grubby when enabling/disabling SELinux.

If the grubby package is not present on the system, the module will only
update the config file and report a warning. Note that even with the
runtime disable functionality removed, setting SELINUX=disabled in the
config file will lead to a system with no SELinux policy loaded, which
will behave in a very similar way as if SELinux was fully disabled, only
there could still be some minor performance impact, since the kernel
hooks will still be active.

More information:
https://lore.kernel.org/selinux/157836784986.560897.13893922675143903084.stgit@chester/
https://fedoraproject.org/wiki/Changes/Remove_Support_For_SELinux_Runtime_Disable

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

selinux module

@Akasurde
Copy link
Member

Akasurde commented Mar 2, 2021

recheck

@WOnder93 WOnder93 force-pushed the selinux-disable-kernel branch 2 times, most recently from dab5587 to 53a6fa0 Compare May 18, 2021 21:01
Copy link
Member

@Akasurde Akasurde left a comment

Choose a reason for hiding this comment

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

Few comments.

@WOnder93 WOnder93 force-pushed the selinux-disable-kernel branch from 53a6fa0 to 33e33c0 Compare June 23, 2021 08:21
@WOnder93
Copy link
Contributor Author

@Akasurde Thanks for the review, both comments should now be addressed. I solved the grubby_bin a little differently, since both the get and set methods need to use it. I also changed it to emit only a warning if the binary is not found (and skip the kernel cmdline config in that case). Please let me know if you're OK with the current code.

@Akasurde
Copy link
Member

cc @quidame @saito-hideki @justjais Could you please review this? Thanks in advance.

Copy link
Member

@Akasurde Akasurde left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@quidame quidame left a comment

Choose a reason for hiding this comment

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

@WOnder93 thanks for implementing this !

As explained right below, I would prefer to see this new feature as optional and explicit, with a dedicated module option :)

plugins/modules/selinux.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@saito-hideki saito-hideki left a comment

Choose a reason for hiding this comment

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

@WOnder93 thank you for the PR!
I have commented about the module description and the item of the result of the module. I hope this helps!

plugins/modules/selinux.py Show resolved Hide resolved
@WOnder93 WOnder93 force-pushed the selinux-disable-kernel branch from 33e33c0 to 80125f1 Compare August 18, 2021 11:22
@WOnder93 WOnder93 force-pushed the selinux-disable-kernel branch from 80125f1 to d465945 Compare September 8, 2021 08:57
@WOnder93 WOnder93 force-pushed the selinux-disable-kernel branch from d465945 to 5a8aa73 Compare September 10, 2021 08:56
Copy link
Collaborator

@saito-hideki saito-hideki left a comment

Choose a reason for hiding this comment

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

LGTM! :)

@WOnder93
Copy link
Contributor Author

@Akasurde @saito-hideki @maxamillion Is there anything else to be done before merging this? I'd be glad if i could finally tick this off as done :)

The ability to disable SELinux from userspace based on the configuration
file is being deprecated in favor of the selinux=0 kernel boot
parameter. (Note that this affects only the "full" disable; switching
to/from permissive mode will work the same as before.)

Therefore, add an 'update_kernel_param' module parameter that will cause
it to set/unset the kernel command-line parameter using grubby when
enabling/disabling SELinux. (An explicit parameter was chosen for
backwards compatibility.)

More information:
https://lore.kernel.org/selinux/157836784986.560897.13893922675143903084.stgit@chester/
https://fedoraproject.org/wiki/Changes/Remove_Support_For_SELinux_Runtime_Disable

Signed-off-by: Ondrej Mosnacek <[email protected]>
@Akasurde Akasurde force-pushed the selinux-disable-kernel branch from 5a8aa73 to 53d47e1 Compare September 20, 2021 04:40
@Akasurde
Copy link
Member

@Akasurde @saito-hideki @maxamillion Is there anything else to be done before merging this? I'd be glad if i could finally tick this off as done :)

Sure. I triggered the build.

@Akasurde
Copy link
Member

recheck

2 similar comments
@saito-hideki
Copy link
Collaborator

recheck

@saito-hideki
Copy link
Collaborator

recheck

@saito-hideki saito-hideki added the gate Gate PR in Zuul CI (Obsolete: Please set "mergeit" instead of "gate") label Sep 24, 2021
Copy link
Contributor

@ansible-zuul ansible-zuul bot left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gate Gate PR in Zuul CI (Obsolete: Please set "mergeit" instead of "gate")
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants