Discussion:
[PATCH 0/5] Build time disabling of auditd network listener
Tyler Hicks
2012-08-01 07:00:19 UTC
Permalink
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
Tyler Hicks
2012-08-01 07:00:20 UTC
Permalink
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
Tyler Hicks
2012-08-01 07:00:21 UTC
Permalink
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
Tyler Hicks
2012-08-01 07:00:22 UTC
Permalink
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
Tyler Hicks
2012-08-01 07:00:23 UTC
Permalink
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
Tyler Hicks
2012-08-01 07:00:24 UTC
Permalink
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
Tyler Hicks
2012-09-10 18:39:10 UTC
Permalink
Post by 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.
Hi Steve - Do you have any thoughts on this idea? Thanks!

Tyler
Post by Tyler Hicks
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
https://www.redhat.com/mailman/listinfo/linux-audit
Steve Grubb
2012-09-11 13:12:25 UTC
Permalink
Post by Tyler Hicks
Post by 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.
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.
Post by Tyler Hicks
Post by Tyler Hicks
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
Post by Tyler Hicks
Post by Tyler Hicks
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
https://www.redhat.com/mailman/listinfo/linux-audit
Tyler Hicks
2012-09-11 17:10:35 UTC
Permalink
Post by Steve Grubb
Post by Tyler Hicks
Post by 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.
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.
Post by Steve Grubb
Post by Tyler Hicks
Post by Tyler Hicks
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
Tyler Hicks
2012-10-26 17:09:24 UTC
Permalink
Post by Tyler Hicks
Post by Steve Grubb
Post by Tyler Hicks
Post by 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.
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.
Post by Steve Grubb
Post by Tyler Hicks
Post by Tyler Hicks
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
Steve Grubb
2012-10-26 17:14:42 UTC
Permalink
Post by Tyler Hicks
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
Steve Grubb
2012-11-05 14:17:34 UTC
Permalink
Post by 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.
Applied.

-Steve

Loading...