pwmt / zathura

Document viewer
https://pwmt.org/projects/zathura
zlib License
1.88k stars 129 forks source link

prevent default GTK dbus connection - [closed] #527

Closed sebastinas closed 2 months ago

sebastinas commented 1 year ago

On GitLab by @valoq on Dec 10, 2022, 24:31


Merges master -> develop

Ok this one is a bit tricky:

GTK by default initiates a connection to dbus which cannot be disabled. However, GTK devs mentioned that it is not an issue to prevent the connection, either by blocking it (like flatpak does) or when there is no dbus session available. https://gitlab.gnome.org/GNOME/gtk/-/issues/4842

As far as I can see, zathura does not use any of the functionality provided by the native GTK dbus interface.

Why would we block it?

Basically to allow an effective sandbox isolation. Currently, we have a file descriptor to dbus open before the seccomp filter can do anything about it and that means that anyone gaining code execution, even in strict sandbox mode, can use said file descriptor for sandbox escape through dbus. The only way to prevent this is to block the connection when gtk_init() is run, which is currently also before the configuration file is read.

I know this looks like a dirty hack at first glance and I'm not really convinced myself about this solution but since GTK does not provide any other option to disable this but also allows blocking the connection attempt, this isn't completely crazy (I hope...) So far I have not noticed any regressions and unless someone find any, this could work.

If necessary, this change could be made dependent on the sandbox mode, however: Since the configuration is read after gtk_init, this would require a rewrite of init_zathura or the sandbox options would need to be changed into a command line argument. Before that I would like some feedback on this approach in general.

The seccomp filter in zathura has become quite mature in my opinion and this dbus socket is the last issue I know of that makes this incomplete (when run on wayland), which is why I would like to solve it.

sebastinas commented 1 year ago

On GitLab by @valoq on Dec 11, 2022, 17:30


added 1 commit

Compare with previous version

sebastinas commented 1 year ago

Besides the fact that the patch relies on under-specified behavior (foo = getenv(...), setenv(...), setenv(foo) is undefined), I'd prefer if people caring about that unset the env variable before launching zathura. I'm not interested in debugging bugs that we introduce by messing with Gtk internals.

sebastinas commented 1 year ago

On GitLab by @valoq on Dec 12, 2022, 01:21


By undefined, are you referring to setenv being unsupported on non POSIX systems?

Since the whole sandbox feature only works on Linux, this should not be an issue if we add a conditional check?

I have been looking into how to rework this feature and consequently I will take care of any issues resulting from it. To minimize the impact, this can be made dependent on a strict sandbox check as well. Since the isolation of dbus is not an issue according to the GTK project, I hope it will be acceptable to keep looking for ways to block this connection, at least in strict sandbox mode.

sebastinas commented 1 year ago

On GitLab by @valoq on Dec 14, 2022, 16:49


Nevermind, we need the connection after all.

Changing the GTK Theme during runtime uses the dbus connection as well and if I'm not mistaken, we actually reuse the dbus connection initiated by gtk for the dbus-service feature as well.

The only way to block this would be in case the sandbox is set to strict, but we have not yet read the configuration file at this point and cant do so until after gtk_init since it is a girara function.

sebastinas commented 1 year ago

On GitLab by @valoq on Dec 14, 2022, 16:49


resolved all threads