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

rework filesystem hardening #555

Merged
merged 3 commits into from
Aug 22, 2022
Merged

Conversation

divialth
Copy link
Contributor

@divialth divialth commented Aug 3, 2022

  • removed a lot duplicated code by using a loop
  • added new hardening options for /tmp
  • added new options "passno" and "dump" for every filesystem.
    currently ansible changed that values to 0 for every fs
    new default depends on fstype, can be overwritten in config
  • removed default fstype in config
    the type will now be autodetected, can be overwritten in config
  • mount src setting is now optional
    the source will now be autodetected, can be overwritten in config
  • it will be now checked, if it is really a mount
  • changed fs reload to handler
  • removed check os_auditd_enabled on /var/log/audit

Notes:

  • The playbook will no longer fail, if a folder does not exists
  • It will now skip mount points, if they are not present on the target system.
    If you have many different configured servers this is very useful
  • The folders are still be hardened regardless if *_enabled is used or not
  • Was there a reason why /var/log/audit was only hardened, when the os_auditd_enabled variable was set to true?

@divialth
Copy link
Contributor Author

divialth commented Aug 3, 2022

i think this would supersede #523

@lbayerlein
Copy link
Contributor

Yes, I was too slow :) Thanks! #523 is deprecated

@lbayerlein
Copy link
Contributor

  • removed a lot duplicated code by using a loop

    • added new hardening options for /tmp

    • added new options "passno" and "dump" for every filesystem.
      currently ansible changed that values to 0 for every fs
      new default depends on fstype, can be overwritten in config

    • removed default fstype in config
      the type will now be autodetected, can be overwritten in config

    • mount src setting is now optional
      the source will now be autodetected, can be overwritten in config

    • it will be now checked, if it is really a mount

    • changed fs reload to handler

    • removed check os_auditd_enabled on /var/log/audit

Notes:

* The playbook will no longer fail, if a folder does not exists

* It will now skip mount points, if they are not present on the target system.
  If you have many different configured servers this is very useful

* The folders are still be hardened regardless if *_enabled  is used or not

* Was there a reason why /var/log/audit was only hardened, when the os_auditd_enabled variable was set to true?

Hi @divialth,

yes there was a reason for me, because /var/log/audit only exists on RHEL derivates. Debian based does not have this folder. And some users do not want to harden /var/log/audit so we set the default to false.

Would you prefer to query a fact for an specific operatingsystems to set this to true?

@divialth
Copy link
Contributor Author

divialth commented Aug 3, 2022

yes there was a reason for me, because /var/log/audit only exists on RHEL derivates. Debian based does not have this folder. And some users do not want to harden /var/log/audit so we set the default to false.

Would you prefer to query a fact for an specific operatingsystems to set this to true?

This should be no longer a problem. The first task in minimize_access_fs.yml only runs, when it is real mount point (or a whitelisted special device like /run, /dev...)
The hardening of the permissions on the directory also does now only run, if it is already existing.

I also did not changed any of the _enabled defaults.
The only thing i removed was the boolean check for os_auditd_enabled.

@lbayerlein
Copy link
Contributor

I think this will work for us. A "go" on my side 👍

Copy link
Contributor

@schurzi schurzi left a comment

Choose a reason for hiding this comment

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

please consider my suggestions as nitpicking.

Aside from my comments I want to say I absolutely like what you did here!

roles/os_hardening/handlers/main.yml Outdated Show resolved Hide resolved
roles/os_hardening/tasks/minimize_access.yml Outdated Show resolved Hide resolved
roles/os_hardening/tasks/minimize_access.yml Outdated Show resolved Hide resolved
roles/os_hardening/tasks/minimize_access.yml Outdated Show resolved Hide resolved
roles/os_hardening/tasks/minimize_access_fs.yml Outdated Show resolved Hide resolved
-  removed a lot duplicated code by using a loop
-  added new hardening options for /tmp
-  added new options "passno" and "dump" for every filesystem.
   currently ansible changed that values to 0 for every fs
   new default depends on fstype, can be overwriten in config
-  removed default fstype in config
   the type will now be autodetected,  can be overwriten in config
-  mount src setting is now optional
   the source will now be autodetected,  can be overwriten in config
-  it will be now checked, if it is really a mount
-  changed fs reload to handler
-  removed check os_auditd_enabled on /var/log/audit

Signed-off-by: divialth <65872926+divialth@users.noreply.github.com>
Signed-off-by: divialth <65872926+divialth@users.noreply.github.com>
Signed-off-by: divialth <65872926+divialth@users.noreply.github.com>
@divialth divialth force-pushed the minimize_access_fs branch from f178915 to 8d9cac4 Compare August 20, 2022 09:50
@divialth
Copy link
Contributor Author

please consider my suggestions as nitpicking.

Aside from my comments I want to say I absolutely like what you did here!

I have implemented your other naming suggestions and also did a rebase to resolve the merge conflicts. The current failing CI checks seems to be unrelated. Please correct me if I am wrong.

@rndmh3ro rndmh3ro merged commit fb8b914 into dev-sec:master Aug 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants