USBGuard / usbguard

USBGuard is a software framework for implementing USB device authorization policies (what kind of USB devices are authorized) as well as method of use policies (how a USB device may interact with the system)
https://usbguard.github.io/
GNU General Public License v2.0
1.1k stars 133 forks source link

Too strict Polkit `auth_self_keep` authorization requirement? #544

Closed mgerstner closed 2 years ago

mgerstner commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

radosroka commented 2 years ago

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 commented 2 years ago

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.

muelli commented 2 years ago

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: https://github.com/USBGuard/usbguard/issues/246#issuecomment-650135986

hartwork commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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