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

Change rule to use variable when auditing faillock #11007

Merged
merged 2 commits into from
Aug 29, 2023

Conversation

Mackemania
Copy link
Contributor

Description:

  • Some benchmarks specify different paths when monitoring the faillock file, depending on if a permanent faillock path has been configured. This updates the audit_rules_login_event_faillock to use the variable var_accounts_passwords_pam_faillock_dir instead of the profile faillock_path

@Mackemania Mackemania requested a review from a team as a code owner August 22, 2023 10:55
@openshift-ci openshift-ci bot added the needs-ok-to-test Used by openshift-ci bot. label Aug 22, 2023
@openshift-ci
Copy link

openshift-ci bot commented Aug 22, 2023

Hi @Mackemania. 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:

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 Aug 22, 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_audit_sudo_log_events' differs.
--- xccdf_org.ssgproject.content_rule_audit_sudo_log_events
+++ xccdf_org.ssgproject.content_rule_audit_sudo_log_events
@@ -2,6 +2,7 @@
 if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ] && rpm --quiet -q audit; then
 
 # Perform the remediation for both possible tools: 'auditctl' and 'augenrules'
+
 
 # Create a list of audit *.rules files that should be inspected for presence and correctness
 # of a particular audit rule. The scheme is as follows:

New content has different text for rule 'xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock'.
--- xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock
+++ xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock
@@ -9,12 +9,12 @@
 default), add the following lines to a file with suffix .rules in the
 directory /etc/audit/rules.d in order to watch for attempted manual
 edits of files involved in storing logon events:
--w /var/log/faillock -p wa -k logins
+-w 'xccdf_org.ssgproject.content_value_var_accounts_passwords_pam_faillock_dir' -p wa -k logins
 If the auditd daemon is configured to use the auditctl
 utility to read audit rules during daemon startup, add the following lines to
 /etc/audit/audit.rules file in order to watch for unattempted manual
 edits of files involved in storing logon events:
--w /var/log/faillock -p wa -k logins
+-w 'xccdf_org.ssgproject.content_value_var_accounts_passwords_pam_faillock_dir' -p wa -k logins
 
 [reference]:
 BP28(R73)

OVAL for rule 'xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock' differs.
--- oval:ssg-audit_rules_login_events_faillock:def:1
+++ oval:ssg-audit_rules_login_events_faillock:def:1
@@ -1,7 +1,7 @@
 criteria OR
 criteria AND
 extend_definition oval:ssg-audit_rules_augenrules:def:1
-criterion oval:ssg-test_arle_faillock_augenrules:tst:1
+criterion oval:ssg-test_arle_var_accounts_passwords_pam_faillock_dir_augenrules:tst:1
 criteria AND
 extend_definition oval:ssg-audit_rules_auditctl:def:1
-criterion oval:ssg-test_arle_faillock_auditctl:tst:1
+criterion oval:ssg-test_arle_var_accounts_passwords_pam_faillock_dir_auditctl:tst:1

OCIL for rule 'xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock' differs.
--- ocil:ssg-audit_rules_login_events_faillock_ocil:questionnaire:1
+++ ocil:ssg-audit_rules_login_events_faillock_ocil:questionnaire:1
@@ -1,7 +1,7 @@
 Verify Red Hat Enterprise Linux 8 generates audit records for all account creations, modifications, disabling, and termination events that affect "/etc/security/opasswd" with the following command:
 
-$ sudo auditctl -l | grep /var/log/faillock
+$ sudo auditctl -l | grep 
 
--w /var/log/faillock -p wa -k logins
+-w  -p wa -k logins
       Is it the case that the command does not return a line, or the line is commented out?
       
bash remediation for rule 'xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock' differs.
--- xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock
+++ xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock
@@ -2,6 +2,9 @@
 if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ] && rpm --quiet -q audit; then
 
 # Perform the remediation for both possible tools: 'auditctl' and 'augenrules'
+
+
+var_accounts_passwords_pam_faillock_dir=''
 
 # Create a list of audit *.rules files that should be inspected for presence and correctness
 # of a particular audit rule. The scheme is as follows:
@@ -26,7 +29,7 @@
 for audit_rules_file in "${files_to_inspect[@]}"
 do
     # Check if audit watch file system object rule for given path already present
-    if grep -q -P -- "^[\s]*-w[\s]+/var/log/faillock" "$audit_rules_file"
+    if grep -q -P -- "^[\s]*-w[\s]+${var_accounts_passwords_pam_faillock_dir}" "$audit_rules_file"
     then
         # Rule is found => verify yet if existing rule definition contains
         # all of the required access type bits
@@ -34,7 +37,7 @@
         # Define BRE whitespace class shortcut
         sp="[[:space:]]"
         # Extract current permission access types (e.g. -p [r|w|x|a] values) from audit rule
-        current_access_bits=$(sed -ne "s#$sp*-w$sp\+/var/log/faillock $sp\+-p$sp\+\([rxwa]\{1,4\}\).*#\1#p" "$audit_rules_file")
+        current_access_bits=$(sed -ne "s#$sp*-w$sp\+${var_accounts_passwords_pam_faillock_dir} $sp\+-p$sp\+\([rxwa]\{1,4\}\).*#\1#p" "$audit_rules_file")
         # Split required access bits string into characters array
         # (to check bit's presence for one bit at a time)
         for access_bit in $(echo "wa" | grep -o .)
@@ -50,12 +53,12 @@
         done
         # Propagate the updated rule's access bits (original + the required
         # ones) back into the /etc/audit/audit.rules file for that rule
-        sed -i "s#\($sp*-w$sp\+/var/log/faillock$sp\+-p$sp\+\)\([rxwa]\{1,4\}\)\(.*\)#\1$current_access_bits\3#" "$audit_rules_file"
+        sed -i "s#\($sp*-w$sp\+${var_accounts_passwords_pam_faillock_dir}$sp\+-p$sp\+\)\([rxwa]\{1,4\}\)\(.*\)#\1$current_access_bits\3#" "$audit_rules_file"
     else
         # Rule isn't present yet. Append it at the end of $audit_rules_file file
         # with proper key
 
-        echo "-w /var/log/faillock -p wa -k logins" >> "$audit_rules_file"
+        echo "-w ${var_accounts_passwords_pam_faillock_dir} -p wa -k logins" >> "$audit_rules_file"
     fi
 done
 # Create a list of audit *.rules files that should be inspected for presence and correctness
@@ -74,7 +77,7 @@
 # If the audit is 'augenrules', then check if rule is already defined
 # If rule is defined, add '/etc/audit/rules.d/*.rules' to list of files for inspection.
 # If rule isn't defined, add '/etc/audit/rules.d/logins.rules' to list of files for inspection.
