Bug 107245 - [RFC] specification extension: ConnectClient()
Summary: [RFC] specification extension: ConnectClient()
Status: RESOLVED MOVED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: D-Bus Maintainers
QA Contact: D-Bus Maintainers
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-07-16 11:18 UTC by Tom Gundersen
Modified: 2018-10-12 21:35 UTC (History)
5 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Tom Gundersen 2018-07-16 11:18:12 UTC
This is a follow-up on the discussion in <https://github.com/systemd/systemd/issues/9503>.

I want to suggest adding a new call, ConnectClient() to the org.freedesktop.DBus interface. The intention behind this is to allow a (privileged) client to connect and claim names on behalf of another (unprivileged) one.

The discussion that precipitated this was the desire to allow PID1 to spawn dbus-clients running as a dynamic user, without having to update the DBus policy dynamically to allow these new users to connect and claim names on the bus.

The proposed API extension would allow PID1 to tell dbus daemon to connect the new client, acquire for it a set of given names, and pin in it its dynamic (unprivileged) credentials, but to use PID1's (privileged) credentials for the policy checks when deciding if the client is allowed to be connected and to acquire the names.

This would solve the immediate problem at hand of dynamic users, but it would also allow other features:

1) Scheduling of well-known names

Currently, if a name is activatable, it is always activatable. There is no way of delaying the availability of a name, or making a name no longer activatable. Short of delaynig the start-up of the whole bus, or shutting the bus down.

2) Socket activation of DBus clients

Allowing DBus clients to be socket-activated is both a simpler scheme than the current name activation, but it would also allow for proper exit-on-idle, which name activation does not allow (without potentially dropping messages).

3) Simplification of XML policy

This is obviously up to the implementers, but this scheme would long-term allow systems without own/user/group directives. And as send_* recv_* directives are already not strictly speaking necessary (one could use polkit instead), this could in the future allow for greatly simplified policies in the common case.

Suggested API (based on suggestion by David Herrmann):

`org.freedesktop.DBus.ConnectClient(h socket, a{sv} args) -> (s unique_name);

This pre-connects, pre-authenticates, and prepares an AF_UNIX socket for use as a D-Bus connection. The socket is taken as input rather than output, to guarantee that the owning-uid/creds are correct. If the socket is not AF_UNIX+SOCK_STREAM+unconnected, the call simply fails.

The args parameter can be extended in the future to modify this call. Examples:

    `RequestNames`: Takes a string array as argument; specifies which names are pre-acquired by the client.

    `AllowUnixFds`: Set the ALLOW_UNIX_FDS flag for the new connection.

The message daemon makes this call available to everyone, but verifies the caller (not the passed in client socket) is allowed to connect and claim the names specified in `RequestNames`.


Note that it isn't expected that existing clients connecting in the traditional way can necessarily be ported over to this new API without further notification. As pointed out by smcv in the above bug, care must be taken with the fact that names are claimed before the client has been initialized. In the common case this shouldn't make a difference, and in the case a client needs to make calls on the bus before processing incomming method calls on its names, it would either have to buffer messages, or use a separate connection to the bus for the initialization before starting to process the main connection (much could be said on that topic, I'm just mentioning it briefly here to note that it is on the radar).

If we agree that we want something like this and on a basic design, I'm happy to write it up as a patch to the spec.
Comment 1 Lennart Poettering 2018-07-16 12:48:24 UTC
So, three questions, some of which are already hinted in the github bug:

1. What about feature negotiation, specifically features that are exclusively a protocol detail between the dbus client library and the broker and should hence be negotiated between them and not spill into the .busname unit file in systemd

2. What about policy? back in kdbus' .busname design that could be configured in the busname unit files themselves, with some reduced language (compared to dbus-daemon XML syntax). I figure we can start with something dead-simple and extend it if needed, but I wonder what the simplest policy to start from would be?

3. What about creds? At the time ConnectClient() is invoked by the service manager, the client is unlikely to exist yet, hence the PID, UID, GID, Selinux label, aux gid list won't be known yet.

4. How would ListActivatableNames() vs. ListNames() deal with such names? What about NameOwnerChange events

5. What about StartServiceByName()?

My personal idea about issue 1 listed above would be to do feature nego via HelloWithFeatures() as discussed in the github item.

My personal idea about issue 3 + 4 would be to keep the initial NUL byte in place, and use it as indicator of handoff between pid1 and the activated service. Thus the SCM_CREDENTIALS associated with the NUL byte would be the ones the broker caches for the connection, and before the NUL byte the connection would appear in ListActivatableNames() and afterwards in ListNames().

My personal idea about issue 5 would be that the broker simply synthesizes a Ping() method in this case which it sends to the not yet activated service. it would be "no-reply" set and be fire-and-forget.
Comment 2 Tom Gundersen 2018-07-16 13:46:54 UTC
1) Hm, it may very well be that I'm missing something fundamental here. but this is my take: I'm not opposed to adding a separate HelloWithFeatures() that allow lazy feature negotiation, if that makes sense. However, if we go for real socket activation, I don't see that being really useful, as the client must be able to receive the first message before receiving any reply regarding feature negotiation. The only way I see it would be something like "UpgradeFeatures()", so the client first starts reading/writing using legacy only, then it can upgrade to get GVariant or whatever else. I'd say this should be a follow-up if turns out to make sense, and to start with we should keep it simple and leave the feature negotiation to the .busname unit.

2) I'd say the simplest possible policy would be none at all, falling back to applying the regular XML policies to the new client. As long as policies for acquiring names are not required, this should actually work just fine as long as one makes sure the send/receive policies only distinguish between root and non-root users (and not rely on any dynamic uids, but rather polkit for anything more finegrained). If it turns out that we want something more powerful, then it would be possible to attach to the a{sv} the precise policy we would want applied to this particular client. This would work even with dynamic uids, as long as PID1 is responsible for resolving the user names to their dynamic uid's before passing the policy to dbus. This is exactly how the API between dbus-broker-launch and dbus-broker works, so we already figured out how to do that. We might want a middle ground here, and actually use a policy like what was in kdbus, my point here is just to say that the whole range of possibilities is open to us from no policy at all to the full policy as used today, but we can figure out what we want in a  future extension.

3) The NUL byte hand-over is quite clever. I would need to think more about that. Just note that that's not how it works now on Linux, the first byte is discarded, and the socket credentials are used. My idea here was to do for the socket what is done for the service in PID1, where one can configure things like User/Group and SELinux labels (which is what matters). Also, note that while the initial handover is one thing, another is sockets surviving the restart of a daemon (exit-on-idle style), which we may also want to support, where a NUL byte type thing wouldn't work.

4) I would say that like a socket being established in the filesystem, a name acquired in this way is no different from one a client has acquired themselves. I.e., the NameOwnerChanged signal is sent out when ConnectClient() is called, and it is always considered owned, regardless of whether the service is actually running (so ActivatableNames is no longer a thing, it is just Names).

5) I'd not consider the names activatable, so not support StartServiceByName: the service is simply already running (from the point of view of the rest of the system). A Ping() would be fine, but I'm not sure I see the need.


My general feeling about this is that we should try as much as possible to keep things simple and in line with the socket-activation ideas, and the more DBus concepts that become redundant, the better (activation of names, handover of names, etc.).
Comment 3 Lennart Poettering 2018-07-16 14:02:24 UTC
I figure ignore my point 1 and 2 for now is indeedacceptable, we can certainly add "late nego" and "complex policy" as an add-on later on.
Comment 4 Lennart Poettering 2018-07-16 14:06:16 UTC
> 3) The NUL byte hand-over is quite clever. I would need to think more about that. Just note that that's not how it works now on Linux, the first byte is discarded, and the socket credentials are used. My idea here was to do for the socket what is done for the service in PID1, where one can configure things like User/Group and SELinux labels (which is what matters). 

That's a misunderstanding. The User= and Group= stuff you configure in .socket units is applied to AF_UNIX sockets in the file system only. It's not used for credentials of the socket itself — the kernel doesn't have any API for manipulating SO_PEERCRED after it was set once. Hence systemd couldn't change it even if it wanted.

> Also, note that while the initial handover is one thing, another is sockets surviving the restart of a daemon (exit-on-idle style), which we may also want to support, where a NUL byte type thing wouldn't work.

Good point. I figure in that case the service manager could inform the broker that the service is gone now. i.e. invoke a call ReconnectClient() or so, which takes the unique id or so, to put the sokcet back into the pre-activated state
Comment 5 Lennart Poettering 2018-07-16 14:15:17 UTC
> 4) I would say that like a socket being established in the filesystem, a name acquired in this way is no different from one a client has acquired themselves. I.e., the NameOwnerChanged signal is sent out when ConnectClient() is called, and it is always considered owned, regardless of whether the service is actually running (so ActivatableNames is no longer a thing, it is just Names).

Hmm, this would mean it wouldn't be useful for example for resolved. There are two kinds of systemd-resolved clients: the activating ones and the non-activating ones. The activating ones are "resolvectl" when it shall show the state and so on. The idea is that in that case we should block on resolved being up, and return the data as soon as it replies. OTOH when nss-resolve calls into resolved it sets the NO_AUTO_START flag, in order to reduce deadlocks, and not trigger (and sync) on resolved, but so that it can fall back to client-side resolving (i.e. a fallback to traditional glibc nss-dns) if resolved is not ready yet. It's a safety feature to avoid deadlocks, for example if nss-resolve is called as part of the startup process of resolved itself (think: resolved's user is resolved with some NSS module which talks to a remote service, which thus means NSS hostname lookups). 

Yes, the ability to schedule activatability of names is super nice, and helps us avoid many such deadlocks, but unfortunately not all of them, as stuff needed to fulfil activation should still not block on activation of itself...

And yes, this is an unsolved problem for classic socket activation...
Comment 6 Tom Gundersen 2018-07-16 14:18:57 UTC
> That's a misunderstanding. The User= and Group= stuff you configure in .socket units is applied to AF_UNIX sockets in the file system only. It's not used for credentials of the socket itself — the kernel doesn't have any API for manipulating SO_PEERCRED after it was set once. Hence systemd couldn't change it even if it wanted.

Sorry, I was unclear. I meant User/Group/SELinuxLabel/etc, as defined in systemd.exec(5), not systemd.socket(5). I.e., we'd need to call socketpair() as the right user/group/selinux context, which I presume would just use the same code path as what is done befoe execve() of a service. This way we would get the kernel to do the credential handling for us, and the behaviour is at least in line with how these things usually work on Linux.
Comment 7 Lennart Poettering 2018-07-16 14:23:01 UTC
(Thinking ahead: so, if we make bus activatability something that can be scheduled from outside of the broker, i.e. systemd, can we go one step further, and split dbus daemon startup from making the classic dbus .service files activatable (i.e. the files in /usr/share/dbus-1/system-services/) too? i.e. so that we can start dbus early on, but not have it make anything activatable at all just yet? Instead, when it gets some notification from the outside later on it would make all the stuff from /usr/share/dbus-1/system-services/ pop up? With that we could finally provide the broker during early boot, which would make me a much happier man)

(Thinking ahead #2: In addition to accepting connection fds via a bus driver call I'd like it if dbus-daemon/broker would accept connection fds also via some protocol of passing fds across execv(), so that we can remove this async mess that PID 1 starts dbus and then waits and waits until dbus is up to then immediately connect to it. if instead it could just pass the already connected socketpair() in via execve() then things would be excellent)
Comment 8 Tom Gundersen 2018-07-16 14:29:40 UTC
The resolved case is a good point. I would need to think more about that. I mean it is a really hairy situation there, and what the "right" solution should be in the big picture is not really clear to me. Though if ever the resolved user is being resolved over the network I'd argue that that's a sign of something being fundamentally wrong ;)
Comment 9 Lennart Poettering 2018-07-16 14:32:36 UTC
(In reply to Tom Gundersen from comment #6)
> > That's a misunderstanding. The User= and Group= stuff you configure in .socket units is applied to AF_UNIX sockets in the file system only. It's not used for credentials of the socket itself — the kernel doesn't have any API for manipulating SO_PEERCRED after it was set once. Hence systemd couldn't change it even if it wanted.
> 
> Sorry, I was unclear. I meant User/Group/SELinuxLabel/etc, as defined in
> systemd.exec(5), not systemd.socket(5). I.e., we'd need to call socketpair()
> as the right user/group/selinux context, which I presume would just use the
> same code path as what is done befoe execve() of a service. This way we
> would get the kernel to do the credential handling for us, and the behaviour
> is at least in line with how these things usually work on Linux.

That can't really work. We don't now the PID that would be used then this early. 

(Also, we don't know the selinux label either as it depends on the label of the binary that is to be executed... so we'd have to read that first of the image, but yuck. We do this for socket activation, and it's a racy mess and I'd prefer not to do this ever again. Similar, for UID/GID it's also a mess as we'd have to look for suid/sgid bits on the binary, and then calculate what the resulting UID/GID would be for the process, taking NNP and whatnot into account. Because it's a frickin mess to do that we don't do that currently for sockets, and I am very sure I don't want to add that. In other words, for socket activation this only works somewhat for selinux labels, but not correctly for UID nor GID nor aux GID list and not at all for the PID... Faking creds is a mess, it always has been, always will.)

There's also the issue that UID/GID assignment generally requires NSS and we are pretty carefully with delaying NSS calls to the latest moment possible, because NSS subsystems might not be up at the moment of activatability but only during activation... Any concept that insists on us knowing the UID/GIDs really early on, at the moment of marking a name as activatable instead of actually activating it is hence highly problematic. (selinux labels don't have this issue, we can do that earlier as we do for socket activation since the database and policy are static and not supplied from network services)
Comment 10 Tom Gundersen 2018-07-16 14:34:34 UTC
(In reply to Lennart Poettering from comment #7)
> (Thinking ahead: so, if we make bus activatability something that can be
> scheduled from outside of the broker, i.e. systemd, can we go one step
> further, and split dbus daemon startup from making the classic dbus .service
> files activatable (i.e. the files in /usr/share/dbus-1/system-services/)
> too? i.e. so that we can start dbus early on, but not have it make anything
> activatable at all just yet? Instead, when it gets some notification from
> the outside later on it would make all the stuff from
> /usr/share/dbus-1/system-services/ pop up? With that we could finally
> provide the broker during early boot, which would make me a much happier man)
> 
> (Thinking ahead #2: In addition to accepting connection fds via a bus driver
> call I'd like it if dbus-daemon/broker would accept connection fds also via
> some protocol of passing fds across execv(), so that we can remove this
> async mess that PID 1 starts dbus and then waits and waits until dbus is up
> to then immediately connect to it. if instead it could just pass the already
> connected socketpair() in via execve() then things would be excellent)

It was always our intention to allow this with dbus-broker. And you could indeed do both those things right now by spawning the broker directly from PID1, rather than starting dbus-broker-launch as a regular service. This would in effect let PID1 be your launcher, which would talk to the internal org.bus1.DBus.Broker API. Though maybe we should move that discussion elsewhere?
Comment 11 Lennart Poettering 2018-07-16 14:36:37 UTC
> The resolved case is a good point. I would need to think more about that. I mean it is a really hairy situation there, and what the "right" solution should be in the big picture is not really clear to me. Though if ever the resolved user is being resolved over the network I'd argue that that's a sign of something being fundamentally wrong ;)

Well, true, but NSS modules are called one by one for each name, and it's not known in generic code ahead whether it's OK to have this resolved over the network or not, nor is ther even an API that allows to pass information about whether network access is OK or not to NSS lookups...
Comment 12 Tom Gundersen 2018-07-16 14:45:19 UTC
> That can't really work. We don't now the PID that would be used then this
> early. 

An option for such non-critical creds is to simply not report them at all for clients connected in this way. Note that PIDs are especially flaky anyway, when we do exit-on-idle. The only thing we really need is UID/GIDs and security label.

> (Also, we don't know the selinux label either as it depends on the label of
> the binary that is to be executed... so we'd have to read that first of the
> image, but yuck. We do this for socket activation, and it's a racy mess and
> I'd prefer not to do this ever again. Similar, for UID/GID it's also a mess
> as we'd have to look for suid/sgid bits on the binary, and then calculate
> what the resulting UID/GID would be for the process, taking NNP and whatnot
> into account. Because it's a frickin mess to do that we don't do that
> currently for sockets, and I am very sure I don't want to add that. In other
> words, for socket activation this only works somewhat for selinux labels,
> but not correctly for UID nor GID nor aux GID list and not at all for the
> PID... Faking creds is a mess, it always has been, always will.)
> 
> There's also the issue that UID/GID assignment generally requires NSS and we
> are pretty carefully with delaying NSS calls to the latest moment possible,
> because NSS subsystems might not be up at the moment of activatability but
> only during activation... Any concept that insists on us knowing the
> UID/GIDs really early on, at the moment of marking a name as activatable
> instead of actually activating it is hence highly problematic. (selinux
> labels don't have this issue, we can do that earlier as we do for socket
> activation since the database and policy are static and not supplied from
> network services)

The more fundamental issue here is that we have a huge mess on our hands with UID/GID resolution in early userspace in general. Moving things "later" helps a bit, but it is still fundamentally not a solved problem. I'd favour being bold on this and say that any user/group configured in systemd configuration must be resolvable statically (/etc/passwd) or DynamicUser=yes.
Comment 13 Lennart Poettering 2018-07-16 14:48:07 UTC

> That can't really work. We don't now the PID that would be used then this
> early. 

An option for such non-critical creds is to simply not report them at all for clients connected in this way. Note that PIDs are especially flaky anyway, when we do exit-on-idle. The only thing we really need is UID/GIDs and security label.

Well, but they are used all over the place. It would be pretty bad if busctl and systemd-inhibit and such tools would suddenly report rubbish PIDs only...
Comment 14 Lennart Poettering 2018-07-16 14:53:15 UTC
(In reply to Tom Gundersen from comment #12)
> > That can't really work. We don't now the PID that would be used then this
> > early. 
> 
> An option for such non-critical creds is to simply not report them at all
> for clients connected in this way. Note that PIDs are especially flaky
> anyway, when we do exit-on-idle. The only thing we really need is UID/GIDs
> and security label.

On top, the owning PID is used for informational purposes at various places. Yes it's usefuless and dangerous for using it for authentication, but it is *really* useful for informational purposes.

(And yes, we do get a lot of complaints about the fact that netstat nowadays reports systemd as being the process that listens on myriads of ports instead of the services actually handling them, due to socket activation)

> The more fundamental issue here is that we have a huge mess on our hands
> with UID/GID resolution in early userspace in general. Moving things "later"
> helps a bit, but it is still fundamentally not a solved problem. I'd favour
> being bold on this and say that any user/group configured in systemd
> configuration must be resolvable statically (/etc/passwd) or DynamicUser=yes.

Well, you can be as bold there as you want. The fact is that NSS modules don't care and are called one-by-one in order, and will go onto the net whenever they feel like it. I mean, just by looking at a name you can't tell if is the user name used by a system service (where we could insist that /etc/passwd and DynamicUser are the only backing sources and simply terminate the query after checking these two) or one belonging to a regular human user (where doing network NSS is fine)
Comment 15 Tom Gundersen 2018-07-16 15:01:10 UTC
>> The more fundamental issue here is that we have a huge mess on our hands
>> with UID/GID resolution in early userspace in general. Moving things "later"
>> helps a bit, but it is still fundamentally not a solved problem. I'd favour
>> being bold on this and say that any user/group configured in systemd
>> configuration must be resolvable statically (/etc/passwd) or DynamicUser=yes.
>
> Well, you can be as bold there as you want. The fact is that NSS modules don't
> care and are called one-by-one in order, and will go onto the net whenever they
> feel like it. I mean, just by looking at a name you can't tell if is the user
> name used by a system service (where we could insist that /etc/passwd and
> DynamicUser are the only backing sources and simply terminate the query after
> checking these two) or one belonging to a regular human user (where doing
> network NSS is fine)

By bold, I meant: don't do NSS from PID1. Use fgetpwent(3) directly.
Comment 16 Lennart Poettering 2018-07-16 15:26:23 UTC
(In reply to Tom Gundersen from comment #15)

> By bold, I meant: don't do NSS from PID1. Use fgetpwent(3) directly.

We actually never do NSS from PID1. Only from forked off children, see our CODING_STYLE about that. systemd is not the only part of the early boot, there are many players, and some will do invariably do NSS might be inserted into weird parts of the boot process, and cause indirect, transient deadlock dep cycles. Convincing everybody in early boot that they should use fgetpwent() is going to be very very hard...

I mean, think about weird storage stuff, that is backing some weird dir that is needed by something we do... We never know what things do. It's not enough that systemd would start avoiding NSS everywhere, everything running before resolved and in particular potentially being inserted between marking-activatable and activation of the resolved bus service would have to care, and you never know what that might be...
Comment 17 Simon McVittie 2018-07-23 13:47:22 UTC
(In reply to Tom Gundersen from comment #0)
> 3) Simplification of XML policy
> 
> This is obviously up to the implementers, but this scheme would long-term
> allow systems without own/user/group directives.

When the XML policy says

    <!-- /usr/share/dbus-1/system.conf -->
    <policy context="default"><deny own="*"/></policy>

    <!-- /{etc,usr/share}/dbus-1/system.d/foo.conf -->
    <policy user="foo"><allow own="com.example.Foo"/></policy>
    (etc.)

then I think there's a significant risk of violating least-astonishment if some names "magically" become allowed. This is what I meant when I said on the systemd issue that the XML policy language is "exhaustive": you can think of an operation and a set of credentials, walk through all the XML files, and come to a conclusion about whether a process with those credentials is going to be allowed to perform that operation. I'm not saying this is *right*, but it's the design we've had for over a decade, and making incompatible changes to it has a fairly heavy cost (particularly since it's security-sensitive).

OK, so what if we said "names that the caller of ConnectClient() asks for are always allowed", with some suitable definition of which processes are sufficiently privileged to call ConnectClient()? Now imagine the sysadmin had added additional XML in /etc (this is ostensibly configuration, remember) so that the policy reads like this:

    <policy context="default"><deny own="*"/></policy>
    <policy user="foo"><allow own="com.example.Foo"/></policy>
    <!-- Foo is undesired on this machine -->
    <policy user="foo"><deny own="com.example.Foo"/></policy>

or like this:

    <policy context="default"><deny own="*"/></policy>
    <policy user="foo"><allow own="com.example.Foo"/></policy>
    <!-- Foo is undesired on this machine -->
    <policy context="mandatory"><deny own="com.example.Foo"/></policy>

A sysadmin might reasonably expect this to prevent the com.example.Foo name from being owned by anyone - but if systemd bypasses that, then they are going to be unpleasantly surprised.

One way round this would be some new rule like

    <policy context="connect-client-caller"><allow own="*"/></policy>

(On OSs that upgrade in-place, that would have to either be in an <include>d file or not be used for a few years, because the dbus-daemon can't re-exec itself, and the old dbus-daemon won't be able to reload if it sees unrecognised syntax in system.conf, whereas syntax errors in <include>d files are non-fatal and result in the file being ignored.)

Another possible way around it would be to configure the name to be ownable by root, and say that ConnectClient() callers are allowed to delegate any name that they themselves can own to the new client.
Comment 18 Simon McVittie 2018-07-23 13:52:05 UTC
(In reply to Tom Gundersen from comment #6)
> I.e., we'd need to call socketpair()
> as the right user/group/selinux context, which I presume would just use the
> same code path as what is done befoe execve() of a service. This way we
> would get the kernel to do the credential handling for us, and the behaviour
> is at least in line with how these things usually work on Linux.

socketpair() doesn't run the same LSM hooks as listen()/accept()/connect(): see Bug #101315.
Comment 19 Simon McVittie 2018-07-23 13:58:26 UTC
(In reply to Lennart Poettering from comment #7)
> (Thinking ahead: so, if we make bus activatability something that can be
> scheduled from outside of the broker, i.e. systemd, can we go one step
> further, and split dbus daemon startup from making the classic dbus .service
> files activatable (i.e. the files in /usr/share/dbus-1/system-services/)
> too?

If that's what you want, add a --pause-activation command-line option and an UnpauseActivation() method call, or something like that. It shouldn't be too difficult to make BusActivation pause until it has been told it can go, while paused treat all bus names as at least potentially activatable, and reload config just before unpausing. (I'm not volunteering to implement this, though.)

I think the hardest part would probably be queuing up ListActivatableNames() calls so they would all be answered after activation is unpaused (dbus-daemon doesn't currently implement any async calls).

> if instead it could just pass the already
> connected socketpair() in via execve() then things would be excellent

dbus-daemon can't know the LSM label of the other end of a socketpair(), so this would work poorly with AppArmor, and probably also SELinux. To have a pre-connected fd with the right credentials you'd have to connect() it the hard way.
Comment 20 Simon McVittie 2018-07-23 14:02:37 UTC
(In reply to Tom Gundersen from comment #0)
> The discussion that precipitated this was the desire to allow PID1 to spawn
> dbus-clients running as a dynamic user, without having to update the DBus
> policy dynamically to allow these new users to connect and claim names on
> the bus.

Would it be sufficient to special-case <policy user=x><allow own=y> rules to be the only ones that can make use of dynamic users?

If so, then using a less efficient (O(n)) code path to check authorization for name ownership would be a lot less costly than using an O(n) code path to check authorization for more frequent operations like message-sending.
Comment 21 Simon McVittie 2018-07-23 14:14:31 UTC
(In reply to Tom Gundersen from comment #0)
>     `RequestNames`: Takes a string array as argument; specifies which names
> are pre-acquired by the client.

If the new client accepts method calls, and it has to do any preparation before it is ready to accept method calls (which, in general, involves contacting other services on the same bus), then it would have to either have two D-Bus sockets, or somehow save up all its method calls (including those for object paths that it might not have created yet!) to be processed when it has done its setup.

This seems awkward. Parallel sockets break the total and causal ordering properties that D-Bus normally has (Telepathy on Maemo devices had some remarkably hard-to-track-down bugs as a result of this), while saving up method calls for later is going to be difficult to implement in APIs that conserve memory and file descriptor quota by having a process-wide shared socket per bus (like libdbus), particularly if that socket is dispatched in a worker thread (like GDBus).

Might it be better for this to be a list of names for which, if/when the client requests them, authorization decisions are made with the credentials of the ConnectClient() caller instead of those of the client itself? That lets the client continue to use the familiar "do setup, export objects, request name" flow.
Comment 22 Simon McVittie 2018-07-23 14:18:22 UTC
(In reply to Tom Gundersen from comment #0)
> Currently, if a name is activatable, it is always activatable. There is no
> way of delaying the availability of a name, or making a name no longer
> activatable. Short of delaynig the start-up of the whole bus, or shutting
> the bus down.

ListActivatableNames() doesn't have any change-notification, so if names are to be dynamically activatable-or-not, it would need to grow change-notification.
Comment 23 Simon McVittie 2018-07-23 14:29:40 UTC
(In reply to Simon McVittie from comment #21)
> Might it be better for this to be a list of names for which, if/when the
> client requests them, authorization decisions are made with the credentials
> of the ConnectClient() caller instead of those of the client itself? That
> lets the client continue to use the familiar "do setup, export objects,
> request name" flow.

Here's a straw-man that would require less elaborate fd-passing plumbing:

RequestName(string name, uint32 flags = DBUS_NAME_FLAG_ACTIVATOR)

    (Extension to the current RequestName())
    Request a name that will be held on behalf of another process.

    If a peer sends a message to a name while the head of the queue
    for that name has the ACTIVATOR flag, the message will not be
    sent to the head of the queue. Instead, the message bus will carry
    out service activation for the name; the activated service is
    expected to call RequestName() as usual, and will be treated as
    though it had the REPLACE_EXISTING flag. When the activated
    service takes over the name, the activation message will be
    delivered to it.

    Similarly, if a peer calls StartServiceByName() while the head of
    the queue for the name has the ACTIVATOR flag, the message bus
    will carry out service activation for the name. The activated
    service is expected to call RequestName() as usual, and when it
    does, it will be treated as though it had the REPLACE_EXISTING
    flag.

    DBUS_NAME_FLAG_ACTIVATOR implies DBUS_NAME_FLAG_ALLOW_REPLACEMENT,
    and requests without the ACTIVATOR flag are treated as though
    they had REPLACE_EXISTING whenever the head of the queue has the
    ACTIVATOR flag. This means that any non-activator successfully
    requesting the name will always go to the head of the queue and
    become the primary owner, replacing one or more activators already
    in the queue.

DelegateNameToUid(string name, uint32 uid)
    Allow connections with the given uid to own the given name as
    though it had the uid of the caller connection.

    The caller must have appropriate privileges to own the given name.
Comment 24 Simon McVittie 2018-07-23 14:31:28 UTC
(In reply to Simon McVittie from comment #23)
>     If a peer sends a message to a name while the head of the queue
>     for that name has the ACTIVATOR flag, the message will not be
>     sent to the head of the queue. Instead, the message bus will carry
>     out service activation for the name

... unless the activating message has the NO_AUTO_START flag, in which case the message bus replies with NameHasNoOwner.
Comment 25 Tom Gundersen 2018-07-23 14:52:40 UTC
(In reply to Simon McVittie from comment #17)
> (In reply to Tom Gundersen from comment #0)
> > 3) Simplification of XML policy
> > 
> > This is obviously up to the implementers, but this scheme would long-term
> > allow systems without own/user/group directives.
> 
> When the XML policy says
> 
>     <!-- /usr/share/dbus-1/system.conf -->
>     <policy context="default"><deny own="*"/></policy>
> 
>     <!-- /{etc,usr/share}/dbus-1/system.d/foo.conf -->
>     <policy user="foo"><allow own="com.example.Foo"/></policy>
>     (etc.)
> 
> then I think there's a significant risk of violating least-astonishment if
> some names "magically" become allowed. This is what I meant when I said on
> the systemd issue that the XML policy language is "exhaustive": you can
> think of an operation and a set of credentials, walk through all the XML
> files, and come to a conclusion about whether a process with those
> credentials is going to be allowed to perform that operation. I'm not saying
> this is *right*, but it's the design we've had for over a decade, and making
> incompatible changes to it has a fairly heavy cost (particularly since it's
> security-sensitive).
> 
> OK, so what if we said "names that the caller of ConnectClient() asks for
> are always allowed", with some suitable definition of which processes are
> sufficiently privileged to call ConnectClient()? Now imagine the sysadmin
> had added additional XML in /etc (this is ostensibly configuration,
> remember) so that the policy reads like this:
> 
>     <policy context="default"><deny own="*"/></policy>
>     <policy user="foo"><allow own="com.example.Foo"/></policy>
>     <!-- Foo is undesired on this machine -->
>     <policy user="foo"><deny own="com.example.Foo"/></policy>
> 
> or like this:
> 
>     <policy context="default"><deny own="*"/></policy>
>     <policy user="foo"><allow own="com.example.Foo"/></policy>
>     <!-- Foo is undesired on this machine -->
>     <policy context="mandatory"><deny own="com.example.Foo"/></policy>
> 
> A sysadmin might reasonably expect this to prevent the com.example.Foo name
> from being owned by anyone - but if systemd bypasses that, then they are
> going to be unpleasantly surprised.

My intention was that the caller of `ConnectClient()` would need to be able to call `RequestName()` on each of the names it requests on behalf of the new client in order for the operation to succeed. This semantics is new, for sure, but it would still follow the same philosophy of the XML policy being exhaustive.

[...]

> Another possible way around it would be to configure the name to be ownable
> by root, and say that ConnectClient() callers are allowed to delegate any
> name that they themselves can own to the new client.

Exactly.
Comment 26 Tom Gundersen 2018-07-23 14:57:45 UTC
(In reply to Simon McVittie from comment #18)
> (In reply to Tom Gundersen from comment #6)
> > I.e., we'd need to call socketpair()
> > as the right user/group/selinux context, which I presume would just use the
> > same code path as what is done befoe execve() of a service. This way we
> > would get the kernel to do the credential handling for us, and the behaviour
> > is at least in line with how these things usually work on Linux.
> 
> socketpair() doesn't run the same LSM hooks as listen()/accept()/connect():
> see Bug #101315.

In (very) recent kernels this now works fine: <https://github.com/torvalds/linux/commit/aae7cfcbb733cf16f3bc9cbb650673b94d5df75f>.
Comment 27 Lennart Poettering 2018-07-23 14:58:52 UTC
(btw, i don't think that LSM mess should stop us from using socketpair(). I mean, it's not going to break selinux/apparmor completely. It just means that they have to update their policies to accept unlabelled sockets for a while until the kernel fixed this case. I mean, policies are "best effort" anyway: as precise as reasonably possible, and when they have to accept "no label" in them then this just means the policy is a bit weaker than they'd like it to be, but it's really up to them to fix that. I mean, this is not some exotic API, but a well-known long established call, and the fact they ignored it for so long is really on them, and not on us)
Comment 28 Simon McVittie 2018-07-23 15:08:03 UTC
(In reply to Tom Gundersen from comment #2)
> My general feeling about this is that we should try as much as possible to
> keep things simple and in line with the socket-activation ideas, and the
> more DBus concepts that become redundant, the better (activation of names,
> handover of names, etc.).

How would you implement, for example, `gnome-shell --replace` without the ability to hand over well-known names?

Typical system services don't need queuing and replacement of well-known names, but it's a feature, and is used for things like D-Bus name ownership as distributed mutex.
Comment 29 Tom Gundersen 2018-07-23 15:19:49 UTC
(In reply to Simon McVittie from comment #21)
> (In reply to Tom Gundersen from comment #0)
> >     `RequestNames`: Takes a string array as argument; specifies which names
> > are pre-acquired by the client.
> 
> If the new client accepts method calls, and it has to do any preparation
> before it is ready to accept method calls (which, in general, involves
> contacting other services on the same bus), then it would have to either
> have two D-Bus sockets, or somehow save up all its method calls (including
> those for object paths that it might not have created yet!) to be processed
> when it has done its setup.
> 
> This seems awkward. Parallel sockets break the total and causal ordering
> properties that D-Bus normally has (Telepathy on Maemo devices had some
> remarkably hard-to-track-down bugs as a result of this), while saving up
> method calls for later is going to be difficult to implement in APIs that
> conserve memory and file descriptor quota by having a process-wide shared
> socket per bus (like libdbus), particularly if that socket is dispatched in
> a worker thread (like GDBus).

It is true that a second "setup socket" would be needed in the general case (though probably not in the common case). And it is true that messages on the two sockets are not ordered against eachother (obviously). However, name activation already breaks total ordering (messages sent to an activatable name before the name has been requested by its eventual owner are not ordered against other messages delivered to the same client (not messages to other yet-to-be requseted activatable names, nor directly to the client). My understanding is that having a separate setup socket does not make anything worse, but makes the (still bad) situation slightly better in two ways: messages delivered to separate activatable names are now always correctly ordered against eachother, and when and how the ordering is broken is made clearer and at least explicit by the fact that there are two sockets.

> Might it be better for this to be a list of names for which, if/when the
> client requests them, authorization decisions are made with the credentials
> of the ConnectClient() caller instead of those of the client itself? That
> lets the client continue to use the familiar "do setup, export objects,
> request name" flow.

This would be possible, and would give the same semantics as now, but as argued above I don't think that's necessarily a good thing. If anything we should make it clear how the ordering is broken, so people are at least aware of the problem.
Comment 30 Tom Gundersen 2018-07-23 15:23:18 UTC
(In reply to Simon McVittie from comment #22)
> (In reply to Tom Gundersen from comment #0)
> > Currently, if a name is activatable, it is always activatable. There is no
> > way of delaying the availability of a name, or making a name no longer
> > activatable. Short of delaynig the start-up of the whole bus, or shutting
> > the bus down.
> 
> ListActivatableNames() doesn't have any change-notification, so if names are
> to be dynamically activatable-or-not, it would need to grow
> change-notification.

Note that my proposal does not use activatable names at all. When ConnectClient() is called, the names are considered active (with the associated NameOwnerChanged signal emmitted), and the actual client is started lazily using regular socket activation. So it is an alternative to activatable names, not an extension.
Comment 31 Tom Gundersen 2018-07-23 15:32:27 UTC
(In reply to Simon McVittie from comment #28)
> (In reply to Tom Gundersen from comment #2)
> > My general feeling about this is that we should try as much as possible to
> > keep things simple and in line with the socket-activation ideas, and the
> > more DBus concepts that become redundant, the better (activation of names,
> > handover of names, etc.).
> 
> How would you implement, for example, `gnome-shell --replace` without the
> ability to hand over well-known names?
> 
> Typical system services don't need queuing and replacement of well-known
> names, but it's a feature, and is used for things like D-Bus name ownership
> as distributed mutex.

I think queuing and handing over names has not turned out to be a tremendously useful feature that's come at a quite considerable conceptual and technical cost, so my argument would be that we should not design new APIs for this usecase. Obviously we don't want to break backwards compatibility, but the current APIs will continue to serve the current users just fine.
Comment 32 GitLab Migration User 2018-10-12 21:35:58 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/dbus/dbus/issues/219.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.