nixos/polkit: don't enable by default#156858
Conversation
github-actions
bot
added
6.topic: nixos
|
@ofborg test gnome gnome-xorg xfce pantheon paslam5 plasma5-systemd-start dnscrypt-wrapper udisk2 prometheus-exporters.modemmanager xmonad lightdm A bunch of things that rely in same way on polkit. |
I guess it would be sane to mkDefault enable polkit in |
ofborg
bot
added
10.rebuild-darwin: 0
|
Or remove the assertion? The commit that introduces it mentions that the xession script calls |
|
Note that systemd itself uses polkit so we should test if it works without it. |
|
Flatpak also needs polkit for escalation prompts of apps. |
Looks like so: instead of this: |
I don't see our module installing polkit rules though. |
mweinelt
force-pushed
the
polkit-mkenable
branch
from
7d52c9a to
fb6b10f
Compare
|
@primeos Not sure about the fix for the sway test. I don't see that sway strictly needs polkit, but the tests are not working without it. But maybe it's something in the tests requiring it? |
mweinelt
force-pushed
the
polkit-mkenable
branch
from
fb6b10f to
a16f9f1
Compare
|
I'm now pretty sure that wayland requires polkit (to access loginctl, the tty, the DRI), so adding it to sway and cage is in fact correct. |
Right, it shouldn't be required for Sway itself. It looks like it might be required for some logind functionality though (doesn't systemd in general rely on Polkit btw? It's a while ago that I peeked into that part but IIRC there are quite some connections between the two, unfortunately (I didn't like it back then either so huge thanks for looking into disabling it!) - ideally Polkit is optional though). I've dumped Sway's output and got this: The problem should be: ( So the problem shouldn't be specific to Sway (dependency wise it's quite nested: sway -> wlroots -> seatd -> systemd/logind). |
|
Ok, so regarding systemd: From a quick peek into the source-code it luckily seems to be optional :) We might be able to avoid these issues by building systemd without polkit support: A few relevant excepts from the release notes to figure out which components/features might be impacted: There are also things like https://github.com/systemd/systemd/blob/5efbd0bf897a990ebe43d7dc69141d87c404ac9a/src/network/systemd-networkd.rules (a complete grep through the source-code might be a good idea). |
No biggie, I think we're fine with enabling polkit in
This is also my experience from Debian setups that runs completely fine without polkit. |
mweinelt
force-pushed
the
polkit-mkenable
branch
from
ce59cec to
1e7fdb7
Compare
github-actions
bot
added
8.has: changelog
|
Added a short mention to the release notes. |
This 85898da is probably the commit that dropped it. |
SUID wrappers really shouldn't be enabled by default, unless a consumer relies on them. So in my opinion this falls upon the desktop environments if needed or a user to explicltly enable this if wanted. Most desktop environments and services like CUPS already enable polkit by default, that should really be sufficient.
Allows user in the networkmanager group to control the daemon.
|
I agree that we don't want to support multiple systemd flavors, and I'm not sure we need to, because systemd seems to handle missing polkit at runtime just fine. I checked several different distros and polkit is optional in their respective systemd packaging, so we're hardly creating a precedent here.
My servers also don't have udisks2 enabled, which at this point also gets enabled by several desktop managers. It's a common scenario to mount your USB devices manually, it's not a biggie really. Most of the machines I deploy are also VMs, where I couldn't even deploy a USB stick easily. Also for most of my server a simple truth is that they don't have any unprivileged interactive users, so I don't see the point in having a privilege elevation framework available, that no deployed service should ever be able to use. |
I don't believe that is part of the fight I'm willing to fight in this PR. I recognize that polkit is useful in setups with interactive usage, especially desktop systems, which is why I added a polkit enabler to sway in Does that make sense from your POV? |
Not not at all tbh. Either you're deliberately ignoring facts or I must somehow be speaking a foreign language or something like that.
We have multiple tests that prove this wrong. This should be considered a systemd bug (unless it's due to our patches, configuration, etc.) but that doesn't automatically make it go away. Someone has to investigate it and report it to upstream. I haven't spend much time on this but the results I posted earlier should be reproducible and clearly prove that systemd doesn't always handle missing Polkit fine at runtime (something in the logind code must be wrong - it shouldn't be too difficult to investigate that but it'll probably require a bit of extra time).
In case that wasn't clear from my previous comments: I don't rally want Polkit on my system (although disabling it can also make security worse because applications like systemd then have to implement some/more of the privilege escalation on their own) and I shouldn't need it (in theory). So I'd welcome this change but that isn't what my comments were about (I just want to clearly state here that I'm not against disabling Polkit). Sway does not require Polkit. Wayland does not require Polkit.
No. The VM test enables neither of them. The output clearly indicates that systemd/logind is the issue and not something else.
You can now either accept those facts and investigate accordingly or you can ignore them and this whole discussion will become pointless (and in that case I'll also refrain from responding as it simply makes no sense at that point). Edit: I took a very brief look into the systemd source-code out of curiosity. The following looks like the relevant bit (the name |
|
tl;dr:
Your expectation of "it should work" is wrong: systemd/logind does handle a missing polkit dependency gracefully: Access Denied for everything. Since polkit is basically systemd's only way for privilege escalation, that is the only sane behavior. Otherwise you could gain privileges via systemd/logind if the polkit service down. (You can't just allow everyone to do everything when your authorization server is down, or the case we are talking about: not installed) So why does logind "just work" when it was compiled without polkit support? You've already found the relevant code in logind: it reads: "if polkit is disabled, we have absolutely no way to check your privileges, so we'll just hand them out to you". This is a rather ugly (insecure?) compromise to make graphical sessions work with logind and no polkit support (to be fair: this is the default polkit rule).
sway/wayland does need some privileged permissions (i think the DRM stuff, but I'm not too informed about that), so we do need some kind of permission granting. Choice one of
So the last case it does have an indirect dependency on polkit: sway -> wlroots -> libseat -> logind (with polkit support) -> polkit. To re-iterate myself: The working logind-without-compiled-support-for-polkit case is imho more of an half-hearted compromise than a solution. What is definitely not a bug: to error out with access denied when your only means for privilege escalation is not unavailable. The (imho proper) solution is to just add polkit as dependency to wlroots based wms (and everything else that depends on logind). I don't see any way to have polkit-less GUI systems without a "split" between systemd with polkit and systemd without polkit. Polkit-less servers on the other hand should be feasible: AFAICT this PR already does most (everything?) to achieve that.
Assuming that we only support systemd with polkit support enabled (current status quo), polkit itself will still be optional for a lot (server/headless) installations. Except for some logind features there is nothing i'm aware of in systemd (with polkit support) which really needs polkit. Setting Pretty much all applications (excluding the logind ones) can deal with missing/unavailable polkit: It is almost the same as a permission deny, and if they misbehave anyway, it is an upstream bug and shouldn't be treated differently than any other upstream bug. I don't think this should be factored into the decision about the support for polkit-less installations. The actual question is whether the effort to identify and keep track of all the services which require polkit (optionally or in case of logind dependencies mandatory) is worth of making polkit optional. AFAICT most packages (which do not depend on polkit via logind) already keep indirectly track of the polkit dependency by adding polkit rules if |
Yes, I could've known better but I wrote that before I looked at the code and it was based on that excerpt:
For the case where polkit does the same, by default, as you mention.
Sure. My point was just that they do not require or depend (code-wise) on Polkit.
That's the issue I'm talking about, yes.
THIS is the problem (that last part). We're not talking about wlroots or Wayland here. This "regression" isn't specific to them. It (potentially) affects every desktop system.
Agreed. The problem is that this PR seems to focus on servers but the change isn't limited to them (via something like a server profile).
One problem is that we have a lot of software (probably mainly Wayland compositors) that doesn't come with a NixOS module (e.g. River). To me it seems unlikely that we'll catch all cases so at least some regressions/breakage will be left (might be acceptable though but I don't want to encourage regressions).
Agreed. That should be time consuming, difficult to test/verify, and currently it looks like there isn't enough motivation (which is why a server profile or workaround might be better for now).
Our tests would hopefully catch other important cases (e.g. it might be required for Anyway tl;dr: I like the idea behind this PR (especially disabling polkit on servers) but it seems like a (over)reaction to the recent polkit CVEs instead of a carefully considered / thought through approach (which is why I felt obliged to interject and spoil the fun as I've seen too many PRs like this that got merged too fast and caused severe regressions that annoyed users). PS: I'm sorry if something sounds aggressive/offensive or is wrong. I just wrote this down very quickly due to lack of time/priority as some of my earlier statements may have come off wrong. |
|
I'm reading all of this and the question keeps getting louder: where are things heading? Is there upstream appetite in systemd/logind or further moving away or avoiding from polkit? What if systemd's behavior were more easily controlled at runtime? wlroot's support for non-logind backends seems to indicate there's support for... avoiding certain components, maybe there's other efforts around this area? It just feels like there might be a few options that involve some work outside NixOS that would give us better choices, and the wlroots ecosystem seems to move fast, and systemd can move fast on things too. Having wrapped my head around the implications of package splits via Also, can we explicitly list out the benefits of achieving polkit-less "server" builds vs just splitting off |
|
I think if we enabled polkit when Once we (and upstream projects) have figured out who actually needs it, we can make it more fine-grained. On non-GUI setups, the worst disabling polkit does is make systemd tools unable to escalate when there is insufficient privilege, right? |
I don't see systemd moving away from polkit, and the current behaviour seems sane to me. The non-logind backends statically grant the necessary permissions to wlroots - there isn't that much from the wlroot side to support, it just needs something that will give it access to KMS/DRM.
Less software to install + no possible privilege escalation: neither through bugs, nor through misconfiguration of component (which is not needed in the first place)
Which is not intuitive if you are running a wayland only environment:. Those would currently affected most: Many rely either directly or indirectly on logind. Splitting packages in nixpkgs to have a polkit and polkit-less variant is imho not feasible. So that leaves only two practical options:
Option 1 does need some work though: Not all packages which require polkit have a module (e.g. "River" was mentioned earlier) and it would be a breaking change for users who depended on the automatic inclusion of polkit. As it is this patchset is currently not enough, because it only works for packages which are somehow represented as a module.
Well, there are some applications, which (optionally) use polkit and technically do not require any GUI. CUPS is the first thing which comes to my mind. |
Either way, enabling any wayland compositor should enable wayland support which should enable polkit. |
|
I don't think this pull request necessarily needs to be complete, as the set of modules we need to touch is unclear and hard to determine. We need to take care of the things we know now and find out the hard way when this goes out to people via the unstable channel. |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
|
Unless somebody has some strong objections that haven't been addressed before, I'm inclined towards merging this. |
|
Should we gate the "default-off" behavior on stateVersion? e.g. "if stateVersion is newer than XX.YY, default = false, else, default = true" Otherwise, if people rely on polkit but don't use one of these small set of modules that have had the |
I believe that'd go contrary to the entire point of unleashing this upon nixos-unstable users. |
|
I'd be against gating this on the |
This was enabled by default in 18a7ce7 with the reason that it would be "useful regardless of the desktop environment.", which I'm not arguing against. The reason why this should not be enabled by default is that there are a lot of systems that NixOS runs on that are not desktop systems. Users on such systems most likely do not want or need this feature and could even consider this an antifeature. Furthermore, it is surprising to them to find out that they have this enabled on their systems. They might be even more surprised to find that they have polkit enabled by default, which was a default that was flipped in a813be0. For some discussion as to why see NixOS#156858. Evidently, this default is not only surprising to users, but also module developers, as most if not all modules for desktop environments already explicity set services.udisks2.enable = true; which they don't need to right now.
The udisks2 service was enabled to fix the test in (c5ebec7). However, cagebreak doesn't require udisks2, just polkit (which the udisks2 module enables and which is why the cagebreak test broke after the udisks2 module was disabled by default). I've documented why polkit is required in this PR: NixOS#156858 In this case the "dependency" chain is basically cagebreak -> wlroots -> libseat -> logind (with polkit support) -> polkit.
The udisks2 service was enabled to fix the test in (c5ebec7). However, cagebreak doesn't require udisks2, just polkit (which the udisks2 module enables and which is why the cagebreak test broke after the udisks2 module was disabled by default). I've documented why polkit is required in this PR: NixOS#156858 In this case the "dependency" chain is basically cagebreak -> wlroots -> libseat -> logind (with polkit support) -> polkit.
14 participants
Motivation for this change
SUID wrappers really shouldn't be enabled by default, unless a consumer
relies on them. So in my opinion this falls upon the desktop
environments if needed or a user to explicltly enable this if wanted.
Most desktop environments and services like CUPS already enable polkit
by default, that should really be sufficient.
Things done
Successfully ran the following tests:
Working after dropping the polkit assertion in x11server.nix
Unrelated failures