ausearch fails to load symlinked config file

[LordGrimmauld]

I started running the audit-testsuite against the NixOS auditd module to try and make stuff work properly. Currently, our audit runs but is broken in a couple subtle ways. Particularly, it does load the config at /etc/audit/auditd.conf, despite it being a symlink! Edit: we do pass -l, this is intended.

One thing i found:

vm-test-run-auditd> machine # Error opening config file (Too many levels of symbolic links)
vm-test-run-auditd> machine # NOTE - using built-in end_of_event_timeout: 2
vm-test-run-auditd> machine # NOTE - using built-in logs: /var/log/audit/audit.log

I suspect part of the issue is in here:

audit-userspace/audisp/plugins/remote/remote-config.c

Lines 244 to 245 in 2b7c23f

	mode = O_RDONLY;
	rc = open(file, mode);

audit-userspace/audisp/plugins/remote/remote-config.c

Line 261 in 2b7c23f

if (fstat(fd, &st) < 0) {

audit-userspace/audisp/plugins/remote/remote-config.c

Line 279 in 2b7c23f

if (!S_ISREG(st.st_mode)) {

There is no O_NOFOLLOW, and the stat syscall is fstat, not lstat, which means it doesn't actually check whether the config file is a symlink.

However, the error code (Too many levels of symbolic links) comes from the kernel:
https://github.com/linux-audit/audit-kernel/blob/729b4babd69517766ae39e21902caa810625e247/net/9p/error.c#L80

This suggests something is actually using O_NOFOLLOW, see the open manual page:

O_NOFOLLOW If path names a symbolic link, fail and set errno to [ELOOP].

I'll either have to patch the kernel module, or patch the userspace audit daemon to pass an absolute path to the kernel. Not sure which of the two options would be better.
Edit: Needs set_allow_links in the ausearch tool to correctly load config, or else O_NOFOLLOW is added.

After #467, plugins from symlinks works. Config from symlinks not working is a little surprising, because until i ran the kernel test suite this never actually complained.

[LordGrimmauld]

So, -l can be passed to the audit daemon to allow links, and that works. Problem is, ausearch does not have a similar option it seems. This means, while auditd itself runs perfectly fine, ausearch throws the above error.

[LordGrimmauld]

I can make this error go away with the following patch:

diff --git a/src/ausearch.c b/src/ausearch.c
index c47b9ed4..41d4ef01 100644
--- a/src/ausearch.c
+++ b/src/ausearch.c
@@ -106,6 +106,7 @@ int main(int argc, char *argv[])
 	setrlimit(RLIMIT_FSIZE, &limit);
 	setrlimit(RLIMIT_CPU, &limit);
 	set_aumessage_mode(MSG_STDERR, DBG_NO);
+	set_allow_links(1); // auditd might have -l flag, ausearch should be lenient here
 	(void) umask( umask( 077 ) | 027 );
 
 	/* Load config so we know where logs are and eoe_timeout */

Not sure this is the ideal fix, but for now this is what i'll use

[Cropi]

I think ausearch isn't the only component impacted, there are also plugins like remote, zos-remote, and filter that rely on configuration files. Plus, aureport comes into play as well. Perhaps auditd ecosystem should adopt auditd's -l flag for consistency. But that still does not fix standalone tools like ausearch and aureport. Does explicitly permitting symlinks for these tools introduce any security risks?

[LordGrimmauld]

Does explicitly permitting symlinks for these tools introduce any security risks?

I don't see how that would be the case. I would argue the opposite is the case: ausearch is useful to search and filter the logs, and to do that it uses some of the config values. That tool falling back to default config values might then report inaccurate audit queries.

The config file still needs to be owned by root and read-only to regular users. This is no different from any other config file commonly found in /etc. The only security impact i could think of would be root setting a symlink which points to a world-writable config file. To me, that falls in the category of "root can do everything, even dumb things", and is not something we should need to protect against.

Further, the symlink check can be bypassed using bind mounts, fuse mounts, or overlayfs. As it stands, i already question the symlink-deny functionality: It makes audit painful to use on systems with extensive symlink use (e.g. NixOS), while not really protecting against anything. I'd much rather symlinked configs be allowed everywhere.

I think ausearch isn't the only component impacted

Fair call, the ausearch just is the only component i have found to be impacted while running the audit-testsuite. There might well be more, but i initially planned to make the test suite pass before continuing my work on audit.

[LordGrimmauld]

Note we do not build any of the zos remote-logging functionality on NixOS, so this is not (yet) a concern.

[stevegrubb]

A lot of the basic code was written ~20 years ago when symlink attacks was a thing. Couple a directory traversal with a write and you can change things in unexpected ways. Since auditd was expected to catch these things, it was hardened to be impervious to this kind of attack. Times are different now. Propose a patch.

[LordGrimmauld]

Propose a patch.

Fair enough, opened #489