Rename account_passwords_pam_faillock_audit

[marcusburghardt]

Description:

All other pam_faillock.so parameters related rules follow a different standard. It was basically renamed account_passwords_pam_faillock_audit to accounts_passwords_pam_faillock_audit.

Rationale:

Better sooner than later.

[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

[marcusburghardt]

I will fix the stable-profiles test result soon.

[jan-cerny]

@marcusburghardt the rename of a rule will break tailoring for people and will cause troubles for insights

[marcusburghardt]

@marcusburghardt the rename of a rule will break tailoring for people and will cause troubles for insights

@jan-cerny , this rule was introduced about just two months ago (2022-06-21). Unfortunately it was merged with this misalignment with other similar rules but since it is pretty fresh, I think the risk of breaking eventual tailoring made this meantime is low and acceptable in favor of keeping the project more organized. If we don't fix it now, it would be much painful to fix in the future, which probably wouldn't happen.

Could you give me more details about the impact on insights, please? So I can investigate and recalculate the risk. If we see the risk is too high, one option is to also keep the former name with an explicit deprecation note.

[jan-cerny]

The problem is that the Insights generate Ansible Playbooks for rules based on the latest (downstream?) release and if that changes they have a mismatch between set of available rules and set of generated Playbooks.

[jan-cerny]

@marcusburghardt I believe that the action plan here would be:

• We avoid hard problems by not removing anything from DS whenever we want to rename / split a rule.
• New and old rules should have “superseeds” and “obsoleted by” warnings for the transitional period.
• We keep the “old” rule in the DS for some time, so it is the tailoring that will get screwed.
• It is imperative that we document those “breaking” changes in release notes, ideally Insights should be able to consume them.
• Changes like PR #9462 are worth doing because it improves consistency.

Am I correct?

[marcusburghardt]

@marcusburghardt I believe that the action plan here would be:

• We avoid hard problems by not removing anything from DS whenever we want to rename / split a rule.
• New and old rules should have “superseeds” and “obsoleted by” warnings for the transitional period.
• We keep the “old” rule in the DS for some time, so it is the tailoring that will get screwed.
• It is imperative that we document those “breaking” changes in release notes, ideally Insights should be able to consume them.
• Changes like PR Rename account_passwords_pam_faillock_audit #9462 are worth doing because it improves consistency.

Am I correct?

Yes, that is correct @jan-cerny . I plan to update these PRs and documentation soon.

[marcusburghardt]

Hi, I plan to resume the work on this PR along the week.

[marcusburghardt]

I rebased the PR and updated it:

• Created a new rule called accounts_passwords_pam_faillock_dir based on the existing account_passwords_pam_faillock_dir rule instead of renaming the existing rule
• Included a deprecation warning in the account_passwords_pam_faillock_dir rule
• Updated the OVAL ids in the account_passwords_pam_faillock_dir rule in order avoid ids duplication

Although the commits ids were changed in favor of better organization after the changes, their contents were not modified beyond the changes described above.

[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_audit'.
--- xccdf_org.ssgproject.content_rule_account_passwords_pam_faillock_audit
+++ xccdf_org.ssgproject.content_rule_account_passwords_pam_faillock_audit
@@ -4,6 +4,11 @@
 
 [description]:
 PAM faillock locks an account due to excessive password failures, this event must be logged.
+
+[warning]:
+This rule is deprecated in favor of the accounts_passwords_pam_faillock_audit 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

OVAL for rule 'xccdf_org.ssgproject.content_rule_account_passwords_pam_faillock_audit' differs.
--- oval:ssg-account_passwords_pam_faillock_audit:def:1
+++ oval:ssg-account_passwords_pam_faillock_audit:def:1
@@ -1,9 +1,9 @@
 criteria OR
 criteria AND
-criterion oval:ssg-test_pam_faillock_audit_parameter_system_auth:tst:1
-criterion oval:ssg-test_pam_faillock_audit_parameter_password_auth:tst:1
-criterion oval:ssg-test_pam_faillock_audit_parameter_no_faillock_conf:tst:1
+criterion oval:ssg-test_account_pam_faillock_audit_parameter_system_auth:tst:1
+criterion oval:ssg-test_account_pam_faillock_audit_parameter_password_auth:tst:1
+criterion oval:ssg-test_account_pam_faillock_audit_parameter_no_faillock_conf:tst:1
 criteria AND
-criterion oval:ssg-test_pam_faillock_audit_parameter_no_pamd_system:tst:1
-criterion oval:ssg-test_pam_faillock_audit_parameter_no_pamd_password:tst:1
-criterion oval:ssg-test_pam_faillock_audit_parameter_faillock_conf:tst:1
+criterion oval:ssg-test_account_pam_faillock_audit_parameter_no_pamd_system:tst:1
+criterion oval:ssg-test_account_pam_faillock_audit_parameter_no_pamd_password:tst:1
+criterion oval:ssg-test_account_pam_faillock_audit_parameter_faillock_conf:tst:1

[marcusburghardt]

Included highlight label in this PR to make sure the changes are properly announced in the release notes.

[marcusburghardt]

@marcusburghardt Thanks for the update. It looks great! Could you please rebase once more because the CentOS stream 9 Testing farm CI job is now fixed so they switched it to required.

Thanks @jan-cerny . Sure, I will rebase it within the next 2 hours.

[marcusburghardt]

@marcusburghardt Thanks for the update. It looks great! Could you please rebase once more because the CentOS stream 9 Testing farm CI job is now fixed so they switched it to required.

Thanks @jan-cerny . Sure, I will rebase it within the next 2 hours.

Done.

[qlty-cloud-legacy[bot]]

Code Climate has analyzed commit 04a12e3d 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.5% (0.0% change).

View more on Code Climate.