Avoid dbus connection in strict sandbox mode #320

Closed sebastinas closed 5 months ago

On GitLab by @valoq on Apr 8, 2022, 23:36

With 'set dbus-service false' in zathurarc the dbus service should not be used.

However, even with this setting set to false, we have an open file descriptor to the dbus socket in '/run/user/UID/bus'

The setting disables the SyncTeX forwarding but seems to still initialize the dbus service connection.

This is relevant when using the strict sandbox setting, as the open socket connection to dbus is an easy way of sandbox escape. Since we dont need this connection when the dbus-service is set to false (or when using the strict sandbox) we shoudl be able to avoid initializing this connection.

Im not quite sure where this initialization happens though

That's a Dbus connection by GTK that we do not control.

On GitLab by @valoq on Apr 11, 2022, 23:04

Is this not the same socket that zathura used for ipc?

Im currently playing around with g_dbus_connection_close[1] though without the intended result so far.

We should be able to close the socket from the same process, right?

[1] https://developer-old.gnome.org/gio/unstable/GDBusConnection.html#g-dbus-connection-close

I don't believe so. Also, closing GTK's dbus connection behind its back sounds like a terrible idea.

On GitLab by @valoq on Apr 17, 2022, 18:53

After looking into this, it seems there is no intended way for GTK applictions to disable the DBus initialisation, though we can still block the access in our sandbox.

On GitLab by @valoq on Apr 17, 2022, 19:27

Aside from more complex sandbox solution that block the file path to the DBus socket, one way to prevent the socket connection is to manipulate DBUS_SESSION_BUS_ADDRESS

However this would have to be done before gtk_init() is executed and I dont see how we could read the zathurarc config before that.

@sebastinas Do you see any way to do this by reading the sandbox setting in zathurarc before gtk_init() ?

On GitLab by @valoq on Dec 14, 2022, 20:33

After investigating this for a while, it turns out that the dbus connection that is initiated by gtk_init is actually the same that connection that is later used for zathura dbus-service feature.

Since we cannot read the configuration to block the connection before initiation, changing the socket connection during zathura_init is the alternative. Fortunately, we already have most of the code required in zathura/dbus-interface.c

Looking through the gtk documentation, manipulating the dbus connection initiated by gtk appears to be supported (and expected) and we could safely close the dbus socket. As a result, some gtk features like changing window decoration during runtime won't work anymore. This functionality would be expected to work for general use, but we can sacrifice it in strict sandbox mode. As a result, the process would be effectively sandboxed so that any malicious code that gained code execution as a result of a parsing error (most likely in poppler, mupdf or one of the media libraries), would need a kernel exploit for one of the few remaining system calls in order to escape the isolated process environment and do actual damage.

I'm still testing this to make sure there are no unexpected consequences, but if anyone has comments on this approach or knows additional conditions to consider, please add them here.

On GitLab by @valoq on Dec 22, 2022, 20:16

While closing the dbus connection is possible, it seems to be unreliable, especially when accessibility services are available as well.

The code below is just for documentation, and in case someone knows why it is unreliable.

// zathura_dbus_close() is called during zathura_init

void zathura_dbus_close()
{
  /* unreliable maybe because it receives some dbus? maybe wayland socket? maybe at bus?*/
  girara_debug("Closing dbus connection for sandbox");
  g_bus_get(G_BUS_TYPE_SESSION, NULL, (GAsyncReadyCallback) bus_connection_received, NULL);
}

/* GAsyncReadyCallback function */
void
bus_connection_received (GDBusConnection* connectionx, GAsyncResult *res, gpointer user_data)
{
  g_dbus_connection_close(g_bus_get_finish(res, NULL), NULL, (GAsyncReadyCallback) bus_connection_closed , NULL);
}

/* GAsyncReadyCallback function */
void
bus_connection_closed (GDBusConnection* connection, GAsyncResult *res, gpointer user_data)
{
  if (g_dbus_connection_close_finish(connection, res, NULL)){
    girara_debug("close successful");
  }
  else{
    girara_debug("close failed");
  }
}

On GitLab by @valoq on Dec 22, 2022, 20:21

Instead of closing the connection, a more reliable and clean solution is to prevent the initiation of dbus during gtk_init, which requires reading the configuration option for the sandbox mode.

Unfortunately, the configuration is read and saved by girara, which requires a ui session that depends on gtk_init.

Adding a simplified configuration check in girara, without saving the configuration and therefore without the need for gtk_init would solve this and possibly also be useful for other use cases.

On GitLab by @valoq on Dec 23, 2022, 20:22

mentioned in merge request girara!8

On GitLab by @valoq on Jan 5, 2023, 22:31

According to the DBus developers, setting DBUS_SESSION_BUS_ADDRESS=disabled: is a reliable solution to this problem and this is also used by chromium to avoid this connection. https://gitlab.freedesktop.org/dbus/dbus/-/issues/429

As soon as !8 is merged to allow an early sandbox config check, we can use the code below to prevent the socket connection in strict sandbox mode before gtk_init

#ifdef WITH_SECCOMP
  if (config_check_setting(config_dir, "sandbox", "strict")) {
    girara_debug("Strict sandbox setting detected, disableing IPC services");
    /* Prevent default gtk dbus connection */
    g_setenv("DBUS_SESSION_BUS_ADDRESS", "disabled:", TRUE);
  }
#endif

I think this goes beyond the scope of zathura. If users want to disable the dbus connection, they should run the command with DBUS_SESSION_BUS_ADDRESS=disabled:.

pwmt / zathura

Avoid dbus connection in strict sandbox mode #320