Attachment 21250 Details for Bug 19141 – More syslog information

[patch] More syslog information

dbus-syslog-extra.patch (text/plain), 15.63 KB, created by Colin Walters on 2008-12-17 13:06:16 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Colin Walters

Created: 2008-12-17 13:06:16 UTC

Size: 15.63 KB

patch

obsolete

>commit e6bbcbd903f9989cf9fe69ca4efa807848a6c11c
>Author: Colin Walters <walters@verbum.org>
>Date:   Wed Dec 17 16:01:28 2008 -0500
>
>    Add uid, pid, and command to security logs
>    
>    Extend the current security logs with even more relevant
>    information than just the message content.  This requires
>    some utility code to look up and cache (as a string)
>    the data such as the uid/pid/command when a connection is
>    authenticated.
>
>diff --git a/bus/bus.c b/bus/bus.c
>index 391acad..b8354e2 100644
>--- a/bus/bus.c
>+++ b/bus/bus.c
>@@ -1176,6 +1176,8 @@ bus_context_check_security_policy (BusContext     *context,
>   int type;
>   dbus_bool_t requested_reply;
>   const char *sender_name;
>+  const char *sender_loginfo;
>+  const char *proposed_recipient_loginfo;
>   
>   type = dbus_message_get_type (message);
>   dest = dbus_message_get_destination (message);
>@@ -1190,9 +1192,20 @@ bus_context_check_security_policy (BusContext     *context,
> 
>   /* Used in logging below */
>   if (sender != NULL)
>-    sender_name = bus_connection_get_name (sender);
>+    {
>+      sender_name = bus_connection_get_name (sender);
>+      sender_loginfo = bus_connection_get_loginfo (sender);
>+    }
>+  else
>+    {
>+      sender_name = NULL;
>+      sender_loginfo = "(bus)";
>+    }
>+  
>+  if (proposed_recipient != NULL)
>+    proposed_recipient_loginfo = bus_connection_get_loginfo (proposed_recipient);
>   else
>-    sender_name = NULL;
>+    proposed_recipient_loginfo = "bus";
>   
>   switch (type)
>     {
>@@ -1355,32 +1368,35 @@ bus_context_check_security_policy (BusContext     *context,
>                                          message, &toggles, &log))
>     {
>       const char *msg = "Rejected send message, %d matched rules; "
>-                        "type=\"%s\", sender=\"%s\" interface=\"%s\" member=\"%s\" error name=\"%s\" destination=\"%s\")";
>-
>+                        "type=\"%s\", sender=\"%s\" (%s) interface=\"%s\" member=\"%s\" error name=\"%s\" destination=\"%s\" (%s))";
> 
>       dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED, msg,
>                       toggles,
>                       dbus_message_type_to_string (dbus_message_get_type (message)),
>                       sender_name ? sender_name : "(unset)",
>+                      sender_loginfo,
>                       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);
>+                      dest ? dest : DBUS_SERVICE_DBUS,
>+                      proposed_recipient_loginfo);
>       /* Needs to be duplicated to avoid calling malloc and having to handle OOM */
>       bus_context_log_security (context, msg,
>                                 toggles,
>                                 dbus_message_type_to_string (dbus_message_get_type (message)),
>                                 sender_name ? sender_name : "(unset)",
>+                                sender_loginfo,
>                                 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);
>+                                dest ? dest : DBUS_SERVICE_DBUS,
>+                                proposed_recipient_loginfo);
>       _dbus_verbose ("security policy disallowing message due to sender policy\n");
>       return FALSE;
>     }
>@@ -1409,35 +1425,39 @@ bus_context_check_security_policy (BusContext     *context,
>                                             message, &toggles))
>     {
>       const char *msg = "Rejected receive message, %d matched rules; "
>-                        "type=\"%s\" sender=\"%s\" interface=\"%s\" member=\"%s\" error name=\"%s\" destination=\"%s\" reply serial=%u requested_reply=%d)";
>+                        "type=\"%s\" sender=\"%s\" (%s) interface=\"%s\" member=\"%s\" error name=\"%s\" reply serial=%u requested_reply=%d destination=\"%s\" (%s))";
> 
>       dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED, msg,
>                       toggles,
>                       dbus_message_type_to_string (dbus_message_get_type (message)),
>                       sender_name ? sender_name : "(unset)",
>+                      sender_loginfo,
>                       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);
>+                      requested_reply,
>+                      dest ? dest : DBUS_SERVICE_DBUS,
>+                      proposed_recipient_loginfo);
>       /* Needs to be duplicated to avoid calling malloc and having to handle OOM */
>       bus_context_log_security (context, msg,
>                                 toggles,
>                                 dbus_message_type_to_string (dbus_message_get_type (message)),
>                                 sender_name ? sender_name : "(unset)",
>+                                sender_loginfo,
>                                 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);
>+                                requested_reply,
>+                                dest ? dest : DBUS_SERVICE_DBUS,
>+                                proposed_recipient_loginfo);
>       _dbus_verbose ("security policy disallowing message due to recipient policy\n");
>       return FALSE;
>     }
>diff --git a/bus/connection.c b/bus/connection.c
>index ed1b139..ab99fa5 100644
>--- a/bus/connection.c
>+++ b/bus/connection.c
>@@ -32,6 +32,9 @@
> #include <dbus/dbus-hash.h>
> #include <dbus/dbus-timeout.h>
> 
>+/* Trim executed commands to this length; we want to keep logs readable */
>+#define MAX_LOG_COMMAND_LEN 50
>+
> static void bus_connection_remove_transactions (DBusConnection *connection);
> 
> typedef struct
>@@ -76,6 +79,7 @@ typedef struct
>   DBusPreallocatedSend *oom_preallocated;
>   BusClientPolicy *policy;
> 
>+  char *cached_loginfo_string;
>   BusSELinuxID *selinux_id;
> 
>   long connection_tv_sec;  /**< Time when we connected (seconds component) */
>@@ -406,6 +410,8 @@ free_connection_data (void *data)
>   if (d->selinux_id)
>     bus_selinux_id_unref (d->selinux_id);
>   
>+  dbus_free (d->cached_loginfo_string);
>+  
>   dbus_free (d->name);
>   
>   dbus_free (d);
>@@ -537,13 +543,73 @@ bus_connections_unref (BusConnections *connections)
>     }
> }
> 
>+/* Used for logging */
>+static dbus_bool_t
>+cache_peer_loginfo_string (BusConnectionData *d, 
>+                           DBusConnection    *connection)
>+{
>+  DBusString loginfo_buf;
>+  unsigned long uid;
>+  unsigned long pid;
>+  char *windows_sid;
>+  dbus_bool_t prev_added;
>+
>+  if (!_dbus_string_init (&loginfo_buf))
>+    return FALSE;
>+  
>+  prev_added = FALSE;
>+  if (dbus_connection_get_unix_user (connection, &uid))
>+    {
>+      if (!_dbus_string_append_printf (&loginfo_buf, "uid=%ld", uid))
>+        goto oom;
>+      else
>+        prev_added = TRUE;
>+    }
>+
>+  if (dbus_connection_get_unix_process_id (connection, &pid))
>+    {
>+      if (prev_added)
>+        {
>+          if (!_dbus_string_append_byte (&loginfo_buf, ' '))
>+            goto oom;
>+        }
>+      if (!_dbus_string_append_printf (&loginfo_buf, "pid=%ld comm=\"", pid))
>+        goto oom;
>+      /* Ignore errors here */
>+      if (_dbus_command_for_pid (pid, &loginfo_buf, MAX_LOG_COMMAND_LEN, NULL))
>+        {
>+          if (!_dbus_string_append_byte (&loginfo_buf, '"'))
>+            goto oom;
>+        }
>+    }
>+
>+  if (dbus_connection_get_windows_user (connection, &windows_sid))
>+    {
>+      if (!_dbus_string_append_printf (&loginfo_buf, "sid=\"%s\" ", windows_sid))
>+        goto oom;
>+      dbus_free (windows_sid);
>+    }
>+
>+  if (!_dbus_string_steal_data (&loginfo_buf, &(d->cached_loginfo_string)))
>+    goto oom;
>+
>+  _dbus_string_free (&loginfo_buf); 
>+
>+  return TRUE;
>+oom:
>+   _dbus_string_free (&loginfo_buf);
>+   return FALSE;
>+}
>+
> dbus_bool_t
> bus_connections_setup_connection (BusConnections *connections,
>                                   DBusConnection *connection)
> {
>+
>   BusConnectionData *d;
>   dbus_bool_t retval;
>   DBusError error;
>+
>   
>   d = dbus_new0 (BusConnectionData, 1);
>   
>@@ -583,7 +649,7 @@ bus_connections_setup_connection (BusConnections *connections,
>       dbus_error_free (&error);
>       goto out;
>     }
>-  
>+
>   if (!dbus_connection_set_watch_functions (connection,
>                                             add_connection_watch,
>                                             remove_connection_watch,
>@@ -842,6 +908,18 @@ bus_connection_is_in_unix_group (DBusConnection *connection,
>   return FALSE;
> }
> 
>+const char *
>+bus_connection_get_loginfo (DBusConnection        *connection)
>+{
>+  BusConnectionData *d;
>+    
>+  d = BUS_CONNECTION_DATA (connection);
>+
>+  if (!bus_connection_is_active (connection))
>+    return "inactive";
>+  return d->cached_loginfo_string;  
>+}
>+
> BusClientPolicy*
> bus_connection_get_policy (DBusConnection *connection)
> {
>@@ -1302,16 +1380,15 @@ bus_connection_complete (DBusConnection   *connection,
>     {
>       if (!adjust_connections_for_uid (d->connections,
>                                        uid, 1))
>-        {
>-          BUS_SET_OOM (error);
>-          dbus_free (d->name);
>-          d->name = NULL;
>-          bus_client_policy_unref (d->policy);
>-          d->policy = NULL;
>-          return FALSE;
>-        }
>+        goto fail;
>     }
>-  
>+
>+  /* Create and cache a string which holds information about the 
>+   * peer process; used for logging purposes.
>+   */
>+  if (!cache_peer_loginfo_string (d, connection))
>+    goto fail;
>+
>   /* Now the connection is active, move it between lists */
>   _dbus_list_unlink (&d->connections->incomplete,
>                      d->link_in_connection_list);
>@@ -1329,6 +1406,14 @@ bus_connection_complete (DBusConnection   *connection,
>   _dbus_assert (bus_connection_is_active (connection));
>   
>   return TRUE;
>+fail:
>+  BUS_SET_OOM (error);
>+  dbus_free (d->name);
>+  d->name = NULL;
>+  if (d->policy)
>+    bus_client_policy_unref (d->policy);
>+  d->policy = NULL;
>+  return FALSE;
> }
> 
> const char *
>diff --git a/bus/connection.h b/bus/connection.h
>index 5099bcf..4f35216 100644
>--- a/bus/connection.h
>+++ b/bus/connection.h
>@@ -50,6 +50,7 @@ BusConnections* bus_connection_get_connections    (DBusConnection
> BusRegistry*    bus_connection_get_registry       (DBusConnection               *connection);
> BusActivation*  bus_connection_get_activation     (DBusConnection               *connection);
> BusMatchmaker*  bus_connection_get_matchmaker     (DBusConnection               *connection);
>+const char *    bus_connection_get_loginfo        (DBusConnection        *connection);
> BusSELinuxID*   bus_connection_get_selinux_id     (DBusConnection               *connection);
> dbus_bool_t     bus_connections_check_limits      (BusConnections               *connections,
>                                                    DBusConnection               *requesting_completion,
>diff --git a/dbus/dbus-sysdeps-util-unix.c b/dbus/dbus-sysdeps-util-unix.c
>index 0062769..7f7b265 100644
>--- a/dbus/dbus-sysdeps-util-unix.c
>+++ b/dbus/dbus-sysdeps-util-unix.c
>@@ -1137,3 +1137,99 @@ _dbus_string_get_dirname  (const DBusString *filename,
> }
> /** @} */ /* DBusString stuff */
> 
>+static void
>+string_squash_nonprintable (DBusString *str)
>+{
>+  char *buf;
>+  int i, len; 
>+  
>+  buf = _dbus_string_get_data (str);
>+  len = _dbus_string_get_length (str);
>+  
>+  for (i = 0; i < len; i++)
>+    if (buf[i] == '\0')
>+      buf[i] = ' ';
>+    else if (buf[i] < 0x20 || buf[i] > 127)
>+      buf[i] = '?';
>+}
>+
>+/**
>+ * Get a printable string describing the command used to execute
>+ * the process with pid.  This string should only be used for
>+ * informative purposes such as logging; it may not be trusted.
>+ * 
>+ * The command is guaranteed to be printable ASCII and no longer
>+ * than max_len.
>+ * 
>+ * @param pid Process id
>+ * @param str Append command to this string
>+ * @param max_len Maximum length of returned command
>+ * @param error return location for errors
>+ * @returns #FALSE on error
>+ */
>+dbus_bool_t 
>+_dbus_command_for_pid (unsigned long  pid,
>+                       DBusString    *str,
>+                       int            max_len,
>+                       DBusError     *error)
>+{
>+  /* This is all Linux-specific for now */
>+  DBusString path;
>+  DBusString cmdline;
>+  int fd;
>+  
>+  if (!_dbus_string_init (&path)) 
>+    {
>+      _DBUS_SET_OOM (error);
>+      return FALSE;
>+    }
>+  
>+  if (!_dbus_string_init (&cmdline))
>+    {
>+      _DBUS_SET_OOM (error);
>+      _dbus_string_free (&path);
>+      return FALSE;
>+    }
>+  
>+  if (!_dbus_string_append_printf (&path, "/proc/%ld/cmdline", pid))
>+    goto oom;
>+  
>+  fd = open (_dbus_string_get_const_data (&path), O_RDONLY);
>+  if (fd < 0) 
>+    {
>+      dbus_set_error (error,
>+                      _dbus_error_from_errno (errno),
>+                      "Failed to open \"%s\": %s",
>+                      _dbus_string_get_const_data (&path),
>+                      _dbus_strerror (errno));
>+      goto fail;
>+    }
>+  
>+  if (!_dbus_read (fd, &cmdline, max_len))
>+    {
>+      dbus_set_error (error,
>+                      _dbus_error_from_errno (errno),
>+                      "Failed to read from \"%s\": %s",
>+                      _dbus_string_get_const_data (&path),
>+                      _dbus_strerror (errno));      
>+      goto fail;
>+    }
>+  
>+  if (!_dbus_close (fd, error))
>+    goto fail;
>+  
>+  string_squash_nonprintable (&cmdline);  
>+  
>+  if (!_dbus_string_copy (&cmdline, 0, str, _dbus_string_get_length (str)))
>+    goto oom;
>+  
>+  _dbus_string_free (&cmdline);  
>+  _dbus_string_free (&path);
>+  return TRUE;
>+oom:
>+  _DBUS_SET_OOM (error);
>+fail:
>+  _dbus_string_free (&cmdline);
>+  _dbus_string_free (&path);
>+  return FALSE;
>+}
>\ No newline at end of file
>diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h
>index 6c96699..b766f3f 100644
>--- a/dbus/dbus-sysdeps.h
>+++ b/dbus/dbus-sysdeps.h
>@@ -412,6 +412,11 @@ dbus_bool_t _dbus_write_pid_to_file_and_pipe (const DBusString *pidfile,
>                                               dbus_pid_t        pid_to_write,
>                                               DBusError        *error);
> 
>+dbus_bool_t _dbus_command_for_pid (unsigned long  pid,
>+                                   DBusString    *str,
>+                                   int            max_len,
>+                                   DBusError     *error);
>+
> /** A UNIX signal handler */
> typedef void (* DBusSignalHandler) (int sig);
>

Actions: View | Splinter Review

Attachments on bug 19141: 21250 | 21255

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.