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

Fix kernel.core_pattern with empty string rule #9396

Merged

Conversation

ggbecker
Copy link
Member

Description:

  • Fix kernel.core_pattern with empty string rule
    • Create custom content for core_pattern with empty string as current template does not work with empty string.

@ggbecker ggbecker added the bugfix Fixes to reported bugs. label Aug 24, 2022
@ggbecker ggbecker added this to the 0.1.64 milestone Aug 24, 2022
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Aug 24, 2022
@openshift-ci
Copy link

openshift-ci bot commented Aug 24, 2022

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@github-actions
Copy link

github-actions bot commented Aug 24, 2022

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

rhel9 (from CTF) Environment (using Fedora as testing environment)
Open in Gitpod

Fedora Testing Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

@github-actions
Copy link

github-actions bot commented Aug 24, 2022

This datastream diff is auto generated by the check Compare DS/Generate Diff

Click here to see the full diff
OCIL for rule 'xccdf_org.ssgproject.content_rule_sysctl_kernel_core_pattern_empty_string' differs:
--- old datastream
+++ new datastream
@@ -1,7 +1,7 @@
 The runtime status of the kernel.core_pattern kernel parameter can be queried
 by running the following command:
-$ sysctl kernel.core_pattern
-''.
+$ sysctl kernel.core_pattern | cat -A
+kernel.core_pattern = $
 
- Is it the case that the returned line does not have a value of ''.?
+ Is it the case that the returned line does not have an empty string?
 
bash remediation for rule 'xccdf_org.ssgproject.content_rule_sysctl_kernel_core_pattern_empty_string' differs:
--- old datastream
+++ new datastream
@@ -1,3 +1,6 @@
+# Remediation is applicable only in certain platforms
+if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ]; then
+
 # Remediation is applicable only in certain platforms
 if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ]; then
 
@@ -18,11 +21,11 @@
 #
 # Set runtime for kernel.core_pattern
 #
-/sbin/sysctl -q -n -w kernel.core_pattern="''"
+/sbin/sysctl -q -n -w kernel.core_pattern=""
 
 #
-# If kernel.core_pattern present in /etc/sysctl.conf, change value to "''"
-# else, add "kernel.core_pattern = ''" to /etc/sysctl.conf
+# If kernel.core_pattern present in /etc/sysctl.conf, change value to empty
+# else, add "kernel.core_pattern =" to /etc/sysctl.conf
 #
 # Test if the config_file is a symbolic link. If so, use --follow-symlinks with sed.
 # Otherwise, regular sed command will do.
@@ -36,7 +39,7 @@
 stripped_key=$(sed 's/[\^=\$,;+]*//g' <<< "^kernel.core_pattern")
 
 # shellcheck disable=SC2059
-printf -v formatted_output "%s = %s" "$stripped_key" "''"
+printf -v formatted_output "%s=" "$stripped_key"
 
 # If the key exists, change it. Otherwise, add it to the config_file.
 # We search for the key string followed by a word boundary (matched by \>),
@@ -46,11 +49,14 @@
 "${sed_command[@]}" "s/^kernel.core_pattern\\>.*/$escaped_formatted_output/gi" "/etc/sysctl.conf"
 else
 # \n is precaution for case where file ends without trailing newline
- cce="CCE-86005-6"
- printf '\n# Per %s: Set %s in %s\n' "$cce" "$formatted_output" "/etc/sysctl.conf" >> "/etc/sysctl.conf"
+
 printf '%s\n' "$formatted_output" >> "/etc/sysctl.conf"
 fi
 
 else
 >&2 echo 'Remediation is not applicable, nothing was done'
 fi
+
+else
+ >&2 echo 'Remediation is not applicable, nothing was done'
+fi

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_sysctl_kernel_core_pattern_empty_string' differs:
--- old datastream
+++ new datastream
@@ -34,10 +34,26 @@
 - reboot_required
 - sysctl_kernel_core_pattern_empty_string
 
