bus: When failing to reload client policy, continue iteration

If we have a large number of connections to the bus, and we fail to
reload the policy for one of them (perhaps because its uid no longer
exists in the system user database), previously we would crash, which
is obviously unintended. After the previous commit, we would stop
iteration through the list of client connections, which doesn't seem
great either: one bad connection shouldn't prevent us from reloading
the rest of our state.

Instead, let's distinguish between new connections (where we want
failure to establish a security policy to be fatal), and pre-existing
connections (where the current security policy is presumably good
enough to keep using if we have nothing better). If we're unable to
reload the policy for a pre-existing connection, log a warning and
carry on iterating.

Helps: https://gitlab.freedesktop.org/dbus/dbus/-/issues/343
Signed-off-by: Simon McVittie <smcv@collabora.com>

diff --git a/bus/bus.c b/bus/bus.c
index 9003860e219be446b6e808d852319e84cc8232e4..11de90dd759a52eb710b81c515632b26921429ff 100644 (file)
--- a/bus/bus.c
+++ b/bus/bus.c
@@ -1412,11 +1412,42 @@ bus_context_get_containers (BusContext *context)
 BusClientPolicy*
 bus_context_create_client_policy (BusContext      *context,
                                   DBusConnection  *connection,
+                                  BusClientPolicy *previous,
                                   DBusError       *error)
 {
+  BusClientPolicy *client;
+  DBusError local_error = DBUS_ERROR_INIT;
+  const char *conn;
+  const char *loginfo;
+
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
-  return bus_policy_create_client_policy (context->policy, connection,
-                                          error);
+
+  client = bus_policy_create_client_policy (context->policy, connection,
+                                            &local_error);
+
+  /* On success, use new policy */
+  if (client != NULL)
+    return client;
+
+  /* On failure while setting up a new connection, fail */
+  if (previous == NULL)
+    {
+      dbus_move_error (&local_error, error);
+      return NULL;
+    }
+
+  /* On failure while reloading, keep the previous policy */
+  conn = bus_connection_get_name (connection);
+  loginfo = bus_connection_get_loginfo (connection);
+
+  if (conn == NULL)
+    conn = "(inactive)";
+
+  bus_context_log (context, DBUS_SYSTEM_LOG_WARNING,
+                   "Unable to reload policy for connection \"%s\" (%s), "
+                   "keeping current policy: %s",
+                   conn, loginfo, local_error.message);
+  return bus_client_policy_ref (previous);
 }
 
 int

diff --git a/bus/bus.h b/bus/bus.h
index b44111809f4d3d13c3aa45c2021bd4be62e975f7..bc8b42c3cbe766e705184d9387f9bc71944486e8 100644 (file)
--- a/bus/bus.h
+++ b/bus/bus.h
@@ -115,6 +115,7 @@ BusContainers    *bus_context_get_containers                     (BusContext
 
 BusClientPolicy*  bus_context_create_client_policy               (BusContext       *context,
                                                                   DBusConnection   *connection,
+                                                                  BusClientPolicy  *previous,
                                                                   DBusError        *error);
 int               bus_context_get_activation_timeout             (BusContext       *context);
 int               bus_context_get_auth_timeout                   (BusContext       *context);

diff --git a/bus/connection.c b/bus/connection.c
index 0e1616d6d4623204d2b10fb83b26f014a8b64a40..b912d89e97a9090ae0311eb74cd3d44d340d9816 100644 (file)
--- a/bus/connection.c
+++ b/bus/connection.c
@@ -1584,6 +1584,7 @@ bus_connection_complete (DBusConnection   *connection,
 
   d->policy = bus_context_create_client_policy (d->connections->context,
                                                 connection,
+                                                NULL,
                                                 error);
 
   /* we may have a NULL policy on OOM or error getting list of
@@ -1669,6 +1670,7 @@ bus_connections_reload_policy (BusConnections *connections,
 
       policy = bus_context_create_client_policy (connections->context,
                                                  connection,
+                                                 d->policy,
                                                  error);
       if (policy == NULL)
         {