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

Manage auditd using service instead of exec #40

Closed
wants to merge 1 commit into from
Closed

Manage auditd using service instead of exec #40

wants to merge 1 commit into from

Conversation

amlodzianowski
Copy link
Contributor

We were running into the following issue on Amazon Linux 2 systems if Puppet was being invoked via cron:
/Stage[main]/Threatstack::Package/Exec[stop_auditd] Could not evaluate: invalid byte sequence in US-ASCII

Looking at the way auditd was managed was very inefficient on RedHat based systems - the execs would be executed on every single Puppet run. This seemed like a clear case to use the Puppet service provider.

There is a small caveat on RedHat 7+ systems - systemd can't be used to start/stop auditd. Details are available here:
https://access.redhat.com/solutions/2664811

@amlodzianowski amlodzianowski changed the title Manage audtid using service instead of exec Manage auditd using service instead of exec Nov 12, 2019
@rockpapergoat
Copy link
Contributor

sounds reasonable. also, good call on the service vs. exec resource there. i feel like the exec was leftover from awhile ago. there was probably a reason to do it that way — not sure it's a good one, though.

will take a look through and get this merged in soon.

disable auditd using the service resource instead of execs

set provider on rhel7 to disable auditd

added comments for auditd provider case

linting fixes to pass travis ci

disable auditd using the service resource instead of execs
@rockpapergoat
Copy link
Contributor

rockpapergoat commented Nov 13, 2019

oh, i remember why that exists. we're not explicitly managing auditd aside from disabling it, and an org may manage it in another class. i'll cherry pick some of your changes and add a few bits in a new branch that takes this into account. also, since the param was ruleset before, i might add a warning that the plural rulesets is more correct and plan to phase it out soon.

@amlodzianowski
Copy link
Contributor Author

@rockpapergoat If someone manages auditd in an outside module, all they need to do is declare the ::threatstack class with disable_auditd => false and we won't be touching the service at all.

@rockpapergoat
Copy link
Contributor

true, true. i need to push some changes to docs related to our last release and will get these changes in. looking at it now.

@rockpapergoat
Copy link
Contributor

@amlodzianowski i just merged in a better way to deal with this and will release 2.0.7 on the forge shortly. will close this PR for now, but let us know what you think of the changes. thanks again for sending us these edits!

@amlodzianowski
Copy link
Contributor Author

Looks good! And great idea with using the auditd module to control the service, no need to manage providers here. Once 2.0.7 is released we will run a test on our side to make sure all issues are gone. Thanks!

@rockpapergoat
Copy link
Contributor

cool, yeah… let us know. what i found in quick tests on centos was that the auditd systemd unit file prevents manually stopping/starting the service once it's up, so using the auditd module to wave a magic wand at the problem was easier in this case. it should be relatively safe to assume the module will know what to do on supported OSes here. if it doesn't, we'll fire up a PR for that module.

@amlodzianowski
Copy link
Contributor Author

I think we might see issues with Amazon Linux 201X / 2.0 according to what's here:
https://github.com/kemra102/puppet-auditd/blob/master/manifests/params.pp#L44

But we'll run a test and confirm

@amlodzianowski
Copy link
Contributor Author

amlodzianowski commented Nov 15, 2019

@rockpapergoat as expected, it is failing on our Amazon Linux 2 instances:
Systemd stop for auditd failed! journalctl log for auditd: Journal file /var/log/journal/ded64cbff86f478990a3dfbb63a8d238/system@1808f4639ae7708bc17bc7a1290933be-9f7ca3f291b62e24-6dbcf3fc3d79b81b.journal corrupted, ignoring file. -- No entries --

Looks like this condition needs to be modified:
https://github.com/kemra102/puppet-auditd/blob/master/manifests/params.pp#L44

Using the legacy-action script, everything works fine:

[root@prod-linuxp4-amazon2cis-ywl bin]# facter -p operatingsystemrelease
2
[root@prod-linuxp4-amazon2cis-ywl bin]# facter -p operatingsystem
Amazon
[root@prod-linuxp4-amazon2cis-ywl bin]# /usr/libexec/initscripts/legacy-actions/auditd/stop
Stopping logging:                                          [  OK  ]
[root@prod-linuxp4-amazon2cis-ywl bin]# service auditd status
Redirecting to /bin/systemctl status auditd.service
● auditd.service - Security Auditing Service
   Loaded: loaded (/usr/lib/systemd/system/auditd.service; enabled; vendor preset: enabled)
   Active: inactive (dead) since Fri 2019-11-15 18:02:04 UTC; 9s ago

Let me know if you'd like me to submit the PR or if you want to take care of it.

The other systems we tested are looking good, progress!

@rockpapergoat
Copy link
Contributor

@amlodzianowski interesting. thanks for the update. would you mind submitting the PR? i can probably tag team the problem with you but am in the middle of something at the moment. so the issue is on amazon linux 2 but not amazon linux 1?

@amlodzianowski
Copy link
Contributor Author

Sure thing, I'll submit the PR a little later today.

And yes, Amazon Linux works 1 works but 2 is failing. For Amazon Linux 2 they finally switched to systemd which we just need to catch in the statement

@amlodzianowski
Copy link
Contributor Author

@rockpapergoat

kemra102/puppet-auditd#65

I'm not sure if that module is still maintained since the last release happened in December 2016 and there are 8 different pull requests that were never reviewed. Let me know how you would like to proceed in case this is not merged in in a timely matter

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.

2 participants