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

Some Elogind actions bypass Polkit rules #149

Closed
Acatorn opened this issue Jan 20, 2020 · 7 comments
Closed

Some Elogind actions bypass Polkit rules #149

Acatorn opened this issue Jan 20, 2020 · 7 comments
Assignees

Comments

@Acatorn
Copy link

@Acatorn Acatorn commented Jan 20, 2020

Hello,
First of all - many thanks for your hard work on Elogind.
To the point, I think it might be better if I put a link to my recent post on the Gentoo forum here:
https://forums.gentoo.org/viewtopic-t-1107000.html
You can find many details there. In short, commands like:

loginctl poweroff
loginctl reboot

seems to be ok, and they work properly with polkit rules. Commands like:

loginctl suspend
loginctl hibernate
loginctl lock-session

completely ignore the polkit rules.

I'm not sure if it's Elogind issue but I think it may be (other applications, like gparted, works correctly with polkit on my machine, after all). I did some tests on different distros and here we go:
Centos 7 - polkit 0.112-22, systemd 219.67 - everything works properly
Voidlinux - polkit 0.116_3, elogind 241.4_1 - same problems as on my Gentoo machine
Artix - polkit 0.116-3, elogind 243.4_1 - ALL loginctl actions ignore polkit rules, total disaster;)

Best Regards,
Acatorn

@Yamakuzure

This comment has been minimized.

Copy link
Collaborator

@Yamakuzure Yamakuzure commented Jan 21, 2020

Some do indeed.

Example: If you are a regular user and the only user logged into your machine, pressing the suspend key on your laptop does not trigger a question for your password. If an inhibitor, like root being logged in on any tty, is present, you simply can't suspend.

Example: I have logged in as root on tty2

 ~ $ loginctl suspend
User root is logged in on tty2.
Please retry operation after closing inhibitors and logging out other users.
Alternatively, ignore inhibitors and users with 'loginctl suspend -i'.

When I try the mentioned "-i" option, the system suspends nevertheless without asking for a password. This way lid-close-events can be used to suspend the system and won't leave your laptop running (and heating) in your backpack just because you forgot to log out root somewhere.

Same is with poweroff. You are the ruler of your system and must be able to power off your system.

Does MacOS ask you for your password when hitting "poweroff"?
Does Windows ask for you password when hitting "poweroff"?

Those three commands are meant to work when issued by a regular user.

