Support for "daemon: dbus"?

jamesh · December 6, 2018, 12:15pm

Recently, I’ve been working on adding support for bus activated D-Bus services, and noticed that the existing systemd code supported dbus as a daemon type.

This generates a systemd .service file using Type=dbus, where the service is considered to have finished starting when it registers a particular well known name on the message bus. I imagine this is the mode most snapped D-Bus services would want to use.

However, Snapcraft complains that dbus is not a valid value for the daemon attribute on an app. I was able to build a snap using passthrough with something like this:

apps:
  system:
    command: ...
    passthrough:
      daemon: dbus
      bus-name: io.snapcraft.SnapDbusService

This produced a working snap with the right mode in the systemd unit, but it looks like review-tools also trips up on dbus type daemons:

$ review-tools.snap-review test-snapd-dbus-service_1.0_amd64.snap 
Errors
------
...
 - lint-snap-v2:daemon:system
    invalid daemon: 'dbus'
Warnings
--------
 - lint-snap-v2:apps_unknown:system
    unknown fields for app 'system': 'bus-name'

Is this just an oversight, or is there a particular reason to discourage use of this daemon type?

jdstrand · December 7, 2018, 8:35pm

This used to be supported by the review-tools but almost two years ago it was removed because it was not properly supported in snapd. This is bug https://bugs.launchpad.net/snappy/+bug/1649399. If ‘daemon: dbus’ is now properly supported in snapd, happy to add it back to the review-tools.

jamesh · December 10, 2018, 6:36am

It certainly seems to be working in current snapd master, but I guess it is worth discussing whether it’s a syntax we want to keep and/or encourage the use of. As I’ve been working on D-Bus service activation, here are a few points to keep in mind:

A daemon must be able to acquire the bus name systemd is using to check that the daemon is active. If we’re only considering strict confinement, then this means the daemon must have a dbus slot matching the bus name. With classic snaps, it is quite possible for a daemon to register a name without a matching slot (although with installation of service activation files tied to the interface, they probably want one anyway).
If a daemon is an activatable dbus service, then the dbus daemon type is the most appropriate method to signal that it is active. If the daemon doesn’t claim the name then it is obviously broken, and once it does claim the name it needs to be able to serve requests from clients.
A daemon can potentially claim multiple bus names. It should be considered active only once it owns both names. In general, this means bus-name should be the last name it tries to claim.

As an example close to home, snap userd tries to register both io.snapcraft.Launcher and io.snapcraft.Settings. By inspecting the source code I can see that it registers io.snapcraft.Settings last, so that is the name systemd should be watching for.

For simple cases it might be possible to infer the details, but (3) requires knowledge of how the daemon initialises itself. So maybe the existing syntax is good enough and just needs to be supplemented with review-tools checks. The following might be a good start:

If a snap is using strict confinement and has an app with daemon: dbus, then bus-name must match one of the app’s dbus interface slots.
If an app is a system daemon, then it should not have any dbus interface slots that specify bus: session.
Once my user daemons PR is merged, any app that is a user daemon must not have any dbus interface slots that specify bus: system.
If an app has a dbus interface slot with activatable: true, and sets daemon: simple then issue a warning suggesting that they use daemon: dbus.

jdstrand · December 17, 2018, 7:55pm

IIRC, this was one of the sticking points. It was somewhat awkward that in the snap.yaml you had to declare two different things (daemon: dbus and the dbus slot, with attributes). There was some talk of trying to unify this, but the way that the backends are all rather independent of one another made this problematic. Conceptually, I have no problem and would encourage snapd to grow the (as described or equivalent) functionally of daemon: dbus. At this point, we probably just need to live with the fact that we need two different things declared, but snapcraft and the review-tools can check for this; your suggestions for the review-tools seem reasonable to me.

I suggest asking @mvo his thoughts (since iirc, he was part of the older discussion) as well as signoff from @pedronis and/or @niemeyer on the path forward.

pedronis · February 7, 2019, 9:21am

@jamesh @jdstrand there are two issues here:

“bus-name” was ported over (but never agreed upon) from pre slots/plugs snappy, it would be more natural for the “daemon: dbus” app entry to actually designate a “dbus-slot” now
we haven’t really deeply discussed what roles plugs/slots should play for classic snaps, while they relate deeply to confinement for things like dbus and because now we have interface hooks they cannot probably be just ignored

jamesh · February 11, 2019, 11:22am

Referencing a type: dbus slot rather than the bus name directly seems like a decent improvement: it would remove the repetition of the bus name, and tie in closer to the interfaces system.

Re. dbus plugs/slots and classic snaps, I think you’re right that they’re probably already needed. For example, if you wanted a strict confined client app to talk to a classic confined D-Bus service, you need a slot on the classic snap for the strict snap to connect to, otherwise the policy for the strict snap will block communication. So that’s another point towards depending on slot definitions.

As a simple extension, we could infer the value of dbus-slot if an app has only a single dbus type slot. This way it would only be needed in cases where there are multiple slots (a situation where we can’t determine the correct value without code inspection).

jdstrand · February 12, 2019, 8:27pm

To be clear, you aren’t talking about the dbus interface are you? It is quite simplistic and doesn’t cover “all things dbus” (esp wrt dbus bus policy). If we wanted to change this interface, yes, we would need to deeply think about and design what the improvements should be to the dbus interface so that on the whole it is coherent (even if immediately we don’t implement everything). If you are talking about something else, that’s potentially fine.

This is a complicated topic in part because classic snaps run under a security policy (which is wide open), so we need to be careful not to accidentally break interactions between snaps. The most complicated bit is the mount namespace for things like the content interface since with classic, they intentionally run in the global mount namespace.

pedronis · February 12, 2019, 8:31pm

I’m talking about this (I hoped it was clear):

apps:
  system:
    command: ...
    passthrough:
      daemon: dbus
      bus-name: io.snapcraft.SnapDbusService

jdstrand · February 12, 2019, 9:41pm

This is missing configuring the dbus bus policy for the snap and therefore this would be incomplete for arbitrary snaps. For systems where the default dbus bus policy is default closed, then the lack of snapd providing a policy means the default bus policy would block its use. On systems where the default dbus policy is default open, then the lack of snapd providing a policy means the default policy would allow its use to all users, when it is likely desired for system services that that the bus policy be locked down to, for example, root.

The dbus interface does provide a simplistic bus policy (root is allowed but all others users are not) that is not currently configurable. I had always hoped that specifying the dbus interface slot would somehow allow opting into systemd type=dbus and BusName=... but we don’t yet since, aiui, there wasn’t a convenient way for an interface slot to influence the systemd backend. IIRC, we aren’t allowing bus-name now precisely because it introduces an awkwardness in the snap.yaml of having to specify bus-name in the command and name in the dbus slot.

That said, if we implement what I always hoped for, that is likely only reasonable for Core and not classic, since I suspect we’ll need more flexibility with defining the dbus bus policy on classic systems.

pedronis · February 12, 2019, 10:07pm

I fear we are bit talking at cross purposes here, what I said is that we shouldn’t specify a “bus-name” but a slot (with dbus interface) with the daemon. So

apps:
  system:
    command: ...
    daemon: dbus
    dbus-slot: my-service-dbus-slot

jdstrand · April 7, 2020, 1:58pm

@jamesh and @pedronis - can we approve the full syntax in one spot? AIUI, it is:

slots:
  my-system-dbus-slot:
    interface: dbus
    bus: system
    name: com.example.system
  my-session-dbus-slot:
    interface: dbus
    bus: session
    name: com.example.session

apps:
  test-system:
    daemon: dbus           # or any existing (eg, 'simple')
    activates-on:
    - my-system-dbus-slot
    slots:
    - my-system-dbus-slot
  test-session:
    # daemon-scope: user   # eventually with PR 5822
    # daemon: ???
    activates-on:
    - my-session-dbus-slot
    slots:
    - my-session-dbus-slot

With the exception of the https://github.com/snapcore/snapd/pull/5822 bits, this is my understanding of what https://github.com/snapcore/snapd/pull/6258 is trying to achieve.

UPDATE: note, https://github.com/snapcore/snapd/pull/6258 also removes ‘bus-name’ in favor of ‘activates-on’; please also comment on if bus-name support is supposed to be removed. If so, we should query the store for occurrences of bus-name.

jamesh · April 8, 2020, 8:13am

@pedronis was in favour of removing bus-name, so that is what the PR currently does. The main reasons are:

daemon: dbus is primarily useful for D-Bus activated services.
For D-Bus activated services, we can determine the bus name(s) the daemon will acquire via activates-on.
It removes the possibility that we’ll write out a systemd service file that references a bus name the daemon is blocked from acquiring via AppArmor.

jdstrand · April 21, 2020, 10:08pm

@pedronis - can you comment? @jamesh didn’t correct my understanding so I think we just need you now.

jamesh · April 22, 2020, 4:12am

I think what you’ve written above is pretty close to what we want. I do want to restrict dbus activation to daemons, which I was planning as a follow up after the two existing PRs were merged (which are both already close to being too large).

I think the main review-tools validation checks that would make sense for this eventual version are:

only allow activates-on for apps where daemon is not empty.
if an app has activates-on, it must only list slots attached to the app in question.
all slots listed in an activates-on property must be of interface type dbus.
a dbus slot can be listed in the activates-on property of at most one app in the snap.

And not directly related to activation:

if a dbus slot is associated with a daemon, the bus must match the daemon scope. That is, daemon-scope: system (the default) corresponds to bus: system, and daemon-scope: user corresponds to bus: session.

It’s possible that (5) might catch some existing snaps, but it isn’t clear how a session bus slot could be useful to a system daemon.

jdstrand · April 22, 2020, 12:30pm

I’ve added this to trello. Thanks!

pedronis · May 6, 2020, 3:49pm

I looked, there is nothing in the store with a stable release that uses bus-name.

pedronis · May 6, 2020, 3:54pm

The main concerns/open questions about that syntax, that I still have, are:

the repetition in activates-on vs slots
will we to support activating on something that is not slot?

rpjday · February 17, 2022, 10:41am

What is the current state of “dbus” type daemons? I ask since the Snapcraft 101 course covers daemons, and it’s not clear if the dbus type should be mentioned if there is no current application of it. Thoughts?