ansible-lockdown / RHEL9-CIS

Ansible role for Red Hat 9 CIS Baseline
https://ansible-lockdown.readthedocs.io
MIT License
109 stars 86 forks source link

fix: idempotency molecule issue fixed for logfiles #173 #175

Closed rjacobs1990 closed 6 months ago

rjacobs1990 commented 6 months ago

Overall Review of Changes: A general description of the changes made that are being requested for merge 4.2.3 | PATCH | Ensure permissions on all logfiles are configured -- this was not idempotent for the audit log. if the audit.conf has log_group set to root it will rotate the permissions back to 0600. (tested with AWS ec2/ Azure vms) This caused a failure in my own molecule testing in the second run.

Issue Fixes: Please list (using linking) any open issues this PR addresses

173 is fixed with this MR

Enhancements: Please list any enhancements/features that are not open issue tickets

How has this been tested?: Please give an overview of how these changes were tested. If they were not please use N/A Tested from a cis-wrapper role which spins up vagrant vms in aws. First run went fine, second run detected changes. This will fix the prevent changes in de second run on the /var/log/audit/audit.log.

rjacobs1990 commented 6 months ago

@uk-bolly could you also verify this PR? This will improve 4.2.3 making it idempotent with molecule (second run) and will not change log files which should be 0600 making this CIS baseline more secure.

uk-bolly commented 6 months ago

hi @rjacobs1990

Thank you for the PR and taking the time to raise this and feedback. I have been looking at this and think that maybe we should approach this the same way we have other controls.

By adding the conditional to the loop

- item.mode != '06(0|4)0'

Does that still work for you?

Many thanks

uk-bolly

rjacobs1990 commented 6 months ago

Hi @uk-bolly ,

That could be an option, but then it will skip over some files. With the MR i provided it will not skip over the items. It will tell everything is OK instead of skipping. This will look better in the output during testing.

If you prefer adding an additional when condition i am OK with it. I did this method on purpose, It will say 0600 is also a valid filemode (OK) instead of skipping!

uk-bolly commented 6 months ago

Hi @uk-bolly ,

That could be an option, but then it will skip over some files. With the MR i provided it will not skip over the items. It will tell everything is OK instead of skipping. This will look better in the output during testing.

If you prefer adding an additional when condition i am OK with it. I did this method on purpose, It will say 0600 is also a valid filemode (OK) instead of skipping!

hi @rjacobs1990

That is an excellent way to consider it and this is exactly why we have discussions. Your way if actually preferred in my eyes and demonstrates an improvement to the we work on other controls.

I am happy to approve and merge this and watch for your work in other up coming changes. :)

Thank you again for your time and feedback.

kindest regards

uk-bolly