btw: Neither the src/login/*.rules nor src/login/org.freedesktop.login1.policy has any significant difference between systemd-243.4 and elogind-243.4.

So much for the basics and defaults.


In the gentoo forum post you wrote that you explicitly wrote polkit rules so that your particular user has to be asked for a password.

Well, this ought to work for reboot and poweroff, but elogind explicitly does not check extra rules for suspending and hibernating. This really is a difference to systemd, and that is so since June 2018.

The reason we removed any extra check but against inhibitors was the questioning of a password when hitting the suspend key on a laptop or closing its lid. It just didn't make any sense.

As far as I can see, the default rules haven't changed. So any user wanting to instantly suspending their system would have to learn to write custom polkit rules.


A possible solution might be to re-add the questioning of the rules while changing the defaults. Without changing the defaults, we'd end up breaking many user setups.

However, I can see why someone would not want any regular user to be able to suspend a multi-user system in an office or something like that.
Well, this was never much of (at least) my concern. I simply can't imagine professional system administrators in corporations to set up anything but commercial distributions like RedHat or SuSE. 🤷‍♂

...maybe it's about time to thinks again?


@markhindley @wingo @kensington @maxice8 @Cogitri
What do you guys think?
I find this detail quite interesting, and it has much to consider.

Personally I think it should be possible for polkit rules author to change the behavior when a user wants to suspend the system.

But how so? Change the defaults so it becomes a (working) opt-in, or leave the defaults like systemd so it becomes a then needed opt-out?

@Cogitri

This comment has been minimized.

Copy link
Contributor

@Cogitri Cogitri commented Jan 21, 2020

I personally I don't have a problem with the current behaviour since I only have single user systems and I don't think many people would want to restrict this, so making it working by default but making it possible to restrict access via polkit seems like the best way to go about this (it certainly is better to allow users to hibernate the system by default than my laptop going into nuclear fusion mode in my backpack :P)

@Acatorn

This comment has been minimized.

Copy link
Author

@Acatorn Acatorn commented Jan 21, 2020

Thank you very much guys for looking into it:) Yeah, nuclear fusion in a backpack is certainly not a good thing:)
After installation of elogind with polkit USE flag enabled the file /usr/share/polkit-1/actions/org.freedesktop.login1.policy gets created. It contains the default behaviour and It's perfectly fine for normal home (single) user:) In my opinion it should stay like that because I do admit that my case of Gentoo based virtual desktops in corporate production environment is probably a minority case;)

As you can see it's not a problem for me (nor it should be for anyone familiar with Polkit) to write a rule that override the defaults. I only need a mechanism to work properly. Because, in current state, even if I change values directly in /usr/share/polkit-1/actions/org.freedesktop.login1.policy (not a good practice to change that file, it's better to write a .rules file instead) to ask everyone for a password when doing 'loginctl suspend' - it is ignored too.

To get crystal clear - I know the many people mind about Polkit. And I agree in regards to personal computer with single user - you are the administrator of your own PC. I also think that Polkit is mostly useless on a server (even in large corporate environment like mine) and that is one of the reasons why I'm trying to avoid RedHat and SUSE on my servers - Polkit is installed there by default and deleting it is not a trivial task. Sudo is sufficient on my servers. But virtual workstations is totally different thing - the admins for thousands of such workstations would be myself and my associate. We do not want users to be able to poweroff/reboot/suspend/hibernate those machines, it would make our job more difficult;) Yet, we still want to use Polkit to grant/deny certain users certain access to some GUI apps.

Wow, another wall of text from me;) I hope I wrote it in easy to understand way (English is not my native language). If I can be of any help in making Elogind fully compatible with Polkit I am more than eager to help:)

@Acatorn

This comment has been minimized.

Copy link
Author

@Acatorn Acatorn commented Jan 24, 2020

Thank you @Yamakuzure for highlighting that issue with that pretty colorful labels:) I just wonder if there are a lot of things to change in the source code to make it work. I'm just a weakling in coding myself but maybe you could point me in right direction? Am I right that there is something to change (uncomment maybe) in /src/login/logind-dbus.c ? Would it be sufficient? Sorry for asking, I just really care about it to make all the org.freedesktop.login1.* actions work properly with polkit (like logind in systemd) :) If that matter is more complicated then I will patiently wait for any news:)

@Yamakuzure

This comment has been minimized.

Copy link
Collaborator

@Yamakuzure Yamakuzure commented Jan 27, 2020

@Acatorn No, there isn't much to change, but to test.
However, as @perfect7gentleman has found issue #150 which really should be fixed ASAP, I'll add this, too.
So the next service release will not only have that issue fixed, but also the checks for suspension re-enabled, making your custom rules do what they are meant to do. 😉

@Acatorn

This comment has been minimized.

Copy link
Author

@Acatorn Acatorn commented Jan 27, 2020

@Yamakuzure Thank you very much sir!:) Looking forward to test it:)

@Yamakuzure Yamakuzure closed this in b187de7 Feb 4, 2020
Yamakuzure added a commit that referenced this issue Feb 4, 2020
When suspending a machine using elogind, the checking off the credentials
was removed to circumvent asking (privileged) users for their password
when hitting the sleep key on their laptops or when closing the lid.

This has the unfortunate side effect to also circumvent any custom policykit
rules an administrator might add to their system to limit the circle of users
being allowed to put the machine to sleep.

Since the check was masked, the mechanics of credential checking has evolved,
and it is no longer neccessary to mask the credential check. Whatever lead to
elogind asking for authorization on machine suspension is fixed in all tests.

Bug: #149
Closes: #149
Signed-off-by: Sven Eden <sven.eden@prydeworx.com>
Cherry-picked-from: b187de7
@Yamakuzure

This comment has been minimized.

Copy link
Collaborator

@Yamakuzure Yamakuzure commented Feb 4, 2020

I have removed the extra check. In my tests it looks like the masking is no longer needed.

Whatever lead to elogind asking for extra credentials on every attempt to suspend my laptop, must have been something that was fixed in the meantime. Even with re-enabled check I could suspend my machine without any changes to the default policykit rules.

@Acatorn: Your custom rules should work with the mentioned commit, which I also cherry-picked into v243-stable branch.

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

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.