Integrate snapd-xdg-open into snapd repository

I was thinking about a similar approach before but for me this looks really hacky and relies on a lot things which we may don’t have cross-distribution.

yes, it is definitely a lot less clean than simply shipping a dbus listener like we do now …

@mvo Thanks a lot for putting that in place! That’s pretty small for being impossible. :grin:

@morphis By all means please feel free to improve on the basic idea provided there, and perhaps @robert.ancell and others more familiar than us on those details will have ideas. We’re all for making it cleaner and simpler. The point here is that this may be done once, inside snapd itself, and then have it working everywhere without requiring every single distribution to not only ship that additional daemon but ensure that it’s installed right next to snapd itself so that people can do something as simple as click on a link. That has been proven to not go well already, so I’m not excited to just bang harder on the same broken door.

seeing the proposal i still wonder what is wrong with the current implementation (which works along freedesktop standards already), apart from the fact that we did not package it into snapd in the first place.

to forward such calls into a session using dbus with a listener is a standard. we do not use any daemon but a forwarder service that is only invoked when an xdg-open request comes in on the bus and only then forwards the request to the actual xdg-open service binary. it uses the implemented and established policykit desktop standards for securing the whole operation and generally works well already except for the fact that the listener is not included in the right package today.

fishing for random environment via logind calls and circumventing default security mechanisnms instead does not seem to be a good replacement IMHO. cant we keep the dbus implementation (the listener .service file is minimal) and simply improve the service binary ?

1 Like

@ogra The rationale was presented above a few times already. Simply asking why again, without addressing the points made, means I’d have to just restate what was already said.

This is also not “circumventing security mechanisms”. If you have real security worries about what is being proposed, then please be specific about how this is a security threat in any sense.

polkit is a default mechanism for admins to deny/allow what their users can access on desktops. by completely working around the policy kit and dbus implementations that are freedesktop standards for such things, the mechanism will not work for snaps … i.e. an enterprise admin denying all xdg-open calls for certain urls or whatnot will surely be surprised that snaps will simply ignore such default mechanisms.

i think the fd.o specifications exist for a reason, and i think working around them with a hack will not really serve us well but will again cause complaints about canonical suffering from NIH.

IMHO we should make sure to integrate with existing technologies accepted by all desktops (even openbox obeys to dbus/polkit policies), which the current implementation does already.

At the end of the strawman by mvo provided above, it’s xdg-open itself that is called. If xdg-open doesn’t work for whatever reason, it will not work for snaps either. If it works, it will work for snaps too, and that sounds pretty good.

in the strawman xdg-open is called directly not using dbus and logind/policykit anymore

Yes, when xdg-open is called, what happens is exactly what happens when xdg-open is called. That’s the goal.

well, indeed xdg-open is what is called in the end in either implementation. the question is rather, do we want to be good citizens and integrate with established standards on the way towards this or do we want to hack around all these established mechanisms that all desktops agree on, pass session info around through the system where it does not belong, just to enforce our own very hackish way to achieve the very same.

the result will in either case be that “xdg-open $URL” is executed indeed (once by dbus proper on demand when the request comes in and once directly by passing info around that should most likely not be passed around to non-session services)

Just to answer to the specific part about polkit - our current snapd-xdg-open code does not use polkit, so the mechanism you describe (enterprise admin) would not work today. There could be code added for that of course but its not there today.

@ogra Again, if you want to contribute to the conversation, you’ll need to provide more details of what specifically is wrong and why, and preferably how to improve Michael’s suggestion so we have a simple implementation. If you can’t, please stop so the rest of us can do that.

@mvo Hmm… can we simply ship the session ID directly from snapctl to snapd? We do have access to the environment there, right?

Note there is a method in logind to determine the session for a given PID (this uses cgroups internally):

$ dbus-send --system --dest=org.freedesktop.login1 --print-reply /org/freedesktop/login1 org.freedesktop.login1.Manager.GetSessionByPID uint32:0

Replace the 0 PID with the PID of the process you are interested in (0 means use the PID of the connecting process).

The bad news is no matter what PID I send it doesn’t seem to find any session. So not sure if this doesn’t work on Ubuntu or is broken?!

The more bad news is even once you know session I’m still not convinced you can / should be able to contact the session bus from a root daemon [1]. That link indicates that the it might not be allowed by d-bus. Also I’m not sure if there is a reliable method of finding where the bus socket is. It happens to be /run/user/<UID>/bus, but that might just be by convention.

[1] http://stackoverflow.com/questions/6496847/access-another-users-d-bus-session

This still doesn’t put away that this is a hack :slight_smile:

I’ve said already above that something like this would be possible but not to be preferred in my mind. The main thing I don’t like is that we’re crossing layers here from the bottom to the top. Todays Linux desktop systems are build in a way that nothing calls into directly into a user session instance. See the references I gave above. In every case where such a bottom-up call is needed an agent-style like approach is chosen to avoid the direct call and delegate the execution to something inside the user-session. The key principle here is that the user session has the responsibility to decide if it wants to register a handler or not. We move this decision now into the system part.

The example @mvo gave is quite nice as it shows what is possible with todays systems but it has a fundamental problem. If the URI we hand to xdg-open requires a program to start which is either not able to accept open requests while already running or doesn’t run yet inside the user session the new process spawned by systemd-run is placed incorrect in the process hierarchy of the system. I did a quick experiment:

# Get out of my current session and into a new one
$ ssh simon@localhost
# Get a root shell without any environment leaking from the user
localhost$ env -i sudo -i
localhost$ systemd-run --scope --uid=1000 --setenv=DISPLAY=:0 --setenv=XAUTHORITY=/home/simon/.Xauthority /usr/bin/xdg-open https://forum.snapcraft.io

That opens up the browser running with the specified user (uid 1000) and shows the webpage. However the process hierarchy now looks like this:

systemd─┬
        ├─sshd───sshd───sshd───zsh───sudo───bash───firefox─┬─{Cache I/O}
        │                                                  ├─{Cache2 I/O}
        │                                                  ├─{Compositor}
[...]

That isn’t really what we want. Using something like

localhost$ systemd-run --slice user-1000.slice --uid=1000 --setenv=DISPLAY=:0 --setenv=XAUTHORITY=/home/simon/.Xauthority /usr/bin/xdg-open https://forum.snapcraft.io

helps a bit but still leaves firefox as a child of system systemd process.

systemd─┬─ModemManager─┬─{gdbus}
        │              └─{gmain}
        ├─NetworkManager─┬─dhclient
        │                ├─dnsmasq
        │                ├─{gdbus}
        │                └─{gmain}
        ├─accounts-daemon─┬─{gdbus}
        │                 └─{gmain}
        ├─acpid
        ├─2*[agetty]
        ├─apt-cacher-ng───2*[{apt-cacher-ng}]
        ├─atd
        ├─avahi-daemon───avahi-daemon
        ├─bluetoothd
        ├─canonical-livep───14*[{canonical-livep}]
        ├─cgmanager
        ├─colord─┬─{gdbus}
        │        └─{gmain}
        ├─cron
        ├─cups-browsed─┬─{gdbus}
        │              └─{gmain}
        ├─cupsd───2*[dbus]
        ├─dbus-daemon
        ├─2*[dnsmasq]
        ├─dockerd─┬─containerd───8*[{containerd}]
        │         └─14*[{dockerd}]
        ├─firefox─┬─{Cache I/O}
        │         ├─{Cache2 I/O}
        │         ├─{Compositor}
[...]
        ├─lightdm─┬─Xorg
        │         ├─lightdm─┬─upstart─┬─thunderbird
[...]

In the process tree we really want to have the launched process below lightdm──upstart (Ubuntu 16.04).

Also this behavior is expected as described in the man page of systemd-run:

If a command is run as transient service unit, it will be started and managed by the service manager like any
other service, and thus shows up in the output of systemctl list-units like any other unit. It will run in a
clean and detached execution environment, with the service manager as its parent process. In this mode,
systemd-run will start the service asynchronously in the background and return after the command has begun
execution.

The --user option for systemd-run can’t be used as not all systems we support use systemd --user to manage user sessions (Ubuntu 16.04 being the best example).

So using systemd-run is not a real option to solve the problem we’re discussing here. As described before we need something inside the user session, below the user session init manager in the process tree which can start a process at the right place in the tree. That is what snapd-xdg-open did or a snapd --user would do. As far as I know there is no way to inject a process into the tree from the outside and if that exists it would be a much more uglier hack.

The other option would be to add support for multiple init systems used for user sessions (upstart, system, …) into snapd but I don’t think that is a real option as we defended adding support for upstart before for system services.

I hope this describes my concerns a bit better to help driving the discussion towards an agreement.

1 Like

AFAIK it does, the receiving dbus session daemon runs under polkit giving the admin full control over the bus via system and user policies.

I looked at the code in https://github.com/snapcore/snapd-xdg-open/blob/master/src/snapd-xdg-open.c - am I looking into the wrong place?

the session dbus bus itself is started inside a logind session which makes its traffic manageable by the policykit running in the same session …

@ogra I’m not sure we are talking about the same thing. Can you point me to an example how https://github.com/snapcore/snapd-xdg-open/blob/master/src/snapd-xdg-open.c could be restricted using polkit? Polkit requires that users of polkit declare “actions” in the polkit xml format and that the application code checks those. Some examples for this would be /usr/share/polkit-1/actions/com.canonical.controlcenter.datetime.policy. Those can be overriden via /etc/polkit-1/localauthority.conf.d. I’m curious to learn how this works just from inside the session dbus. Any pointers appreciated.

snapd-xdg-open as it is today was developed by the desktop team (see the commit log on github) with the required knowledge around desktop standards in mind. the current implementation follows expected established fd.o standards to spawn processes in running sessions that all desktops from openbox to KDE support on all distros.

the proposal is simply neglecting that these standards exist, uses a hack to fish out info from /proc environment from processes running in this particular session and makes us break with the expected standards.

@robert.ancell as well as @morphis have brought up enough technical info in their last posts why this is a bad idea. my remaining main concern here is the NIH attitude to not go with the expected standard and the assumption that the desktop team did not pick the most proper implementation in the first place when designing this.

but since you prefer this, i’ll just shut up now … go with whatever fits you best.

likewise you can put a “com.canonical.SafeLauncher.policy” file in place as a system admin to manage any communication to the shipped com.canonical.SafeLauncher.service file that is in the snapd-xdg-open package at https://github.com/snapcore/snapd-xdg-open/blob/master/data/Makefile.am