aureport/ausearch: check for non-input stdin pipes

[ematsumiya]

Calling aureport or ausearch on a remote host e.g.:

# ssh host aureport

will make aureport hang on read() because stdin is seen as a pipe (from SSH). This can be worked around with "ssh -t", but the aforementioned behaviour is not expected anyway.

Fix this by checking if stdin is a pipe, and, if so, poll it to check for available data to read, which will return false for the reproducer.

Following examples now works, or continue to work, successfully:

# aureport
# cat audit.log | aureport
# ssh host aureport
# ssh host "cat audit.log | aureport"
# cat audit.log | ssh host aureport

[stevegrubb]

There is the --input-logs option which tells it ignore the pipe and use the logs from auditd.conf

[ematsumiya]

Then --input-logs can be deprecated? :-P I'm half joking.

On the non-joke half part:
I'm aware of --input-logs, but recommending it to users is the same as recommending to use "ssh -t" instead; highly inflexible, especially on modern deployments where everything is automated. Even greater impact on large deployments.

Not counting the 100% unexpected/undesired behaviour for "ssh host aureport" hanging without warnings/errors.

What's wrong with making stdin input a try-read instead of "it's a pipe, read from it"? i.e. what's wrong with having this patch?

[stevegrubb]

I'll look at this next week.

[Cropi]

Wouldn’t it be possible to resolve the issue with a straightforward is_pipe redesign and not touch main at all, like this:

diff --git a/src/aureport.c b/src/aureport.c
index 3849e049..8bff2ceb 100644
--- a/src/aureport.c
+++ b/src/aureport.c
@@ -35,6 +35,7 @@
 #include <sys/stat.h>
 #include <locale.h>
 #include <sys/param.h>
+#include <poll.h>
 #include "libaudit.h"
 #include "auditd-config.h"
 #include "aureport-options.h"
@@ -69,14 +70,20 @@ extern int force_logs;
  */
 extern time_t arg_eoe_timeout;
 
-
 static int is_pipe(int fd)
 {
 	struct stat st;
 
 	if (fstat(fd, &st) == 0) {
-		if (S_ISFIFO(st.st_mode))
-			return 1;
+		if (S_ISFIFO(st.st_mode)) {
+			struct pollfd p;
+
+			p.fd = fd;
+			p.events = POLLIN;
+			if (poll(&p, 1, 0) > 0 && (p.revents & POLLIN))
+				return 1;
+			return 0;
+		}
 	}
 	return 0;
 }

Of course, this could be refactored into a common function and used by both aureport and ausearch. I tried a few combinations.

[stevegrubb]

@Cropi The only drawback is that this is based on time. To test this, I used a log file from 2021 and piped that to aureport --log. It reported 2025 time range. I gave it 1000 milliseconds and it reported 2021. Then I remembered when I worked on audit full time. I had audit logging 100MB sized files and 200 of them. This was to simulate the environment where someone really uses auditing so I could make sure the utilities were fast. Even in that case, it sometimes took over a minute before it started finding the records of interest. So, I don't know if there is any good way because it can take some time to start outputting in large environments.

One idea is maybe add a new command line option --stdin to tell it don't even test but go directly to stdin for input. Then we can have the automatic detection that works most of the time and manual overrides when it doesn't.

If we stay with the automatic method, I like @Cropi patch better. But I'd give it like 250 milliseconds in case the input is a little slow to arrive.

[ematsumiya]

Wouldn’t it be possible to resolve the issue with a straightforward is_pipe redesign and not touch main at all,

Sure, but why not touch main? I just took the opportunity to do a small refactoring in that if/else if/else.

@stevegrubb adding a --stdin switch would be great and it was indeed my first approach to this, but would require (IMHO) a redesign to "don't ever read stdin, unless --stdin", which would break current behaviour.

As for the slow input concern, you're correct. Even though I did test such cases to avoid false negatives, apparently my tests were wrong.

Note, however, that current code only works with slow input because it (tries to) just read straight from the pipe, regardless if there's data or not. Which is what causes the bug I originally wanted to fix here.

I'll rewrite the patch to yours and @Cropi 's suggestions, but to accomodate the timing issue, may I also suggest something like (pseudocode):

if not S_ISFIFO(stdin):
    return 0
poll stdin for 0ms
if POLLIN:
    # cases:
    # > cat audit.log | aureport
    # > cat audit.log | ssh host aureport
    # > ssh host "cat audit.log | aureport"
    return 1
if POLLHUP:
    # case: echo | aureport
    warn "stdin was closed before data was available"
    return -1
# increase poll timeout
poll stdin for 250ms
if POLLIN:
    # case: ./fetch_audit_logs_from_server.sh | aureport
    return 1
# false result mark
info "reading from default log file"
return 0

Like this, we have a fast, binary path for stdin pipe (readable data or hangup), and an acceptable delay for slow input cases.
The "false result mark" is where it gets tricky, as there's no definitive (*) way to tell if 250ms was too little (input data taking >250ms to arrivee) or too much (e.g. "ssh host aureport").

(*) unless a new command line argument is added

I'll wait for your thoughts to refresh the patch.