Duplicated rules for pam_faillock dir parameter

[marcusburghardt]

Description:

There are two rules for the same purpose of assessing the pam_faillock.so dir parameter. The account_passwords_pam_faillock_dir (without s in account) only contains the rule definition, but no OVAL or remediation. The accounts_passwords_pam_faillock_dir (with s in account) is more complete, containing OVAL, remediation and test scenario scripts.

This PR includes some missing fields in the more complete rule and fixes some small issues on it to finally update the relevant profiles in order to use the more complete rule instead of the one with only rule description.

Rationale:

There are two rules for the same purpose and one should be deprecated in favor of the more complete rule.

[github-actions[bot]]

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

rhel8 (from CTF) Environment (using Fedora as testing environment)

Fedora Testing Environment

Oracle Linux 8 Environment

[github-actions[bot]]

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

Click here to see the full diff

New content has different text for rule 'xccdf_org.ssgproject.content_rule_account_passwords_pam_faillock_dir'.
--- xccdf_org.ssgproject.content_rule_account_passwords_pam_faillock_dir
+++ xccdf_org.ssgproject.content_rule_account_passwords_pam_faillock_dir
@@ -5,20 +5,16 @@
 [description]:
 By setting a `dir` in the faillock configuration account lockouts will persist across reboots.
 
+[warning]:
+This rule is deprecated in favor of the accounts_passwords_pam_faillock_dir rule.
+Please consider replacing this rule in your files as it is not expected to receive
+updates as of version 0.1.65.
+
 [reference]:
 CCI-000044
 
 [reference]:
 AC-7 (a)
-
-[reference]:
-SRG-OS-000021-GPOS-00005
-
-[reference]:
-RHEL-08-020017
-
-[reference]:
-SV-230339r743975_rule
 
 [rationale]:
 Having lockouts persist across reboots ensures that account is only unlocked by an administrator.

New content has different text for rule 'xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_dir'.
--- xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_dir
+++ xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_dir
@@ -4,7 +4,7 @@
 
 [description]:
 This rule ensures that the system lock out accounts using pam_faillock.so persist
-after system reboot. From "Pam_Faillock" man pages:
+after system reboot. From "pam_faillock" man pages:
 Note that the default directory that "pam_faillock" uses is usually cleared on system
 boot so the access will be reenabled after system reboot. If that is undesirable, a different
 tally directory must be set with the "dir" option.
@@ -45,12 +45,15 @@
 SRG-OS-000329-GPOS-00128
 
 [reference]:
-RHEL-08-020016
+RHEL-08-020017
 
 [reference]:
-SV-230338r627750_rule
+SV-230339r743975_rule
 
 [rationale]:
 Locking out user accounts after a number of incorrect attempts prevents direct password
 guessing attacks. In combination with the silent option, user enumeration attacks
 are also mitigated.
+
+[ident]:
+CCE-86067-6

OCIL for rule 'xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_dir' differs.
--- ocil:ssg-accounts_passwords_pam_faillock_dir_ocil:questionnaire:1
+++ ocil:ssg-accounts_passwords_pam_faillock_dir_ocil:questionnaire:1
@@ -1,5 +1,5 @@
 To ensure the tally directory is configured correctly, run the following command:
-$ grep 'dir =' /etc/security/faillock.conf
-The output should show that dir is set to something different to "/var/run/faillock"
- Is it the case that dir is not set or is set to /var/run/faillock?
+$ sudo grep 'dir =' /etc/security/faillock.conf
+The output should show that dir is set to something other than "/var/run/faillock"
+ Is it the case that the "dir" option is not set to a non-default documented tally log directory, is missing or commented out?
 
bash remediation for rule 'xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_dir' differs.
--- xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_dir
+++ xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_dir
@@ -1,5 +1,8 @@
 # Remediation is applicable only in certain platforms
 if rpm --quiet -q pam; then
+
+var_accounts_passwords_pam_faillock_dir=''
+
 
 if [ -f /usr/bin/authselect ]; then
 if ! authselect check; then
@@ -25,16 +28,15 @@
 sed -Ei 's/(auth.*)(\[default=die\])(.*pam_faillock\.so)/\1required \3/g' "$pam_file"
 done
 fi
- 
 AUTH_FILES=("/etc/pam.d/system-auth" "/etc/pam.d/password-auth")
 FAILLOCK_CONF="/etc/security/faillock.conf"
 if [ -f $FAILLOCK_CONF ]; then
 regex="^\s*dir\s*="
- line="dir = /var/log/faillock"
+ line="dir = $var_accounts_passwords_pam_faillock_dir"
 if ! grep -q $regex $FAILLOCK_CONF; then
 echo $line >> $FAILLOCK_CONF
 else
