Add syslog of security denials and configuration file reloads

We need to start logging denials so that they become more easily trackable
and debuggable.

13 files changed, 170 insertions, 23 deletions

diff --git a/bus/bus.c b/bus/bus.c
index a28a2672..195a6fd4 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;
 };
 
 static dbus_int32_t server_data_slot = -1;
@@ -384,6 +385,7 @@ process_config_first_time_only (BusContext      *context,
     }
 
   context->fork = bus_config_parser_get_fork (parser);
+  context->syslog = bus_config_parser_get_syslog (parser);
   
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
   retval = TRUE;
@@ -826,7 +828,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;
@@ -1107,6 +1112,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.
  *
@@ -1131,8 +1162,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);
   
@@ -1143,6 +1176,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)
     {
@@ -1185,8 +1224,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) ?
@@ -1304,16 +1344,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) ?
@@ -1321,6 +1361,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;
     }
@@ -1331,16 +1382,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) ?
@@ -1350,6 +1401,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 6e4bb701..ce590861 100644
--- a/bus/config-parser-common.c
+++ b/bus/config-parser-common.c
@@ -114,6 +114,10 @@ bus_config_parser_element_name_to_type (const char *name)
     {
       return ELEMENT_ASSOCIATE;
     }
+  else if (strcmp (name, "syslog") == 0)
+    {
+      return ELEMENT_SYSLOG;
+    }
   return ELEMENT_NONE;
 }
 
@@ -162,7 +166,9 @@ bus_config_parser_element_type_to_name (ElementType type)
       return "selinux";
     case ELEMENT_ASSOCIATE:
       return "associate";
-    }
+    case ELEMENT_SYSLOG:
+      return "syslog";
+   }
 
   _dbus_assert_not_reached ("bad element type");
 
diff --git a/bus/config-parser-common.h b/bus/config-parser-common.h
index 3718c958..4ecaa8d8 100644
--- a/bus/config-parser-common.h
+++ b/bus/config-parser-common.h
@@ -47,7 +47,8 @@ typedef enum
   ELEMENT_SELINUX,
   ELEMENT_ASSOCIATE,
   ELEMENT_STANDARD_SESSION_SERVICEDIRS,
-  ELEMENT_STANDARD_SYSTEM_SERVICEDIRS
+  ELEMENT_STANDARD_SYSTEM_SERVICEDIRS,
+  ELEMENT_SYSLOG
 } ElementType;
 
 ElementType bus_config_parser_element_name_to_type (const char *element_name);
diff --git a/bus/config-parser.c b/bus/config-parser.c
index f9e0b7d7..f4d7c501 100644
--- a/bus/config-parser.c
+++ b/bus/config-parser.c
@@ -111,6 +111,8 @@ struct BusConfigParser
 
   unsigned int fork : 1; /**< TRUE to fork into daemon mode */
 
+  unsigned int syslog : 1; /**< TRUE to enable syslog */
+
   unsigned int is_toplevel : 1; /**< FALSE if we are a sub-config-file inside another one */
 };
 
@@ -698,6 +700,21 @@ start_busconfig_child (BusConfigParser   *parser,
       
       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)
     {
       if (!check_no_attributes (parser, "pidfile", attribute_names, attribute_values, error))
@@ -1947,6 +1964,7 @@ bus_config_parser_end_element (BusConfigParser   *parser,
     case ELEMENT_ALLOW:
     case ELEMENT_DENY:
     case ELEMENT_FORK:
+    case ELEMENT_SYSLOG:
     case ELEMENT_SELINUX:
     case ELEMENT_ASSOCIATE:
     case ELEMENT_STANDARD_SESSION_SERVICEDIRS:
@@ -2232,6 +2250,7 @@ bus_config_parser_content (BusConfigParser   *parser,
     case ELEMENT_ALLOW:
     case ELEMENT_DENY:
     case ELEMENT_FORK:
+    case ELEMENT_SYSLOG:
     case ELEMENT_STANDARD_SESSION_SERVICEDIRS:    
     case ELEMENT_STANDARD_SYSTEM_SERVICEDIRS:    
     case ELEMENT_SELINUX:
@@ -2554,6 +2573,12 @@ bus_config_parser_get_fork (BusConfigParser   *parser)
   return parser->fork;
 }
 
+dbus_bool_t
+bus_config_parser_get_syslog (BusConfigParser   *parser)
+{
+  return parser->syslog;
+}
+
 const char *
 bus_config_parser_get_pidfile (BusConfigParser   *parser)
 {
diff --git a/bus/config-parser.h b/bus/config-parser.h
index ec0dfed1..fcc5f5dc 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);
 const char* bus_config_parser_get_pidfile      (BusConfigParser *parser);
 const char* bus_config_parser_get_servicehelper (BusConfigParser *parser);
 DBusList**  bus_config_parser_get_service_dirs (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 @@
   <!-- Write a pid file -->
   <pidfile>@DBUS_SYSTEM_PID_FILE@</pidfile>
 
+  <!-- Enable logging to syslog -->
+  <syslog/>
+
   <!-- Only allow socket-credentials-based authentication -->
   <auth>EXTERNAL</auth>
 
diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c
index 24a3774f..ccb84832 100644
--- a/dbus/dbus-sysdeps-unix.c
+++ b/dbus/dbus-sysdeps-unix.c
@@ -2780,7 +2780,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 0343a90c..3f2a2330 100644
--- a/dbus/dbus-sysdeps-util-unix.c
+++ b/dbus/dbus-sysdeps-util-unix.c
@@ -451,6 +451,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 80236f05..5f4b00e1 100644
--- a/dbus/dbus-sysdeps.h
+++ b/dbus/dbus-sysdeps.h
@@ -420,6 +420,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..1cb640a2 100644
--- a/test/name-test/tmp-session-like-system.conf
+++ b/test/name-test/tmp-session-like-system.conf
@@ -8,9 +8,7 @@
   <!-- Our well-known bus type, don't change this -->
   <type>session</type>
 
-  <!-- If we fork, keep the user's original umask to avoid affecting
-       the behavior of child processes. -->
-  <keep_umask/>
+  <syslog/>
 
   <listen>unix:tmpdir=/tmp</listen>