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

Misc CIS/SLE* related updates #10161

Merged
merged 18 commits into from
May 10, 2023
Merged

Conversation

truzzon
Copy link
Contributor

@truzzon truzzon commented Feb 3, 2023

Description:

  • Provide fixes to make auditing SLES OS systems especially in CIS context more accurate

Rationale:

  • Split up banner auditing, to allow separate regex for each file. (MotD/issue/issue.net) - Updated templates accordingly
  • Add /etc/shadow mode 640 for SLES OS
  • Add sle products to faillog rule
  • Update rules called by cis_sle15 to comply to current CIS requirements
  • Create separate rule for sles 'rsyslog-module-gtls' pkg, to properly execute the check, since updating the package name in the original rule did not work out during my testing.

@truzzon truzzon requested a review from a team as a code owner February 3, 2023 11:12
@openshift-ci openshift-ci bot added the needs-ok-to-test Used by openshift-ci bot. label Feb 3, 2023
@openshift-ci
Copy link

openshift-ci bot commented Feb 3, 2023

Hi @truzzon. 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

github-actions bot commented Feb 3, 2023

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

rhel8 (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 Feb 3, 2023

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

Click here to see the full diff
bash remediation for rule 'xccdf_org.ssgproject.content_rule_banner_etc_issue_net' differs.
--- xccdf_org.ssgproject.content_rule_banner_etc_issue_net
+++ xccdf_org.ssgproject.content_rule_banner_etc_issue_net
@@ -1,22 +1,22 @@
 # Remediation is applicable only in certain platforms
 if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ]; then
 
-login_banner_text=''
+remote_login_banner_text=''
 
 
 # Multiple regexes transform the banner regex into a usable banner
 # 0 - Remove anchors around the banner text
-login_banner_text=$(echo "$login_banner_text" | sed 's/^\^\(.*\)\$$/\1/g')
+remote_login_banner_text=$(echo "$remote_login_banner_text" | sed 's/^\^\(.*\)\$$/\1/g')
 # 1 - Keep only the first banners if there are multiple
 # (dod_banners contains the long and short banner)
-login_banner_text=$(echo "$login_banner_text" | sed 's/^(\(.*\.\)|.*)$/\1/g')
+remote_login_banner_text=$(echo "$remote_login_banner_text" | sed 's/^(\(.*\.\)|.*)$/\1/g')
 # 2 - Add spaces ' '. (Transforms regex for "space or newline" into a " ")
-login_banner_text=$(echo "$login_banner_text" | sed 's/\[\\s\\n\]+/ /g')
+remote_login_banner_text=$(echo "$remote_login_banner_text" | sed 's/\[\\s\\n\]+/ /g')
 # 3 - Adds newlines. (Transforms "(?:\[\\n\]+|(?:\\n)+)" into "\n")
-login_banner_text=$(echo "$login_banner_text" | sed 's/(?:\[\\n\]+|(?:\\\\n)+)/\n/g')
+remote_login_banner_text=$(echo "$remote_login_banner_text" | sed 's/(?:\[\\n\]+|(?:\\\\n)+)/\n/g')
 # 4 - Remove any leftover backslash. (From any parethesis in the banner, for example).
-login_banner_text=$(echo "$login_banner_text" | sed 's/\\//g')
-formatted=$(echo "$login_banner_text" | fold -sw 80)
+remote_login_banner_text=$(echo "$remote_login_banner_text" | sed 's/\\//g')
+formatted=$(echo "$remote_login_banner_text" | fold -sw 80)
 
 cat <<EOF >/etc/issue.net
 $formatted

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_banner_etc_issue_net' differs.
--- xccdf_org.ssgproject.content_rule_banner_etc_issue_net
+++ xccdf_org.ssgproject.content_rule_banner_etc_issue_net
@@ -1,13 +1,13 @@
-- name: XCCDF Value login_banner_text # promote to variable
+- name: XCCDF Value remote_login_banner_text # promote to variable
 set_fact:
- login_banner_text: !!str 
+ remote_login_banner_text: !!str 
 tags:
 - always
 
 - name: Modify the System Login Banner for Remote Connections - ensure correct banner
 copy:
 dest: /etc/issue.net
- content: '{{ login_banner_text | regex_replace("^\^(.*)\$$", "\1") | regex_replace("^\((.*\.)\|.*\)$",
+ content: '{{ remote_login_banner_text | regex_replace("^\^(.*)\$$", "\1") | regex_replace("^\((.*\.)\|.*\)$",
 "\1") | regex_replace("\[\\s\\n\]\+"," ") | regex_replace("\(\?:\[\\n\]\+\|\(\?:\\\\n\)\+\)",
 "\n") | regex_replace("\\", "") | wordwrap() }}'
 when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]

