[PATCH 0/5] Build time disabling of auditd network listener

[PATCH 0/5] Build time disabling of auditd network listener

[Tyler Hicks]

Hello Steve - This is a patch set that allows --disable-listener to be passed
to the configure script to disable the auditd network listener code at build
time. The reasoning is that a large number of users do not need centralized
audit logging and removing the network listening code from a root-owned auditd
process is appealing from a security perspective.

The existing implementation clearly does not initialize the listener when
tcp_listen_port is undefined in auditd.conf, but I still think there is value
in not having the listening code present in all auditd installations.

The first three patches in the set are refactoring patches to move nearly all of
the listening code into auditd-listen.c in order to minimize the number of
ifdefs that would need to be scattered throughout C source files. The fourth
patch is an optional cleanup patch. The last patch introduces the
--disable-listener option.

The auditd listener code is still enabled by default so that existing distro
packaging recipes will not need to be updated.

I look forward to your feedback. Thanks!

Tyler

[PATCH 1/5] Move auditd listener reconfigure code into auditd-listen.c

[Tyler Hicks]

This allows for easier build-time disabling of the listener-specific
code in auditd-event.c.
---
 src/auditd-event.c  |   23 ++---------------------
 src/auditd-listen.c |   28 +++++++++++++++++++++++++++-
 src/auditd-listen.h |    3 ++-
 3 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/src/auditd-event.c b/src/auditd-event.c
index b1b2f0a..acf5aa1 100644
--- a/src/auditd-event.c
+++ b/src/auditd-event.c
@@ -1177,27 +1177,8 @@ static void reconfigure(struct auditd_consumer_data *data)
 		}
 	}
 
-	/* Look at network things that do not need restarting */
-	if (oconf->tcp_client_min_port != nconf->tcp_client_min_port ||
-		    oconf->tcp_client_max_port != nconf->tcp_client_max_port ||
-		    oconf->tcp_max_per_addr != nconf->tcp_max_per_addr) {
-		oconf->tcp_client_min_port = nconf->tcp_client_min_port;
-		oconf->tcp_client_max_port = nconf->tcp_client_max_port;
-		oconf->tcp_max_per_addr = nconf->tcp_max_per_addr;
-		auditd_set_ports(oconf->tcp_client_min_port,
-				oconf->tcp_client_max_port,
-				oconf->tcp_max_per_addr);
-	}
-	if (oconf->tcp_client_max_idle != nconf->tcp_client_max_idle) {
-		oconf->tcp_client_max_idle = nconf->tcp_client_max_idle;
-		periodic_reconfigure();
-	}
-	if (oconf->tcp_listen_port != nconf->tcp_listen_port ||
-			oconf->tcp_listen_queue != nconf->tcp_listen_queue) {
-		oconf->tcp_listen_port = nconf->tcp_listen_port;
-		oconf->tcp_listen_queue = nconf->tcp_listen_queue;
-		// FIXME: need to restart the network stuff
-	}
+	// network listener
+	auditd_tcp_listen_reconfigure(nconf, oconf);
 	
 	/* At this point we will work on the items that are related to 
 	 * a single log file. */
diff --git a/src/auditd-listen.c b/src/auditd-listen.c
index 741c424..0caf324 100644
--- a/src/auditd-listen.c
+++ b/src/auditd-listen.c
@@ -866,7 +866,7 @@ static void auditd_tcp_listen_handler( struct ev_loop *loop,
 	send_audit_event(AUDIT_DAEMON_ACCEPT, emsg);
 }
 
-void auditd_set_ports(int minp, int maxp, int max_p_addr)
+static void auditd_set_ports(int minp, int maxp, int max_p_addr)
 {
 	min_port = minp;
 	max_port = maxp;
@@ -1009,3 +1009,29 @@ void auditd_tcp_listen_check_idle (struct ev_loop *loop )
 		free(ev);
 	}
 }
+
+void auditd_tcp_listen_reconfigure ( struct daemon_conf *nconf,
+				     struct daemon_conf *oconf )
+{
+	/* Look at network things that do not need restarting */
+	if (oconf->tcp_client_min_port != nconf->tcp_client_min_port ||
+		    oconf->tcp_client_max_port != nconf->tcp_client_max_port ||
+		    oconf->tcp_max_per_addr != nconf->tcp_max_per_addr) {
+		oconf->tcp_client_min_port = nconf->tcp_client_min_port;
+		oconf->tcp_client_max_port = nconf->tcp_client_max_port;
+		oconf->tcp_max_per_addr = nconf->tcp_max_per_addr;
+		auditd_set_ports(oconf->tcp_client_min_port,
+				oconf->tcp_client_max_port,
+				oconf->tcp_max_per_addr);
+	}
+	if (oconf->tcp_client_max_idle != nconf->tcp_client_max_idle) {
+		oconf->tcp_client_max_idle = nconf->tcp_client_max_idle;
+		periodic_reconfigure();
+	}
+	if (oconf->tcp_listen_port != nconf->tcp_listen_port ||
+			oconf->tcp_listen_queue != nconf->tcp_listen_queue) {
+		oconf->tcp_listen_port = nconf->tcp_listen_port;
+		oconf->tcp_listen_queue = nconf->tcp_listen_queue;
+		// FIXME: need to restart the network stuff
+	}
+}
diff --git a/src/auditd-listen.h b/src/auditd-listen.h
index 81e0ad3..440b6ab 100644
--- a/src/auditd-listen.h
+++ b/src/auditd-listen.h
@@ -25,9 +25,10 @@
 #define AUDITD_LISTEN_H
 
 #include "ev.h"
-void auditd_set_ports(int minp, int maxp, int max_p_addr);
 int auditd_tcp_listen_init ( struct ev_loop *loop, struct daemon_conf *config );
 void auditd_tcp_listen_uninit ( struct ev_loop *loop );
 void auditd_tcp_listen_check_idle ( struct ev_loop *loop );
+void auditd_tcp_listen_reconfigure ( struct daemon_conf *nconf,
+				     struct daemon_conf *oconf );
 
 #endif
-- 
1.7.9.5

[PATCH 2/5] Store daemon config pointer in the periodic watcher's private data

[Tyler Hicks]

In preparation for moving the periodic watcher to auditd-listen.c,
periodic_handler() cannot rely on having access to the config global
variable. That can be solved by using the private data pointer built
into libev watchers.
---
 src/auditd.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/auditd.c b/src/auditd.c
index a5b8b2c..e0ee702 100644
--- a/src/auditd.c
+++ b/src/auditd.c
@@ -445,7 +445,9 @@ static void netlink_handler(struct ev_loop *loop, struct ev_io *io,
 static void periodic_handler(struct ev_loop *loop, struct ev_periodic *per,
 			int revents )
 {
-	if (config.tcp_client_max_idle)
+	struct daemon_conf *config = (struct daemon_conf *) per->data;
+
+	if (config->tcp_client_max_idle)
 		auditd_tcp_listen_check_idle (loop);
 }
 
@@ -719,6 +721,7 @@ int main(int argc, char *argv[])
 
 	ev_periodic_init (&periodic_watcher, periodic_handler,
 			  0, config.tcp_client_max_idle, NULL);
+	periodic_watcher.data = &config;
 	if (config.tcp_client_max_idle)
 		ev_periodic_start (loop, &periodic_watcher);
 
-- 
1.7.9.5

[PATCH 3/5] Move periodic watcher into auditd-listen.c

[Tyler Hicks]

Move the periodic watcher (un)initialization and handler code into
auditd-listen.c to allow for easy disabling at build time. The
(un)initialization is now handled by auditd_tcp_listen_init() and
auditd_tcp_listen_uninit().
---
 src/auditd-config.h |    2 --
 src/auditd-listen.c |   40 +++++++++++++++++++++++++++++++++++++---
 src/auditd-listen.h |    4 ++--
 src/auditd.c        |   32 +-------------------------------
 4 files changed, 40 insertions(+), 38 deletions(-)

diff --git a/src/auditd-config.h b/src/auditd-config.h
index 9bf6698..f58a521 100644
--- a/src/auditd-config.h
+++ b/src/auditd-config.h
@@ -96,7 +96,5 @@ int start_config_manager(struct auditd_reply_list *rep);
 void shutdown_config(void);
 void free_config(struct daemon_conf *config);
 
-void periodic_reconfigure(void);
-
 #endif
 
diff --git a/src/auditd-listen.c b/src/auditd-listen.c
index 0caf324..01c14a0 100644
--- a/src/auditd-listen.c
+++ b/src/auditd-listen.c
@@ -75,6 +75,7 @@ typedef struct ev_tcp {
 
 static int listen_socket;
 static struct ev_io tcp_listen_watcher;
+static struct ev_periodic periodic_watcher;
 static int min_port, max_port, max_per_addr;
 static int use_libwrap = 1;
 #ifdef USE_GSSAPI
@@ -87,6 +88,8 @@ static char msgbuf[MAX_AUDIT_MESSAGE_LENGTH + 1];
 
 static struct ev_tcp *client_chain = NULL;
 
+static void auditd_tcp_listen_check_idle (struct ev_loop *loop );
+
 static char *sockaddr_to_ipv4(struct sockaddr_in *addr)
 {
 	unsigned char *uaddr = (unsigned char *)&(addr->sin_addr);
@@ -873,11 +876,26 @@ static void auditd_set_ports(int minp, int maxp, int max_p_addr)
 	max_per_addr = max_p_addr;
 }
 
+static void periodic_handler(struct ev_loop *loop, struct ev_periodic *per,
+			int revents )
+{
+	struct daemon_conf *config = (struct daemon_conf *) per->data;
+
+	if (config->tcp_client_max_idle)
+		auditd_tcp_listen_check_idle (loop);
+}
+
 int auditd_tcp_listen_init ( struct ev_loop *loop, struct daemon_conf *config )
 {
 	struct sockaddr_in address;
 	int one = 1;
 
+	ev_periodic_init (&periodic_watcher, periodic_handler,
+			  0, config->tcp_client_max_idle, NULL);
+	periodic_watcher.data = config;
+	if (config->tcp_client_max_idle)
+		ev_periodic_start (loop, &periodic_watcher);
+
 	/* If the port is not set, that means we aren't going to
 	  listen for connections.  */
 	if (config->tcp_listen_port == 0)
@@ -963,7 +981,8 @@ int auditd_tcp_listen_init ( struct ev_loop *loop, struct daemon_conf *config )
 	return 0;
 }
 
-void auditd_tcp_listen_uninit ( struct ev_loop *loop )
+void auditd_tcp_listen_uninit ( struct ev_loop *loop,
+				struct daemon_conf *config )
 {
 #ifdef USE_GSSAPI
 	OM_uint32 status;
@@ -987,9 +1006,12 @@ void auditd_tcp_listen_uninit ( struct ev_loop *loop )
 		ev_io_stop (loop, &client_chain->io);
 		close_client (client_chain);
 	}
+
+	if (config->tcp_client_max_idle)
+		ev_periodic_stop (loop, &periodic_watcher);
 }
 
-void auditd_tcp_listen_check_idle (struct ev_loop *loop )
+static void auditd_tcp_listen_check_idle (struct ev_loop *loop )
 {
 	struct ev_tcp *ev, *next = NULL;
 	int active;
@@ -1010,6 +1032,18 @@ void auditd_tcp_listen_check_idle (struct ev_loop *loop )
 	}
 }
 
