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

accounts_password_set_min_life_existing: Avoid system accounts #9955

Merged
merged 9 commits into from
Jan 6, 2023

Conversation

dodys
Copy link
Contributor

@dodys dodys commented Dec 12, 2022

Description:

@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 Dec 12, 2022

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_set_min_life_existing' differs.
--- xccdf_org.ssgproject.content_rule_accounts_password_set_min_life_existing
+++ xccdf_org.ssgproject.content_rule_accounts_password_set_min_life_existing
@@ -3,5 +3,5 @@
 
 
 while IFS= read -r i; do
- passwd -n $var_accounts_minimum_age_login_defs $i
-done < <(awk -v var="$var_accounts_minimum_age_login_defs" -F: '$4 < var || $4 == "" {print $1}' /etc/shadow)
+ chage -m $var_accounts_minimum_age_login_defs $i
+done < <(awk -v var="$var_accounts_minimum_age_login_defs" -F: '(/^[^:]+:[^!*]/ && ($4 < var || $4 == "")) {print $1}' /etc/shadow)

@dodys
Copy link
Contributor Author

dodys commented Dec 13, 2022

/retest

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.

The errors in testing-farm:centos-stream-8-x86_64 and testing-farm:centos-stream-9-x86_64 seems to be legit. The rule is still failing after the Ansible remediation. Could you check it, please?

@dodys
Copy link
Contributor Author

dodys commented Dec 14, 2022

@marcusburghardt I'm not really sure why it is failing for fedora and CentOS, do you have any idea?

@marcusburghardt
Copy link
Member

@marcusburghardt I'm not really sure why it is failing for fedora and CentOS, do you have any idea?

No yet but I can take a look on this.

@freddieRv
Copy link
Contributor

Hi! Doing some testing on our side with these commits backported we found an issue were the correct_stig_noninteractive.pass.sh test results in a false negative when there are no interactive users on the system (other than root).

This issue is also present on the accounts_password_set_max_life_existing rule with the changes from #9954

Changing the check_existence attribute of the OVAL tests to any_exist fixes this. However with this change the incorrect_min_pass_age.fail.sh test reports that "Rule evaluation resulted in pass, instead of expected fail during initial stage". An interactive user could be added on this test to make sure there is at least one ill-configured user to fail the test.

@dodys
Copy link
Contributor Author

dodys commented Dec 19, 2022

Hi! Doing some testing on our side with these commits backported we found an issue were the correct_stig_noninteractive.pass.sh test results in a false negative when there are no interactive users on the system (other than root).

This issue is also present on the accounts_password_set_max_life_existing rule with the changes from #9954

Changing the check_existence attribute of the OVAL tests to any_exist fixes this. However with this change the incorrect_min_pass_age.fail.sh test reports that "Rule evaluation resulted in pass, instead of expected fail during initial stage". An interactive user could be added on this test to make sure there is at least one ill-configured user to fail the test.

Thanks Freddie for testing this PR. Indeed your suggestions make sense, so I add a new commit changing the existence check to any_exist and simplified to incorrect_min_pass_age.fail.sh test.
Let me know in case of issues.
I will be doing the same for max life.

@dodys dodys added Ansible Ansible remediation update. OVAL OVAL update. Related to the systems assessments. Bash Bash remediation update. labels Dec 19, 2022
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.

Please, take a look in the comments from #9954

test user account.

Suggested-by: Marcus Burghardt <maburgha@redhat.com>
@codeclimate
Copy link

codeclimate bot commented Jan 6, 2023

Code Climate has analyzed commit 2ef1c60 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.9% (0.1% change).

View more on Code Climate.

@marcusburghardt marcusburghardt added this to the 0.1.66 milestone Jan 6, 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.

Thanks for the improvements in this rule @dodys .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ansible Ansible remediation update. Bash Bash remediation update. OVAL OVAL update. Related to the systems assessments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants