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

auditd service fails to start because augenrules says rules have not changed #355

Closed
ShellCode33 opened this issue Feb 17, 2024 · 16 comments

Comments

@ShellCode33
Copy link

ShellCode33 commented Feb 17, 2024

Hey, I'm facing an issue which is I believe related to audit-rules.service.

I'm not sure what the initial trigger for this bug is but I believe it is due to the fact that auditd failed to start at boot because the kernel ran out of buffer space:

Feb 15 22:52:42 laptop systemd[1]: Starting Security Audit Logging Service...
░░ Subject: A start job for unit auditd.service has begun execution
░░ Defined-By: systemd
░░ Support: https://lists.freedesktop.org/mailman/listinfo/systemd-devel
░░
░░ A start job for unit auditd.service has begun execution.
░░
░░ The job identifier is 152.
Feb 15 22:52:42 laptop auditd[819]: No plugins found, not dispatching events
Feb 15 22:52:42 laptop auditd[819]: Error receiving audit netlink packet (No buffer space available)
Feb 15 22:52:42 laptop auditd[819]: Error setting audit daemon pid (No buffer space available)
Feb 15 22:52:42 laptop auditd[819]: Unable to set audit pid, exiting
Feb 15 22:52:42 laptop auditd[819]: The audit daemon is exiting.
Feb 15 22:52:42 laptop auditd[816]: Cannot daemonize (Success)
Feb 15 22:52:42 laptop auditd[816]: The audit daemon is exiting.
Feb 15 22:52:42 laptop systemd[1]: auditd.service: Control process exited, code=exited, status=1/FAILURE

So I tried to start the auditd.service again using systemctl start auditd, and now it fails with:

Feb 17 16:53:10 laptop systemd[1]: Dependency failed for Security Audit Logging Service.
░░ Subject: A start job for unit auditd.service has failed
░░ Defined-By: systemd
░░ Support: https://lists.freedesktop.org/mailman/listinfo/systemd-devel
░░ 
░░ A start job for unit auditd.service has finished with a failure.
░░ 
░░ The job identifier is 13326 and the job result is dependency.
Feb 17 16:53:10 laptop systemd[1]: auditd.service: Job auditd.service/start failed with result 'dependency'.

The only dependency auditd.service has is audit-rules.service according to:

Requires=audit-rules.service

And indeed if I look at audit-rules logs I can see the following:

Feb 17 16:53:10 laptop systemd[1]: Starting Load Audit Rules...
░░ Subject: A start job for unit audit-rules.service has begun execution
░░ Defined-By: systemd
░░ Support: https://lists.freedesktop.org/mailman/listinfo/systemd-devel
░░ 
░░ A start job for unit audit-rules.service has begun execution.
░░ 
░░ The job identifier is 13327.
Feb 17 16:53:10 laptop augenrules[407697]: /usr/bin/augenrules: No change
Feb 17 16:53:10 laptop systemd[1]: audit-rules.service: Main process exited, code=exited, status=1/FAILURE

augenrules reports that rules haven't changed: /usr/bin/augenrules: No change which is fine, right ? Still, it exits with code 1, resulting in a service failure.

Might be worth mentioning that I set my rules to be immutable using -e 2.

I'm not sure what is the correct way to fix this issue, should augenrules not exit with status 1 if it detects that rules haven't changed ? What does it even mean No change ? Is it a "I was unable to change the rules" or is it a "rules are already set" ?
Or should the systemd service not fail if status 1 is returned ?

I'm using auditd 4.0-1 on Linux hardened 6.7.3.

Let me know if you need any additional info, cheers.

@stevegrubb
Copy link
Contributor

When augenrules says no change, it means that the rules on disk have not changed since the last time it compiled the rules from the pieces. Next it tries to load the rules. If you have -e 2, that is likely the reason why it fails. That said, if rules are loaded and they have not changed, it should not be a failure. Things are good.

I'll look to see how we can change the code of augenrules to avoid failing when the rules are immutable or not changed. Auditd should still start no matter what happened in the audit-rules.service.

@ShellCode33
Copy link
Author

Thanks for your fast answer. Maybe the solution would be to replace Requires= with After= in auditd.service ?

@stevegrubb
Copy link
Contributor

stevegrubb commented Feb 18, 2024

Commit 62b30cc should solve the problem. This has been there a long time unnoticed. I'll have another look at the systemd man pages to see if Requires is correct. There is a firm relationship between the 2 services that I really want enforced.

@ShellCode33
Copy link
Author

ShellCode33 commented Feb 18, 2024

Mmmh, I don't know much about auditd and its ecosystem, so I might be wrong here, but that patch feels wrong to me. How am I supposed to update the rules now ? The way I see it, immutable rules means you cannot alter the behavior at runtime, but you should still be able to edit them, and they will be enforced the next time the system boots. Immutable rules should be (AFAIU) a kernel thing only, not something userland utilities should check.

Unless the rules are recompiled at boot without using augenrules ?

@ShellCode33
Copy link
Author

I feel like the proper fix for this issue would be to replace these two lines:

try_load
exit $RETVAL

with exit 0

Why should the script call try_load if it detected that rules didn't change ?

@ShellCode33
Copy link
Author

ShellCode33 commented Feb 18, 2024

Actually, the whole if/else statement is mixed up: (EDIT: didn't notice the $OnlyCheck)

cmp -s "${TmpRules}" ${DestinationFile} > /dev/null 2>&1
if [ $? -eq 0 ]; then
echo "$0: No change"
rm -f "${TmpRules}"
try_load
exit $RETVAL
elif [ $OnlyCheck -eq 1 ] ; then
echo "$0: Rules have changed and should be updated"
rm -f "${TmpRules}"
exit 0
fi

I believe it should be:

cmp -s "${TmpRules}" ${DestinationFile} > /dev/null 2>&1 
 if [ $? -eq 0 ]; then 
 	echo "$0: No change"
 	rm -f "${TmpRules}" 
 	exit 0 
 elif [ $OnlyCheck -eq 1 ] ; then 
 	echo "$0: Rules have changed and should be updated" 
 	rm -f "${TmpRules}" 
 	exit 0
 fi

@stevegrubb
Copy link
Contributor

And commit a3d436f will allow checking in immutable mode.

@stevegrubb
Copy link
Contributor

The reason it has to call try_load is because the rules need to get loaded. Normally the rules never change but they need to get into the kernel. Maybe troubleshooting has been done and the admin wants to get back to the normal baseline. For immutable rules, there is the system boot where nothing is loaded yet. Immutable rules can only be loaded at boot. They are used to prevent someone from modifying rules that would catch a specific action. I moved the exit so that at least a simple status message can be emitted before exiting. Please give the current file a test. I think it does what you need it to do.

@ShellCode33
Copy link
Author

ShellCode33 commented Feb 19, 2024

At first the script didn't work, the following error is emitted:

./fix_augenrules: 50: test: 2: unexpected operator

I replaced:

if test $(auditctl -s | awk '$1 == "enabled" { print $2 }') == "2"

With:

if [ "$(auditctl -s | awk '$1 == "enabled" { print $2 }')" = "2" ]

I also added quotes around the subshell command to prevent word globbing, as advised by shellcheck (even though that's unlikely considering you are telling awk to print a single token, but better be warning-free right?)

Otherwise it works fine, thanks a lot ! I was a bit worried that it wouldn't be picked up by the kernel (next reboot) if auditctl -R was not run by the script, but apparently there's some other mechanism involved I'm unaware of.

While I'm here, shellcheck warns about the signal trap 13 and says that the name of the signal should be used instead.
Here's the rationale: https://www.shellcheck.net/wiki/SC2172

@stevegrubb
Copy link
Contributor

Strange, I don't get a warning on either F38 or F40 at line 50. How are you getting that? What version of bash do you have? The "test" and "[" commands are almost identical.

@ShellCode33
Copy link
Author

ShellCode33 commented Feb 21, 2024

What do you mean by F38 and F40 ?

How are you getting that?

I just ran the script like so:

sudo ./fix_augenrules

Which uses the shebang defined at the top of the file.

What version of bash do you have?

The augenrules script doesn't use bash, it uses sh, on my system sh is a symlink to dash, a POSIX compliant shell as well.

@Pierre-Gronau-ndaal
Copy link
Contributor

maybe you have e.g. to replace echo with printf if you wand to be POSIX compliant
or to quote consequently also in the echo/print statements as well

@stevegrubb
Copy link
Contributor

stevegrubb commented Feb 22, 2024

F38 and F40 are abbreviations for "Fedora 38" and "Fedora 40". They use bash 5.2.26 as the shell. There is no fix_augenrules. Fedora uses augenrules as shipped in this repo. Since you are on a distro using dash as the shell, that probably explains why you are seeing an issue where I'm not seeing a problem. Even shellcheck doesn't complain about line 50 not being posix compliant. It mentions trapping signals by number.

I wanted to understand how you were getting a warning before I make any changes. I'll see how I can reproduce it so I know the fix is right.

@ShellCode33
Copy link
Author

ShellCode33 commented Feb 23, 2024

F38 and F40 are abbreviations for "Fedora 38" and "Fedora 40"

Alright, well I'm running Arch.


There is no fix_augenrules. Fedora uses augenrules as shipped in this repo

That's just the name I gave to the script, I copy/pasted it from this repo.


Since you are on a distro using dash as the shell, that probably explains why you are seeing an issue where I'm not seeing a problem

I'm not sure you got my point, my point is that at the beginning of the augenrules script there's a #!/bin/sh which means your script shouldn't use any bash-ism, it should be 100% POSIX compliant, not use bash. Otherwise you can modify the shebang to be #!/bin/bash.


Even shellcheck doesn't complain about line 50 not being posix compliant.

Indeed, it's my bad, it's not shellcheck giving me a warning, it's bashls, my editor LSP for shell scripts:

image

Which is a warning, but not a POSIX compliance issue. But that's not the main issue, the main issue is that you are not using test properly, you are using == but this is not supported by test. If you run man test you can see the following:

image

So you can use the following if you want to, with a single = :

if test "$(auditctl -s | awk '$1 == "enabled" { print $2 }')" = "2"; then

But I advise against using test for homogeneity purposes. Everywhere else in the script, test is not used, [ is used. So I invite you to use the following instead, just like all the other conditions in the script:

if [ "$(auditctl -s | awk '$1 == "enabled" { print $2 }')" = "2" ]

But really that's fine, use test if you want to, but please replace == with =. == is a bashism and should not be used in scripts that start with #!/bin/sh.

@stevegrubb
Copy link
Contributor

stevegrubb commented Feb 23, 2024

OK. I think it's all fixed now. Thanks for reporting the issue.

it's bashls

Never heard of it. But now that I have, it looks useful.

@ShellCode33
Copy link
Author

Thanks a lot ! Have a nice week-end

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

No branches or pull requests

3 participants