OCIL for rule 'xccdf_org.ssgproject.content_rule_banner_etc_motd' differs.
--- ocil:ssg-banner_etc_motd_ocil:questionnaire:1
+++ ocil:ssg-banner_etc_motd_ocil:questionnaire:1
@@ -1,4 +1,4 @@
-To check if the system login banner is compliant,
+To check if the system motd banner is compliant,
 run the following command:
 $ cat /etc/motd
 Is it the case that it does not display the required banner?

bash remediation for rule 'xccdf_org.ssgproject.content_rule_banner_etc_motd' differs.
--- xccdf_org.ssgproject.content_rule_banner_etc_motd
+++ xccdf_org.ssgproject.content_rule_banner_etc_motd
@@ -1,22 +1,22 @@
 # Remediation is applicable only in certain platforms
 if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ]; then
 
-login_banner_text=''
+motd_banner_text=''
 
 
 # Multiple regexes transform the banner regex into a usable banner
 # 0 - Remove anchors around the banner text
-login_banner_text=$(echo "$login_banner_text" | sed 's/^\^\(.*\)\$$/\1/g')
+motd_banner_text=$(echo "$motd_banner_text" | sed 's/^\^\(.*\)\$$/\1/g')
 # 1 - Keep only the first banners if there are multiple
 # (dod_banners contains the long and short banner)
-login_banner_text=$(echo "$login_banner_text" | sed 's/^(\(.*\.\)|.*)$/\1/g')
+motd_banner_text=$(echo "$motd_banner_text" | sed 's/^(\(.*\.\)|.*)$/\1/g')
 # 2 - Add spaces ' '. (Transforms regex for "space or newline" into a " ")
-login_banner_text=$(echo "$login_banner_text" | sed 's/\[\\s\\n\]+/ /g')
+motd_banner_text=$(echo "$motd_banner_text" | sed 's/\[\\s\\n\]+/ /g')
 # 3 - Adds newlines. (Transforms "(?:\[\\n\]+|(?:\\n)+)" into "\n")
-login_banner_text=$(echo "$login_banner_text" | sed 's/(?:\[\\n\]+|(?:\\\\n)+)/\n/g')
+motd_banner_text=$(echo "$motd_banner_text" | sed 's/(?:\[\\n\]+|(?:\\\\n)+)/\n/g')
 # 4 - Remove any leftover backslash. (From any parethesis in the banner, for example).
-login_banner_text=$(echo "$login_banner_text" | sed 's/\\//g')
-formatted=$(echo "$login_banner_text" | fold -sw 80)
+motd_banner_text=$(echo "$motd_banner_text" | sed 's/\\//g')
+formatted=$(echo "$motd_banner_text" | fold -sw 80)
 
 cat <<EOF >/etc/motd
 $formatted

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_banner_etc_motd' differs.
--- xccdf_org.ssgproject.content_rule_banner_etc_motd
+++ xccdf_org.ssgproject.content_rule_banner_etc_motd
@@ -1,13 +1,13 @@
-- name: XCCDF Value login_banner_text # promote to variable
+- name: XCCDF Value motd_banner_text # promote to variable
 set_fact:
- login_banner_text: !!str 
+ motd_banner_text: !!str 
 tags:
 - always
 
 - name: Modify the System Message of the Day Banner - ensure correct banner
 copy:
 dest: /etc/motd
- content: '{{ login_banner_text | regex_replace("^\^(.*)\$$", "\1") | regex_replace("^\((.*\.)\|.*\)$",
+ content: '{{ motd_banner_text | regex_replace("^\^(.*)\$$", "\1") | regex_replace("^\((.*\.)\|.*\)$",
 "\1") | regex_replace("\[\\s\\n\]\+"," ") | regex_replace("\(\?:\[\\n\]\+\|\(\?:\\\\n\)\+\)",
 "\n") | regex_replace("\\", "") | wordwrap() }}'
 when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]

OCIL for rule 'xccdf_org.ssgproject.content_rule_audit_privileged_commands_init' differs.
--- ocil:ssg-audit_privileged_commands_init_ocil:questionnaire:1
+++ ocil:ssg-audit_privileged_commands_init_ocil:questionnaire:1
@@ -2,6 +2,6 @@
 
 $ sudo auditctl -l | grep init
 
