Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFE: API to acquire a connection fd from dbus-broker with pre-applied bus names and policies #259

Open
poettering opened this issue Feb 17, 2021 · 11 comments

Comments

@poettering
Copy link

I'd like to add support for a ".busname" concept in systemd, where people can declare a dbus service name to acquire as unit file, and it would list the full service name plus the matching policy. PID 1 would then parse that, and pass this to dbus-broker via some driver API, and get an fd back that can be used via socket activation.

The would fix a bunch of issues for us:

  1. Solve the reordering mess when bus names are acquired that already have queued messages
  2. Fixes issues with exit-on-idle: messages would remain enqueued on the bus connection while the service goes down, and it may continue where it left of — except if half-read messages remain
  3. provide compatibility with the DynamicUser= concept of PID 1, as well as the "portable service" concept, as policy could be attached to the unit file itself

I'd suggest to drop the SASL and feature negotiation phase for sockets of this kind, i.e. sockets come readily set up for the main protocol of D-Bus. feature negotiation would have to take place in the unit files too, I guess.

Idea: dbus-broker's driver service would gain:

RegisterActivationService("sauaubas") → "has"

input:

 s → name to acquire
 au → list of uids that shall get access to this service
 au → list of gids that shall get access to this service
 b → boolean that decides if anyone else gets access
 as → feature flags requested ("unix-fd", …)

output:

  h → file handle of connection
  as → agreed on feature flags ("unix-fd", …), subset of same array of input
@poettering
Copy link
Author

(one more idea: maybe generate SOCK_SEQPACKET sockets this way, so that half-queued datagrams can't remain)

@bluca
Copy link
Contributor

bluca commented Feb 17, 2021

Previous discussions on the systemd bug tracker:

systemd/systemd#9503

and dbus-daemon:

https://gitlab.freedesktop.org/dbus/dbus/-/issues/219

@poettering
Copy link
Author

ah, nice, forgot where this was discussed before...

Anyway, maybe we can agree on something in dbus-broker, and then later standardize this and add it to the spec or so, after figuring out how and if things work. i certainly would be willing to provide the .busname side in pid 1, and the sd-bus client side to take over the connections.

@poettering
Copy link
Author

/cc @smcv

@dvdhrm
Copy link
Member

dvdhrm commented Feb 17, 2021

Few comments:

  1. SOCK_SEQPACKET has the problem of maximum packet size. I don't think it is suitable for D-Bus.

  2. If the broker creates the UDS socket, it will be created with its credentials. This means all those FDs will be accounted on the user of the broker. I would much rather prefer if the caller creates the socket and passes it in. Otherwise, you can easily DoS the broker.

  3. I would make the argument an array of names+properties, so you can claim multiple names on one socket atomically. On the systemd-side, this can be tricky to merge multiple .busname units into a single socket which is then passed to a unit. Not sure what the implications on the systemd side are, but I would prefer if the API allows it.

@poettering
Copy link
Author

1. `SOCK_SEQPACKET` has the problem of maximum packet size. I don't think it is suitable for D-Bus.

if you get EMSGSIZE on sendmsg() allocate memfd, and send empty dgram with only a memfd in SCM_RIGHTS.

@poettering
Copy link
Author

2\. If the broker creates the UDS socket, it will be created with its credentials. This means all those FDs will be accounted on the user of the broker. I would much rather prefer if the caller creates the socket and passes it in. Otherwise, you can easily DoS the broker.

This would mean, basically all are credited to PID 1. Which isn't much better, is it?

@dvdhrm
Copy link
Member

dvdhrm commented Feb 18, 2021

  1. If the broker creates the UDS socket, it will be created with its credentials. This means all those FDs will be accounted on the user of the broker. I would much rather prefer if the caller creates the socket and passes it in. Otherwise, you can easily DoS the broker.

This would mean, basically all are credited to PID 1. Which isn't much better, is it?

It would mean the caller controls what they are credited to. dbus-broker runs as unprivileged user, so it will never have any chance to account them on anything but itself. But the caller is free to create those sockets as they wish.

I mean, I don't mind much if systemd ignores that matter and just creates the sockets as root user. But at least the API allows us to improve on this in the future. If we wouldn't pass the socket it, we would be locked to the current behavior.

Is there any downside to passing the socket in, rather than passing it out?

  1. SOCK_SEQPACKET has the problem of maximum packet size. I don't think it is suitable for D-Bus.

if you get EMSGSIZE on sendmsg() allocate memfd, and send empty dgram with only a memfd in SCM_RIGHTS.

Ah, like the journal-API you mean. Right, that would work. You still run into the same EMFILE / ENFILE (I can never remember which one is per-user, and which per-process) issue, because the other side could just start hoarding FDs, thus triggering a DoS in the sender.
I mean, this is a stupid issue and probably needs a better fix in the kernel, so maybe we can just ignore it. It feels really wrong to not be able to move resources across non-trusting user boundaries.

@poettering
Copy link
Author

poettering commented Feb 18, 2021

hmm so linux AF_UNIX will actually send a new SCM_CREDENTIALS message whenever the sender on an AF_UNIX/SOCK_STREAM socket changes. We use that in the journal for attributing output on an stdout/stderr stream that is written to by multiple processes (e.g. shell script with its subprocesses) back to the originating processes. Maybe a simple solution would be to rely on that: always charge against the current SCM_CREDENTIALS data (and in absence of any SO_PEERCRED). This would then mean that initially PID 1 would be responsible, until the first byte is written, in which moment ownership would be switched over.

@poettering
Copy link
Author

(btw, we seriously bumped RLIMIT_NOFILE by default for all userspace now (at least the hard limit, soft limit we can't because select() is stupid), i.e. all processes spawned by systemd, hence the memfd exhaustion thing might not be as easy to trigger — though it of course still exists)

bluca added a commit to bluca/systemd that referenced this issue Dec 28, 2022
Due to policy checks against system users this cannot currently work, and it is non-obvious.
In the future it might be implemented if support is added to dbus-broker/dbus-daemon, e.g.:

bus1/dbus-broker#259
bluca added a commit to bluca/systemd that referenced this issue Jan 3, 2023
Due to policy checks against system users this cannot currently work, and it is non-obvious.
In the future it might be implemented if support is added to dbus-broker/dbus-daemon, e.g.:

bus1/dbus-broker#259
bluca added a commit to bluca/systemd that referenced this issue Jan 4, 2023
Due to policy checks against system users this cannot currently work, and it is non-obvious.
In the future it might be implemented if support is added to dbus-broker/dbus-daemon, e.g.:

bus1/dbus-broker#259
poettering pushed a commit to systemd/systemd that referenced this issue Jan 4, 2023
Due to policy checks against system users this cannot currently work, and it is non-obvious.
In the future it might be implemented if support is added to dbus-broker/dbus-daemon, e.g.:

bus1/dbus-broker#259
d-hatayama pushed a commit to d-hatayama/systemd that referenced this issue Feb 15, 2023
Due to policy checks against system users this cannot currently work, and it is non-obvious.
In the future it might be implemented if support is added to dbus-broker/dbus-daemon, e.g.:

bus1/dbus-broker#259
@poettering
Copy link
Author

btw, my current thinking about this, is that pid 1 allocates a socket, then connects it to the broker, does some minimal handshake, claims names, sets policies, and then waits for POLLIN. But the originally suggested method call would work too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants