search

bus1 / dbus-broker

Linux D-Bus Message Broker
https://github.com/bus1/dbus-broker/wiki
Apache License 2.0
685 stars 84 forks source link

Transient Unit name for applications #302

Open swick opened 1 year ago

swick commented 1 year ago

Dbus activated processes currently live in transient units with the scheme dbus-<unique>-<ServiceName>@<instance>.service. The systemd Desktop Environments Integration specification says that applications should be in service or scopes which follow the following scheme app[-<launcher>]-<ApplicationID>[@<RANDOM>].service.

xdg-desktop-portal relies on this convention to derive the app id of a calling process (when the application is not authenticated by a sandboxing specific code path). This however breaks for apps started by dbus-broker. It would be nice if it followed the convention.

Can we change the schema to app-dbus-<ServiceName>@<unique><instance>.service? Not all services are applications. Should we change the schema only if the service has a corresponding desktop file?

dvdhrm commented 1 year ago

Sounds good! I created a PR to implement this. I added a dash between <unique> and <instance>, but didn't check whether systemd actually allows this in the template argument. Anyway, please let me know if that PR looks good!

Btw., we should document that this becomes API and should not be changed in the future.

smcv commented 1 year ago

Am I right in thinking that for services with SystemdService=foo.service, dbus-broker will start the systemd service named foo.service (the same as dbus-daemon does), with transient units only used for services that lack a SystemdService (as dbus-broker's equivalent of dbus-daemon's "traditional activation" in a child process)?

If that's true, then xdg-desktop-portal will not always be able to rely on this convention anyway, both for dbus-broker and for dbus-daemon, because foo.service doesn't indicate an app ID.

dvdhrm commented 1 year ago

Correct. I would mostly see this as an effort to make the naming scheme of dbus-broker API, and then stick to the spec as closely as possible.

smcv commented 1 year ago

I would mostly see this as an effort to make the naming scheme of dbus-broker API

Wouldn't that mean that adding a SystemdService=fooapp.service to an existing session service would be a functional regression on dbus-broker systems, because consumers of this "API" would no longer be able to identify its app-ID?

Something like this:

I think the ideal thing might be if there was a way for xdg-desktop-portal to inspect fooapp.service and find out its app-ID from metadata, which could work equally well for dbus-broker and dbus-daemon? And then dbus-broker could put that same metadata on its transient units too?

smcv commented 1 year ago

now add SystemdService=fooapp.service

Or is the intention that this is deprecated, and if anything with an app-ID is installing a user-service, it should be named like /usr/lib/systemd/user/app-com.example.FooApp.service and therefore SystemdService=app-com.example.FooApp.service?

If that is the intention, then it doesn't seem to have stuck yet. At the moment, on my Debian testing/unstable system, I have:

swick commented 1 year ago

If that's true, then xdg-desktop-portal will not always be able to rely on this convention anyway, both for dbus-broker and for dbus-daemon, because foo.service doesn't indicate an app ID.

True, we already don't hard rely on this. Users can also start binaries directly from their command line etc.

I don't think anyone disagrees that we should still try to make applications start in services/slices which adhere to this convention whenever possible.

I think the ideal thing might be if there was a way for xdg-desktop-portal to inspect fooapp.service and find out its app-ID from metadata, which could work equally well for dbus-broker and dbus-daemon? And then dbus-broker could put that same metadata on its transient units too?

This doesn't solve the issue with systemd services, does it? Even if the dbus service file contains the appid, if the systemd service name will always be whatever the systemd service file is.

This might still be a good idea. It makes sure only apps end up with the "app-" prefix and not random services which don't happen do have a corresponding systemd service. On the other hand this opt-in requires the app developers to adjust to it. Just looking for a corresponding desktop file doesn't have those issues (but requires the .service and .desktop files to be named to same).

Or is the intention that this is deprecated, and if anything with an app-ID is installing a user-service, it should be named like /usr/lib/systemd/user/app-com.example.FooApp.service and therefore SystemdService=app-com.example.FooApp.service?

Isn't this basically already the case according to the systemd desktop integration spec? It says:

  • Application units should follow the scheme [...]
  • Using .service units instead of .scope units, i.e. allowing systemd to start the process on behalf of the caller, instead of the caller starting the process and letting systemd know about it, is encouraged.

Apparently @benzea wrote this. Any opinion?

If that is the intention, then it doesn't seem to have stuck yet.

Yeah, I think we should push for that but I'm not sure what that would involve.

smcv commented 1 year ago

This doesn't solve the issue with systemd services, does it? Even if the dbus service file contains the appid, if the systemd service name will always be whatever the systemd service file is.

Sorry, I wasn't clear enough about fooapp.service being the systemd service file, not the D-Bus service file. What I was suggesting was having x-d-p look at the metadata of the systemd service (which could contain the app ID), and not just its name.

I don't think adding the app ID to the D-Bus service file would be useful, because the important thing on D-Bus is the bus name, which should match the app ID anyway. The D-Bus service file should ideally always be ${bus_name}.service, although a few older D-Bus services don't follow that pattern, and that links them back to the app ID. The filename of the D-Bus service file is also not very interesting, because it's not part of the API in the way that the names of systemd services are.

The names of systemd service files have traditionally used the same short name you might use in /usr/bin, /usr/libexec or /etc/init.d, like dbus.service, gpg-agent.service and pipewire.service, so they're unconnected to the app ID.

dvdhrm commented 1 year ago

Wouldn't that mean that adding a SystemdService=fooapp.service to an existing session service would be a functional regression on dbus-broker systems, because consumers of this "API" would no longer be able to identify its app-ID?

The API would guarantee that a missing SystemdService= line produces a predictable service-name based on the dbus-activation-file. I would expect an application to check the dbus-activation-file and decide based on the presence of SystemdService= how to react. IOW, it would enable a predictable service-naming. It would not allow shortcutting the dbus-service-file. Hence, I don't believe adding SystemdService= would break the API.

The current naming-scheme already allows this, but as @swick correctly pointed out, this has not been part of our API guarantees in any way and is really fragile to rely upon. Hence, the proposed switch to a documented naming scheme plus documenting this as ABI.

I am open to other suggestions, especially if they allow reliably deducing the appid from the dbus-activation file.

swick commented 1 year ago

Re-reading it with the knowledge that the service is a systemd service...

I think the ideal thing might be if there was a way for xdg-desktop-portal to inspect fooapp.service and find out its app-ID from metadata, which could work equally well for dbus-broker and dbus-daemon? And then dbus-broker could put that same metadata on its transient units too?

That would be giving up on the systemd naming convention and we would have yet another convention of how to get the app id from a process. I would much prefer to move everything over to the systemd naming convention if at all possible.

The transient unit case is low hanging fruit. The systemd service case is a bit more interesting.

The names of systemd service files have traditionally used the same short name you might use in /usr/bin, /usr/libexec or /etc/init.d, like dbus.service, gpg-agent.service and pipewire.service, so they're unconnected to the app ID.

Sure, but on the other hand none of those are what I would consider applications. None of them have any reason to use portals and if they do, having a host app with an empty app id should just be considered authorized.

That's why I was also asking if we should only enforce the systemd naming convention for transient units if the dbus service file has a corresponding desktop file (maybe require NoDisplay=false).

Why do you prefer to add another key to the systemd service file instead of renaming the file according to the systemd naming convention?

smcv commented 1 year ago

Why do you prefer to add another key to the systemd service file instead of renaming the file according to the systemd naming convention?

I don't prefer it, exactly, but there are two reasons I thought it should be considered:

smcv commented 1 year ago

I would expect an application to check the dbus-activation-file and decide based on the presence of SystemdService= how to react

This seems odd to me: usually D-Bus service files are written by application authors and only read by a message bus implementation (dbus-daemon or dbus-broker), and occasionally read/written by a sandboxed-app framework like Flatpak that needs to transform the Exec line. Applications usually only care about whether the name is activatable or not, which they canonically find out from ListActivatableNames() rather than by reading D-Bus service files themselves.

swick commented 1 year ago

a small number of user services named with a reversed-DNS D-Bus-style name with no app- prefix, like org.gnome.Evince.service

I checked my install and there is literally no service for what I would consider an application. I suspect there are not many which ship with a systemd service file. Could we reasonable rename them upstream?

it seemed strange to me that with the change that is proposed here, a change that is usually positive or at worst neutral (adding a SystemdService to the D-Bus service file to tie it to a systemd unit) becomes negative

Not if you follow the naming schema for the systemd service file.

swick commented 1 year ago

This is still relevant. Not sure how to push it forward though. I think there are two problems:

  1. How do we deal with systemd services
  2. How do we decide if something is an app and not a service etc

I would say we can just ignore 1. and then in the future change the service file names when it makes sense. For the second issue maybe one of those is a reasonable solution:

swick commented 1 month ago

This is tow years old now and nothing has changed in any way for the better and I don't have any other ideas on how to improve the situation. Should we just move forward with it? It's not ideal but I think we have to do something to improve the situation.

Thoughts?

@dvdhrm @smcv

benzea commented 1 month ago

Aren't you overthinking it? Maybe just assume it is an application and if it really is a service, let it define a systemd unit that should be activated. It'll get a different naming then.

This is more about conventions to e.g. be able to show a desktop file belonging to a systemd unit/cgroup.

swick commented 1 month ago

Peronally I think the suggestion in the first post is still not a bad idea. The change is small and it would improve the situation significantly. We could revisit other measures later on.

I also think that systemd services usually are not apps so them not getting the app unit naming seems like a win to me, and if they are, they can name the service files accordingly.

@benzea I think you're agreeing with this? It's not entirely clear to me what you think is "overthinking".

benzea commented 1 month ago

No, that is fine. I was thinking that it is not worth trying to decide between service and application. Just let the developers provide a service file for a service.

dvdhrm commented 1 month ago

I think this matter boils down to the question that smcv raised:

Wouldn't that mean that adding a SystemdService=fooapp.service to an existing session service would be a functional regression on dbus-broker systems, because consumers of this "API" would no longer be able to identify its app-ID?

I have to say I am very much confused by the specification. Smcv is certainly right that the service file name of an application is part of its API. And a service might certainly be right in refusing to name its service file app-<appid>@.service. I don't believe there is any precedence in that. This is certainly suitable for autogenerated units, but feels odd for manual units.

Further, apps might be restricted in their unit-names for other reasons. The desktop-specification is not the only place that uses unit-names as identification.

Now what we could do is use unit-aliases of systemd. However, this would require runtime detection to iterate all names of a unit, rather than just check its service-ID. This is available in the PID1 D-Bus api:

  interface org.freedesktop.systemd1.Unit {
[...]
    properties:
      readonly s Id = 'avahi-daemon.service';
      readonly as Names = ['avahi-daemon.service'];
[...]

If this is already what the portal does, I think it would be suitable to apply this change. In this case adding SystemdService=foobar.service would still be a breaking change, but could be easily fixed by adding Alias=app-FoobarAppId.service (or the equivalent template). Ideally, we would be able to do that from dbus-broker-launch, but I don't think that is currently possible in a sensible manner. We could drop aliases in /run, but we would override any user-aliases, which is a bit unfortunate.

benzea commented 1 month ago

So, there were various points of doing this kind of convention:

  1. Initially I wanted to do a app-.service drop-in to move everything into app.slice. However, the default slice is app.slice these days, IIRC.
  2. The other big point was being able to map both unit and cgroup to the .desktop file.

Further, apps might be restricted in their unit-names for other reasons. The desktop-specification is not the only place that uses unit-names as identification.

I don't think that this is really an API. If someone doesn't conform, we just cannot inspect it. You'll be stuck into a generic "System" group in a task manager like GNOME Usage.

Also, not sure what other restrictions/conventions there are and how much of a conflict it is in the end.

swick commented 1 month ago

Mh, the goal here isn't to make every cgroup follow the app[-<launcher>]-<ApplicationID>[@<RANDOM>].service pattern but ideally only the ones which are actual apps. On my system right now for example the only thing that I would consider an app which got dbus activated and isn't a flatpak is gnome software.

If avahi-daemon followed the same pattern that wouldn't be useful to us. Maybe the right route is to just fix up those cases and either add a systemd service file or a new thing to the dbus service file?

The use case here is xdg-desktop-portal which does inspect the service file to get the AppID which then is used as a key for permissions. I think it's not very uniform what we do when we fail to match. Maybe we need to rethink that part as well.

smcv commented 1 month ago

On my system right now for example the only thing that I would consider an app which got dbus activated and isn't a flatpak is gnome software.

I think you might use more Flatpak apps and fewer system apps than I do, but /usr/lib/systemd/user on a Debian 12 GNOME system already includes Evince (GNOME's PDF viewer), and /usr/share/dbus-1/services on the same system includes several more user-facing apps that in principle would ideally be systemd-activated rather than just D-Bus-activated, even when using dbus-daemon:

Several of those don't really make sense to sandbox - in particular, Nautilus really wants to be part of the TCB, because a file manager that can't access all of your files is not very useful.

the goal here isn't to make every cgroup follow the app[-launcher>]-<ApplicationID>[@<RANDOM].service pattern but ideally only the ones which are actual apps

I don't think we can tell which is which, at the moment. There is not enough information in a D-Bus .service file to distinguish between these D-Bus-activatable apps, and the non-app services like org.gnome.keyring.service that also have D-Bus .service files. If more information is needed, it could be standardized via https://dbus.freedesktop.org/doc/dbus-specification.html (like the SystemdService field was), but at the moment that doesn't exist.

There is, unfortunately, also not enough information in a D-Bus .service file to be able to generate a correct systemd user service in all cases: D-Bus services are allowed to double-fork and daemonize (this is discouraged but allowed, and I believe there are real examples of services that do this), and as a result the reference dbus-daemon has a special-case code path to treat a D-Bus service exiting with status 0 before it has requested its name as not indicating failure. I believe that's the main reason why dbus-daemon's support for systemd activation (which was contributed by a systemd maintainer) needed to be opt-in via SystemdService instead of being automatic or opt-out.

In a much earlier message, I wrote:

I think the ideal thing might be if there was a way for xdg-desktop-portal to inspect [the systemd unit] fooapp.service and find out its app-ID from metadata, which could work equally well for dbus-broker and dbus-daemon? And then dbus-broker could put that same metadata on its transient units too?

Runtime detection of the list of aliases, as @dvdhrm described in https://github.com/bus1/dbus-broker/issues/302#issuecomment-2406892629, would be one possible implementation of inspecting fooapp.service to find out who/what it is.

swick commented 1 month ago

Runtime detection of the list of aliases, as @dvdhrm described in https://github.com/bus1/dbus-broker/issues/302#issuecomment-2406892629, would be one possible implementation of inspecting fooapp.service to find out who/what it is.

I would really like to avoid this and rather have a single thing that can be looked up because there are multiple things that want do look up this kind of information and trying to implement all those different strategies isn't viable IMO.

I don't think we can tell which is which, at the moment. There is not enough information in a D-Bus .service file to distinguish between these D-Bus-activatable apps, and the non-app services like org.gnome.keyring.service that also have D-Bus .service files. If more information is needed, it could be standardized via https://dbus.freedesktop.org/doc/dbus-specification.html (like the SystemdService field was), but at the moment that doesn't exist.

That sounds like the cleanest solution to me. A new DesktopFile property which can be used by the dbus server to put the service in the right app-dbus-<ApplicationID>[@<RANDOM>].service cgroup.