--a always,exit -F path=/usr/sbin/init -F perm=x -F auid>=1000 -F auid!=unset -k privileged-init
+-a always,exit -F path=/init -F perm=x -F auid>=1000 -F auid!=unset -k privileged-init
 Is it the case that the command does not return a line, or the line is commented out?
 
OCIL for rule 'xccdf_org.ssgproject.content_rule_audit_privileged_commands_poweroff' differs.
--- ocil:ssg-audit_privileged_commands_poweroff_ocil:questionnaire:1
+++ ocil:ssg-audit_privileged_commands_poweroff_ocil:questionnaire:1
@@ -2,6 +2,6 @@
 
 $ sudo auditctl -l | grep poweroff
 
--a always,exit -F path=/usr/sbin/poweroff -F perm=x -F auid>=1000 -F auid!=unset -k privileged-poweroff
+-a always,exit -F path=/poweroff -F perm=x -F auid>=1000 -F auid!=unset -k privileged-poweroff
 Is it the case that the command does not return a line, or the line is commented out?
 
OCIL for rule 'xccdf_org.ssgproject.content_rule_audit_privileged_commands_reboot' differs.
--- ocil:ssg-audit_privileged_commands_reboot_ocil:questionnaire:1
+++ ocil:ssg-audit_privileged_commands_reboot_ocil:questionnaire:1
@@ -2,6 +2,6 @@
 
 $ sudo auditctl -l | grep reboot
 
--a always,exit -F path=/usr/sbin/reboot -F perm=x -F auid>=1000 -F auid!=unset -k privileged-reboot
+-a always,exit -F path=/reboot -F perm=x -F auid>=1000 -F auid!=unset -k privileged-reboot
 Is it the case that the command does not return a line, or the line is commented out?
 
OCIL for rule 'xccdf_org.ssgproject.content_rule_audit_privileged_commands_shutdown' differs.
--- ocil:ssg-audit_privileged_commands_shutdown_ocil:questionnaire:1
+++ ocil:ssg-audit_privileged_commands_shutdown_ocil:questionnaire:1
@@ -2,6 +2,6 @@
 
 $ sudo auditctl -l | grep shutdown
 
--a always,exit -F path=/usr/sbin/shutdown -F perm=x -F auid>=1000 -F auid!=unset -k privileged-shutdown
+-a always,exit -F path=/shutdown -F perm=x -F auid>=1000 -F auid!=unset -k privileged-shutdown
 Is it the case that the command does not return a line, or the line is commented out?
 
New content has different text for rule 'xccdf_org.ssgproject.content_rule_package_rsyslog-gnutls_installed'.
--- xccdf_org.ssgproject.content_rule_package_rsyslog-gnutls_installed
+++ xccdf_org.ssgproject.content_rule_package_rsyslog-gnutls_installed
@@ -4,6 +4,7 @@
 
 [description]:
 TLS protocol support for rsyslog is installed.
+
 The rsyslog-gnutls package can be installed with the following command:
 
 $ sudo yum install rsyslog-gnutls

OCIL for rule 'xccdf_org.ssgproject.content_rule_package_rsyslog-gnutls_installed' differs.
--- ocil:ssg-package_rsyslog-gnutls_installed_ocil:questionnaire:1
+++ ocil:ssg-package_rsyslog-gnutls_installed_ocil:questionnaire:1
@@ -1,3 +1,4 @@
-Run the following command to determine if the rsyslog-gnutls package is installed: $ rpm -q rsyslog-gnutls
- Is it the case that the package is not installed?
+Run the following command to determine if the rsyslog-gnutls package is installed:
+$ rpm -q rsyslog-gnutls
+ Is it the case that the package is installed?

@truzzon
Copy link
Contributor Author

truzzon commented Feb 3, 2023

I am currently updating the control files to account for my changes to the banner tests

@truzzon truzzon requested a review from a team as a code owner February 3, 2023 12:25
@truzzon
Copy link
Contributor Author

truzzon commented Feb 3, 2023

Hello, the logs if the failing tests all show the same error:

ERROR - Rule 'xccdf_org.ssgproject.content_rule_package_rsyslog-module-gtls_installed' isn't present in benchmark 'xccdf_org.ssgproject.content_benchmark_RHEL-8' in '/tmp/ssgts-ds-dy34idmd'

I'm thinking to myself "It isn't supposed to..."

Can you have a look at it?

@jan-cerny
Copy link
Collaborator

@truzzon Yes, it's fine and it's expected because the rule package_rsyslog-module-gtls_installed is only for SLE 12 and SLE 15.

