From bb2a464067c6843320f367b590b0e4cb00225e50 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 10 Dec 2008 14:17:02 -0500 Subject: [PATCH] Add syslog of security denials and configuration file reloads We need to start logging denials so that they become more easily trackable and debuggable. --- bus/bus.c | 90 ++++++++++++++++++--- bus/bus.h | 6 ++ bus/config-parser-common.c | 6 ++ bus/config-parser-common.h | 1 + bus/config-parser.c | 25 ++++++ bus/config-parser.h | 1 + bus/policy.c | 10 ++- bus/policy.h | 6 +- bus/system.conf.in | 3 + dbus/dbus-sysdeps-unix.c | 1 - dbus/dbus-sysdeps-util-unix.c | 32 ++++++++ dbus/dbus-sysdeps.h | 4 + test/name-test/tmp-session-like-system.conf | 2 + 13 files changed, 169 insertions(+), 18 deletions(-) diff --git a/bus/bus.c b/bus/bus.c index 42cc295f..c6fd693e 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -54,6 +54,7 @@ struct BusContext BusMatchmaker *matchmaker; BusLimits limits; unsigned int fork : 1; + unsigned int syslog : 1; unsigned int keep_umask : 1; unsigned int allow_anonymous : 1; }; @@ -389,6 +390,7 @@ process_config_first_time_only (BusContext *context, } context->fork = bus_config_parser_get_fork (parser); + context->syslog = bus_config_parser_get_syslog (parser); context->keep_umask = bus_config_parser_get_keep_umask (parser); context->allow_anonymous = bus_config_parser_get_allow_anonymous (parser); @@ -834,7 +836,10 @@ bus_context_reload_config (BusContext *context, } ret = TRUE; + bus_context_log_info (context, "Reloaded configuration"); failed: + if (!ret) + bus_context_log_info (context, "Unable to reload configuration: %s", error->message); if (parser != NULL) bus_config_parser_unref (parser); return ret; @@ -1115,6 +1120,32 @@ bus_context_get_reply_timeout (BusContext *context) return context->limits.reply_timeout; } +void +bus_context_log_info (BusContext *context, const char *msg, ...) +{ + va_list args; + + va_start (args, msg); + + if (context->syslog) + _dbus_log_info (msg, args); + + va_end (args); +} + +void +bus_context_log_security (BusContext *context, const char *msg, ...) +{ + va_list args; + + va_start (args, msg); + + if (context->syslog) + _dbus_log_security (msg, args); + + va_end (args); +} + /* * addressed_recipient is the recipient specified in the message. * @@ -1139,8 +1170,10 @@ bus_context_check_security_policy (BusContext *context, { BusClientPolicy *sender_policy; BusClientPolicy *recipient_policy; + dbus_int32_t toggles; int type; dbus_bool_t requested_reply; + const char *sender_name; type = dbus_message_get_type (message); @@ -1151,6 +1184,12 @@ bus_context_check_security_policy (BusContext *context, _dbus_assert (type == DBUS_MESSAGE_TYPE_SIGNAL || addressed_recipient != NULL || strcmp (dbus_message_get_destination (message), DBUS_SERVICE_DBUS) == 0); + + /* Used in logging below */ + if (sender != NULL) + sender_name = bus_connection_get_name (sender); + else + sender_name = NULL; switch (type) { @@ -1193,8 +1232,9 @@ bus_context_check_security_policy (BusContext *context, dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED, "An SELinux policy prevents this sender " "from sending this message to this recipient " - "(rejected message had interface \"%s\" " + "(rejected message had sender \"%s\" interface \"%s\" " "member \"%s\" error name \"%s\" destination \"%s\")", + sender_name ? sender_name : "(unset)", dbus_message_get_interface (message) ? dbus_message_get_interface (message) : "(unset)", dbus_message_get_member (message) ? @@ -1312,16 +1352,16 @@ bus_context_check_security_policy (BusContext *context, context->registry, requested_reply, proposed_recipient, - message)) + message, &toggles)) { const char *dest; + const char *msg = "Rejected send message, %d matched rules; " + "sender=\"%s\" interface=\"%s\" member=\"%s\" error name=\"%s\" destination=\"%s\")"; dest = dbus_message_get_destination (message); - dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED, - "A security policy in place prevents this sender " - "from sending this message to this recipient, " - "see message bus configuration file (rejected message " - "had interface \"%s\" member \"%s\" error name \"%s\" destination \"%s\")", + dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED, msg, + toggles, + sender_name ? sender_name : "(unset)", dbus_message_get_interface (message) ? dbus_message_get_interface (message) : "(unset)", dbus_message_get_member (message) ? @@ -1329,6 +1369,17 @@ bus_context_check_security_policy (BusContext *context, dbus_message_get_error_name (message) ? dbus_message_get_error_name (message) : "(unset)", dest ? dest : DBUS_SERVICE_DBUS); + /* Needs to be duplicated to avoid calling malloc and having to handle OOM */ + bus_context_log_security (context, msg, + toggles, + sender_name ? sender_name : "(unset)", + dbus_message_get_interface (message) ? + dbus_message_get_interface (message) : "(unset)", + dbus_message_get_member (message) ? + dbus_message_get_member (message) : "(unset)", + dbus_message_get_error_name (message) ? + dbus_message_get_error_name (message) : "(unset)", + dest ? dest : DBUS_SERVICE_DBUS); _dbus_verbose ("security policy disallowing message due to sender policy\n"); return FALSE; } @@ -1339,16 +1390,16 @@ bus_context_check_security_policy (BusContext *context, requested_reply, sender, addressed_recipient, proposed_recipient, - message)) + message, &toggles)) { + const char *msg = "Rejected receive message, %d matched rules; " + "sender=\"%s\" interface=\"%s\" member=\"%s\" error name=\"%s\" destination=\"%s\" reply serial=%u requested_reply=%d)"; const char *dest; dest = dbus_message_get_destination (message); - dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED, - "A security policy in place prevents this recipient " - "from receiving this message from this sender, " - "see message bus configuration file (rejected message " - "had interface \"%s\" member \"%s\" error name \"%s\" destination \"%s\" reply serial %u requested_reply=%d)", + dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED, msg, + toggles, + sender_name ? sender_name : "(unset)", dbus_message_get_interface (message) ? dbus_message_get_interface (message) : "(unset)", dbus_message_get_member (message) ? @@ -1358,6 +1409,19 @@ bus_context_check_security_policy (BusContext *context, dest ? dest : DBUS_SERVICE_DBUS, dbus_message_get_reply_serial (message), requested_reply); + /* Needs to be duplicated to avoid calling malloc and having to handle OOM */ + bus_context_log_security (context, msg, + toggles, + sender_name ? sender_name : "(unset)", + dbus_message_get_interface (message) ? + dbus_message_get_interface (message) : "(unset)", + dbus_message_get_member (message) ? + dbus_message_get_member (message) : "(unset)", + dbus_message_get_error_name (message) ? + dbus_message_get_error_name (message) : "(unset)", + dest ? dest : DBUS_SERVICE_DBUS, + dbus_message_get_reply_serial (message), + requested_reply); _dbus_verbose ("security policy disallowing message due to recipient policy\n"); return FALSE; } diff --git a/bus/bus.h b/bus/bus.h index ad231040..74bdb821 100644 --- a/bus/bus.h +++ b/bus/bus.h @@ -107,6 +107,12 @@ int bus_context_get_max_services_per_connection (BusContext int bus_context_get_max_match_rules_per_connection (BusContext *context); int bus_context_get_max_replies_per_connection (BusContext *context); int bus_context_get_reply_timeout (BusContext *context); +void bus_context_log_info (BusContext *context, + const char *msg, + ...); +void bus_context_log_security (BusContext *context, + const char *msg, + ...); dbus_bool_t bus_context_check_security_policy (BusContext *context, BusTransaction *transaction, DBusConnection *sender, diff --git a/bus/config-parser-common.c b/bus/config-parser-common.c index a12642c4..4965c192 100644 --- a/bus/config-parser-common.c +++ b/bus/config-parser-common.c @@ -118,6 +118,10 @@ bus_config_parser_element_name_to_type (const char *name) { return ELEMENT_KEEP_UMASK; } + else if (strcmp (name, "syslog") == 0) + { + return ELEMENT_SYSLOG; + } else if (strcmp (name, "allow_anonymous") == 0) { return ELEMENT_ALLOW_ANONYMOUS; @@ -172,6 +176,8 @@ bus_config_parser_element_type_to_name (ElementType type) return "associate"; case ELEMENT_KEEP_UMASK: return "keep_umask"; + case ELEMENT_SYSLOG: + return "syslog"; case ELEMENT_ALLOW_ANONYMOUS: return "allow_anonymous"; } diff --git a/bus/config-parser-common.h b/bus/config-parser-common.h index ebdc3c95..2c296433 100644 --- a/bus/config-parser-common.h +++ b/bus/config-parser-common.h @@ -49,6 +49,7 @@ typedef enum ELEMENT_STANDARD_SESSION_SERVICEDIRS, ELEMENT_STANDARD_SYSTEM_SERVICEDIRS, ELEMENT_KEEP_UMASK, + ELEMENT_SYSLOG, ELEMENT_ALLOW_ANONYMOUS } ElementType; diff --git a/bus/config-parser.c b/bus/config-parser.c index 64b8f6f1..4e3be531 100644 --- a/bus/config-parser.c +++ b/bus/config-parser.c @@ -113,6 +113,8 @@ struct BusConfigParser unsigned int keep_umask : 1; /**< TRUE to keep original umask when forking */ + unsigned int syslog : 1; /**< TRUE to enable syslog */ + unsigned int is_toplevel : 1; /**< FALSE if we are a sub-config-file inside another one */ unsigned int allow_anonymous : 1; /**< TRUE to allow anonymous connections */ @@ -718,6 +720,21 @@ start_busconfig_child (BusConfigParser *parser, parser->keep_umask = TRUE; + return TRUE; + } + else if (element_type == ELEMENT_SYSLOG) + { + if (!check_no_attributes (parser, "syslog", attribute_names, attribute_values, error)) + return FALSE; + + if (push_element (parser, ELEMENT_SYSLOG) == NULL) + { + BUS_SET_OOM (error); + return FALSE; + } + + parser->syslog = TRUE; + return TRUE; } else if (element_type == ELEMENT_PIDFILE) @@ -1984,6 +2001,7 @@ bus_config_parser_end_element (BusConfigParser *parser, case ELEMENT_DENY: case ELEMENT_FORK: case ELEMENT_KEEP_UMASK: + case ELEMENT_SYSLOG: case ELEMENT_SELINUX: case ELEMENT_ASSOCIATE: case ELEMENT_STANDARD_SESSION_SERVICEDIRS: @@ -2271,6 +2289,7 @@ bus_config_parser_content (BusConfigParser *parser, case ELEMENT_DENY: case ELEMENT_FORK: case ELEMENT_KEEP_UMASK: + case ELEMENT_SYSLOG: case ELEMENT_STANDARD_SESSION_SERVICEDIRS: case ELEMENT_STANDARD_SYSTEM_SERVICEDIRS: case ELEMENT_ALLOW_ANONYMOUS: @@ -2600,6 +2619,12 @@ bus_config_parser_get_keep_umask (BusConfigParser *parser) return parser->keep_umask; } +dbus_bool_t +bus_config_parser_get_syslog (BusConfigParser *parser) +{ + return parser->syslog; +} + dbus_bool_t bus_config_parser_get_allow_anonymous (BusConfigParser *parser) { diff --git a/bus/config-parser.h b/bus/config-parser.h index 6d69f3da..1f2d3d9e 100644 --- a/bus/config-parser.h +++ b/bus/config-parser.h @@ -65,6 +65,7 @@ const char* bus_config_parser_get_type (BusConfigParser *parser); DBusList** bus_config_parser_get_addresses (BusConfigParser *parser); DBusList** bus_config_parser_get_mechanisms (BusConfigParser *parser); dbus_bool_t bus_config_parser_get_fork (BusConfigParser *parser); +dbus_bool_t bus_config_parser_get_syslog (BusConfigParser *parser); dbus_bool_t bus_config_parser_get_keep_umask (BusConfigParser *parser); const char* bus_config_parser_get_pidfile (BusConfigParser *parser); const char* bus_config_parser_get_servicehelper (BusConfigParser *parser); diff --git a/bus/policy.c b/bus/policy.c index caa544e7..2c1a3541 100644 --- a/bus/policy.c +++ b/bus/policy.c @@ -866,7 +866,8 @@ bus_client_policy_check_can_send (BusClientPolicy *policy, BusRegistry *registry, dbus_bool_t requested_reply, DBusConnection *receiver, - DBusMessage *message) + DBusMessage *message, + dbus_int32_t *toggles) { DBusList *link; dbus_bool_t allowed; @@ -876,6 +877,7 @@ bus_client_policy_check_can_send (BusClientPolicy *policy, */ _dbus_verbose (" (policy) checking send rules\n"); + *toggles = 0; allowed = FALSE; link = _dbus_list_get_first_link (&policy->rules); @@ -1026,6 +1028,7 @@ bus_client_policy_check_can_send (BusClientPolicy *policy, /* Use this rule */ allowed = rule->allow; + (*toggles)++; _dbus_verbose (" (policy) used rule, allow now = %d\n", allowed); @@ -1044,7 +1047,8 @@ bus_client_policy_check_can_receive (BusClientPolicy *policy, DBusConnection *sender, DBusConnection *addressed_recipient, DBusConnection *proposed_recipient, - DBusMessage *message) + DBusMessage *message, + dbus_int32_t *toggles) { DBusList *link; dbus_bool_t allowed; @@ -1059,6 +1063,7 @@ bus_client_policy_check_can_receive (BusClientPolicy *policy, */ _dbus_verbose (" (policy) checking receive rules, eavesdropping = %d\n", eavesdropping); + *toggles = 0; allowed = FALSE; link = _dbus_list_get_first_link (&policy->rules); @@ -1223,6 +1228,7 @@ bus_client_policy_check_can_receive (BusClientPolicy *policy, /* Use this rule */ allowed = rule->allow; + (*toggles)++; _dbus_verbose (" (policy) used rule, allow now = %d\n", allowed); diff --git a/bus/policy.h b/bus/policy.h index adb9a059..91fde99f 100644 --- a/bus/policy.h +++ b/bus/policy.h @@ -141,14 +141,16 @@ dbus_bool_t bus_client_policy_check_can_send (BusClientPolicy *policy, BusRegistry *registry, dbus_bool_t requested_reply, DBusConnection *receiver, - DBusMessage *message); + DBusMessage *message, + dbus_int32_t *toggles); dbus_bool_t bus_client_policy_check_can_receive (BusClientPolicy *policy, BusRegistry *registry, dbus_bool_t requested_reply, DBusConnection *sender, DBusConnection *addressed_recipient, DBusConnection *proposed_recipient, - DBusMessage *message); + DBusMessage *message, + dbus_int32_t *toggles); dbus_bool_t bus_client_policy_check_can_own (BusClientPolicy *policy, DBusConnection *connection, const DBusString *service_name); diff --git a/bus/system.conf.in b/bus/system.conf.in index 1b6e716a..41e1bb1a 100644 --- a/bus/system.conf.in +++ b/bus/system.conf.in @@ -29,6 +29,9 @@ @DBUS_SYSTEM_PID_FILE@ + + + EXTERNAL diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c index fb40d5ab..01516a1b 100644 --- a/dbus/dbus-sysdeps-unix.c +++ b/dbus/dbus-sysdeps-unix.c @@ -2786,7 +2786,6 @@ _dbus_full_duplex_pipe (int *fd1, #endif } - /** * Measure the length of the given format string and arguments, * not including the terminating nul. diff --git a/dbus/dbus-sysdeps-util-unix.c b/dbus/dbus-sysdeps-util-unix.c index d8718c21..00627695 100644 --- a/dbus/dbus-sysdeps-util-unix.c +++ b/dbus/dbus-sysdeps-util-unix.c @@ -456,6 +456,38 @@ _dbus_change_to_daemon_user (const char *user, return FALSE; } +void +_dbus_init_system_log (void) +{ + openlog ("dbus", LOG_PID, LOG_DAEMON); +} + +/** + * Log an informative message. Intended for use primarily by + * the system bus. + * + * @param msg a printf-style format string + * @param args arguments for the format string + */ +void +_dbus_log_info (const char *msg, va_list args) +{ + vsyslog (LOG_DAEMON|LOG_NOTICE, msg, args); +} + +/** + * Log a security-related message. Intended for use primarily by + * the system bus. + * + * @param msg a printf-style format string + * @param args arguments for the format string + */ +void +_dbus_log_security (const char *msg, va_list args) +{ + vsyslog (LOG_AUTH|LOG_NOTICE, msg, args); +} + /** Installs a UNIX signal handler * * @param sig the signal to handle diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h index 469b5e52..6c966992 100644 --- a/dbus/dbus-sysdeps.h +++ b/dbus/dbus-sysdeps.h @@ -421,6 +421,10 @@ void _dbus_set_signal_handler (int sig, dbus_bool_t _dbus_user_at_console (const char *username, DBusError *error); +void _dbus_init_system_log (void); +void _dbus_log_info (const char *msg, va_list args); +void _dbus_log_security (const char *msg, va_list args); + /* Define DBUS_VA_COPY() to do the right thing for copying va_list variables. * config.h may have already defined DBUS_VA_COPY as va_copy or __va_copy. */ diff --git a/test/name-test/tmp-session-like-system.conf b/test/name-test/tmp-session-like-system.conf index 96bbf764..f48b7513 100644 --- a/test/name-test/tmp-session-like-system.conf +++ b/test/name-test/tmp-session-like-system.conf @@ -12,6 +12,8 @@ the behavior of child processes. --> + + unix:tmpdir=/tmp -- 2.34.1