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

Too strict Polkit auth_self_keep authorization requirement? #544

Closed
mgerstner opened this issue Mar 29, 2022 · 10 comments · Fixed by #545
Closed

Too strict Polkit auth_self_keep authorization requirement? #544

mgerstner opened this issue Mar 29, 2022 · 10 comments · Fixed by #545

Comments

@mgerstner
Copy link

In the Polkit policy the read-only operations are guarded by auth_self_keep authorization requirements.

Is this adding any security / was this a conscious decision or would it be also okay to grant yes for locally logged in sessions?

@hartwork
Copy link
Contributor

Hi @mgerstner my work in pull request #531 was meant to preserve the existing threat model. We'll need someone else to shed light on the original threat model and its intentions. Is this about all read-only methods/actions or about a particular one?

@mgerstner
Copy link
Author

Hi, it is about all occurences of auth_self_keep in the Polkit policy. Looking only at the Polkit level this doesn't seem to really add any security, except maybe a hardening. In case of a rogue process or on a physically hijacked machine the attacker would then still need to enter the user's password. Apart from that the setting only reduces usability.

We are currently integrating the D-Bus bridge into the openSUSE packaging of usbguard and a colleague complained about these auth_self_keep settings that he deems unnecessary. I just wanted to check back if there was any special intention behind these settings.

@hartwork
Copy link
Contributor

@mgerstner I understand. @radosroka what do you think?

@radosroka
Copy link
Member

Well security/hardening is always against usability. It's up to you/admin how to find a balance between these two. I personally don't have a problem with writing a password even for read-ops. If you feel like the benefits of password less operation will overweight the security concerns then you can use it as you wish.

@mgerstner
Copy link
Author

We will naturally do so. I just wanted to make sure there is not some special consideration behind this that I don't know of, since auth_self is rather unusual - for the active user at any rate.

mgerstner added a commit to mgerstner/polkit-default-privs that referenced this issue Apr 1, 2022
The auth_self_keep setting is unusual and not user friendly, offering
little benefits security wise. Upstream does not seem to have a special
reason for this:

USBGuard/usbguard#544 (comment)

So let's relax these settings to 'yes' instead.
@muelli
Copy link
Contributor

muelli commented Apr 10, 2022

yeah, this is probably becoming problematic for GNOME users, too.
https://gitlab.gnome.org/GNOME/gnome-settings-daemon/-/issues/676

Can we get to a point where the currently active user can list the policy and authorise (or deny) devices?

Well security/hardening is always against usability

I'd like to disagree somewhat. Our goal should be to unify security and usability. To the point that users don't even notice how the protection mechanism unfolds its powers. A thing that's most secure but cannot be used by anybody doesn't increase the overall protection. In that sense, requiring all downstreams to patch the defaults in order for the thing to be actually useful, is a waste of energy.

I feel reminded of another discussion that also balanced usability and security: #246 (comment)

@hartwork
Copy link
Contributor

hartwork commented Apr 10, 2022

@radosroka would you accept a pull request replacing auth_self_keep by yes three times for read-only methods listDevices, listRules, getParameter?

@mgerstner is this about all three of them^^ or which ones?

@mgerstner
Copy link
Author

@radosroka would you accept a pull request replacing auth_self_keep by yes three times for read-only methods listDevices, listRules, getParameter?

@mgerstner is this about all three of them^^ or which ones?

Yes all three of them.

@radosroka
Copy link
Member

@radosroka would you accept a pull request replacing auth_self_keep by yes three times for read-only methods listDevices, listRules, getParameter?

@mgerstner is this about all three of them^^ or which ones?

Yes, no problem.

@hartwork
Copy link
Contributor

@radosroka would you accept a pull request replacing auth_self_keep by yes three times for read-only methods listDevices, listRules, getParameter?

Yes, no problem.

Please see PR #545

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