- sed -i --follow-symlinks 's|^\s*\(dir\s*=\s*\)\(\S\+\)|\1'"/var/log/faillock"'|g' $FAILLOCK_CONF
+ sed -i --follow-symlinks 's|^\s*\(dir\s*=\s*\)\(\S\+\)|\1'"$var_accounts_passwords_pam_faillock_dir"'|g' $FAILLOCK_CONF
 fi
 for pam_file in "${AUTH_FILES[@]}"
 do
@@ -85,15 +87,29 @@
 for pam_file in "${AUTH_FILES[@]}"
 do
 if ! grep -qE '^\s*auth.*pam_faillock\.so (preauth|authfail).*dir' "$pam_file"; then
- sed -i --follow-symlinks '/^auth.*required.*pam_faillock\.so.*preauth.*silent.*/ s/$/ dir='"/var/log/faillock"'/' "$pam_file"
- sed -i --follow-symlinks '/^auth.*required.*pam_faillock\.so.*authfail.*/ s/$/ dir='"/var/log/faillock"'/' "$pam_file"
+ sed -i --follow-symlinks '/^auth.*required.*pam_faillock\.so.*preauth.*silent.*/ s/$/ dir='"$var_accounts_passwords_pam_faillock_dir"'/' "$pam_file"
+ sed -i --follow-symlinks '/^auth.*required.*pam_faillock\.so.*authfail.*/ s/$/ dir='"$var_accounts_passwords_pam_faillock_dir"'/' "$pam_file"
 else
- sed -i --follow-symlinks 's/\(^auth.*required.*pam_faillock\.so.*preauth.*silent.*\)\('"dir"'=\)[0-9]\+\(.*\)/\1\2'"/var/log/faillock"'\3/' "$pam_file"
- sed -i --follow-symlinks 's/\(^auth.*required.*pam_faillock\.so.*authfail.*\)\('"dir"'=\)[0-9]\+\(.*\)/\1\2'"/var/log/faillock"'\3/' "$pam_file"
+ sed -i --follow-symlinks 's/\(^auth.*required.*pam_faillock\.so.*preauth.*silent.*\)\('"dir"'=\)[0-9]\+\(.*\)/\1\2'"$var_accounts_passwords_pam_faillock_dir"'\3/' "$pam_file"
+ sed -i --follow-symlinks 's/\(^auth.*required.*pam_faillock\.so.*authfail.*\)\('"dir"'=\)[0-9]\+\(.*\)/\1\2'"$var_accounts_passwords_pam_faillock_dir"'\3/' "$pam_file"
 fi
 done
 fi
 
+if ! rpm -q --quiet "python3-libselinux" ; then
+ yum install -y "python3-libselinux"
+fi
+if ! rpm -q --quiet "python3-policycoreutils" ; then
+ yum install -y "python3-policycoreutils"
+fi
+if ! rpm -q --quiet "policycoreutils-python-utils" ; then
+ yum install -y "policycoreutils-python-utils"
+fi
+
+mkdir -p "$var_accounts_passwords_pam_faillock_dir"
+semanage fcontext -a -t faillog_t "$var_accounts_passwords_pam_faillock_dir(/.*)?"
+restorecon -R -v "$var_accounts_passwords_pam_faillock_dir"
+
 else
 >&2 echo 'Remediation is not applicable, nothing was done'
 fi

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_dir' differs.
--- xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_dir
+++ xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_dir
@@ -2,7 +2,8 @@
 package_facts:
 manager: auto
 tags:
- - DISA-STIG-RHEL-08-020016
+ - CCE-86067-6
+ - DISA-STIG-RHEL-08-020017
 - NIST-800-53-AC-7(a)
 - NIST-800-53-AC-7(b)
 - NIST-800-53-AC-7.1(ii)
@@ -19,7 +20,8 @@
 register: result_authselect_present
 when: '"pam" in ansible_facts.packages'
 tags:
- - DISA-STIG-RHEL-08-020016
+ - CCE-86067-6
+ - DISA-STIG-RHEL-08-020017
 - NIST-800-53-AC-7(a)
 - NIST-800-53-AC-7(b)
 - NIST-800-53-AC-7.1(ii)
@@ -83,7 +85,8 @@
 - '"pam" in ansible_facts.packages'
 - result_authselect_present.stat.exists
 tags:
- - DISA-STIG-RHEL-08-020016
+ - CCE-86067-6
+ - DISA-STIG-RHEL-08-020017
 - NIST-800-53-AC-7(a)
 - NIST-800-53-AC-7(b)
 - NIST-800-53-AC-7.1(ii)
@@ -148,7 +151,8 @@
 - '"pam" in ansible_facts.packages'
 - not result_authselect_present.stat.exists
 tags:
