launcher: start unit immediately as unprivileged user

[bluca]

It is better to never have privileges rather than start with them and remove them later, as the attack surface is reduced, and there are fewer things to do before being 'ready'. Nowadays systemd can run the service as the appropriate user/group out of the box.

When starting as root files in /proc/self/fdinfo/ will be owned as root and set to 400, so we cannot read them. Nowadays it is not necessary to start as root when running under systemd, so just add User/Group with the configured user to the system unit. Add a meson option to let users configure the user, and default to the same as dbus-daemon's default, 'messagebus'.

If libaudit support is enabled, add AmbientCapabilities=CAP_AUDIT_WRITE so that we can still write to the audit log.

Same change for dbus-daemon: https://gitlab.freedesktop.org/dbus/dbus/-/merge_requests/399

[dvdhrm]

I would actually prefer to always set the user and let distributions strip the lines from the files if they decide against it (is there precedent? Does anyone not want this?). Similarly, I would always include CAP_AUDIT_WRITE, but I am fine with your approach as well.

The only issue I have is that if the configuration contains a user= line, the daemon will fail changing to that user. And since we do not ship a dbus-configuration file, but use the reference configuration, this is a bit of an issue. Furthermore, are you sure this works with NSS? So far, the launcher has been running as root to ensure compatibility with existing NSS modules.

[bluca]

I would actually prefer to always set the user and let distributions strip the lines from the files if they decide against it (is there precedent? Does anyone not want this?). Similarly, I would always include CAP_AUDIT_WRITE, but I am fine with your approach as well.

Yeah I think everyone uses the same, and if not adding a drop-in is trivial, so I've dropped the new meson option. I've left the ambient caps setting conditional on the build time audit option, as I think it's nice to add the additional permission only if it can actually be used, and omit it otherwise, least privilege principle and all that.

The only issue I have is that if the configuration contains a user= line, the daemon will fail changing to that user. And since we do not ship a dbus-configuration file, but use the reference configuration, this is a bit of an issue.

I'd argue that's a configuration mistake - if the user= in the config file and the User= in the unit are not configured to be the same, then it's a packaging/build problem. The unit can be trivially adapted with a drop-in. There's immediately an error on startup, so it should be hard to miss.

Furthermore, are you sure this works with NSS? So far, the launcher has been running as root to ensure compatibility with existing NSS modules.

What do you mean, specifically? I tested this in a Debian testing VM and didn't see any issue (including logging audit messages), but if there's something specific you want me to try I can do that

[bluca]

FYI, corresponding change was merged in dbus-daemon: https://gitlab.freedesktop.org/dbus/dbus/-/merge_requests/399

[teg]

I like this very much!

If the logic is that we respect the systemd unit and fail if the dbus config don't match, couldn't we check/fail early and delete a bunch of code that deals with dropping privileges?

[bluca]

I can add a check as soon as the config is parsed yes. Regarding the other point, it depends on whether you want to retain the optional ability to start as root or not. If you don't, then I can remove it.

[teg]

I don't see the point of keeping the ability to start as root if the service files we ship anyway won't do that. But maybe I'm missing something... I'll let @dvdhrm comment...

[bluca]

I agree, the lesser code around to deal with privileges the better, but I'll leave the decision to you folks

[dvdhrm]

With this change, the dbus-connection used by the launcher will no longer be annotated as root. Hence, this will lack privileges to start transient units, right? And it will trigger polkit-queries from systemd, and thus can lead to deadlocks.

I also don't think we can start regular units, but I am not sure how systemd performs priv-checks for those.

I didn't test this, but I don't think this works on the system bus, or am I missing something?

[bluca]

ooft you are indeed right, in the images where I was testing all this I was just starting polkit with additional debugs so I did not notice. I'm not sure how to fix this, perhaps before asking polkit on StartUnit&friends we could special case it to see if the request is coming from dbus[-broker].service and auth it? We can reliably tell with the pidfd stuff, and we have a few other special-casing for dbus anyway. But it would need gradual rollout.

Any other suggestion?

[dvdhrm]

I mean, we do this privilege-drop-delay explicitly to ensure everything is set up with the correct permissions. Not sure any alternative would make this scenario simpler. Somehow a root-process needs to call socket() + connect() to ensure the privileges are correct.

dbus-daemon also requires root to spawn non-systemd units, but I think they avoid this by using a setuid-helper. We could do something similar, but it feels more brittle than the current way.

[bluca]

Yeah it uses setuid, but I very very much prefer to avoid that, it wouldn't be worth it.

But there's still one bit I am not following: the priv drop when system.conf sets the user to messagebus (ie, pretty much always) is done immediately on launcher startup before going into the main loop, and after fork/before exec of the broker. I can see both processes running as messagebus. How is it able to start units then?

[bluca]

Ah found it - because GetConnectionCreds still says it's running as root

[dvdhrm]

I think one strategy to solve this is to add polkit-rules that allow the dbus user to run StartTransientUnit() and StartUnit(). I don't think we need any other privileges. However, being able to run StartTransientUnit() seems equivalent to root-privileges.

Yes, I see the advantage in running as non-root, even if we have enough privileges to spawn root-shells. But given that we drop privileges so early, this is very low-priority for me.

If someone wants to implement this, please go ahead! I am going to close this for now, since this implementation cannot be merged as it is. But If you want to pick this up, please feel free to re-open.