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.

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

Preserve default ownership and dir mode for /var/log on Ubuntu #615

Merged
merged 4 commits into from
Jan 23, 2023
Merged

Preserve default ownership and dir mode for /var/log on Ubuntu #615

merged 4 commits into from
Jan 23, 2023

Conversation

stdtom
Copy link
Contributor

@stdtom stdtom commented Jan 3, 2023

Closes #614.

@stdtom
linting
13e5957 
Signed-off-by: stdtom <[email protected]>
@stdtom
Copy link
Contributor Author

stdtom commented Jan 16, 2023

I see the ansible check failing but not due to the change in this PR.
@rndmh3ro Is there something I could do to get this PR merged?

@rndmh3ro
Copy link
Member

I'll try to check this PR this week.

@rndmh3ro
Copy link
Member

I don't like the variable inside the variable in the defaults:

os_mnt_boot_dir_mode: "{{ lookup('vars', 'os_mnt_boot_dir_mode_override' | lower, default='0700') }}"

We already had this but decided against it and changed it in this PR: #353

Can you please define the variables separately for the OS in the vars? I know this will lead to more code, but it's more readble this way. Thanks!

@stdtom
Copy link
Contributor Author

stdtom commented Jan 19, 2023

Can you please define the variables separately for the OS in the vars? I know this will lead to more code, but it's more readble this way. Thanks!

I hope I understood it correctly and this commit meets your request.

@rndmh3ro
Copy link
Member

Yes, that looks correct, thank you!

One thing I wonder: Does this problem only happen on Ubuntu or on Debian, too? Do you know this? If not, I'll have to test.

@rndmh3ro
Copy link
Member

There are some errors in the CI. Seems that RHEL/Centos/etc. do not have the syslog-group. I guess the default in most cased should be root.

@stdtom
Copy link
Contributor Author

stdtom commented Jan 20, 2023

There are some errors in the CI. Seems that RHEL/Centos/etc. do not have the syslog-group. I guess the default in most cased should be root.

OMG. Of course thats what I did in my first commit. I guess I copy/pasted the Ubuntu values instead of the Debian values. Suppose it was too late last night... 🙈 🙊

@stdtom
Copy link
Contributor Author

stdtom commented Jan 20, 2023

One thing I wonder: Does this problem only happen on Ubuntu or on Debian, too? Do you know this? If not, I'll have to test.

I tested on Debian before I submitted the PR and could not reproduce the issue. AFAIR syslog on Debian runs as root.

@rndmh3ro rndmh3ro merged commit 9d2c68e into dev-sec:master Jan 23, 2023
@rndmh3ro
Copy link
Member

Thanks!

rndmh3ro pushed a commit that referenced this pull request Jan 23, 2023
@stdtom
Preserve default ownership and dir mode for /var/log on Ubuntu (#615)
e86a5cc 
* Preserve default ownership and dir mode for /var/log on Ubuntu

Signed-off-by: stdtom <[email protected]>

* linting

Signed-off-by: stdtom <[email protected]>

* Define vars for each OS instead of using defaults.

Signed-off-by: stdtom <[email protected]>

* Fix values for os_mnt_var_log_dir_mode and os_mnt_var_log_group

Signed-off-by: stdtom <[email protected]>

Signed-off-by: stdtom <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement minor os_hardening
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

os_hardening is setting wrong ownership for /var/log on Ubuntu
2 participants
@stdtom @rndmh3ro