-readarray -t matches < <(grep -HP "[\s]*-w[\s]+/var/log/faillock" /etc/audit/rules.d/*.rules)
+readarray -t matches < <(grep -HP "[\s]*-w[\s]+${var_accounts_passwords_pam_faillock_dir}" /etc/audit/rules.d/*.rules)
 
 # For each of the matched entries
 for match in "${matches[@]}"
@@ -103,7 +106,7 @@
 for audit_rules_file in "${files_to_inspect[@]}"
 do
     # Check if audit watch file system object rule for given path already present
-    if grep -q -P -- "^[\s]*-w[\s]+/var/log/faillock" "$audit_rules_file"
+    if grep -q -P -- "^[\s]*-w[\s]+${var_accounts_passwords_pam_faillock_dir}" "$audit_rules_file"
     then
         # Rule is found => verify yet if existing rule definition contains
         # all of the required access type bits
@@ -111,7 +114,7 @@
         # Define BRE whitespace class shortcut
         sp="[[:space:]]"
         # Extract current permission access types (e.g. -p [r|w|x|a] values) from audit rule
-        current_access_bits=$(sed -ne "s#$sp*-w$sp\+/var/log/faillock $sp\+-p$sp\+\([rxwa]\{1,4\}\).*#\1#p" "$audit_rules_file")
+        current_access_bits=$(sed -ne "s#$sp*-w$sp\+${var_accounts_passwords_pam_faillock_dir} $sp\+-p$sp\+\([rxwa]\{1,4\}\).*#\1#p" "$audit_rules_file")
         # Split required access bits string into characters array
         # (to check bit's presence for one bit at a time)
         for access_bit in $(echo "wa" | grep -o .)
@@ -127,12 +130,12 @@
         done
         # Propagate the updated rule's access bits (original + the required
         # ones) back into the /etc/audit/audit.rules file for that rule
-        sed -i "s#\($sp*-w$sp\+/var/log/faillock$sp\+-p$sp\+\)\([rxwa]\{1,4\}\)\(.*\)#\1$current_access_bits\3#" "$audit_rules_file"
+        sed -i "s#\($sp*-w$sp\+${var_accounts_passwords_pam_faillock_dir}$sp\+-p$sp\+\)\([rxwa]\{1,4\}\)\(.*\)#\1$current_access_bits\3#" "$audit_rules_file"
     else
         # Rule isn't present yet. Append it at the end of $audit_rules_file file
         # with proper key
 
-        echo "-w /var/log/faillock -p wa -k logins" >> "$audit_rules_file"
+        echo "-w ${var_accounts_passwords_pam_faillock_dir} -p wa -k logins" >> "$audit_rules_file"
     fi
 done
 

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock' differs.
--- xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock
+++ xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock
@@ -17,11 +17,17 @@
   - medium_severity
   - reboot_required
   - restrict_strategy
-
-- name: Check if watch rule for /var/log/faillock already exists in /etc/audit/rules.d/
+- name: XCCDF Value var_accounts_passwords_pam_faillock_dir # promote to variable
+  set_fact:
+    var_accounts_passwords_pam_faillock_dir: !!str 
+  tags:
+    - always
+
+- name: Check if watch rule for {{ var_accounts_passwords_pam_faillock_dir }} already
+    exists in /etc/audit/rules.d/
   find:
     paths: /etc/audit/rules.d
-    contains: ^\s*-w\s+/var/log/faillock\s+-p\s+wa(\s|$)+
+    contains: ^\s*-w\s+{{ var_accounts_passwords_pam_faillock_dir }}\s+-p\s+wa(\s|$)+
     patterns: '*.rules'
   register: find_existing_watch_rules_d
   when:
@@ -124,10 +130,10 @@
   - reboot_required
   - restrict_strategy
 
-- name: Add watch rule for /var/log/faillock in /etc/audit/rules.d/
+- name: Add watch rule for {{ var_accounts_passwords_pam_faillock_dir }} in /etc/audit/rules.d/
   lineinfile:
     path: '{{ all_files[0] }}'
-    line: -w /var/log/faillock -p wa -k logins
+    line: -w {{ var_accounts_passwords_pam_faillock_dir }} -p wa -k logins
     create: true
     mode: '0640'
   when:
@@ -152,10 +158,11 @@
   - reboot_required
   - restrict_strategy
 
-- name: Check if watch rule for /var/log/faillock already exists in /etc/audit/audit.rules
+- name: Check if watch rule for {{ var_accounts_passwords_pam_faillock_dir }} already
+    exists in /etc/audit/audit.rules
   find:
     paths: /etc/audit/
-    contains: ^\s*-w\s+/var/log/faillock\s+-p\s+wa(\s|$)+
+    contains: ^\s*-w\s+{{ var_accounts_passwords_pam_faillock_dir }}\s+-p\s+wa(\s|$)+
     patterns: audit.rules
   register: find_existing_watch_audit_rules
   when:
@@ -178,9 +185,9 @@
   - reboot_required
   - restrict_strategy
 
-- name: Add watch rule for /var/log/faillock in /etc/audit/audit.rules
+- name: Add watch rule for {{ var_accounts_passwords_pam_faillock_dir }} in /etc/audit/audit.rules
   lineinfile:
-    line: -w /var/log/faillock -p wa -k logins
+    line: -w {{ var_accounts_passwords_pam_faillock_dir }} -p wa -k logins
     state: present
     dest: /etc/audit/audit.rules
     create: true

bash remediation for rule 'xccdf_org.ssgproject.content_rule_audit_rules_login_events_lastlog' differs.
--- xccdf_org.ssgproject.content_rule_audit_rules_login_events_lastlog
+++ xccdf_org.ssgproject.content_rule_audit_rules_login_events_lastlog
@@ -2,6 +2,7 @@
 if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ] && rpm --quiet -q audit; then
 
 # Perform the remediation for both possible tools: 'auditctl' and 'augenrules'
+
 
 # Create a list of audit *.rules files that should be inspected for presence and correctness
 # of a particular audit rule. The scheme is as follows:

bash remediation for rule 'xccdf_org.ssgproject.content_rule_audit_rules_login_events_tallylog' differs.
--- xccdf_org.ssgproject.content_rule_audit_rules_login_events_tallylog
+++ xccdf_org.ssgproject.content_rule_audit_rules_login_events_tallylog
@@ -2,6 +2,7 @@
 if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ] && rpm --quiet -q audit; then
 
 # Perform the remediation for both possible tools: 'auditctl' and 'augenrules'
+
 
 # Create a list of audit *.rules files that should be inspected for presence and correctness
 # of a particular audit rule. The scheme is as follows:

@jan-cerny
Copy link
Collaborator

I think it would be better to modify the audit_rules_login_events template in a way that the template could generate content that uses a variable. That way we would avoid code duplication.

@Mab879 Mab879 self-assigned this Aug 22, 2023
@Mackemania
Copy link
Contributor Author

I think it would be better to modify the audit_rules_login_events template in a way that the template could generate content that uses a variable. That way we would avoid code duplication.

I will check it out, thanks

@Mackemania Mackemania force-pushed the master branch 2 times, most recently from 2782a60 to ceb7ea6 Compare August 22, 2023 14:52
@Mab879 Mab879 added this to the 0.1.70 milestone Aug 22, 2023
@Mackemania
Copy link
Contributor Author

I don't think either of the failed tests are due to this PR.

@ggbecker
Copy link
Member

/packit retest-failed

@Mab879
Copy link
Member

Mab879 commented Aug 23, 2023

I ran the Automatus tests locally and got the following results

Setting console output to log level INFO
INFO - The base image option has not been specified, choosing libvirt-based test environment.
INFO - Logging into /home/mburket/code/ComplianceAsCode/content/tests/logs/rule-custom-2023-08-23-0817/test_suite.log
INFO - xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock
INFO - Script auditctl_wrong_rule.fail.sh using profile (all) OK
INFO - Script auditctl_wrong_rule_without_key.fail.sh using profile (all) OK
ERROR - Script auditctl_correct_without_key.pass.sh using profile (all) found issue:
ERROR - Rule evaluation resulted in fail, instead of expected pass during initial stage 
ERROR - The initial scan failed for rule 'xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock'.
INFO - Script auditctl_remove_all_rules.fail.sh using profile (all) OK
ERROR - Script augenrules_correct_extra_permission.pass.sh using profile (all) found issue:
ERROR - Rule evaluation resulted in fail, instead of expected pass during initial stage 
ERROR - The initial scan failed for rule 'xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock'.
INFO - Script augenrules_wrong_rule_without_key.fail.sh using profile (all) OK
ERROR - Script augenrules_correct_without_key.pass.sh using profile (all) found issue:
ERROR - Rule evaluation resulted in fail, instead of expected pass during initial stage 
ERROR - The initial scan failed for rule 'xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock'.
ERROR - Script auditctl_correct.pass.sh using profile (all) found issue:
ERROR - Rule evaluation resulted in fail, instead of expected pass during initial stage 
ERROR - The initial scan failed for rule 'xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock'.
ERROR - Script auditctl_correct_extra_permission.pass.sh using profile (all) found issue:
ERROR - Rule evaluation resulted in fail, instead of expected pass during initial stage 
ERROR - The initial scan failed for rule 'xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock'.
INFO - Script augenrules_wrong_rule.fail.sh using profile (all) OK
INFO - Script augenrules_remove_all_rules.fail.sh using profile (all) OK
ERROR - Script augenrules_correct.pass.sh using profile (all) found issue:
ERROR - Rule evaluation resulted in fail, instead of expected pass during initial stage 
ERROR - The initial scan failed for rule 'xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock'.

They do pass on master (2ea9497).

@Mackemania
Copy link
Contributor Author

I ran the Automatus tests locally and got the following results

Setting console output to log level INFO
INFO - The base image option has not been specified, choosing libvirt-based test environment.
INFO - Logging into /home/mburket/code/ComplianceAsCode/content/tests/logs/rule-custom-2023-08-23-0817/test_suite.log
INFO - xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock
INFO - Script auditctl_wrong_rule.fail.sh using profile (all) OK
INFO - Script auditctl_wrong_rule_without_key.fail.sh using profile (all) OK
ERROR - Script auditctl_correct_without_key.pass.sh using profile (all) found issue:
ERROR - Rule evaluation resulted in fail, instead of expected pass during initial stage 
ERROR - The initial scan failed for rule 'xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock'.
INFO - Script auditctl_remove_all_rules.fail.sh using profile (all) OK
ERROR - Script augenrules_correct_extra_permission.pass.sh using profile (all) found issue:
ERROR - Rule evaluation resulted in fail, instead of expected pass during initial stage 
ERROR - The initial scan failed for rule 'xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock'.
INFO - Script augenrules_wrong_rule_without_key.fail.sh using profile (all) OK
ERROR - Script augenrules_correct_without_key.pass.sh using profile (all) found issue:
ERROR - Rule evaluation resulted in fail, instead of expected pass during initial stage 
ERROR - The initial scan failed for rule 'xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock'.
ERROR - Script auditctl_correct.pass.sh using profile (all) found issue:
ERROR - Rule evaluation resulted in fail, instead of expected pass during initial stage 
ERROR - The initial scan failed for rule 'xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock'.
ERROR - Script auditctl_correct_extra_permission.pass.sh using profile (all) found issue:
ERROR - Rule evaluation resulted in fail, instead of expected pass during initial stage 
ERROR - The initial scan failed for rule 'xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock'.
INFO - Script augenrules_wrong_rule.fail.sh using profile (all) OK
INFO - Script augenrules_remove_all_rules.fail.sh using profile (all) OK
ERROR - Script augenrules_correct.pass.sh using profile (all) found issue:
ERROR - Rule evaluation resulted in fail, instead of expected pass during initial stage 
ERROR - The initial scan failed for rule 'xccdf_org.ssgproject.content_rule_audit_rules_login_events_faillock'.

They do pass on master (2ea9497).

Thank you, I will check out what's causing the issues

@Mackemania Mackemania force-pushed the master branch 2 times, most recently from a778871 to 1efd42e Compare August 24, 2023 14:08
@Mackemania Mackemania changed the title Change rule to use variable when auditing faillock Draft: Change rule to use variable when auditing faillock Aug 24, 2023
@Mackemania Mackemania changed the title Draft: Change rule to use variable when auditing faillock Change rule to use variable when auditing faillock Aug 28, 2023
Some benchmarks specify different paths when monitoring the faillock
file, depending on if a permanent faillock path has been configured.
This updates the audit_rules_login_event_faillock to use the variable
var_accounts_passwords_pam_faillock_dir instead of the profile
faillock_path
The tests were moved to shared to be usable when the faillock path differs
in a profile from the globally set faillock path. The test now sets specific
profiles path before running the tests, since the XCCDF varaibles are not
available when running tests
@codeclimate
Copy link

codeclimate bot commented Aug 29, 2023

Code Climate has analyzed commit 5304dcb 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 53.3% (0.0% change).

View more on Code Climate.

@Mackemania
Copy link
Contributor Author

/packit retest-failed

@Mackemania
Copy link
Contributor Author

The Automatus SLE tests now pass when run locally. Though I can't seem to figure out why they fail in these tests.

@Mab879 Mab879 merged commit 2a682d3 into ComplianceAsCode:master Aug 29, 2023
ggbecker added a commit to ggbecker/content that referenced this pull request Oct 2, 2023
…les.

Rules as audit_rules_login_events_faillock were changed the behavior
in ComplianceAsCode#11007 and didn't have the content updated for RHEL7 according to the
previous default value of
https://github.com/ComplianceAsCode/content/blob/d044199f1b037e632197893adbe3ab2b78ee1138/ssg/constants.py#L475.
@Mab879 Mab879 added Update Rule Issues or pull requests related to Rules updates. CIS CIS Benchmark related. labels Oct 12, 2023
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. Update Rule Issues or pull requests related to Rules updates.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants