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 PAM control flag for pam_pwhistory.so #10021

Merged

Conversation

marcusburghardt
Copy link
Member

Description:

Newer versions of RHEL8 and RHEL9 are using authselect for PAM configurations.
When the feature with_pwhistory for pam_pwhistory is available, it uses requisite as control flag for pam_pwhistory.so module instead of required.

Rationale:

Although the man for pam_pwhistory.so contains examples to configure the module using required control flag, this is not the default control flag in RHEL8 and RHEL9 when using authselect.
For RHEL7 there is no authselect. However, it makes sense to keep it aligned.

In practical, changing from required to requisite has no technical impact for this case but only user experience impact since using the requisite control flag the user will be informed immediately.

requisite is the control flag required by CIS and also the default for
authselect profiles. This commit updates the RHEL8 and RHEL7
controlfiles to use requisite instead of required.
Newer versions of RHEL8 and RHEL9 are using authselect for PAM
configurations. When the feature for pam_pwhistory is available, it uses
requisite as control flag for pam_pwhistory.so module. The STIG
benchmark for RHEL8 should be updated soon.
The lines referring to pam_pwhistory.so are now getting the control flag
from the variable.
There was one extra parameter in pam_pwhistory related macro which was
impacting in the Bash remediation effectiveness for systems without
authselect.
The parameter had a different name in the macro description. Since the
name used in the description was more self-explanatory than the name
used in the macro, the macro content was updated. This change has no
technical impact but aligns the macro description to the respective
macro content.
@marcusburghardt marcusburghardt added bugfix Fixes to reported bugs. RHEL Red Hat Enterprise Linux product related. labels Jan 4, 2023
@marcusburghardt marcusburghardt added this to the 0.1.66 milestone Jan 4, 2023
@marcusburghardt marcusburghardt requested a review from a team as a code owner January 4, 2023 13:44
@github-actions
Copy link

github-actions bot commented Jan 4, 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 Jan 4, 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_accounts_password_pam_pwhistory_remember_password_auth' differs.
--- xccdf_org.ssgproject.content_rule_accounts_password_pam_pwhistory_remember_password_auth
+++ xccdf_org.ssgproject.content_rule_accounts_password_pam_pwhistory_remember_password_auth
@@ -56,7 +56,12 @@
 # The control is updated only if one single line matches.
 sed -i -E --follow-symlinks 's/^(\s*password\s+).*(\bpam_pwhistory.so.*)/\1'"$var_password_pam_remember_control_flag"' \2/' "$PAM_FILE_PATH"
 else
- echo 'password '"$var_password_pam_remember_control_flag"' pam_pwhistory.so' >> "$PAM_FILE_PATH"
+ LAST_MATCH_LINE=$(grep -nP "^password.*requisite.*pam_pwquality\.so" "$PAM_FILE_PATH" | tail -n 1 | cut -d: -f 1)
+ if [ ! -z $LAST_MATCH_LINE ]; then
+ sed -i --follow-symlinks $LAST_MATCH_LINE' a password '"$var_password_pam_remember_control_flag"' pam_pwhistory.so' "$PAM_FILE_PATH"
+ else
+ echo 'password '"$var_password_pam_remember_control_flag"' pam_pwhistory.so' >> "$PAM_FILE_PATH"
+ fi
 fi
 fi
 fi
@@ -67,7 +72,12 @@
 # The control is updated only if one single line matches.
 sed -i -E --follow-symlinks 's/^(\s*password\s+).*(\bpam_pwhistory.so.*)/\1'"$var_password_pam_remember_control_flag"' \2/' "/etc/pam.d/password-auth"
 else
- echo 'password '"$var_password_pam_remember_control_flag"' pam_pwhistory.so' >> "/etc/pam.d/password-auth"
+ LAST_MATCH_LINE=$(grep -nP "^password.*requisite.*pam_pwquality\.so" "/etc/pam.d/password-auth" | tail -n 1 | cut -d: -f 1)
+ if [ ! -z $LAST_MATCH_LINE ]; then
+ sed -i --follow-symlinks $LAST_MATCH_LINE' a password '"$var_password_pam_remember_control_flag"' pam_pwhistory.so' "/etc/pam.d/password-auth"
+ else
+ echo 'password '"$var_password_pam_remember_control_flag"' pam_pwhistory.so' >> "/etc/pam.d/password-auth"
+ fi
 fi
 fi
 fi

bash remediation for rule 'xccdf_org.ssgproject.content_rule_accounts_password_pam_pwhistory_remember_system_auth' differs.
--- xccdf_org.ssgproject.content_rule_accounts_password_pam_pwhistory_remember_system_auth
+++ xccdf_org.ssgproject.content_rule_accounts_password_pam_pwhistory_remember_system_auth
@@ -56,7 +56,12 @@
 # The control is updated only if one single line matches.
 sed -i -E --follow-symlinks 's/^(\s*password\s+).*(\bpam_pwhistory.so.*)/\1'"$var_password_pam_remember_control_flag"' \2/' "$PAM_FILE_PATH"
 else
- echo 'password '"$var_password_pam_remember_control_flag"' pam_pwhistory.so' >> "$PAM_FILE_PATH"
+ LAST_MATCH_LINE=$(grep -nP "^password.*requisite.*pam_pwquality\.so" "$PAM_FILE_PATH" | tail -n 1 | cut -d: -f 1)
+ if [ ! -z $LAST_MATCH_LINE ]; then
+ sed -i --follow-symlinks $LAST_MATCH_LINE' a password '"$var_password_pam_remember_control_flag"' pam_pwhistory.so' "$PAM_FILE_PATH"
+ else
+ echo 'password '"$var_password_pam_remember_control_flag"' pam_pwhistory.so' >> "$PAM_FILE_PATH"
+ fi
 fi
 fi
 fi
@@ -67,7 +72,12 @@
 # The control is updated only if one single line matches.
 sed -i -E --follow-symlinks 's/^(\s*password\s+).*(\bpam_pwhistory.so.*)/\1'"$var_password_pam_remember_control_flag"' \2/' "/etc/pam.d/system-auth"
 else
- echo 'password '"$var_password_pam_remember_control_flag"' pam_pwhistory.so' >> "/etc/pam.d/system-auth"
+ LAST_MATCH_LINE=$(grep -nP "^password.*requisite.*pam_pwquality\.so" "/etc/pam.d/system-auth" | tail -n 1 | cut -d: -f 1)
+ if [ ! -z $LAST_MATCH_LINE ]; then
+ sed -i --follow-symlinks $LAST_MATCH_LINE' a password '"$var_password_pam_remember_control_flag"' pam_pwhistory.so' "/etc/pam.d/system-auth"
+ else
+ echo 'password '"$var_password_pam_remember_control_flag"' pam_pwhistory.so' >> "/etc/pam.d/system-auth"
+ fi
 fi
 fi
 fi

bash remediation for rule 'xccdf_org.ssgproject.content_rule_accounts_password_pam_unix_remember' differs.
--- xccdf_org.ssgproject.content_rule_accounts_password_pam_unix_remember
+++ xccdf_org.ssgproject.content_rule_accounts_password_pam_unix_remember
@@ -53,7 +53,12 @@
 # The control is updated only if one single line matches.
 sed -i -E --follow-symlinks 's/^(\s*password\s+).*(\bpam_pwhistory.so.*)/\1'"requisite"' \2/' "$PAM_FILE_PATH"
 else
- echo 'password '"requisite"' pam_pwhistory.so' >> "$PAM_FILE_PATH"
+ LAST_MATCH_LINE=$(grep -nP "^password.*requisite.*pam_pwquality\.so" "$PAM_FILE_PATH" | tail -n 1 | cut -d: -f 1)
+ if [ ! -z $LAST_MATCH_LINE ]; then
+ sed -i --follow-symlinks $LAST_MATCH_LINE' a password '"requisite"' pam_pwhistory.so' "$PAM_FILE_PATH"
+ else
+ echo 'password '"requisite"' pam_pwhistory.so' >> "$PAM_FILE_PATH"
+ fi
 fi
 fi
 fi
@@ -64,7 +69,12 @@
 # The control is updated only if one single line matches.
 sed -i -E --follow-symlinks 's/^(\s*password\s+).*(\bpam_pwhistory.so.*)/\1'"requisite"' \2/' "/etc/pam.d/system-auth"
 else
- echo 'password '"requisite"' pam_pwhistory.so' >> "/etc/pam.d/system-auth"
+ LAST_MATCH_LINE=$(grep -nP "^password.*requisite.*pam_pwquality\.so" "/etc/pam.d/system-auth" | tail -n 1 | cut -d: -f 1)
+ if [ ! -z $LAST_MATCH_LINE ]; then
+ sed -i --follow-symlinks $LAST_MATCH_LINE' a password '"requisite"' pam_pwhistory.so' "/etc/pam.d/system-auth"
+ else
+ echo 'password '"requisite"' pam_pwhistory.so' >> "/etc/pam.d/system-auth"
+ fi
 fi
 fi
 fi

@mildas
Copy link
Contributor

mildas commented Jan 4, 2023

I understand there is no technical impact, but might it be a problem for DISA? @Mab879

For RHEL7 (CentOS7), do you think it will fix https://artifacts.dev.testing-farm.io/7a51726e-fd3d-400c-ac21-e72905b06a77/ ? If so, it would be amazing because it would unblock #9942

@marcusburghardt
Copy link
Member Author

I understand there is no technical impact, but might it be a problem for DISA? @Mab879

For RHEL7 (CentOS7), do you think it will fix https://artifacts.dev.testing-farm.io/7a51726e-fd3d-400c-ac21-e72905b06a77/ ? If so, it would be amazing because it would unblock #9942

Yes, it fixes these rules in RHEL7 too. I successfully reproduced the error and confirmed the fix locally.

@marcusburghardt
Copy link
Member Author

I understand there is no technical impact, but might it be a problem for DISA? @Mab879

For RHEL7 (CentOS7), do you think it will fix https://artifacts.dev.testing-farm.io/7a51726e-fd3d-400c-ac21-e72905b06a77/ ? If so, it would be amazing because it would unblock #9942

@Mab879 @ggbecker , we should communicate DISA to update this control flag in the "Fix Text". The assessment is already fine. For CIS I already proposed the change for RHEL7. CIS RHEL8 and RHEL9 are already aligned to use requisite.

@marcusburghardt
Copy link
Member Author

/retest

1 similar comment
@marcusburghardt
Copy link
Member Author

/retest

@vojtapolasek vojtapolasek self-assigned this Jan 5, 2023
Copy link
Contributor

@mildas mildas left a comment

Choose a reason for hiding this comment

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

Changes look good. The rules are passing in TF CI now (they are not being waived anymore).

@Mab879
Copy link
Member

Mab879 commented Jan 5, 2023

I understand there is no technical impact, but might it be a problem for DISA? @Mab879
For RHEL7 (CentOS7), do you think it will fix https://artifacts.dev.testing-farm.io/7a51726e-fd3d-400c-ac21-e72905b06a77/ ? If so, it would be amazing because it would unblock #9942

@Mab879 @ggbecker , we should communicate DISA to update this control flag in the "Fix Text". The assessment is already fine. For CIS I already proposed the change for RHEL7. CIS RHEL8 and RHEL9 are already aligned to use requisite.

We can get this in our next round of feedback to them.

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 discovered one failing test scenario in RHEL7:
ERROR - Script rhel7_correct_value_cis_l2.pass.sh using profile xccdf_org.ssgproject.content_profile_cis found issue:
ERROR - Rule evaluation resulted in fail, instead of expected pass during initial stage

@codeclimate
Copy link

codeclimate bot commented Jan 6, 2023

Code Climate has analyzed commit 17563c2 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 49.8% (0.0% change).

View more on Code Climate.

@marcusburghardt
Copy link
Member Author

/retest

@vojtapolasek
Copy link
Collaborator

I am merging this PR, thank you @marcusburghardt for the fix. The Ansible lint test is broken now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixes to reported bugs. RHEL Red Hat Enterprise Linux product related.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pam_pwhistory rules are failing after cis_workstation_l2 hardening
4 participants