From 3cc671cd2b245bf8d85e32af46f64e5977665888 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 19 Feb 2015 12:04:26 +0000 Subject: [PATCH 1/2] 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. --- bus/Makefile.am | 2 + bus/apparmor.c | 39 ++-------- bus/apparmor.h | 1 - bus/audit.c | 180 +++++++++++++++++++++++++++++++++++++++++++++++ bus/audit.h | 31 ++++++++ bus/bus.c | 9 +-- bus/main.c | 2 + bus/selinux.c | 136 +++++------------------------------ bus/selinux.h | 3 - cmake/bus/CMakeLists.txt | 2 + 10 files changed, 243 insertions(+), 162 deletions(-) create mode 100644 bus/audit.c create mode 100644 bus/audit.h diff --git a/bus/Makefile.am b/bus/Makefile.am index 9d3cb00..9d29c41 100644 --- a/bus/Makefile.am +++ b/bus/Makefile.am @@ -73,6 +73,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 a1b3621..3ba84d8 100644 --- a/bus/apparmor.c +++ b/bus/apparmor.c @@ -43,10 +43,10 @@ #include #ifdef HAVE_LIBAUDIT -#include #include #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 4a47aec..18f3ee7 100644 --- a/bus/apparmor.h +++ b/bus/apparmor.h @@ -29,7 +29,6 @@ #include #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 0000000..97f7d1c --- /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 +#include "audit.h" + +#ifdef HAVE_ERRNO_H +#include +#endif + +#ifdef HAVE_LIBAUDIT +#include +#include +#endif + +#include +#ifdef DBUS_UNIX +#include +#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 0000000..367d7e7 --- /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 + +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 68de1b2..31e4e5e 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 #include @@ -971,13 +972,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 b48f03f..68eaf0c 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; @@ -657,6 +658,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 2b8e5db..ef627c8 100644 --- a/bus/selinux.c +++ b/bus/selinux.c @@ -28,6 +28,7 @@ #include #endif #include "selinux.h" +#include "audit.h" #include "services.h" #include "policy.h" #include "utils.h" @@ -50,7 +51,6 @@ #include #endif /* HAVE_SELINUX */ #ifdef HAVE_LIBAUDIT -#include #include #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 3bab36d..e44c97e 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 40f6690..7e8cf57 100644 --- a/cmake/bus/CMakeLists.txt +++ b/cmake/bus/CMakeLists.txt @@ -39,6 +39,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 -- 2.1.4