Add audit_rules_unsuccessful_file_modification_detailed remediation scripts

[redhatrises]

No description provided.

[pep8speaks]

Hello @redhatrises! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file shared/templates/create_audit_rules_unsuccessful_file_modification_detailed.py:

Line 56:13: E265 block comment should start with '# '
Line 62:13: E265 block comment should start with '# '
Line 77:100: E501 line too long (101 > 99 characters)
Line 79:13: E265 block comment should start with '# '
Line 85:13: E265 block comment should start with '# '

Comment last updated at 2019-03-06 17:06:01 UTC

[jan-cerny]

Why is this labeled as a "blocker"? AFAIK pull requests with "blocker" label are impediments to upstream release like failure to build content, or content built is out of standard syntax. Your patch only adds new remediation scripts. I think it isn't a blocker then. What am I missing here?

[scrutinizer-notifier]

The inspection completed: 2 new issues, 1 updated code elements

[jan-cerny]

Regarding the bash remediations, for RHEL and OL this PR has been replaced by https://github.com/ComplianceAsCode/content/pull/4107/files.

What about other platforms? Wouldn't it be better to just add platforms to list of platforms to remediation scripts introduced in #4107 ?

Should we use the Ansible template introduced here to have Ansible remediations for those rules?

[jan-cerny]

Any opinions?

[jan-cerny]

bump @redhatrises

[redhatrises]

Not sure what the ask is here. I consider #4107 as an interim fix, and the fixes should be updated to not rely on having ospp.rules installed at all.

[jan-cerny]

AFAIK using fix_audit_syscall_rule doesn't ensure ordering of audit rules. The patch in #4107 ensures the order, so this change would be a regression.

I see #4107 was introduced as a "provisional" fix. @matejak or @yuumasato , do you remember what is a proper fix for audit_rules_unsuccessful_file_modification_detailed?

and the fixes should be updated to not rely on having ospp.rules installed at all.

We don't rely on the file installed, we have included the contents of the file in the remediation. https://github.com/ComplianceAsCode/content/blob/master/shared/bash_remediation_functions/create_audit_remediation_unsuccessful_file_modification_detailed.sh

[yuumasato]

I see #4107 was introduced as a "provisional" fix. @matejak or @yuumasato , do you remember what is a proper fix for audit_rules_unsuccessful_file_modification_detailed?

Well...The proper fix is to have Bash and Ansible scripts that can re-order the audit rules if all of them are present. By all of them, consider all of the audit rules specific for a syscall.

[yuumasato]

I'm removing Blocker label from this PR, as we have an interim fix #4107.

[jan-cerny]

All the affected rules already have remediations, and this one doesn't address the problem of ordering rules anyway, so I suggest closing it.

[jcerny@thinkpad scap-security-guide{master}]$ find . -name audit_rules_unsuccessful_file_modification*_o_creat | xargs tree 
./linux_os/guide/system/auditing/auditd_configure_rules/audit_file_modification/audit_rules_unsuccessful_file_modification_open_by_handle_at_o_creat
├── ansible
│   └── shared.yml
├── bash
│   └── shared.sh
└── rule.yml
./linux_os/guide/system/auditing/auditd_configure_rules/audit_file_modification/audit_rules_unsuccessful_file_modification_open_o_creat
├── ansible
│   └── shared.yml
├── bash
│   └── shared.sh
└── rule.yml
./linux_os/guide/system/auditing/auditd_configure_rules/audit_file_modification/audit_rules_unsuccessful_file_modification_openat_o_creat
├── ansible
│   └── shared.yml
├── bash
│   └── shared.sh
└── rule.yml

6 directories, 9 files
[jcerny@thinkpad scap-security-guide{master}]$ find . -name audit_rules_unsuccessful_file_modification*_o_trunc_write | xargs tree 
./linux_os/guide/system/auditing/auditd_configure_rules/audit_file_modification/audit_rules_unsuccessful_file_modification_open_o_trunc_write
├── ansible
│   └── shared.yml
├── bash
│   └── shared.sh
└── rule.yml
./linux_os/guide/system/auditing/auditd_configure_rules/audit_file_modification/audit_rules_unsuccessful_file_modification_open_by_handle_at_o_trunc_write
├── ansible
│   └── shared.yml
├── bash
│   └── shared.sh
└── rule.yml
./linux_os/guide/system/auditing/auditd_configure_rules/audit_file_modification/audit_rules_unsuccessful_file_modification_openat_o_trunc_write
├── ansible
│   └── shared.yml
├── bash
│   └── shared.sh
└── rule.yml

6 directories, 9 files

[redhatrises]

All the affected rules already have remediations, and this one doesn't address the problem of ordering rules anyway, so I suggest closing it.

[jcerny@thinkpad scap-security-guide{master}]$ find . -name audit_rules_unsuccessful_file_modification*_o_creat | xargs tree 
./linux_os/guide/system/auditing/auditd_configure_rules/audit_file_modification/audit_rules_unsuccessful_file_modification_open_by_handle_at_o_creat
├── ansible
│   └── shared.yml
├── bash
│   └── shared.sh
└── rule.yml
./linux_os/guide/system/auditing/auditd_configure_rules/audit_file_modification/audit_rules_unsuccessful_file_modification_open_o_creat
├── ansible
│   └── shared.yml
├── bash
│   └── shared.sh
└── rule.yml
./linux_os/guide/system/auditing/auditd_configure_rules/audit_file_modification/audit_rules_unsuccessful_file_modification_openat_o_creat
├── ansible
│   └── shared.yml
├── bash
│   └── shared.sh
└── rule.yml

6 directories, 9 files
[jcerny@thinkpad scap-security-guide{master}]$ find . -name audit_rules_unsuccessful_file_modification*_o_trunc_write | xargs tree 
./linux_os/guide/system/auditing/auditd_configure_rules/audit_file_modification/audit_rules_unsuccessful_file_modification_open_o_trunc_write
├── ansible
│   └── shared.yml
├── bash
│   └── shared.sh
└── rule.yml
./linux_os/guide/system/auditing/auditd_configure_rules/audit_file_modification/audit_rules_unsuccessful_file_modification_open_by_handle_at_o_trunc_write
├── ansible
│   └── shared.yml
├── bash
│   └── shared.sh
└── rule.yml
./linux_os/guide/system/auditing/auditd_configure_rules/audit_file_modification/audit_rules_unsuccessful_file_modification_openat_o_trunc_write
├── ansible
│   └── shared.yml
├── bash
│   └── shared.sh
└── rule.yml

6 directories, 9 files

Current rules are interim, copy a config file, and need to be replaced without requiring a config file. Ordering happens through a different rule.

[jan-cerny]

Current rules are interim, copy a config file, and need to be replaced without requiring a config file.

Yes, I know, I also agree that copying a file is wrong approach.

Ordering happens through a different rule.

The rules audit_rules_unsuccessful_file_modification_openat_rule_order, audit_rules_unsuccessful_file_modification_open_by_handle_at_rule_order,
audit_rules_unsuccessful_file_modification_open_rule_order have only bash remediation which again copy the config file.
My understanding is that this remediation would overwrite the effect of the remediation you propose.

@yuumasato said that:

The proper fix is to have Bash and Ansible scripts that can re-order the audit rules if all of them are present. By all of them, consider all of the audit rules specific for a syscall.

@redhatrises It isn't clear to me how this PR improves the situation. How does it ensure that the remediation puts the audit rule in the right place in the audit config file?

[adelton]

Could you please rebase on the latest master?

[redhatrises]

Closing this PR for now. This is going to have to be done anyways for all audit rules.