-- name: Ensure sysctl kernel.core_pattern is set to ''
+- name: Comment out any occurrences of kernel.core_pattern with value from /etc/sysctl.conf
+ files
+ replace:
+ path: /etc/sysctl.conf
+ regexp: ^[\s]*kernel.core_pattern([ \t]*=[ \t]*\S+)
+ replace: '#kernel.core_pattern\1'
+ when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+ tags:
+ - CCE-86005-6
+ - disable_strategy
+ - low_complexity
+ - medium_disruption
+ - medium_severity
+ - reboot_required
+ - sysctl_kernel_core_pattern_empty_string
+
+- name: Ensure sysctl kernel.core_pattern is set to empty
 sysctl:
 name: kernel.core_pattern
- value: ''''''
+ value: ' '
 state: present
 reload: true
 when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]

@ggbecker ggbecker force-pushed the fix-core-pattern-empty-string branch from 43f59ae to 796d363 Compare August 24, 2022 16:29
@ggbecker ggbecker marked this pull request as ready for review August 24, 2022 16:33
@ggbecker ggbecker requested a review from a team as a code owner August 24, 2022 16:33
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Aug 24, 2022
Mab879
Mab879 previously requested changes Aug 24, 2022
Copy link
Member

@Mab879 Mab879 left a comment

Choose a reason for hiding this comment

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

OpenSCAP Error: Invalid SCAP Source Datastream (1.3) content in /__w/content/content/build/ssg-centos8-ds.xml. [oscap_source.c:343]

Please take a look at the test failures. Looks like there is a SCAP validation error.

Copy link
Collaborator

@vojtapolasek vojtapolasek left a comment

Choose a reason for hiding this comment

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

I reviewed files more from stylistic point of view. See comments.

@vojtapolasek
Copy link
Collaborator

For some reason, the test "validate-parse-affected" expects only one tag in OVAL checks. We need to either rewrite the test or rewrite this particular OVAL. I am more for rewriting of the test because the sysctl template generates basically the same OVAL check you write here.

@ggbecker ggbecker force-pushed the fix-core-pattern-empty-string branch from ef8503d to 8d6176c Compare August 25, 2022 13:16
@pep8speaks
Copy link

pep8speaks commented Aug 25, 2022

Hello @ggbecker! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-08-25 15:27:52 UTC

@ggbecker ggbecker force-pushed the fix-core-pattern-empty-string branch from 6e47df8 to c5bcea3 Compare August 25, 2022 13:21
@ggbecker ggbecker force-pushed the fix-core-pattern-empty-string branch from 7358c0f to 243347a Compare August 25, 2022 15:27
@codeclimate
Copy link

codeclimate bot commented Aug 25, 2022

Code Climate has analyzed commit 243347a and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Style 1

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 42.5% (0.0% change).

View more on Code Climate.

@matusmarhefka matusmarhefka self-assigned this Aug 25, 2022
@matusmarhefka matusmarhefka dismissed stale reviews from vojtapolasek and Mab879 August 25, 2022 16:40

Feedback has been addressed

@matusmarhefka matusmarhefka merged commit 66996f8 into ComplianceAsCode:master Aug 25, 2022
@comps
Copy link
Collaborator

comps commented Aug 29, 2022

@ggbecker While looking at the generated fix for this, I noticed that an applicability check is now present twice:

# Remediation is applicable only in certain platforms
if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ]; then

# Remediation is applicable only in certain platforms
if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ]; then

...

else
    >&2 echo 'Remediation is not applicable, nothing was done'
fi

else
    >&2 echo 'Remediation is not applicable, nothing was done'
fi

Not a big deal, but, if possible, consider looking into it. Thanks!

@ggbecker
Copy link
Member Author

@ggbecker While looking at the generated fix for this, I noticed that an applicability check is now present twice:

# Remediation is applicable only in certain platforms
if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ]; then

# Remediation is applicable only in certain platforms
if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ]; then

...

else
    >&2 echo 'Remediation is not applicable, nothing was done'
fi

else
    >&2 echo 'Remediation is not applicable, nothing was done'
fi

Not a big deal, but, if possible, consider looking into it. Thanks!

I'll fix on a follow up PR, thanks for noticing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixes to reported bugs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants