Update bash remediation to fix bug into account_disable_inactivity* #12134
Conversation
The account_disable_inactivity_password_auth & account_disable_inactivity_system_auth bash remediation has a bug when ensure the order of lines into file, if the value of pam_lastlog.so is after pam_unix.so the remediation insert a new line after each line. Signed-off-by: Armando Acosta <[email protected]>
|
Hi @mrkanon. 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 Once the patch is verified, the new status will be reflected by the 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-sigs/prow repository. |
|
Start a new ephemeral environment with changes proposed in this pull request: ol8 (from CTF) Environment (using Fedora as testing environment) Fedora Testing Environment Oracle Linux 8 Environment |
|
This datastream diff is auto generated by the check Click here to see the full diffbash remediation for rule 'xccdf_org.ssgproject.content_rule_account_disable_inactivity_password_auth' differs.
--- xccdf_org.ssgproject.content_rule_account_disable_inactivity_password_auth
+++ xccdf_org.ssgproject.content_rule_account_disable_inactivity_password_auth
@@ -51,15 +51,23 @@
# The control is updated only if one single line matches.
sed -i -E --follow-symlinks 's/^(\s*auth\s+).*(\bpam_unix.so.*)/\1'"sufficient"' \2/' "$PAM_FILE_PATH"
else
- echo 'auth '"sufficient"' pam_unix.so' >> "$PAM_FILE_PATH"
+ LAST_MATCH_LINE=$(grep -nP "^\s*auth.*required.*pam_lastlog\.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 auth '"sufficient"' pam_unix.so' "$PAM_FILE_PATH"
+ else
+ echo 'auth '"sufficient"' pam_unix.so' >> "$PAM_FILE_PATH"
+ fi
fi
fi
# Ensure pam_unix.so is configured after pam_lastlog.so
if ! grep -Pz \
"auth\s*required\s*pam_lastlog\.so[^#]*inactive=35[\s\S]*\n\s*auth\s*sufficient\s*pam_unix\.so"\
"$PAM_FILE_PATH" ; then
- PAM_LASTLOG_LINE="$(grep -oP '^\s*auth.*pam_lastlog\.so.*' $PAM_FILE_PATH)"
- sed -i "0,/^\s*auth.*pam_unix\.so.*/i$PAM_LASTLOG_LINE" "$PAM_FILE_PATH"
+ readarray -t pam_lastlog_lines <<< "$(grep -oP '^\s*auth.*pam_lastlog\.so[^#]*inactive=35.*' $PAM_FILE_PATH)"
+ sed -i "/^\s*auth.*pam_lastlog\.so[^#]*inactive=35.*/d" "$PAM_FILE_PATH"
+ for line in "${pam_lastlog_lines[@]}"; do
+ sed -i "/^\s*auth.*pam_unix\.so.*/i$line" "$PAM_FILE_PATH"
+ done
fi
if [ -f /usr/bin/authselect ]; then
authselect apply-changes -b --backup=after-hardening-pam_lastlog.so-inactive.backup
bash remediation for rule 'xccdf_org.ssgproject.content_rule_account_disable_inactivity_system_auth' differs.
--- xccdf_org.ssgproject.content_rule_account_disable_inactivity_system_auth
+++ xccdf_org.ssgproject.content_rule_account_disable_inactivity_system_auth
@@ -51,15 +51,23 @@
# The control is updated only if one single line matches.
sed -i -E --follow-symlinks 's/^(\s*auth\s+).*(\bpam_unix.so.*)/\1'"sufficient"' \2/' "$PAM_FILE_PATH"
else
- echo 'auth '"sufficient"' pam_unix.so' >> "$PAM_FILE_PATH"
+ LAST_MATCH_LINE=$(grep -nP "^\s*auth.*required.*pam_lastlog\.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 auth '"sufficient"' pam_unix.so' "$PAM_FILE_PATH"
+ else
+ echo 'auth '"sufficient"' pam_unix.so' >> "$PAM_FILE_PATH"
+ fi
fi
fi
# Ensure pam_unix.so is configured after pam_lastlog.so
if ! grep -Pz \
"auth\s*required\s*pam_lastlog\.so[^#]*inactive=35[\s\S]*\n\s*auth\s*sufficient\s*pam_unix\.so"\
"$PAM_FILE_PATH" ; then
- PAM_LASTLOG_LINE="$(grep -oP '^\s*auth.*pam_lastlog\.so.*' $PAM_FILE_PATH)"
- sed -i "0,/^\s*auth.*pam_unix\.so.*/i$PAM_LASTLOG_LINE" "$PAM_FILE_PATH"
+ readarray -t pam_lastlog_lines <<< "$(grep -oP '^\s*auth.*pam_lastlog\.so[^#]*inactive=35.*' $PAM_FILE_PATH)"
+ sed -i "/^\s*auth.*pam_lastlog\.so[^#]*inactive=35.*/d" "$PAM_FILE_PATH"
+ for line in "${pam_lastlog_lines[@]}"; do
+ sed -i "/^\s*auth.*pam_unix\.so.*/i$line" "$PAM_FILE_PATH"
+ done
fi
if [ -f /usr/bin/authselect ]; then
authselect apply-changes -b --backup=after-hardening-pam_lastlog.so-inactive.backup |
|
🤖 A k8s content image for this PR is available at: Click here to see how to deploy itIf you alread have Compliance Operator deployed: Otherwise deploy the content and operator together by checking out ComplianceAsCode/compliance-operator and: |
marcusburghardt
added
Bash
| # Ensure pam_unix.so is configured after pam_lastlog.so | ||
| if ! grep -Pz \ | ||
| "auth\s*required\s*pam_lastlog\.so[^#]*inactive=35[\s\S]*\n\s*auth\s*sufficient\s*pam_unix\.so"\ | ||
| "$PAM_FILE_PATH" ; then | ||
| PAM_LASTLOG_LINE="$(grep -oP '^\s*auth.*pam_lastlog\.so.*' $PAM_FILE_PATH)" | ||
| sed -i "0,/^\s*auth.*pam_unix\.so.*/i$PAM_LASTLOG_LINE" "$PAM_FILE_PATH" | ||
| readarray -t pam_lastlog_lines <<< $(grep -oP '^\s*auth.*pam_lastlog\.so[^#]*inactive=35.*' $PAM_FILE_PATH) |
There was a problem hiding this comment.
the shellcheck warns about this, see the CI results
There was a problem hiding this comment.
The scenario is the test wrong_order.fail.sh, in that test we have as initial file
# Generated by authselect on Tue Jul 30 19:05:11 2024
# Do not modify this file manually.
auth required pam_env.so
auth required pam_faildelay.so delay=2000000
auth sufficient pam_unix.so nullok
auth required pam_lastlog.so inactive=35
auth required pam_deny.so
account required pam_unix.so
password requisite pam_pwquality.so
password sufficient pam_unix.so sha512 shadow nullok use_authtok
password required pam_deny.so
session optional pam_keyinit.so revoke
session required pam_limits.so
-session optional pam_systemd.so
session [success=1 default=ignore] pam_succeed_if.so service in crond quiet use_uid
session required pam_unix.so
and after the bash remediation we have
# Generated by authselect on Tue Jul 30 19:09:20 2024
# Do not modify this file manually.
auth required pam_lastlog.so inactive=35
auth required pam_env.so
auth required pam_lastlog.so inactive=35
auth required pam_faildelay.so delay=2000000
auth required pam_lastlog.so inactive=35
auth required pam_lastlog.so inactive=35
auth sufficient pam_unix.so nullok
auth required pam_lastlog.so inactive=35
auth required pam_deny.so
account required pam_unix.so
password requisite pam_pwquality.so
password sufficient pam_unix.so sha512 shadow nullok use_authtok
password required pam_deny.so
The automatus tool mark that test as correct because the condition pass but not as expected.
There was a problem hiding this comment.
OK makes sense
So just please fix the shellcheck issues
Fix missing quote in account_disable_inactivity_system_auth & account_disable_inactivity_password_auth bash remediation Signed-off-by: Armando Acosta <[email protected]>
|
Code Climate has analyzed commit b2cc9e6 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 59.4% (0.0% change). View more on Code Climate. |
There was a problem hiding this comment.
I have add these rules to RHEL 9 default profile and I have added the bash remediations to RHEL. Then, I have executed the test scenarios.
jcerny@fedora:~/work/git/scap-security-guide (pr/12134)$ python3 tests/automatus.py rule --libvirt qemu:///system ssgts_rhel9 account_disable_inactivity_password_auth account_disable_inactivity_system_auth
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/jcerny/work/git/scap-security-guide/logs/rule-custom-2024-08-02-0814/test_suite.log
INFO - xccdf_org.ssgproject.content_rule_account_disable_inactivity_password_auth
INFO - Script correct_value.pass.sh using profile (all) OK
INFO - Script line_not_there.fail.sh using profile (all) OK
INFO - Script stricter_value.pass.sh using profile (all) OK
INFO - Script wrong_order.fail.sh using profile (all) OK
INFO - Script wrong_value.fail.sh using profile (all) OK
INFO - xccdf_org.ssgproject.content_rule_account_disable_inactivity_system_auth
INFO - Script correct_value.pass.sh using profile (all) OK
INFO - Script line_not_there.fail.sh using profile (all) OK
INFO - Script stricter_value.pass.sh using profile (all) OK
INFO - Script wrong_order.fail.sh using profile (all) OK
INFO - Script wrong_value.fail.sh using profile (all) OK
Description:
Update bash remediation to fix bug into
account_disable_inactivity_system_authandaccount_disable_inactivity_password_auth.Fix the way to delete and insert the existent lines.
Rationale:
The
account_disable_inactivity_password_authandaccount_disable_inactivity_system_authbash remediation has a bug when ensure the correct order of lines into file. The script inserts thepam_lastlog.soline after each line from 0 to where it finds thepam_unix.solineNote:
I believe this error can be resolved with a change to the
bash_ensure_pam_module_linemacro. That macro accepts a regular expression to ensure the expected line is inserted this regex match, but only if the expected line is not there, it doesn't reorder in case order was wrong.