- - DISA-STIG-RHEL-08-020016
+ - CCE-86067-6
+ - DISA-STIG-RHEL-08-020017
 - NIST-800-53-AC-7(a)
 - NIST-800-53-AC-7(b)
 - NIST-800-53-AC-7.1(ii)
@@ -171,7 +175,8 @@
 register: result_faillock_conf_check
 when: '"pam" in ansible_facts.packages'
 tags:
- - DISA-STIG-RHEL-08-020016
+ - CCE-86067-6
+ - DISA-STIG-RHEL-08-020017
 - NIST-800-53-AC-7(a)
 - NIST-800-53-AC-7(b)
 - NIST-800-53-AC-7.1(ii)
@@ -192,7 +197,8 @@
 - '"pam" in ansible_facts.packages'
 - result_faillock_conf_check.stat.exists
 tags:
- - DISA-STIG-RHEL-08-020016
+ - CCE-86067-6
+ - DISA-STIG-RHEL-08-020017
 - NIST-800-53-AC-7(a)
 - NIST-800-53-AC-7(b)
 - NIST-800-53-AC-7.1(ii)
@@ -536,7 +542,8 @@
 - '"pam" in ansible_facts.packages'
 - result_faillock_conf_check.stat.exists
 tags:
- - DISA-STIG-RHEL-08-020016
+ - CCE-86067-6
+ - DISA-STIG-RHEL-08-020017
 - NIST-800-53-AC-7(a)
 - NIST-800-53-AC-7(b)
 - NIST-800-53-AC-7.1(ii)
@@ -620,13 +627,95 @@
 - '"pam" in ansible_facts.packages'
 - not result_faillock_conf_check.stat.exists
 tags:
- - DISA-STIG-RHEL-08-020016
- - NIST-800-53-AC-7(a)
- - NIST-800-53-AC-7(b)
- - NIST-800-53-AC-7.1(ii)
- - accounts_passwords_pam_faillock_dir
- - configure_strategy
- - low_complexity
- - low_disruption
- - medium_severity
- - no_reboot_needed
+ - CCE-86067-6
+ - DISA-STIG-RHEL-08-020017
+ - NIST-800-53-AC-7(a)
+ - NIST-800-53-AC-7(b)
+ - NIST-800-53-AC-7.1(ii)
+ - accounts_passwords_pam_faillock_dir
+ - configure_strategy
+ - low_complexity
+ - low_disruption
+ - medium_severity
+ - no_reboot_needed
+
+- name: Lock Accounts Must Persist - Ensure necessary SELinux packages are installed
+ ansible.builtin.package:
+ name: '{{ item }}'
+ state: present
+ with_items:
+ - python3-libselinux
+ - python3-policycoreutils
+ - policycoreutils-python-utils
+ when: '"pam" in ansible_facts.packages'
+ tags:
+ - CCE-86067-6
+ - DISA-STIG-RHEL-08-020017
+ - NIST-800-53-AC-7(a)
+ - NIST-800-53-AC-7(b)
+ - NIST-800-53-AC-7.1(ii)
+ - accounts_passwords_pam_faillock_dir
+ - configure_strategy
+ - low_complexity
+ - low_disruption
+ - medium_severity
+ - no_reboot_needed
+
+- name: Lock Accounts Must Persist - Create the tally directory if it does not exist
+ ansible.builtin.file:
+ path: '{{ var_accounts_passwords_pam_faillock_dir }}'
+ state: directory
+ setype: faillog_t
+ when: '"pam" in ansible_facts.packages'
+ tags:
+ - CCE-86067-6
+ - DISA-STIG-RHEL-08-020017
+ - NIST-800-53-AC-7(a)
+ - NIST-800-53-AC-7(b)
+ - NIST-800-53-AC-7.1(ii)
+ - accounts_passwords_pam_faillock_dir
+ - configure_strategy
+ - low_complexity
+ - low_disruption
+ - medium_severity
+ - no_reboot_needed
+
+- name: Lock Accounts Must Persist - Ensure SELinux file context is permanently set
+ ansible.builtin.command:
+ cmd: semanage fcontext -a -t faillog_t "{{ var_accounts_passwords_pam_faillock_dir
+ }}(/.*)?"
+ register: result_accounts_passwords_pam_faillock_dir_semanage
+ ignore_errors: true
+ changed_when:
+ - result_accounts_passwords_pam_faillock_dir_semanage.rc == 0
+ when: '"pam" in ansible_facts.packages'
+ tags:
+ - CCE-86067-6
+ - DISA-STIG-RHEL-08-020017
+ - NIST-800-53-AC-7(a)
+ - NIST-800-53-AC-7(b)
+ - NIST-800-53-AC-7.1(ii)
+ - accounts_passwords_pam_faillock_dir
+ - configure_strategy
+ - low_complexity
+ - low_disruption
+ - medium_severity
+ - no_reboot_needed
+
+- name: Lock Accounts Must Persist - Ensure SELinux file context is applied
+ ansible.builtin.command:
+ cmd: restorecon -R "{{ var_accounts_passwords_pam_faillock_dir }}"
+ register: result_accounts_passwords_pam_faillock_dir_restorecon
+ when: '"pam" in ansible_facts.packages'
+ tags:
+ - CCE-86067-6
+ - DISA-STIG-RHEL-08-020017
+ - NIST-800-53-AC-7(a)
+ - NIST-800-53-AC-7(b)
+ - NIST-800-53-AC-7.1(ii)
+ - accounts_passwords_pam_faillock_dir
+ - configure_strategy
+ - low_complexity
+ - low_disruption
+ - medium_severity
+ - no_reboot_needed

