8000 Remove specific metadata in shared Bash remediations by matejak · Pull Request #7254 · ComplianceAsCode/content · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Remove specific metadata in shared Bash remediations #7254

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 27, 2021
Merged

Remove specific metadata in shared Bash remediations #7254

merged 1 commit into from
Jul 27, 2021

Conversation

matejak
Copy link
Member
@matejak matejak commented Jul 15, 2021 •
edited
Loading

This PR improves the problematic situation with remediations for different products and the respective metadata.

When a rule has exactly one shared.sh file, then cases when remediation shouldn't be performed are extremely rare, and even then they should be handled by means of the more flexible jinja macros.

accounts_password_warn_age_login_defs and accounts_minimum_age_login_defs got their remediations that were specialized for no good reason unified.

Question: Is there any reason that e.g. https://github.com/ComplianceAsCode/content/blob/master/linux_os/guide/system/network/network-iptables/iptables_ruleset_modifications/set_iptables_default_rule_forward/rule.yml had RHEV-only remediation? The history doesn't indicate it at all. (answered below - there is no reason).

Fixes: #7246

This PR has been separated from its Ansible counterpart, as Ansible remediations in the project are more tricky regarding product-specificity.

@matejak matejak added this to the 0.1.57 milestone Jul 15, 2021
@matejak matejak added the RHEL9 Red Hat Enterprise Linux 9 product related. label Jul 15, 2021
@yuumasato
Copy link
Member

Question: Is there any reason that e.g. https://github.com/ComplianceAsCode/content/blob/master/linux_os/guide/system/network/network-iptables/iptables_ruleset_modifications/set_iptables_default_rule_forward/rule.yml had RHEV-only remediation? The history doesn't indicate it at all.

From https://github.com/ComplianceAsCode/content/commits/master/linux_os/guide/system/network/network-iptables/iptables_ruleset_modifications/set_iptables_default_rule_forward/bash

It seems to me that it was not intentional.
The remediation was marked as rhel6 only, then the rhv4 was added.
But when rhel6 product was removed, the rhel6 marking went away and only rhv4 remained.
As the rule has no specific prodtype the remediation very likely applies to many products.

yuumasato
yuumasato approved these changes Jul 15, 2021
View reviewed changes
Copy link
Member
@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@matejak
Copy link
Member Author
matejak commented Jul 15, 2021

I did the same with Ansible, and that will be it.

@matejak matejak force-pushed the remediation_metadata branch from fba10e0 to 63c44e5 Compare July 16, 2021 11:29
vojtapolasek
vojtapolasek requested changes Jul 16, 2021
View reviewed changes
Copy link
Collaborator
@vojtapolasek vojtapolasek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, at some places I just pointed out some misalignments, fell free to comment.

...ounts-physical/screen_locking/smart_card_login/install_smartcard_packages/ansible/shared.yml Outdated
@@ -1,4 +1,4 @@
# platform = multi_platform_sle
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is originally only for SLE. Do you want to include it also in rhel?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely a problem. The rule.yml uses package installed template. This should either be reverted or the file should be split into two: sle12.yml and sle15.yml

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ggbecker @matejak Just as a note, in #6792 we allowed the generic sle.yml so you don't have to worry about duplicated content.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now actually documented in #7291

10000

...nts/accounts-restrictions/password_expiration/accounts_minimum_age_login_defs/bash/shared.sh Outdated
. /usr/share/scap-security-guide/remediation_functions
declare var_accounts_minimum_age_login_defs
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is needed anymore.

...counts-restrictions/password_expiration/accounts_password_warn_age_login_defs/bash/shared.sh Outdated
. /usr/share/scap-security-guide/remediation_functions
declare var_accounts_password_warn_age_login_defs
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is needed anymore.

linux_os/guide/system/accounts/accounts-session/accounts_tmout/bash/shared.sh Outdated
@@ -1,4 +1,4 @@
# platform = Red Hat Enterprise Linux 7,Red Hat Enterprise Linux 8,Red Hat Virtualization 4,multi_platform_fedora,multi_platform_ol,multi_platform_wrlinux
# platform = multi_platform_rhel,Red Hat Virtualization 4,multi_platform_fedora,multi_platform_ol,multi_platform_wrlinux
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not multi_platform_all here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, the rule has an opt-in prodtype, so there is no reason to be too careful here.

...ssions/files/permissions_within_important_dirs/dir_ownership_library_dirs/ansible/shared.yml Outdated
@@ -1,4 +1,4 @@
# platform = multi_platform_sle
# platform = multi_platform_all
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is originally for SLE only... is that OK?

8000
.../guide/system/software/integrity/software-integrity/aide/aide_verify_acls/ansible/shared.yml Outdated
@@ -1,4 +1,4 @@
# platform = multi_platform_sle
# platform = multi_platform_all
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this one originally SLE only?

...tem/software/integrity/software-integrity/aide/aide_verify_ext_attributes/ansible/shared.yml Outdated
@@ -1,4 +1,4 @@
# platform = multi_platform_sle
# platform = multi_platform_all
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this is SLE only

@matejak
Copy link
Member Author
matejak commented Jul 16, 2021

I can't answer the Ansible applicability questions before I take off for my PTO, so if you don't want to seek the answers by yourselves, then please split the PR and use only the first commit.

@matejak matejak force-pushed the remediation_metadata branch from 63c44e5 to 5a88638 Compare July 27, 2021 09:32
@matejak matejak changed the title Remove specific metadata in shared remediations Remove specific metadata in shared Bash remediations Jul 27, 2021
@matejak matejak mentioned this pull request Jul 27, 2021
Closed
@matejak
Remove specific metadata in shared Bash remediations
1958119 
When a rule has exactly one shared.sh file, then cases when remediation
shouldn't be performed are extremely rare, and they should be handled
by means of the more flexible jinja macros.
@matejak matejak force-pushed the remediation_metadata branch from 5a88638 to 1958119 Compare July 27, 2021 09:50
@matejak
Copy link
Member Author
matejak commented Jul 27, 2021

I have split the more controversial Ansible remediations to another PR.

vojtapolasek
vojtapolasek approved these changes Jul 27, 2021
View reviewed changes
Copy link
Collaborator
@vojtapolasek vojtapolasek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now, merging.

@vojtapolasek vojtapolasek merged commit 6446ebb into ComplianceAsCode:master Jul 27, 2021
@vojtapolasek vojtapolasek added backported-into-stabilization PRs which were cherry-picked during stabilization process. Bash Bash remediation update. labels Jul 27, 2021
vojtapolasek added a commit that referenced this pull request Jul 27, 2021
@vojtapolasek
Merge pull request #7254 from matejak/remediation_metadata
dae5726 
Remove specific metadata in shared Bash remediations

(cherry picked from commit 6446ebb)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cipherboy cipherboy cipherboy left review comments

@ggbecker ggbecker ggbecker left review comments

@vojtapolasek vojtapolasek vojtapolasek approved these changes

@yuumasato yuumasato yuumasato approved these changes

Assignees
No one assigned
Labels
backported-into-stabilization PRs which were cherry-picked during stabilization process. Bash Bash remediation update. RHEL9 Red Hat Enterprise Linux 9 product related.
Projects
None yet
Milestone
0.1.57
Development

Successfully merging this pull request may close these issues.

Remediation of rules fails on RHEL 9, likely due to missing platform listing
5 participants
@matejak @yuumasato @cipherboy @vojtapolasek @ggbecker
0