Change rules related to /etc/shadow to check only local user configuration #10838
Conversation
jan-cerny
added
bugfix
|
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 |
|
This datastream diff is auto generated by the check Click here to see the full diffOVAL for rule 'xccdf_org.ssgproject.content_rule_accounts_password_set_max_life_existing' differs.
--- oval:ssg-accounts_password_set_max_life_existing:def:1
+++ oval:ssg-accounts_password_set_max_life_existing:def:1
@@ -1,3 +1,4 @@
criteria AND
-criterion oval:ssg-test_password_max_life_existing:tst:1
-criterion oval:ssg-test_password_max_life_existing_minimum:tst:1
+criterion oval:ssg-test_accounts_password_set_max_life_existing_password_max_life_existing:tst:1
+criterion oval:ssg-test_accounts_password_set_max_life_existing_password_max_life_existing_minimum:tst:1
+criterion oval:ssg-test_accounts_password_set_max_life_existing_password_max_life_not_empty:tst:1
OVAL for rule 'xccdf_org.ssgproject.content_rule_accounts_password_set_min_life_existing' differs.
--- oval:ssg-accounts_password_set_min_life_existing:def:1
+++ oval:ssg-accounts_password_set_min_life_existing:def:1
@@ -1,3 +1,4 @@
criteria AND
-criterion oval:ssg-test_password_min_life_existing:tst:1
-criterion oval:ssg-test_password_min_life_existing_maximum:tst:1
+criterion oval:ssg-test_accounts_password_set_min_life_existing_password_max_life_existing:tst:1
+criterion oval:ssg-test_accounts_password_set_min_life_existing_password_max_life_existing_minimum:tst:1
+criterion oval:ssg-test_accounts_password_set_min_life_existing_password_max_life_not_empty:tst:1 |
|
I have removed a parameter that has a low meaning. |
Change OVAL in rule `accounts_password_set_max_life_existing` so that it doesn't use the OVAL "unix:shadow_object" element which checks also remote users via NSS. Instead, it will parse `/etc/shadow` using the "ind:textfilecontent54_object" which will include only local users. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2213958
Moving the OVAL code to a new jinja macro `etc_shadow_test` will allow us to reduce the code in other similar rules.
Change OVAL in rule `accounts_password_set_min_life_existing` so that it doesn't use the OVAL "unix:shadow_object" element which checks also remote users via NSS. Instead, it will parse `/etc/shadow` using the "ind:textfilecontent54_object" which will include only local users. This change is similar to the recent change that has been done in `accounts_password_set_max_life_existing` and leverages the OVAL macro. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2213958
Change OVAL in rule `accounts_password_set_warn_age_existing` so that it doesn't use the OVAL "unix:shadow_object" element which checks also remote users via NSS. Instead, it will parse `/etc/shadow` using the "ind:textfilecontent54_object" which will include only local users. This change is similar to the recent change that has been done in `accounts_password_set_max_life_existing` and `accounts_password_set_min_life_existing`. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2213958
Extracts some OVAL code to a new macro textfilecontent54_test_etc_shadow which can be reused later.
In rule accounts_password_set_warn_age_existing we will replace an OVAL test with a macro textfilecontent54_test_etc_shadow to reduce code duplication.
Change OVAL in rule `accounts_set_post_pw_existing` so that it doesn't use the OVAL "unix:shadow_object" element which checks also remote users via NSS. Instead, it will parse `/etc/shadow` using the "ind:textfilecontent54_object" which will include only local users. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2213958
Unify the OVAL code used in rules accounts_password_set_warn_age_existing and accounts_set_post_pw_existing. The code is very similar so it can be easily replaced by a Jinja macro. That prevents a code duplication.
We will remove the `field` parameter of the macro `etc_shadow_test` because this parameter is only substituted into an XML comment in OVAL and doesn't impact the actual behavior, so its added value is low I think.
|
I have rebased this PR on the top of the latest upstream master branch. |
shared/macros/10-oval.jinja
Outdated
|
|
||
| <ind:textfilecontent54_object id="object_{{{ test_id }}}" version="1"> | ||
| <ind:filepath>/etc/shadow</ind:filepath> | ||
| <!-- filters away accounts with no passwords and locked passwords (passwords are located in the 2nd field)--> |
There was a problem hiding this comment.
@jan-cerny I don't understand this comment. Can you explain please? It confuses me because the next regexp is supplied in form of macro argument.
There was a problem hiding this comment.
You're right that it's confusing. It's left there from the previous version where this wasn't a macro. Although in the current state it generates a correct statement in the generated OVAL, it doesn't make sense to have it here. It would be better to remove it and instead add a similar comment to the place where this is called.
|
@vojtapolasek I have resolved the confusing comment by removing the comment from the macro code and instead adding comments to places where the regular expressions are defined. |
|
Code Climate has analyzed commit e7424a0 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.4% (0.0% change). View more on Code Climate. |
Description:
This PR changes OVAL checks in rules that are related to
/etc/shadowconfiguration in a way that the OVAL check will use OVAL textfilecontent54_test to parse data directly from/etc/shadowinstead of using the OVAL shadow_test.This PR changes the following rules:
Moreover, some OVAL code was refactored to Jinja macros in order to share code and prevent code duplication.
Rationale:
Using OVAL shadow_test means that the checks also consider remote (LDAP) users, because the implementation of the shadow probe in OpenSCAP uses the
getpwent()function which read all users from NSS maps, including non-local users. That isn't desired. The rules should check local configuration. The remediation can't fix remote users, only local ones.I think it is reasonable to change scope of those rules so that remediation and check are aligned.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2213958
Review Hints: