Rename account_passwords_pam_faillock_audit #9462

marcusburghardt · 2022-09-02T06:59:39Z

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 · 2022-09-02T07:00:47Z

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 · 2022-09-02T07:39:37Z

I will fix the stable-profiles test result soon.

jan-cerny · 2022-09-02T12:02:52Z

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

marcusburghardt · 2022-09-02T12:42:20Z

@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 · 2022-09-05T08:18:11Z

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 · 2022-09-22T06:59:08Z

@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?

marcusburghardt · 2022-09-22T10:13:46Z

@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 · 2022-10-24T08:00:20Z

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

marcusburghardt · 2022-10-25T09:04:31Z

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 · 2022-10-25T09:05:01Z

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 · 2022-10-25T09:05:55Z

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

jan-cerny

@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.

marcusburghardt · 2022-10-31T09:04:23Z

@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.

This rule was based on a existing rule with similar name: account_passwords_pam_faillock_audit The existing rule name was not aligned to the other similar rules making it prone to confusion. This commit duplicates code but it is worth in order to improve the project consistency. This will be solved in the future by deprecating the rule with inconsistent name.

The accounts_passwords_pam_faillock_audit rule was copied from account_passwords_pam_faillock_audit and both will coexist in the project for some time. Therefore, the OVAL ids used in the deprecated rule were renamed to avoid conflicts while the new rule kept the ids in order to keep consistency with other pam_faillock related rules.

To keep the project more consistent, a new rule was created based on an existing rule but using a name aligned to other similar rules. The old rule can't be removed in short term, but the new rule must replace the old one in control files.

marcusburghardt · 2022-10-31T09:24:51Z

@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.

codeclimate · 2022-10-31T10:17:40Z

Code Climate has analyzed commit 04a12e3 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.

marcusburghardt added Oracle Linux Oracle Linux product related. RHEL9 Red Hat Enterprise Linux 9 product related. RHEL8 Red Hat Enterprise Linux 8 product related. STIG STIG Benchmark related. labels Sep 2, 2022

marcusburghardt added this to the 0.1.64 milestone Sep 2, 2022

marcusburghardt requested a review from a team as a code owner September 2, 2022 06:59

jan-cerny self-assigned this Sep 2, 2022

vojtapolasek modified the milestones: 0.1.64, 0.1.65 Sep 19, 2022

marcusburghardt force-pushed the faillock_audit_rename branch from ee0364b to b7539aa Compare October 25, 2022 08:57

marcusburghardt added the Highlight This PR/Issue should make it to the featured changelog. label Oct 25, 2022

jan-cerny requested changes Oct 31, 2022

View reviewed changes

marcusburghardt added 6 commits October 31, 2022 10:24

Include deprecation warning in disaligned rule

df9f967

Update rule name in RHEL8 STIG profile

bd48cc1

Update rule name in OL8 STIG profile

158ddd6

Update SRG to use rule with consistent name

ff5d1d3

To keep the project more consistent, a new rule was created based on an existing rule but using a name aligned to other similar rules. The old rule can't be removed in short term, but the new rule must replace the old one in control files.

Update profiles for profile_stability tests

04a12e3

marcusburghardt force-pushed the faillock_audit_rename branch from b7539aa to 04a12e3 Compare October 31, 2022 09:24

marcusburghardt requested a review from jan-cerny October 31, 2022 09:24

jan-cerny approved these changes Oct 31, 2022

View reviewed changes

jan-cerny merged commit 40b629d into ComplianceAsCode:master Oct 31, 2022

marcusburghardt deleted the faillock_audit_rename branch October 31, 2022 12:24

marcusburghardt mentioned this pull request Nov 3, 2022

Duplicated rules for pam_faillock dir parameter #9466

Merged

yuumasato mentioned this pull request Nov 7, 2022

Remove STIGIDs from deprecated rule #9775

Merged

marcusburghardt mentioned this pull request Nov 9, 2022

Document rule deprecation instructions and agreements #9797

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Rename account_passwords_pam_faillock_audit #9462

Rename account_passwords_pam_faillock_audit #9462

Uh oh!

marcusburghardt commented Sep 2, 2022

Uh oh!

github-actions bot commented Sep 2, 2022 •

edited

Loading

Uh oh!

marcusburghardt commented Sep 2, 2022

Uh oh!

jan-cerny commented Sep 2, 2022

Uh oh!

marcusburghardt commented Sep 2, 2022 •

edited

Loading

Uh oh!

jan-cerny commented Sep 5, 2022

Uh oh!

jan-cerny commented Sep 22, 2022

Uh oh!

marcusburghardt commented Sep 22, 2022

Uh oh!

marcusburghardt commented Oct 24, 2022

Uh oh!

marcusburghardt commented Oct 25, 2022

Uh oh!

github-actions bot commented Oct 25, 2022

Uh oh!

marcusburghardt commented Oct 25, 2022

Uh oh!

jan-cerny left a comment

Uh oh!

marcusburghardt commented Oct 31, 2022

Uh oh!

marcusburghardt commented Oct 31, 2022

Uh oh!

codeclimate bot commented Oct 31, 2022

Uh oh!

Uh oh!

Rename account_passwords_pam_faillock_audit #9462

Rename account_passwords_pam_faillock_audit #9462

Uh oh!

Conversation

marcusburghardt commented Sep 2, 2022

Description:

Rationale:

Uh oh!

github-actions bot commented Sep 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marcusburghardt commented Sep 2, 2022

Uh oh!

jan-cerny commented Sep 2, 2022

Uh oh!

marcusburghardt commented Sep 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jan-cerny commented Sep 5, 2022

Uh oh!

jan-cerny commented Sep 22, 2022

Uh oh!

marcusburghardt commented Sep 22, 2022

Uh oh!

marcusburghardt commented Oct 24, 2022

Uh oh!

marcusburghardt commented Oct 25, 2022

Uh oh!

github-actions bot commented Oct 25, 2022

Uh oh!

marcusburghardt commented Oct 25, 2022

Uh oh!

jan-cerny left a comment

Choose a reason for hiding this comment

Uh oh!

marcusburghardt commented Oct 31, 2022

Uh oh!

marcusburghardt commented Oct 31, 2022

Uh oh!

codeclimate bot commented Oct 31, 2022

Uh oh!

Uh oh!

github-actions bot commented Sep 2, 2022 •

edited

Loading

marcusburghardt commented Sep 2, 2022 •

edited

Loading