@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 
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.
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
@ogra I added the file now (http://paste.ubuntu.com/24420390/). There appears to be no filtering happening, am I using this wrong? I still suspect this needs application support but I’m happy to be convinced otherwise.
no, i would expect this to work (and i know we used PK like this a lot on phones, even with just scripts as backends, that had no particular extra integration). perhaps someone from the desktop team can chime in here.
Note that polkit is not a wrapper. It is a mechanism that a service is deliberately coded to use so that the service may make decisions based on polkit’s response. In other words, services must be designed to use polkit: the methods they care to protect all call out to polkit to ask if it is ok, polkit responds yes or no based on the policy defined by the application or overridden by the admin, and then the service makes a decision based on that response (eg, allowing the action or telling the client it was not allowed by polkit). Polkit is used to protect privileged resources that are running on the system bus (see ‘man polkit’) and indeed, there is no org.freedesktop.PolicyKit1 service running on the session bus for session services to use (you can verify in /usr/share/dbus-1/services). xdg-open runs in the user’s session and traditionally anything in the user’s session is trusted. I’m not aware of any polkit integration with xdg-open (and I spot checked the code).
As for the phone, polkit was crippled in that while it was present, it only supported enforcement and didn’t support prompting. I suspect what was used in shell scripts on the phone was pkexec which allows running the command as another user (eg, root) and it is possible to say ‘pkexec /usr/bin/foo’ and ‘pkexec /usr/bin/bar’ and have different policy for authorizing use of foo and bar. See ‘man pkexec’ for details.
Thanks @jdstrand! This is my understanding as well how polkit works. Happy to see this clarified and that we can put the polkit discussion at rest.
I really don’t see what’s wrong with our current approach for XDG stuff. The only thing I have a problem with is the name of the D-Bus path being com.canonical.* instead of being like everything else in using io.snapcraft.*.
I’m extremely annoyed by the cavalier attitude to all the desktop stuff. Yes, I get that snaps weren’t designed for the desktop case, but now that we have desktop apps on there, playing nice with users’ (and developers’) expectations should be something that we should strive for.
It took far too many years to get most (if not all) desktops to roughly conform to XDG specifications, and now that we’re here, we should leverage these things. The Portals concept from Flatpak offers a similar mechanism to what snapd-xdg-open does for desktop integration bits, and I fully expect to see more of this kind of stuff rather than less in the Snappy ecosystem.
Can we just skip all the bickering and go to fixing up snapd-xdg-open? The desktop people all basically agree that it’s sound, I certainly see no problems with the approach, and as far as I can tell, the only objection is that it’s not written in monster-Go to be part of the hydra that is the snapd repository.
Per the discussion above, we were exploring options with the goal of making it easier to implement it evenly and simply across all distributions, because our current approach seems to have failed to do exactly that so far. In different words: the reason this discussion even started is is precisely because the method “all desktop people agree” is still broken despite being available for a long time.
That said, this was an exploration. We haven’t found any mechanisms so far that wouldn’t open up different cans of worms. The feedback we got from several ends saying “let’s please keep the current method” is also part of that process and being heard. So, sure: what do you suggest we do next to fix up the current mechanism? If the idea is integrating the tool into the snapd repository, I’d like to have something slightly more generic. Some “snapd-foo-bar” that may do more than simply opening links in the future, as we don’t want a new tool for every trivial need like this. Suggestions? “snapd-user-helper”?
Quick question, is there any issue apart from packaging? I wasn’t aware of any bugs in that code.
No that I know. snapd-xdg-open as is should work.
I would suggest a two step approach:
com.canonical.* to io.snapcraft.*
snapd-xdg-open itself so it integrates better with the rest of the code. My proposal would be a snapd --user switch which allows us to start a second snapd instance within a different context which will take care of different things. For this step this would only mean to exactly replicate what snapd-xdg-open did and nothing else. In the future we can add additional xdg DBus interfaces or any other user session related bits in here. This can be either a dbus-activated service or one started by the user session init system which is then abstracted through proper packaging but systemd/upstart should fit almost everything.@niemeyer If that sounds good to you and also for everyone else I can come with a more detail proposal / PR for 2 once I am through other bits of my work list. 1. is something we can do very easily today and bring snapd-xdg-open back into all distributions with very low cost for packaging and a good integration story.
I’m curious about this. Do you plan to keep the current C code around or really spawn a 2nd copy of snapd that has (perhaps) different APIs (e.g. to expose DBus services).
I would vote for a second snapd instance which doesn’t load/provide anything of what the system instance does. It will load a different mode which provides for now just the dbus endpoint snapd-xdg-open provided before. Over time we can grow that in different other things we may want to introduce for the user session (like the xdg-desktop portals @sitter proposed here). If the implementation is in C or Go, I don’t really care but giving the nature of the existing snapd code base Go sounds the way to go.