@@ -27,9 +27,11 @@ references:
nist: AU-12(c)
srg: SRG-OS-000477-GPOS-00222

{{{ ocil_fix_srg_privileged_command("init", "/usr/sbin/") }}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

The removal of /usr/sbin from this place doesn't seem to be right because the default value of this parameter is /usr/bin, that means that the generated OCIL and fixtext in RHEL 9 (and probably other products as well) contain an incorrect path. Can you take a look into this problem and make sure that you don't break RHEL 9 or other products?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added a Jinja macro, that sets the path for the rule, based on the os. I hope it is a viable solution.

@@ -27,9 +27,11 @@ references:
nist: AU-12(c)
srg: SRG-OS-000477-GPOS-00222

{{{ ocil_fix_srg_privileged_command("poweroff", "/usr/sbin/") }}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This appears here many times.

@marcusburghardt marcusburghardt added SLES SUSE Linux Enterprise Server product related. CIS CIS Benchmark related. labels Feb 8, 2023
Copy link
Member

@marcusburghardt marcusburghardt left a comment

Choose a reason for hiding this comment

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

Besides the comments, could you give more context about the banners changes, please?

I saw you created new variables but they seem to have exactly the same content. I see value doing that when there is different content for these banners but I can't see this in most of the benchmarks. If there is indeed a demand to differentiate the content, I would agree to create this flexibility. Otherwise, we are only duplicating content in the project and increasing the maintainability if we have to update any message. Maybe you have more context about this to help me understand the proposal. Thanks.

@marcusburghardt marcusburghardt added the RHEL Red Hat Enterprise Linux product related. label Feb 8, 2023
@marcusburghardt marcusburghardt requested a review from a team February 8, 2023 10:08
@truzzon
Copy link
Contributor Author

truzzon commented Feb 8, 2023

Besides the comments, could you give more context about the banners changes, please?

I saw you created new variables but they seem to have exactly the same content. I see value doing that when there is different content for these banners but I can't see this in most of the benchmarks. If there is indeed a demand to differentiate the content, I would agree to create this flexibility. Otherwise, we are only duplicating content in the project and increasing the maintainability if we have to update any message. Maybe you have more context about this to help me understand the proposal. Thanks.

I'm using the scap workbench to customize the files to customer needs. That's where the whole idea came from.

I understand the issues with duplicate templates and so on. But I was running into issues, where customers had different content for each file that had to be checked. I hoped to achieve a better overview and make it easier to edit in the final result.

@truzzon truzzon changed the title Misc CIS/SLE* related updates WIP: Misc CIS/SLE* related updates Feb 10, 2023
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Feb 10, 2023
@truzzon truzzon changed the title WIP: Misc CIS/SLE* related updates Misc CIS/SLE* related updates Feb 14, 2023
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Feb 14, 2023
@openshift-merge-robot openshift-merge-robot added the needs-rebase Used by openshift-ci bot. label Feb 16, 2023
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Used by openshift-ci bot. label Feb 17, 2023
@openshift-merge-robot openshift-merge-robot added the needs-rebase Used by openshift-ci bot. label Feb 24, 2023
Copy link
Collaborator

@jan-cerny jan-cerny left a comment

Choose a reason for hiding this comment

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

Please resolve the conflict and see comments below.

Copy link
Contributor

@teacup-on-rockingchair teacup-on-rockingchair left a comment

Choose a reason for hiding this comment

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

Overall this is great effort with lots of improvement IMHO, I only had some hard time testing the motd related regex since the rule seems to accept almost anything in /etc/motd, besides referring to another distro :)
But this seems to be preserved behaviour from before the MR, and adding the separate variables just made the code more maintainable so kudos on that 🙇

@marcusburghardt
Copy link
Member

@truzzon , could you rebase this PR and resolve the conflict, please? I believe it would be ready to be merged after that.

@codeclimate
Copy link

codeclimate bot commented May 5, 2023

Code Climate has analyzed commit e53a1ea 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 52.4% (0.0% change).

View more on Code Climate.

@marcusburghardt marcusburghardt dismissed jan-cerny’s stale review May 10, 2023 12:03

Requested changes were addressed

@marcusburghardt marcusburghardt merged commit 7b1efa9 into ComplianceAsCode:master May 10, 2023
@truzzon truzzon deleted the misc_fixes branch May 15, 2023 15:24
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CIS CIS Benchmark related. needs-ok-to-test Used by openshift-ci bot. RHEL Red Hat Enterprise Linux product related. SLES SUSE Linux Enterprise Server product related.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants