auparse: report feed consumption errors

auparse_feed() and auparse_flush_feed() only reported buffering failures even though consume_feed() could encounter parser failures or skip malformed feed records. They also allowed feed data to be consumed without a registered callback, which could silently discard ready events.

Convert consume_feed() to return status, require feed callers to have an AUSOURCE_FEED parser with a callback, propagate parser failures with errno, and report malformed feed records as EBADMSG after continuing to consume the available feed data. Malformed records remain rejected and are not delivered to the callback.

Add auparse_extra_test coverage for missing callbacks and malformed feed records, and document the stricter feed contract, errno meanings, malformed-record disposition, and errno success caveat in the feed man pages.

Author: stevegrubb

ChangeLog

@@ -3,6 +3,7 @@
 - Add KillMode=mixed to the auditd unit file (Issue 272)
 - Update perm table to add syscalls to it
 - Auditd - prevent netlink congestion
+- Propogate feed consumption errors into auparse_feed and auparse_flush_feed
 
 4.1.4
 - Update syscalls and io_uring tables for the 7.0 kernel (Hilko Bengen)

auparse/auparse.c

@@ -46,6 +46,8 @@ static int debug = 0;
 
 static time_t	eoe_timeout = EOE_TIMEOUT;
 
+static int au_auparse_next_event(auparse_state_t *au, int *malformed);
+
 /* like strchr except string is delimited by length, not null byte */
 static char *strnchr(const char *s, int c, size_t n)
 {
@@ -603,19 +605,60 @@ void auparse_add_callback(auparse_state_t *au, auparse_callback_ptr callback,
 	au->callback_user_data_destroy = user_destroy_func;
 }
 
-static void consume_feed(auparse_state_t *au, int flush)
+/* validate_feed - check whether @au is ready to consume feed data
+ * @au: parser state machine to validate
+ *
+ * Return: 0 on success, -1 on failure with errno set.
+ */
+static int validate_feed(const auparse_state_t *au)
+{
+	if (au == NULL || au->source != AUSOURCE_FEED ||
+						au->callback == NULL) {
+		errno = EINVAL;
+		return -1;
+	}
+
+	return 0;
+}
+
+/* consume_feed - process buffered feed data and optionally flush events
+ * @au: parser state machine
+ * @flush: non-zero to terminate and deliver outstanding events
+ *
+ * Return: 0 on success, -1 on failure with errno set.
+ */
+static int consume_feed(auparse_state_t *au, int flush)
 {
+	int rc, malformed, saved_errno = 0;
+
 	//if (debug) printf("consume feed, flush %d\n", flush);
-	while (auparse_next_event(au) > 0) {
-		if (au->callback) {
+	do {
+		/*
+		 * This calls the lower-level parser so malformed feed
+		 * records can be reported while preserving auparse_next_event()
+		 * normalizer state handling.
+		 */
+		malformed = 0;
+		clear_normalizer(&au->norm_data);
+		rc = au_auparse_next_event(au, &malformed);
+		if (malformed && saved_errno == 0)
+			saved_errno = EBADMSG;
+
+		if (rc > 0)
 			(*au->callback)(au, AUPARSE_CB_EVENT_READY,
 					au->callback_user_data);
-		}
+	} while (rc > 0);
+
+	if (rc < 0) {
+		if (errno == 0)
+			errno = EIO;
+		return -1;
 	}
+
 	if (flush) {
 		/* Terminate all outstanding events, as we are at end of input
-		 * (ie mark BUILDING events as COMPLETE events) then if we
-		 * have a callback execute the callback on each event
+		 * (ie mark BUILDING events as COMPLETE events) then execute
+		 * the callback on each event
 		 */
 		event_list_t	*l;
 
@@ -630,12 +673,17 @@ static void consume_feed(auparse_state_t *au, int flush)
 			load_interpretation_list(au, r->interp);
 			aup_list_first_field(l);
 
-			if (au->callback) {
-				(*au->callback)(au, AUPARSE_CB_EVENT_READY,
+			(*au->callback)(au, AUPARSE_CB_EVENT_READY,
 					au->callback_user_data);
-			}
 		}
 	}
+
+	if (saved_errno) {
+		errno = saved_errno;
+		return -1;
+	}
+
+	return 0;
 }
 
 int auparse_new_buffer(auparse_state_t *au, const char *data, size_t data_len)
@@ -651,18 +699,39 @@ int auparse_new_buffer(auparse_state_t *au, const char *data, size_t data_len)
 	return 0;
 }
 
+/* auparse_feed - append data to the feed parser and consume what is ready
+ * @au: parser state machine
+ * @data: feed data to append
+ * @data_len: length of @data
+ *
+ * Return: 0 on success, -1 on failure with errno set.
+ */
 int auparse_feed(auparse_state_t *au, const char *data, size_t data_len)
 {
-	if (databuf_append(&au->databuf, data, data_len) < 0)
+	if (data == NULL && data_len != 0) {
+		errno = EINVAL;
 		return -1;
-	consume_feed(au, 0);
-	return 0;
+	}
+	if (validate_feed(au) < 0)
+		return -1;
+	if (databuf_append(&au->databuf, data, data_len) < 0) {
+		if (errno == 0)
+			errno = ENOMEM;
+		return -1;
+	}
+	return consume_feed(au, 0);
 }
 
+/* auparse_flush_feed - flush pending feed data through the parser
+ * @au: parser state machine
+ *
+ * Return: 0 on success, -1 on failure with errno set.
+ */
 int auparse_flush_feed(auparse_state_t *au)
 {
-	consume_feed(au, 1);
-	return 0;
+	if (validate_feed(au) < 0)
+		return -1;
+	return consume_feed(au, 1);
 }
 
 // If there is any data in the state machine, return 1.
@@ -696,11 +765,21 @@ int auparse_feed_has_ready_event(auparse_state_t *au)
 	return 0;
 }
 
+/* auparse_feed_age_events - complete feed events aged out by the clock
+ * @au: parser state machine
+ *
+ * Return: none.
+ */
 void auparse_feed_age_events(auparse_state_t *au)
 {
-	time_t t = time(NULL);
+	time_t t;
+
+	if (validate_feed(au) < 0)
+		return;
+
+	t = time(NULL);
 	au_check_events(au, t);
-	consume_feed(au, 0);
+	(void)consume_feed(au, 0);
 }
 
 void auparse_set_escape_mode(auparse_state_t *au, auparse_esc_t mode)
@@ -1476,16 +1555,14 @@ int ausearch_next_event(auparse_state_t *au)
 	return 0;
 }
 
-/*
- * au_auparse_next_event - Get the next complete event
- * Args:
- * 	au - the parser state machine
- * Rtns:
- *	< 0	- error
- *	== 0	- no data
- *	> 0	- we have an event and it's set to the 'current event' au->le
+/* au_auparse_next_event - get the next complete event
+ * @au: parser state machine
+ * @malformed: optional flag set when a malformed record is skipped
+ *
+ * Return: less than 0 on error, 0 if no data is available, or greater
+ * than 0 if an event is available and set as the current event.
  */
-static int au_auparse_next_event(auparse_state_t *au)
+static int au_auparse_next_event(auparse_state_t *au, int *malformed)
 {
 	int rc, i, built;
 	event_list_t *l;
@@ -1621,6 +1698,8 @@ static int au_auparse_next_event(auparse_state_t *au)
 			if (debug)
 				printf("Malformed line:%s\n", au->cur_buf);
 #endif	/* LOL_EVENTS_DEBUG01 */
+			if (malformed)
+				*malformed = 1;
 			continue;
 		}
 
@@ -1716,7 +1795,7 @@ static int au_auparse_next_event(auparse_state_t *au)
 int auparse_next_event(auparse_state_t *au)
 {
 	clear_normalizer(&au->norm_data);
-	return au_auparse_next_event(au);
+	return au_auparse_next_event(au, NULL);
 }
 
 /* Accessors to event data */

auparse/test/auparse_extra_test.c

@@ -1,5 +1,6 @@
 #include "config.h"
 #include <assert.h>
+#include <errno.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
@@ -33,6 +34,7 @@ static void test_feed_state(void)
 	const char buf[] = "type=LOGIN msg=audit(1143146623.787:142): pid=1\n";
 	auparse_state_t *au = auparse_init(AUSOURCE_FEED, NULL);
 	assert(au != NULL);
+	cb_count = 0;
 	auparse_add_callback(au, ready_cb, NULL, NULL);
 	assert(auparse_feed_has_data(au) == 0);
 	assert(auparse_feed(au, buf, strlen(buf)) == 0);
@@ -42,6 +44,46 @@ static void test_feed_state(void)
 	auparse_destroy(au);
 }
 
+static void test_feed_requires_callback(void)
+{
+	const char buf[] = "type=LOGIN msg=audit(1143146623.787:142): pid=1\n";
+	auparse_state_t *au = auparse_init(AUSOURCE_FEED, NULL);
+
+	assert(au != NULL);
+	errno = 0;
+	assert(auparse_feed(au, buf, strlen(buf)) == -1);
+	assert(errno == EINVAL);
+	assert(auparse_feed_has_data(au) == 0);
+
+	errno = 0;
+	assert(auparse_flush_feed(au) == -1);
+	assert(errno == EINVAL);
+
+	auparse_destroy(au);
+}
+
+static void test_feed_rejects_malformed_record(void)
+{
+	const char buf[] =
+		"not an audit record\n"
+		"type=LOGIN msg=audit(1143146623.787:142): pid=1\n";
+	auparse_state_t *au = auparse_init(AUSOURCE_FEED, NULL);
+
+	assert(au != NULL);
+	cb_count = 0;
+	auparse_add_callback(au, ready_cb, NULL, NULL);
+
+	errno = 0;
+	assert(auparse_feed(au, buf, strlen(buf)) == -1);
+	assert(errno == EBADMSG);
+	assert(cb_count == 0);
+
+	assert(auparse_flush_feed(au) == 0);
+	assert(cb_count == 1);
+
+	auparse_destroy(au);
+}
+
 static void test_normalize(void)
 {
 	auparse_state_t *au = auparse_init(AUSOURCE_FILE, "./test.log");
@@ -218,6 +260,8 @@ int main(void)
 {
 	test_new_buffer();
 	test_feed_state();
+	test_feed_requires_callback();
+	test_feed_rejects_malformed_record();
 	test_normalize();
 	test_compare();
 	test_timestamp_milli();

docs/auparse_feed.3

@@ -27,6 +27,8 @@ number of bytes in
 supplies new data for the parser to consume.
 .I auparse_init()
 must have been called with a source type of AUSOURCE_FEED and a NULL pointer.
+.I auparse_add_callback()
+must be called before feeding data so that complete events have a consumer.
 .br
 .sp
 The parser consumes as much data
@@ -38,10 +40,58 @@ each time the parser recognizes a complete event in the data stream. Data not fu
 prepended to the next feed data. After all data has been feed to the parser
 .I auparse_flush_feed
 should be called to signal the end of input data and flush any pending parse data through the parsing system.
+Malformed feed records are rejected and are not passed to the callback.
+If malformed records are found, parsing continues for the rest of the
+available feed data before
+.I auparse_feed
+returns an error.
 
 .SH "RETURN VALUE"
 
 Returns \-1 if an error occurs; otherwise, 0 for success.
+When \-1 is returned,
+.I errno
+is set to indicate the error. The value of
+.I errno
+is meaningful only after an error return; callers should not assume it is
+cleared after a successful call.
+Applications that may run with libauparse from before audit-4.1.5 should set
+.I errno
+to 0 before calling
+.I auparse_feed.
+If
+.I auparse_feed
+returns \-1 and
+.I errno
+is still 0, the runtime library is using the older error behavior and did not
+set
+.I errno.
+Clearing
+.I errno
+first avoids treating a pre-existing value as if it were set by
+.I auparse_feed.
+
+.SH "ERRORS"
+
+.TP
+.B EINVAL
+The parser state is invalid, the parser was not initialized with
+.B AUSOURCE_FEED,
+the input arguments are invalid, or no callback has been registered.
+
+.TP
+.B ENOMEM
+Memory allocation failed while buffering or parsing feed data.
+
+.TP
+.B EBADMSG
+A malformed feed record was found. The malformed record was rejected and was
+not passed to the callback. Parsing continues for the rest of the available
+feed data before this error is returned.
+
+.TP
+.B EIO
+An internal parser failure occurred without a more specific error code.
 
 .SH "EXAMPLE"
 .nf

docs/auparse_feed_age_events.3

@@ -9,7 +9,10 @@ auparse_feed_age_events \- check events for complete based on time.
 .SH "DESCRIPTION"
 
 .I auparse_feed_age_events
-should be called to see if any events are complete based on the current clock time. Any newly complete events will be passed to the callback function.
+should be called to see if any events are complete based on the current clock time.
+.I auparse_add_callback()
+must be called before aging feed events so that complete events have a consumer.
+Any newly complete events will be passed to the callback function.
 
 .SH "RETURN VALUE"