[marcusburghardt]

/retest

[marcusburghardt]

Hello, this looks reasonable. I have one point though: I don't think you can reuse the CCE identifier in this case. Exceprt from the project documentation:

There should only be one CCE mapped to a rule as a global identifier. Any other usage of CCE is no longer considered a best practice. CCEs are also product dependent which means that a different CCE must be used for each different product and product version. For example if `cce@rhel7: 80328-8` exists in a rule, that CCE cannot be used for another product or version (e.g. rhel9), and the CCE MUST be retired
    with the rule.

But maybe @matejak knows more.

Thanks for remembering about this documentation @vojtapolasek . You are correct, the approach here should be to use a new CCE. I will update the PR soon.

[marcusburghardt]

I rebased the PR and updated it:

• The CCEs from account_passwords_pam_faillock_dir were preserved and new CCEs were included in the accounts_passwords_pam_faillock_dir rule
• The release version in the deprecation warning was updated from 0.1.64 to 0.1.65
• Minor correction in the accounts_passwords_pam_faillock_dir rule description. Man page name changed to lowercase

[vojtapolasek]

Thank you for updating the PR. I have some remarks:

1. Please remove STIG related parts of account_passwords_pam_faillock_dir, e.g. policy file, specific elements from the rule.yml, stigid reference etc. Or maybe better, move them to accounts_passwords_pam_faillock_dir.
2. The rule accounts_passwords_pam_faillock_dir has stigid reference but it is not present in RHEL8 STIG profile. Is this intentional?

[marcusburghardt]

Thank you for updating the PR. I have some remarks:

1. Please remove STIG related parts of account_passwords_pam_faillock_dir, e.g. policy file, specific elements from the rule.yml, stigid reference etc. Or maybe better, move them to accounts_passwords_pam_faillock_dir.
2. The rule accounts_passwords_pam_faillock_dir has stigid reference but it is not present in RHEL8 STIG profile. Is this intentional?

Good point @vojtapolasek . Removing the references from the account_passwords_pam_faillock_dir avoids confusion. I am just updating and also rebasing the PR.

Regarding the item 2, the rule was not there before so I didn't include it now. Probably it is not part of STIG for RHEL8. We can include in the future if necessary.

[marcusburghardt]

The testing-farm:centos-stream-9-x86_64 is failing with the account_passwords_pam_faillock_dir, which I believe shouldn't be tested anymore since it was replaced by accounts_passwords_pam_faillock_dir. I have to investigate this. Any hint is welcome!

[marcusburghardt]

I repeat. The testing-farm:centos-stream-9-x86_64 is a great test. Fortunately it can be set as required again. The error is legit. It is happening because in the first scan there is no faillock_dir configured. It is configured by the remediation but the SELinux context is not ensured. Therefore, in the second scan the account_password_selinux_faillock_dir rule correctly fails.

I will increment the accounts_passwords_pam_faillock_dir to cover this case.

[marcusburghardt]

@jan-cerny , this PR is somehow related to #9462

[marcusburghardt]

Rebased

[qlty-cloud-legacy[bot]]

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

View more on Code Climate.

[openshift-ci[bot]]

@marcusburghardt: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-rhcos4-moderate	c1773d532fa145630e3c50160dc6deb38d4be1cc	link	true	/test e2e-aws-rhcos4-moderate
ci/prow/e2e-aws-rhcos4-e8	c1773d532fa145630e3c50160dc6deb38d4be1cc	link	true	/test e2e-aws-rhcos4-e8

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

[jan-cerny]

/retest

[qlty-cloud-legacy[bot]]

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

View more on Code Climate.

[openshift-ci[bot]]

@marcusburghardt: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-rhcos4-moderate	bc9599039298580d6f20a6a4c390a51597d7e63b	link	true	/test e2e-aws-rhcos4-moderate

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.