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.

Sign up for GitHub

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

Jump to bottom

SLE platforms use drop in file for sysctl variables for SLE platforms #10367

Merged
merged 6 commits into from
Apr 4, 2023
Merged

SLE platforms use drop in file for sysctl variables for SLE platforms #10367

merged 6 commits into from
Apr 4, 2023

Conversation

teacup-on-rockingchair
Copy link
Contributor

Description:

  • Instead of modifying the global /etc/sysctl.conf file in sysctl bash template, add the modification in a separate file for each sysctl variables

Rationale:

  • Modification recommended by system engineers with the motivation, this approach provides cleaner setup for support and upgrade the system

@teacup-on-rockingchair
Use drop in file for sysctl variables for SLE platforms
b20efdc 
Instead of modifying the global /etc/sysctl.conf file for each one of them
Modification recommended by system engineers with the motivation, this approach provides cleaner setup for support and upgrade the system
@openshift-ci openshift-ci bot added the needs-ok-to-test Used by openshift-ci bot. label Mar 26, 2023
@openshift-ci
Copy link

openshift-ci bot commented Mar 26, 2023

Hi @teacup-on-rockingchair. Thanks for your PR.

I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@github-actions
Copy link

Start a new ephemeral environment with changes proposed in this pull request:

Fedora Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

@marcusburghardt marcusburghardt added this to the 0.1.68 milestone Mar 28, 2023
@marcusburghardt marcusburghardt added Bash Bash remediation update. SLES SUSE Linux Enterprise Server product related. labels Mar 28, 2023
@marcusburghardt
Copy link
Member

I believe this approach would be good for other products as well. If so, we can remove the conditional for sle products and keep it as default. @yuumasato what do you think? If I am not wrong, you have worked in this template recently.

@yuumasato yuumasato self-assigned this Mar 28, 2023
yuumasato
yuumasato requested changes Mar 28, 2023
View reviewed changes
Copy link
Member

@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

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

@teacup-on-rockingchair Do you plan to make the changes to Ansible as well?
And please, update the behavior of the template in the documentation as well.

shared/templates/sysctl/bash.template Outdated Show resolved Hide resolved
shared/templates/sysctl/bash.template Outdated Show resolved Hide resolved
@teacup-on-rockingchair
Copy link
Contributor Author

teacup-on-rockingchair commented Mar 28, 2023 •
edited by yuumasato
Loading

@teacup-on-rockingchair Do you plan to make the changes to Ansible as well? And please, update the behavior of the template in the documentation as well.

Just to be sure we are on the same page here, to update the documentation that for sle the sysctl configuration goes into a var-specific file right?

yuumasato: Sorry, I edited your comment by mistake

@yuumasato
Copy link
Member

@teacup-on-rockingchair Do you plan to make the changes to Ansible as well? And please, update the behavior of the template in the documentation as well.

Just to be sure we are on the same page here, to update the documentation that for sle the sysctl configuration goes into a var-specific file right?

Yes.
If the property is not added, this would be about stating that the sysctl template remediates into the drop-in file for SLE.
But if the property is added, it would be about mentioning the property, and what it does when defined.

@codeclimate
Copy link

codeclimate bot commented Apr 2, 2023

Code Climate has analyzed commit d14940f and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 51.8% (-0.1% change).

View more on Code Climate.

@teacup-on-rockingchair teacup-on-rockingchair requested a review from a team as a code owner April 2, 2023 07:58
@teacup-on-rockingchair
Use sysctl_remediate_drop_in_file property
ab54b42 
Instead of using per product logn conditions in templates and rules/fixes, use property defined on per product basis.
Thanks to @yuumasato for the hint
@teacup-on-rockingchair teacup-on-rockingchair force-pushed the sysctl_rules_dropin_file_remediation branch from c1d8884 to ab54b42 Compare April 2, 2023 08:06
yuumasato
yuumasato requested changes Apr 3, 2023
View reviewed changes
Copy link
Member

@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

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

@teacup-on-rockingchair Thank you for adding the property.

ssg/products.py Show resolved Hide resolved
yuumasato
yuumasato approved these changes Apr 4, 2023
View reviewed changes
Copy link
Member

@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

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

Thank you for adding the template customization property.

@yuumasato
Copy link
Member

Based on #10417, I'm waving Gate / Build, Test on Fedora Latest .

@yuumasato yuumasato merged commit 557f974 into ComplianceAsCode:master Apr 4, 2023
yuumasato added a commit to yuumasato/scap-security-guide that referenced this pull request Jul 31, 2023
@yuumasato
sysctl_kernel_kptr_restrict is PASS by default
e48bed9 
In ComplianceAsCode#10367 the sysctl template was updated to check whether a
sysctl is set on /usr/lib/sysctl.d without being overwritten.

kernel.kptr_restrict is set to 1 by default in
/usr/lib/sysctl.d/50-redhat.conf

Before, the template didn't check for sysctls in that path and resulted in
Fail evaluation, requiring remediation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yuumasato yuumasato yuumasato approved these changes

@ggbecker ggbecker Awaiting requested review from ggbecker

Assignees

@yuumasato yuumasato

Labels
Bash Bash remediation update. needs-ok-to-test Used by openshift-ci bot. SLES SUSE Linux Enterprise Server product related.
Projects
None yet
Milestone
0.1.68
Development

Successfully merging this pull request may close these issues.
3 participants
@teacup-on-rockingchair @marcusburghardt @yuumasato