bus: move shared libaudit code to a new audit.[ch]

This fixes various duplicated libaudit interactions in both
SELinux and AppArmor code paths, including opening two audit sockets
if both SELinux and AppArmor were enabled at compile time.
In particular, audit.c is now the only user of libcap-ng.

This commit is not intended to introduce any functional changes,
except for the de-duplication.

The actual audit_log_user_avc_message() call is still duplicated,
because the SELinux and AppArmor code paths use different mechanisms
to compose the audit message: the SELinux path uses a statically-sized
buffer on the stack which might be subject to truncation, whereas
the AppArmor path uses malloc() (via DBusString) and falls back to
using syslog on a memory allocation failure.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89225
Reviewed-by: Colin Walters <walters@verbum.org>
[smcv: minor issues raised during review are subsequently fixed]
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>

10 files changed, 243 insertions, 162 deletions

diff --git a/bus/Makefile.am b/bus/Makefile.am
index 38c0558f..33af09b0 100644
--- a/bus/Makefile.am
+++ b/bus/Makefile.am
@@ -78,6 +78,8 @@ BUS_SOURCES=					\
 	activation-exit-codes.h			\
 	apparmor.c				\
 	apparmor.h				\
+	audit.c					\
+	audit.h					\
 	bus.c					\
 	bus.h					\
 	config-parser.c				\
diff --git a/bus/apparmor.c b/bus/apparmor.c
index a1b3621a..3ba84d8d 100644
--- a/bus/apparmor.c
+++ b/bus/apparmor.c
@@ -43,10 +43,10 @@
 #include <unistd.h>
 
 #ifdef HAVE_LIBAUDIT
-#include <cap-ng.h>
 #include <libaudit.h>
 #endif /* HAVE_LIBAUDIT */
 
+#include "audit.h"
 #include "connection.h"
 #include "utils.h"
 
@@ -62,10 +62,6 @@ typedef enum {
 /* Store the value of the AppArmor mediation mode in the bus configuration */
 static AppArmorConfigMode apparmor_config_mode = APPARMOR_ENABLED;
 
-#ifdef HAVE_LIBAUDIT
-static int audit_fd = -1;
-#endif
-
 /* The AppArmor context, consisting of a label and a mode. */
 struct BusAppArmorConfinement
 {
@@ -105,25 +101,6 @@ bus_apparmor_confinement_new (char       *label,
   return confinement;
 }
 
-void
-bus_apparmor_audit_init (void)
-{
-#ifdef HAVE_LIBAUDIT
-  audit_fd = audit_open ();
-
-  if (audit_fd < 0)
-    {
-      /* If kernel doesn't support audit, bail out */
-      if (errno == EINVAL || errno == EPROTONOSUPPORT || errno == EAFNOSUPPORT)
-        return;
-      /* If user bus, bail out */
-      if (errno == EPERM && getuid () != 0)
-        return;
-      _dbus_warn ("Failed opening connection to the audit subsystem");
-    }
-#endif /* HAVE_LIBAUDIT */
-}
-
 /*
  * Return TRUE on successful check, FALSE on OOM.
  * Set *is_supported to whether AA has D-Bus features.
@@ -189,6 +166,9 @@ static void
 log_message (dbus_bool_t allow, const char *op, DBusString *data)
 {
   const char *mstr;
+#ifdef HAVE_LIBAUDIT
+  int audit_fd;
+#endif
 
   if (allow)
     mstr = "ALLOWED";
@@ -196,14 +176,12 @@ log_message (dbus_bool_t allow, const char *op, DBusString *data)
     mstr = "DENIED";
 
 #ifdef HAVE_LIBAUDIT
+  audit_fd = bus_audit_get_fd ();
+
   if (audit_fd >= 0)
   {
     DBusString avc;
 
-    capng_get_caps_process ();
-    if (!capng_have_capability (CAPNG_EFFECTIVE, CAP_AUDIT_WRITE))
-      goto syslog;
-
     if (!_dbus_string_init (&avc))
       goto syslog;
 
@@ -510,11 +488,6 @@ bus_apparmor_shutdown (void)
 
   bus_apparmor_confinement_unref (bus_con);
   bus_con = NULL;
-
-#ifdef HAVE_LIBAUDIT
-  audit_close (audit_fd);
-#endif /* HAVE_LIBAUDIT */
-
 #endif /* HAVE_APPARMOR */
 }
 
diff --git a/bus/apparmor.h b/bus/apparmor.h
index 4a47aecc..18f3ee79 100644
--- a/bus/apparmor.h
+++ b/bus/apparmor.h
@@ -29,7 +29,6 @@
 #include <dbus/dbus.h>
 #include "bus.h"
 
-void bus_apparmor_audit_init (void);
 dbus_bool_t bus_apparmor_pre_init (void);
 dbus_bool_t bus_apparmor_set_mode_from_config (const char *mode,
                                                DBusError *error);
diff --git a/bus/audit.c b/bus/audit.c
new file mode 100644
index 00000000..97f7d1c4
--- /dev/null
+++ b/bus/audit.c
@@ -0,0 +1,180 @@
+/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*-
+ * audit.c - libaudit integration for SELinux and AppArmor
+ *
+ * Based on apparmor.c, selinux.c
+ *
+ * Copyright © 2014-2015 Canonical, Ltd.
+ * Copyright © 2015 Collabora Ltd.
+ *
+ * Licensed under the Academic Free License version 2.1
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#include <config.h>
+#include "audit.h"
+
+#ifdef HAVE_ERRNO_H
+#include <errno.h>
+#endif
+
+#ifdef HAVE_LIBAUDIT
+#include <cap-ng.h>
+#include <libaudit.h>
+#endif
+
+#include <dbus/dbus-internals.h>
+#ifdef DBUS_UNIX
+#include <dbus/dbus-userdb.h>
+#endif
+
+#ifdef HAVE_LIBAUDIT
+static int audit_fd = -1;
+#endif
+
+/**
+ * Open the libaudit fd if appropriate.
+ */
+void
+bus_audit_init(void)
+{
+#ifdef HAVE_LIBAUDIT
+  audit_fd = audit_open ();
+
+  if (audit_fd < 0)
+    {
+      /* If kernel doesn't support audit, bail out */
+      if (errno == EINVAL || errno == EPROTONOSUPPORT || errno == EAFNOSUPPORT)
+        return;
+      /* If user bus, bail out */
+      if (errno == EPERM && getuid() != 0)
+        return;
+      _dbus_warn ("Failed opening connection to the audit subsystem");
+    }
+#endif /* HAVE_LIBAUDIT */
+}
+
+/**
+ * If libaudit is in use and it would be appropriate to write audit records,
+ * return the libaudit fd. Otherwise return -1.
+ */
+int
+bus_audit_get_fd (void)
+{
+#ifdef HAVE_LIBAUDIT
+  if (audit_fd >= 0)
+  {
+    capng_get_caps_process ();
+
+    if (!capng_have_capability (CAPNG_EFFECTIVE, CAP_AUDIT_WRITE))
+      return -1;
+
+    return audit_fd;
+  }
+#endif
+
+  return -1;
+}
+
+/**
+ * Close the libaudit fd.
+ */
+void
+bus_audit_shutdown (void)
+{
+#ifdef HAVE_LIBAUDIT
+  audit_close (audit_fd);
+#endif /* HAVE_LIBAUDIT */
+}
+
+/* The !HAVE_LIBAUDIT case lives in dbus-sysdeps-util-unix.c */
+#ifdef HAVE_LIBAUDIT
+/**
+ * Changes the user and group the bus is running as.
+ *
+ * @param user the user to become
+ * @param error return location for errors
+ * @returns #FALSE on failure
+ */
+dbus_bool_t
+_dbus_change_to_daemon_user  (const char    *user,
+                              DBusError     *error)
+{
+  dbus_uid_t uid;
+  dbus_gid_t gid;
+  DBusString u;
+
+  _dbus_string_init_const (&u, user);
+
+  if (!_dbus_get_user_id_and_primary_group (&u, &uid, &gid))
+    {
+      dbus_set_error (error, DBUS_ERROR_FAILED,
+                      "User '%s' does not appear to exist?",
+                      user);
+      return FALSE;
+    }
+
+  /* If we were root */
+  if (_dbus_geteuid () == 0)
+    {
+      int rc;
+      int have_audit_write;
+
+      have_audit_write = capng_have_capability (CAPNG_PERMITTED, CAP_AUDIT_WRITE);
+      capng_clear (CAPNG_SELECT_BOTH);
+      /* Only attempt to retain CAP_AUDIT_WRITE if we had it when
+       * starting.  See:
+       * https://bugs.freedesktop.org/show_bug.cgi?id=49062#c9
+       */
+      if (have_audit_write)
+        capng_update (CAPNG_ADD, CAPNG_EFFECTIVE | CAPNG_PERMITTED,
+                      CAP_AUDIT_WRITE);
+      rc = capng_change_id (uid, gid, CAPNG_DROP_SUPP_GRP);
+      if (rc)
+        {
+          switch (rc) {
+            default:
+              dbus_set_error (error, DBUS_ERROR_FAILED,
+                              "Failed to drop capabilities: %s\n",
+                              _dbus_strerror (errno));
+              break;
+            case -4:
+              dbus_set_error (error, _dbus_error_from_errno (errno),
+                              "Failed to set GID to %lu: %s", gid,
+                              _dbus_strerror (errno));
+              break;
+            case -5:
+              _dbus_warn ("Failed to drop supplementary groups: %s\n",
+                          _dbus_strerror (errno));
+              break;
+            case -6:
+              dbus_set_error (error, _dbus_error_from_errno (errno),
+                              "Failed to set UID to %lu: %s", uid,
+                              _dbus_strerror (errno));
+              break;
+            case -7:
+              dbus_set_error (error, _dbus_error_from_errno (errno),
+                              "Failed to unset keep-capabilities: %s\n",
+                              _dbus_strerror (errno));
+              break;
+          }
+          return FALSE;
+        }
+    }
+
+ return TRUE;
+}
+#endif
diff --git a/bus/audit.h b/bus/audit.h
new file mode 100644
index 00000000..367d7e7a
--- /dev/null
+++ b/bus/audit.h
@@ -0,0 +1,31 @@
+/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*-
+ * audit.h - libaudit integration for SELinux and AppArmor
+ *
+ * Licensed under the Academic Free License version 2.1
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
+ *
+ */
+
+#ifndef BUS_AUDIT_H
+#define BUS_AUDIT_H
+
+#include <dbus/dbus.h>
+
+void bus_audit_init (void);
+int bus_audit_get_fd (void);
+void bus_audit_shutdown (void);
+
+#endif
diff --git a/bus/bus.c b/bus/bus.c
index 1aa893b6..098985a6 100644
--- a/bus/bus.c
+++ b/bus/bus.c
@@ -35,6 +35,7 @@
 #include "signals.h"
 #include "selinux.h"
 #include "apparmor.h"
+#include "audit.h"
 #include "dir-watch.h"
 #include <dbus/dbus-list.h>
 #include <dbus/dbus-hash.h>
@@ -972,13 +973,7 @@ bus_context_new (const DBusString *config_file,
 	  goto failed;
 	}
 
-#ifdef HAVE_SELINUX
-      /* FIXME - why not just put this in full_init() below? */
-      bus_selinux_audit_init ();
-#endif
-#ifdef HAVE_APPARMOR
-      bus_apparmor_audit_init ();
-#endif
+      bus_audit_init ();
     }
 
   dbus_server_free_data_slot (&server_data_slot);
diff --git a/bus/main.c b/bus/main.c
index cb08055d..ee5e1ebf 100644
--- a/bus/main.c
+++ b/bus/main.c
@@ -40,6 +40,7 @@
 #endif
 #include "selinux.h"
 #include "apparmor.h"
+#include "audit.h"
 
 static BusContext *context;
 
@@ -661,6 +662,7 @@ main (int argc, char **argv)
   bus_context_unref (context);
   bus_selinux_shutdown ();
   bus_apparmor_shutdown ();
+  bus_audit_shutdown ();
 
   return 0;
 }
diff --git a/bus/selinux.c b/bus/selinux.c
index 2b8e5db2..ef627c8c 100644
--- a/bus/selinux.c
+++ b/bus/selinux.c
@@ -28,6 +28,7 @@
 #include <dbus/dbus-userdb.h>
 #endif
 #include "selinux.h"
+#include "audit.h"
 #include "services.h"
 #include "policy.h"
 #include "utils.h"
@@ -50,7 +51,6 @@
 #include <grp.h>
 #endif /* HAVE_SELINUX */
 #ifdef HAVE_LIBAUDIT
-#include <cap-ng.h>
 #include <libaudit.h>
 #endif /* HAVE_LIBAUDIT */
 
@@ -115,53 +115,35 @@ static const struct avc_lock_callback lock_cb =
  */
 #ifdef HAVE_SELINUX
 
-#ifdef HAVE_LIBAUDIT
-static int audit_fd = -1;
-#endif
-
-void
-bus_selinux_audit_init(void)
-{
-#ifdef HAVE_LIBAUDIT  
-  audit_fd = audit_open ();
-
-  if (audit_fd < 0)
-    {
-      /* If kernel doesn't support audit, bail out */
-      if (errno == EINVAL || errno == EPROTONOSUPPORT || errno == EAFNOSUPPORT)
-        return;
-      /* If user bus, bail out */
-      if (errno == EPERM && getuid() != 0)
-        return;
-      _dbus_warn ("Failed opening connection to the audit subsystem");
-    }
-#endif /* HAVE_LIBAUDIT */
-}
-
 static void 
 log_callback (const char *fmt, ...) 
 {
   va_list ap;
+#ifdef HAVE_LIBAUDIT
+  int audit_fd;
+#endif
 
   va_start(ap, fmt);
 
 #ifdef HAVE_LIBAUDIT
+  audit_fd = bus_audit_get_fd ();
+
   if (audit_fd >= 0)
   {
-    capng_get_caps_process();
-    if (capng_have_capability(CAPNG_EFFECTIVE, CAP_AUDIT_WRITE))
-    {
-      char buf[PATH_MAX*2];
-    
-      /* FIXME: need to change this to show real user */
-      vsnprintf(buf, sizeof(buf), fmt, ap);
-      audit_log_user_avc_message(audit_fd, AUDIT_USER_AVC, buf, NULL, NULL,
-                               NULL, getuid());
-      goto out;
-    }
+    /* This should really be a DBusString, but DBusString allocates
+     * memory dynamically; before switching it, we need to check with
+     * SELinux people that it would be OK for this to fall back to
+     * syslog if OOM, like the equivalent AppArmor code does. */
+    char buf[PATH_MAX*2];
+
+    /* FIXME: need to change this to show real user */
+    vsnprintf(buf, sizeof(buf), fmt, ap);
+    audit_log_user_avc_message(audit_fd, AUDIT_USER_AVC, buf, NULL, NULL,
+                             NULL, getuid());
+    goto out;
   }
 #endif /* HAVE_LIBAUDIT */
-  
+
   vsyslog (LOG_USER | LOG_INFO, fmt, ap);
 
 out:
@@ -1031,88 +1013,6 @@ bus_selinux_shutdown (void)
       bus_avc_print_stats ();
 
       avc_destroy ();
-#ifdef HAVE_LIBAUDIT
-      audit_close (audit_fd);
-#endif /* HAVE_LIBAUDIT */
     }
 #endif /* HAVE_SELINUX */
 }
-
-/* The !HAVE_LIBAUDIT case lives in dbus-sysdeps-util-unix.c */
-#ifdef HAVE_LIBAUDIT
-/**
- * Changes the user and group the bus is running as.
- *
- * @param user the user to become
- * @param error return location for errors
- * @returns #FALSE on failure
- */
-dbus_bool_t
-_dbus_change_to_daemon_user  (const char    *user,
-                              DBusError     *error)
-{
-  dbus_uid_t uid;
-  dbus_gid_t gid;
-  DBusString u;
-
-  _dbus_string_init_const (&u, user);
-
-  if (!_dbus_get_user_id_and_primary_group (&u, &uid, &gid))
-    {
-      dbus_set_error (error, DBUS_ERROR_FAILED,
-                      "User '%s' does not appear to exist?",
-                      user);
-      return FALSE;
-    }
-
-  /* If we were root */
-  if (_dbus_geteuid () == 0)
-    {
-      int rc;
-      int have_audit_write;
-
-      have_audit_write = capng_have_capability (CAPNG_PERMITTED, CAP_AUDIT_WRITE);
-      capng_clear (CAPNG_SELECT_BOTH);
-      /* Only attempt to retain CAP_AUDIT_WRITE if we had it when
-       * starting.  See:
-       * https://bugs.freedesktop.org/show_bug.cgi?id=49062#c9
-       */
-      if (have_audit_write)
-        capng_update (CAPNG_ADD, CAPNG_EFFECTIVE | CAPNG_PERMITTED,
-                      CAP_AUDIT_WRITE);
-      rc = capng_change_id (uid, gid, CAPNG_DROP_SUPP_GRP);
-      if (rc)
-        {
-          switch (rc) {
-            default:
-              dbus_set_error (error, DBUS_ERROR_FAILED,
-                              "Failed to drop capabilities: %s\n",
-                              _dbus_strerror (errno));
-              break;
-            case -4:
-              dbus_set_error (error, _dbus_error_from_errno (errno),
-                              "Failed to set GID to %lu: %s", gid,
-                              _dbus_strerror (errno));
-              break;
-            case -5:
-              _dbus_warn ("Failed to drop supplementary groups: %s\n",
-                          _dbus_strerror (errno));
-              break;
-            case -6:
-              dbus_set_error (error, _dbus_error_from_errno (errno),
-                              "Failed to set UID to %lu: %s", uid,
-                              _dbus_strerror (errno));
-              break;
-            case -7:
-              dbus_set_error (error, _dbus_error_from_errno (errno),
-                              "Failed to unset keep-capabilities: %s\n",
-                              _dbus_strerror (errno));
-              break;
-          }
-          return FALSE;
-        }
-    }
-
- return TRUE;
-}
-#endif
diff --git a/bus/selinux.h b/bus/selinux.h
index 3bab36de..e44c97ed 100644
--- a/bus/selinux.h
+++ b/bus/selinux.h
@@ -66,7 +66,4 @@ dbus_bool_t bus_selinux_allows_send            (DBusConnection *sender,
 BusSELinuxID* bus_selinux_init_connection_id (DBusConnection *connection,
                                               DBusError      *error);
 
-
-void bus_selinux_audit_init(void);
-
 #endif /* BUS_SELINUX_H */
diff --git a/cmake/bus/CMakeLists.txt b/cmake/bus/CMakeLists.txt
index 935881ae..0dcae65a 100644
--- a/cmake/bus/CMakeLists.txt
+++ b/cmake/bus/CMakeLists.txt
@@ -41,6 +41,8 @@ set (BUS_SOURCES
 	${BUS_DIR}/activation.h				
 	${BUS_DIR}/apparmor.c
 	${BUS_DIR}/apparmor.h
+	${BUS_DIR}/audit.c
+	${BUS_DIR}/audit.h
 	${BUS_DIR}/bus.c					
 	${BUS_DIR}/bus.h					
 	${BUS_DIR}/config-parser.c