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 26fdba4bc5a8880fd7630166c910cbf4d003517c..62d58f8e98b8f3aefd30e8675511cecb46bef015 100644 (file)
--- a/bus/bus.c
+++ b/bus/bus.c
@@ -1414,11 +1414,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 4aec1659938f3287f645294f15c997578f5c094c..88451ebbf3b0acddb4eb8e60028212c782defd11 100644 (file)
--- a/bus/bus.h
+++ b/bus/bus.h
@@ -117,6 +117,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 a95e1fad0852bea23b63518a81fe7fcc893df9a6..3643f0c61eb47d0f6b0b3370cb568683c68b6cb1 100644 (file)
--- a/bus/connection.c
+++ b/bus/connection.c
@@ -1586,6 +1586,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
@@ -1671,6 +1672,7 @@ bus_connections_reload_policy (BusConnections *connections,
 
       policy = bus_context_create_client_policy (connections->context,
                                                  connection,
+                                                 d->policy,
                                                  error);
       if (policy == NULL)
         {