Skip to content
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.

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

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

Merged

Conversation

rjacobs1990
Copy link
Contributor

@rjacobs1990 rjacobs1990 commented Feb 12, 2024

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.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congrats on opening your first pull request and thank you for taking the time to help improve Ansible-Lockdown!
Please join in the conversation happening on the Discord Server as well.

Copy link
Member

@bbaassssiiee bbaassssiiee left a comment

Choose a reason for hiding this comment

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

Nice work!

@rjacobs1990
Copy link
Contributor Author

rjacobs1990 commented Feb 19, 2024

@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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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

Copy link
Member

@uk-bolly uk-bolly left a comment

Choose a reason for hiding this comment

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

Excellent

@uk-bolly uk-bolly merged commit 06ec3de into ansible-lockdown:devel Feb 19, 2024
6 checks passed
uk-bolly added a commit that referenced this pull request Feb 19, 2024
@uk-bolly uk-bolly mentioned this pull request Feb 20, 2024
uk-bolly added a commit that referenced this pull request Feb 20, 2024
* change logic thanks to @rjacobs1990 see #175

Signed-off-by: Mark Bolwell <[email protected]>

* thanks to @ipruteani-sie #134

Signed-off-by: Mark Bolwell <[email protected]>

* Thanks to @stwongst #125

Signed-off-by: Mark Bolwell <[email protected]>

* thanks to @sgomez86 #146

Signed-off-by: Mark Bolwell <[email protected]>

* Added updates from #115

Signed-off-by: Mark Bolwell <[email protected]>

* removed rp_filter in post added in error

Signed-off-by: Mark Bolwell <[email protected]>

* updated yamllint precommit

Signed-off-by: Mark Bolwell <[email protected]>

* updated fqcn fo json_query

Signed-off-by: Mark Bolwell <[email protected]>

* updated

Signed-off-by: Mark Bolwell <[email protected]>

* fix typo for virt type query

Signed-off-by: Mark Bolwell <[email protected]>

---------

Signed-off-by: Mark Bolwell <[email protected]>
ipruteanu-sie pushed a commit to siemens/RHEL9-CIS that referenced this pull request Feb 21, 2024
* change logic thanks to @rjacobs1990 see ansible-lockdown#175

Signed-off-by: Mark Bolwell <[email protected]>

* thanks to @ipruteani-sie ansible-lockdown#134

Signed-off-by: Mark Bolwell <[email protected]>

* Thanks to @stwongst ansible-lockdown#125

Signed-off-by: Mark Bolwell <[email protected]>

* thanks to @sgomez86 ansible-lockdown#146

Signed-off-by: Mark Bolwell <[email protected]>

* Added updates from ansible-lockdown#115

Signed-off-by: Mark Bolwell <[email protected]>

* removed rp_filter in post added in error

Signed-off-by: Mark Bolwell <[email protected]>

* updated yamllint precommit

Signed-off-by: Mark Bolwell <[email protected]>

* updated fqcn fo json_query

Signed-off-by: Mark Bolwell <[email protected]>

* updated

Signed-off-by: Mark Bolwell <[email protected]>

* fix typo for virt type query

Signed-off-by: Mark Bolwell <[email protected]>

---------

Signed-off-by: Mark Bolwell <[email protected]>
uk-bolly added a commit that referenced this pull request Mar 5, 2024
uk-bolly added a commit that referenced this pull request Mar 6, 2024
* change logic thanks to @rjacobs1990 see #175

* 1.2.1 force gpg import rhel

* fix missing facts

---------

Signed-off-by: Mark Bolwell <[email protected]>
ipruteanu-sie pushed a commit to siemens/RHEL9-CIS that referenced this pull request Mar 11, 2024
* change logic thanks to @rjacobs1990 see ansible-lockdown#175

Signed-off-by: Mark Bolwell <[email protected]>

* thanks to @ipruteani-sie ansible-lockdown#134

Signed-off-by: Mark Bolwell <[email protected]>

* Thanks to @stwongst ansible-lockdown#125

Signed-off-by: Mark Bolwell <[email protected]>

* thanks to @sgomez86 ansible-lockdown#146

Signed-off-by: Mark Bolwell <[email protected]>

* Added updates from ansible-lockdown#115

Signed-off-by: Mark Bolwell <[email protected]>

* removed rp_filter in post added in error

Signed-off-by: Mark Bolwell <[email protected]>

* updated yamllint precommit

Signed-off-by: Mark Bolwell <[email protected]>

* updated fqcn fo json_query

Signed-off-by: Mark Bolwell <[email protected]>

* updated

Signed-off-by: Mark Bolwell <[email protected]>

* fix typo for virt type query

Signed-off-by: Mark Bolwell <[email protected]>

---------

Signed-off-by: Mark Bolwell <[email protected]>
ipruteanu-sie pushed a commit to siemens/RHEL9-CIS that referenced this pull request Mar 11, 2024
* change logic thanks to @rjacobs1990 see ansible-lockdown#175

* 1.2.1 force gpg import rhel

* fix missing facts

---------

Signed-off-by: Mark Bolwell <[email protected]>
Signed-off-by: Pruteanu <[email protected]>
@uk-bolly uk-bolly mentioned this pull request Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants