bus: move shared libaudit code to a new audit.[ch]
authorSimon McVittie <simon.mcvittie@collabora.co.uk>
Thu, 19 Feb 2015 12:04:26 +0000 (12:04 +0000)
committerSimon McVittie <simon.mcvittie@collabora.co.uk>
Thu, 6 Aug 2015 16:12:36 +0000 (17:12 +0100)
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>
bus/Makefile.am
bus/apparmor.c
bus/apparmor.h
bus/audit.c [new file with mode: 0644]
bus/audit.h [new file with mode: 0644]
bus/bus.c
bus/main.c
bus/selinux.c
bus/selinux.h
cmake/bus/CMakeLists.txt

index 38c0558..33af09b 100644 (file)
@@ -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                         \
index a1b3621..3ba84d8 100644 (file)
 #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 */
 }
 
index 4a47aec..18f3ee7 100644 (file)
@@ -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 (file)
index 0000000..97f7d1c
--- /dev/null
@@ -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 (file)
index 0000000..367d7e7
--- /dev/null
@@ -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
index 1aa893b..098985a 100644 (file)
--- 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);
index cb08055..ee5e1eb 100644 (file)
@@ -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;
 }
index 2b8e5db..ef627c8 100644 (file)
@@ -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
index 3bab36d..e44c97e 100644 (file)
@@ -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 */
index 935881a..0dcae65 100644 (file)
@@ -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