+static void periodic_reconfigure(struct daemon_conf *config)
+{
+	struct ev_loop *loop = ev_default_loop (EVFLAG_AUTO);
+	if (config->tcp_client_max_idle) {
+		ev_periodic_set (&periodic_watcher, ev_now (loop),
+				 config->tcp_client_max_idle, NULL);
+		ev_periodic_start (loop, &periodic_watcher);
+	} else {
+		ev_periodic_stop (loop, &periodic_watcher);
+	}
+}
+
 void auditd_tcp_listen_reconfigure ( struct daemon_conf *nconf,
 				     struct daemon_conf *oconf )
 {
@@ -1026,7 +1060,7 @@ void auditd_tcp_listen_reconfigure ( struct daemon_conf *nconf,
 	}
 	if (oconf->tcp_client_max_idle != nconf->tcp_client_max_idle) {
 		oconf->tcp_client_max_idle = nconf->tcp_client_max_idle;
-		periodic_reconfigure();
+		periodic_reconfigure(oconf);
 	}
 	if (oconf->tcp_listen_port != nconf->tcp_listen_port ||
 			oconf->tcp_listen_queue != nconf->tcp_listen_queue) {
diff --git a/src/auditd-listen.h b/src/auditd-listen.h
index 440b6ab..024fd6f 100644
--- a/src/auditd-listen.h
+++ b/src/auditd-listen.h
@@ -26,8 +26,8 @@
 
 #include "ev.h"
 int auditd_tcp_listen_init ( struct ev_loop *loop, struct daemon_conf *config );
-void auditd_tcp_listen_uninit ( struct ev_loop *loop );
-void auditd_tcp_listen_check_idle ( struct ev_loop *loop );
+void auditd_tcp_listen_uninit ( struct ev_loop *loop,
+				struct daemon_conf *config );
 void auditd_tcp_listen_reconfigure ( struct daemon_conf *nconf,
 				     struct daemon_conf *oconf );
 
diff --git a/src/auditd.c b/src/auditd.c
index e0ee702..a369434 100644
--- a/src/auditd.c
+++ b/src/auditd.c
@@ -68,7 +68,6 @@ static int do_fork = 1;
 static struct auditd_reply_list *rep = NULL;
 static int hup_info_requested = 0, usr1_info_requested = 0;
 static char subj[SUBJ_LEN];
-static struct ev_periodic periodic_watcher;
 
 /* Local function prototypes */
 int send_audit_event(int type, const char *str);
@@ -442,27 +441,6 @@ static void netlink_handler(struct ev_loop *loop, struct ev_io *io,
 	}
 }
 
-static void periodic_handler(struct ev_loop *loop, struct ev_periodic *per,
-			int revents )
-{
-	struct daemon_conf *config = (struct daemon_conf *) per->data;
-
-	if (config->tcp_client_max_idle)
-		auditd_tcp_listen_check_idle (loop);
-}
-
-void periodic_reconfigure(void)
-{
-	struct ev_loop *loop = ev_default_loop (EVFLAG_AUTO);
-	if (config.tcp_client_max_idle) {
-		ev_periodic_set (&periodic_watcher, ev_now (loop),
-				 config.tcp_client_max_idle, NULL);
-		ev_periodic_start (loop, &periodic_watcher);
-	} else {
-		ev_periodic_stop (loop, &periodic_watcher);
-	}
-}
-
 int main(int argc, char *argv[])
 {
 	struct sigaction sa;
@@ -719,12 +697,6 @@ int main(int argc, char *argv[])
 	ev_signal_init (&sigchld_watcher, child_handler, SIGCHLD);
 	ev_signal_start (loop, &sigchld_watcher);
 
-	ev_periodic_init (&periodic_watcher, periodic_handler,
-			  0, config.tcp_client_max_idle, NULL);
-	periodic_watcher.data = &config;
-	if (config.tcp_client_max_idle)
-		ev_periodic_start (loop, &periodic_watcher);
-
 	if (auditd_tcp_listen_init (loop, &config)) {
 		char emsg[DEFAULT_BUF_SZ];
 		if (*subj)
@@ -755,15 +727,13 @@ int main(int argc, char *argv[])
 	if (!stop)
 		ev_loop (loop, 0);
 
-	auditd_tcp_listen_uninit (loop);
+	auditd_tcp_listen_uninit (loop, &config);
 
 	// Tear down IO watchers Part 1
 	ev_signal_stop (loop, &sighup_watcher);
 	ev_signal_stop (loop, &sigusr1_watcher);
 	ev_signal_stop (loop, &sigusr2_watcher);
 	ev_signal_stop (loop, &sigterm_watcher);
-	if (config.tcp_client_max_idle)
-		ev_periodic_stop (loop, &periodic_watcher);
 
 	/* Write message to log that we are going down */
 	rc = audit_request_signal_info(fd);
-- 
1.7.9.5

[PATCH 4/5] Consolidate periodic handler code

[Tyler Hicks]

Clean up the handler code by consolidating it to a single function.
---
 src/auditd-listen.c |   44 +++++++++++++++++++-------------------------
 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/src/auditd-listen.c b/src/auditd-listen.c
index 01c14a0..d1977c6 100644
--- a/src/auditd-listen.c
+++ b/src/auditd-listen.c
@@ -88,8 +88,6 @@ static char msgbuf[MAX_AUDIT_MESSAGE_LENGTH + 1];
 
 static struct ev_tcp *client_chain = NULL;
 
-static void auditd_tcp_listen_check_idle (struct ev_loop *loop );
-
 static char *sockaddr_to_ipv4(struct sockaddr_in *addr)
 {
 	unsigned char *uaddr = (unsigned char *)&(addr->sin_addr);
@@ -880,9 +878,26 @@ static void periodic_handler(struct ev_loop *loop, struct ev_periodic *per,
 			int revents )
 {
 	struct daemon_conf *config = (struct daemon_conf *) per->data;
+	struct ev_tcp *ev, *next = NULL;
+	int active;
 
-	if (config->tcp_client_max_idle)
-		auditd_tcp_listen_check_idle (loop);
+	if (!config->tcp_client_max_idle)
+		return;
+
+	for (ev = client_chain; ev; ev = next) {
+		active = ev->client_active;
+		ev->client_active = 0;
+		if (active)
+			continue;
+
+		audit_msg(LOG_NOTICE,
+			"client %s idle too long - closing connection\n",
+			sockaddr_to_addr4(&(ev->addr)));
+		ev_io_stop (loop, &ev->io);
+		release_client(ev);
+		next = ev->next;
+		free(ev);
+	}
 }
 
 int auditd_tcp_listen_init ( struct ev_loop *loop, struct daemon_conf *config )
@@ -1011,27 +1026,6 @@ void auditd_tcp_listen_uninit ( struct ev_loop *loop,
 		ev_periodic_stop (loop, &periodic_watcher);
 }
 
-static void auditd_tcp_listen_check_idle (struct ev_loop *loop )
-{
-	struct ev_tcp *ev, *next = NULL;
-	int active;
-
-	for (ev = client_chain; ev; ev = next) {
-		active = ev->client_active;
-		ev->client_active = 0;
-		if (active)
-			continue;
-
-		audit_msg(LOG_NOTICE,
-			"client %s idle too long - closing connection\n",
-			sockaddr_to_addr4(&(ev->addr)));
-		ev_io_stop (loop, &ev->io);
-		release_client(ev);
-		next = ev->next;
-		free(ev);
-	}
-}
-
 static void periodic_reconfigure(struct daemon_conf *config)
 {
 	struct ev_loop *loop = ev_default_loop (EVFLAG_AUTO);
-- 
1.7.9.5

[PATCH 5/5] Conditionally build auditd network listener support

[Tyler Hicks]

Add the --disable-listener configure option to leave the network
listener code out of auditd. By default, the listener code is still
included in auditd. When the listener is disabled, the listener init,
uninit, and reconfigure functions are stubbed out.

ifdefs are used in auditd-config.c to disable the listener-specific
parsers, following the style of the krb5 parser functions.
---
 configure.ac        |   14 ++++++++++++++
 src/Makefile.am     |    5 ++++-
 src/auditd-config.c |   35 +++++++++++++++++++++++++++++++++++
 src/auditd-listen.h |   21 +++++++++++++++++++++
 4 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index e14df60..76eaa26 100644
--- a/configure.ac
+++ b/configure.ac
@@ -104,6 +104,20 @@ fi
 fi
 AM_CONDITIONAL(HAVE_PYTHON, test ${python_found} = "yes")
 
+#auditd listener
+AC_MSG_CHECKING(whether to include auditd network listener support)
+AC_ARG_ENABLE(listener,
+	      [AS_HELP_STRING([--disable-listener],
+			      [Disable auditd network listener support])],
+	      enable_listener=$enableval,
+	      enable_listener=yes)
+if test "x$enable_listener" != "xno"; then
+	AC_DEFINE(USE_LISTENER, 1,
+		  [Define if you want to use the auditd network listener.])
+fi
+AM_CONDITIONAL(ENABLE_LISTENER, test "x$enable_listener" != "xno")
+AC_MSG_RESULT($enable_listener)
+
 #gssapi
 AC_ARG_ENABLE(gssapi_krb5,
 	[AS_HELP_STRING([--enable-gssapi-krb5],[Enable GSSAPI Kerberos 5 support @<:@default=no@:>@])],
diff --git a/src/Makefile.am b/src/Makefile.am
index 57ddd27..fdfa5cf 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -28,7 +28,10 @@ sbin_PROGRAMS = auditd auditctl aureport ausearch autrace
 AM_CFLAGS = -D_GNU_SOURCE
 noinst_HEADERS = auditd-config.h auditd-event.h auditd-listen.h ausearch-llist.h ausearch-options.h auditctl-llist.h aureport-options.h ausearch-parse.h aureport-scan.h ausearch-lookup.h ausearch-int.h auditd-dispatch.h ausearch-string.h ausearch-nvpair.h ausearch-common.h ausearch-avc.h ausearch-time.h ausearch-lol.h
 
-auditd_SOURCES = auditd.c auditd-event.c auditd-config.c auditd-reconfig.c auditd-sendmail.c auditd-dispatch.c auditd-listen.c
+auditd_SOURCES = auditd.c auditd-event.c auditd-config.c auditd-reconfig.c auditd-sendmail.c auditd-dispatch.c
+if ENABLE_LISTENER
+auditd_SOURCES += auditd-listen.c
+endif
 auditd_CFLAGS = -fPIE -DPIE -g -D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pthread
 auditd_LDFLAGS = -pie -Wl,-z,relro -Wl,-z,now
 auditd_DEPENDENCIES = mt/libauditmt.a libev/libev.a
diff --git a/src/auditd-config.c b/src/auditd-config.c
index 9569378..13220bf 100644
--- a/src/auditd-config.c
+++ b/src/auditd-config.c
@@ -1189,6 +1189,12 @@ static int tcp_listen_port_parser(struct nv_pair *nv, int line,
 	audit_msg(LOG_DEBUG, "tcp_listen_port_parser called with: %s",
 		  nv->value);
 
+#ifndef USE_LISTENER
+	audit_msg(LOG_DEBUG,
+		"Listener support is not enabled, ignoring value at line %d",
+		line);
+	return 0;
+#else
 	/* check that all chars are numbers */
 	for (i=0; ptr[i]; i++) {
 		if (!isdigit(ptr[i])) {
@@ -1223,6 +1229,7 @@ static int tcp_listen_port_parser(struct nv_pair *nv, int line,
 	}
 	config->tcp_listen_port = (unsigned int)i;
 	return 0;
+#endif
 }
 
 static int tcp_listen_queue_parser(struct nv_pair *nv, int line,
@@ -1234,6 +1241,12 @@ static int tcp_listen_queue_parser(struct nv_pair *nv, int line,
 	audit_msg(LOG_DEBUG, "tcp_listen_queue_parser called with: %s",
 		  nv->value);
 
+#ifndef USE_LISTENER
+	audit_msg(LOG_DEBUG,
+		"Listener support is not enabled, ignoring value at line %d",
+		line);
+	return 0;
+#else
 	/* check that all chars are numbers */
 	for (i=0; ptr[i]; i++) {
 		if (!isdigit(ptr[i])) {
@@ -1270,6 +1283,7 @@ static int tcp_listen_queue_parser(struct nv_pair *nv, int line,
 	}
 	config->tcp_listen_queue = (unsigned int)i;
 	return 0;
+#endif
 }
 
 
@@ -1282,6 +1296,12 @@ static int tcp_max_per_addr_parser(struct nv_pair *nv, int line,
 	audit_msg(LOG_DEBUG, "tcp_max_per_addr_parser called with: %s",
 		  nv->value);
 
+#ifndef USE_LISTENER
+	audit_msg(LOG_DEBUG,
+		"Listener support is not enabled, ignoring value at line %d",
+		line);
+	return 0;
+#else
 	/* check that all chars are numbers */
 	for (i=0; ptr[i]; i++) {
 		if (!isdigit(ptr[i])) {
@@ -1318,6 +1338,7 @@ static int tcp_max_per_addr_parser(struct nv_pair *nv, int line,
 	}
 	config->tcp_max_per_addr = (unsigned int)i;
 	return 0;
+#endif
 }
 
 static int use_libwrap_parser(struct nv_pair *nv, int line,
@@ -1348,6 +1369,12 @@ static int tcp_client_ports_parser(struct nv_pair *nv, int line,
 	audit_msg(LOG_DEBUG, "tcp_listen_queue_parser called with: %s",
 		  nv->value);
 
+#ifndef USE_LISTENER
+	audit_msg(LOG_DEBUG,
+		"Listener support is not enabled, ignoring value at line %d",
+		line);
+	return 0;
+#else
 	/* check that all chars are numbers, with an optional inclusive '-'. */
 	for (i=0; ptr[i]; i++) {
 		if (i > 0 && ptr[i] == '-' && ptr[i+1] != '\0') {
@@ -1412,6 +1439,7 @@ static int tcp_client_ports_parser(struct nv_pair *nv, int line,
 	config->tcp_client_min_port = (unsigned int)minv;
 	config->tcp_client_max_port = (unsigned int)maxv;
 	return 0;
+#endif
 }
 
 static int tcp_client_max_idle_parser(struct nv_pair *nv, int line,
@@ -1423,6 +1451,12 @@ static int tcp_client_max_idle_parser(struct nv_pair *nv, int line,
 	audit_msg(LOG_DEBUG, "tcp_client_max_idle_parser called with: %s",
 		  nv->value);
 
+#ifndef USE_LISTENER
+	audit_msg(LOG_DEBUG,
+		"Listener support is not enabled, ignoring value at line %d",
+		line);
+	return 0;
+#else
 	/* check that all chars are numbers */
 	for (i=0; ptr[i]; i++) {
 		if (!isdigit(ptr[i])) {
@@ -1453,6 +1487,7 @@ static int tcp_client_max_idle_parser(struct nv_pair *nv, int line,
 	}
 	config->tcp_client_max_idle = (unsigned int)i;
 	return 0;
+#endif
 }
 
 static int enable_krb5_parser(struct nv_pair *nv, int line,
diff --git a/src/auditd-listen.h b/src/auditd-listen.h
index 024fd6f..69f9310 100644
--- a/src/auditd-listen.h
+++ b/src/auditd-listen.h
@@ -25,10 +25,31 @@
 #define AUDITD_LISTEN_H
 
 #include "ev.h"
+
+#ifdef USE_LISTENER
 int auditd_tcp_listen_init ( struct ev_loop *loop, struct daemon_conf *config );
 void auditd_tcp_listen_uninit ( struct ev_loop *loop,
 				struct daemon_conf *config );
 void auditd_tcp_listen_reconfigure ( struct daemon_conf *nconf,
 				     struct daemon_conf *oconf );
+#else
+static inline int auditd_tcp_listen_init ( struct ev_loop *loop,
+					   struct daemon_conf *config )
+{
+	return 0;
+}
+
+static inline void auditd_tcp_listen_uninit ( struct ev_loop *loop,
+					      struct daemon_conf *config )
+{
+	return;
+}
+
+static inline void auditd_tcp_listen_reconfigure ( struct daemon_conf *nconf,
+						   struct daemon_conf *oconf )
+{
+	return;
+}
+#endif /* USE_LISTENER */
 
 #endif
-- 
1.7.9.5

Re: [PATCH 0/5] Build time disabling of auditd network listener

[Tyler Hicks]

[-- Attachment #1.1: Type: text/plain, Size: 1385 bytes --]

On 2012-08-01 00:00:19, Tyler Hicks wrote:
> Hello Steve - This is a patch set that allows --disable-listener to be passed
> to the configure script to disable the auditd network listener code at build
> time. The reasoning is that a large number of users do not need centralized
> audit logging and removing the network listening code from a root-owned auditd
> process is appealing from a security perspective.
> 
> The existing implementation clearly does not initialize the listener when
> tcp_listen_port is undefined in auditd.conf, but I still think there is value
> in not having the listening code present in all auditd installations.

Hi Steve - Do you have any thoughts on this idea? Thanks!

Tyler

> 
> The first three patches in the set are refactoring patches to move nearly all of
> the listening code into auditd-listen.c in order to minimize the number of
> ifdefs that would need to be scattered throughout C source files. The fourth
> patch is an optional cleanup patch. The last patch introduces the
> --disable-listener option.
> 
> The auditd listener code is still enabled by default so that existing distro
> packaging recipes will not need to be updated.
> 
> I look forward to your feedback. Thanks!
> 
> Tyler
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 0/5] Build time disabling of auditd network listener

[Steve Grubb]

On Monday, September 10, 2012 11:39:10 AM Tyler Hicks wrote:
> On 2012-08-01 00:00:19, Tyler Hicks wrote:
> > Hello Steve - This is a patch set that allows --disable-listener to be
> > passed to the configure script to disable the auditd network listener
> > code at build time. The reasoning is that a large number of users do not
> > need centralized audit logging and removing the network listening code
> > from a root-owned auditd process is appealing from a security
> > perspective.

My thoughts are that if  tcp_listen_port is not set up, the callback is not 
registered and none of the networking code comes into play. By configuration, 
admins are able to reduce the attack surface. The real effect of the patch is 
that it reduces binary image size.


> > The existing implementation clearly does not initialize the listener when
> > tcp_listen_port is undefined in auditd.conf, but I still think there is
> > value in not having the listening code present in all auditd
> > installations.
> Hi Steve - Do you have any thoughts on this idea? Thanks!

I was getting to this patch set. Are you planning to turn off networking for 
Ubuntu? Just curious if the patch is going to be used rather than just be an 
academic exercise. :-)   I don't see us turning it off any time soon.

Thanks,
-Steve


> > The first three patches in the set are refactoring patches to move nearly
> > all of the listening code into auditd-listen.c in order to minimize the
> > number of ifdefs that would need to be scattered throughout C source
> > files. The fourth patch is an optional cleanup patch. The last patch
> > introduces the
> > --disable-listener option.
> > 
> > The auditd listener code is still enabled by default so that existing
> > distro packaging recipes will not need to be updated.
> > 
> > I look forward to your feedback. Thanks!
> > 
> > Tyler
> > 
> > --
> > Linux-audit mailing list
> > Linux-audit@redhat.com
> > https://www.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH 0/5] Build time disabling of auditd network listener

[Tyler Hicks]

[-- Attachment #1.1: Type: text/plain, Size: 2135 bytes --]

On 2012-09-11 09:12:25, Steve Grubb wrote:
> On Monday, September 10, 2012 11:39:10 AM Tyler Hicks wrote:
> > On 2012-08-01 00:00:19, Tyler Hicks wrote:
> > > Hello Steve - This is a patch set that allows --disable-listener to be
> > > passed to the configure script to disable the auditd network listener
> > > code at build time. The reasoning is that a large number of users do not
> > > need centralized audit logging and removing the network listening code
> > > from a root-owned auditd process is appealing from a security
> > > perspective.
> 
> My thoughts are that if  tcp_listen_port is not set up, the callback is not 
> registered and none of the networking code comes into play. By configuration, 
> admins are able to reduce the attack surface. The real effect of the patch is 
> that it reduces binary image size.

I still see this as more than just reducing binary image size. I agree
about the tcp_listen_port configuration option, but eliminating
potential misconfiguration issues by removing the lesser used networking
code is a security win. 

> 
> 
> > > The existing implementation clearly does not initialize the listener when
> > > tcp_listen_port is undefined in auditd.conf, but I still think there is
> > > value in not having the listening code present in all auditd
> > > installations.
> > Hi Steve - Do you have any thoughts on this idea? Thanks!
> 
> I was getting to this patch set. Are you planning to turn off networking for 
> Ubuntu? Just curious if the patch is going to be used rather than just be an 
> academic exercise. :-)   I don't see us turning it off any time soon.

Yes, we plan to use the patch. The idea is to have two auditd binary
packages - auditd and auditd-base (package names aren't set in stone at
this point). The auditd package would be the fully functional daemon,
with network listener support, and auditd-base would be built with
--disable-listener to provide a daemon with less of an attack surface.

The auditd-base package would promoted to "Main" and we'd encourage the
majority of users to use it, rather than auditd.

Tyler

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 0/5] Build time disabling of auditd network listener

[Tyler Hicks]

[-- Attachment #1.1: Type: text/plain, Size: 2499 bytes --]

On 2012-09-11 10:10:35, Tyler Hicks wrote:
> On 2012-09-11 09:12:25, Steve Grubb wrote:
> > On Monday, September 10, 2012 11:39:10 AM Tyler Hicks wrote:
> > > On 2012-08-01 00:00:19, Tyler Hicks wrote:
> > > > Hello Steve - This is a patch set that allows --disable-listener to be
> > > > passed to the configure script to disable the auditd network listener
> > > > code at build time. The reasoning is that a large number of users do not
> > > > need centralized audit logging and removing the network listening code
> > > > from a root-owned auditd process is appealing from a security
> > > > perspective.
> > 
> > My thoughts are that if  tcp_listen_port is not set up, the callback is not 
> > registered and none of the networking code comes into play. By configuration, 
> > admins are able to reduce the attack surface. The real effect of the patch is 
> > that it reduces binary image size.
> 
> I still see this as more than just reducing binary image size. I agree
> about the tcp_listen_port configuration option, but eliminating
> potential misconfiguration issues by removing the lesser used networking
> code is a security win. 
> 
> > 
> > 
> > > > The existing implementation clearly does not initialize the listener when
> > > > tcp_listen_port is undefined in auditd.conf, but I still think there is
> > > > value in not having the listening code present in all auditd
> > > > installations.
> > > Hi Steve - Do you have any thoughts on this idea? Thanks!
> > 
> > I was getting to this patch set. Are you planning to turn off networking for 
> > Ubuntu? Just curious if the patch is going to be used rather than just be an 
> > academic exercise. :-)   I don't see us turning it off any time soon.
> 
> Yes, we plan to use the patch. The idea is to have two auditd binary
> packages - auditd and auditd-base (package names aren't set in stone at
> this point). The auditd package would be the fully functional daemon,
> with network listener support, and auditd-base would be built with
> --disable-listener to provide a daemon with less of an attack surface.
> 
> The auditd-base package would promoted to "Main" and we'd encourage the
> majority of users to use it, rather than auditd.

Hello Steve - I wanted to follow up on this patch set. I will be moving
forward with the process of getting auditd into Ubuntu's main repo soon
and I'm not clear on the status of these patches. Do you plan on merging
them?

Thanks!

Tyler

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 0/5] Build time disabling of auditd network listener

[Steve Grubb]

On Friday, October 26, 2012 10:09:24 AM Tyler Hicks wrote:
> Hello Steve - I wanted to follow up on this patch set. I will be moving
> forward with the process of getting auditd into Ubuntu's main repo soon
> and I'm not clear on the status of these patches. Do you plan on merging
> them?

Yes, they will be in the next release. Amazing coincidence as I was just 
looking for them today to apply...but forgot the title of this email thread. I 
am trying to get everything ready for a new audit package update in November.

Thanks,
-Steve

Re: [PATCH 0/5] Build time disabling of auditd network listener

[Steve Grubb]

On Wednesday, August 01, 2012 12:00:19 AM Tyler Hicks wrote:
> Hello Steve - This is a patch set that allows --disable-listener to be
> passed to the configure script to disable the auditd network listener code
> at build time. The reasoning is that a large number of users do not need
> centralized audit logging and removing the network listening code from a
> root-owned auditd process is appealing from a security perspective.
> 
> The existing implementation clearly does not initialize the listener when
> tcp_listen_port is undefined in auditd.conf, but I still think there is
> value in not having the listening code present in all auditd installations.
> 
> The first three patches in the set are refactoring patches to move nearly
> all of the listening code into auditd-listen.c in order to minimize the
> number of ifdefs that would need to be scattered throughout C source files.
> The fourth patch is an optional cleanup patch. The last patch introduces
> the  --disable-listener option.
> 
> The auditd listener code is still enabled by default so that existing distro
> packaging recipes will not need to be updated.

Applied.

-Steve