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

Allow admin users to remove packages without password prompt #404

Closed
wants to merge 1 commit into from
Closed

Allow admin users to remove packages without password prompt #404

wants to merge 1 commit into from

Conversation

mcatanzaro
Copy link
Contributor

@mcatanzaro mcatanzaro commented Jun 3, 2020

A local, active admin user can install packages without a password
prompt, but has to enter the admin password to remove packages. This
doesn't make much sense. It should be parallel.

Note that this change has no effect on what users are able to do,
because it only applies to admin users. The password only protects
against unlocked workstation attackers, where an attacker gains physical
access to an unlocked desktop. It's pretty weird to prevent such an
attacker from removing software, but allow installing new stuff.

https://pagure.io/fedora-workstation/issue/137#comment-640697

Edit: better issue: https://pagure.io/fedora-workstation/issue/233

A local, active admin user can install packages without a password
prompt, but has to enter the admin password to remove packages. This
doesn't make much sense. It should be parallel.

Note that this change has no effect on what users are able to do,
because it only applies to admin users. The password only protects
against unlocked workstation attackers, where an attacker gains physical
access to an unlocked desktop. It's pretty weird to prevent such an
attacker from removing software, but allow installing new stuff.

https://pagure.io/fedora-workstation/issue/137#comment-640697
@hughsie
Copy link
Collaborator

hughsie commented Jun 5, 2020

This was NAK'd by both the Fedora and Suse security teams as it would allow users to remove security added to the machine by the administrator. I think the solution here is allow users to install flapaks with --user without any kind of password, and do the existing privilege checks for flatpak --system installs and removals.

@mcatanzaro
Copy link
Contributor Author

mcatanzaro commented Jun 5, 2020

This was NAK'd by both the Fedora and Suse security teams as it would allow users to remove security added to the machine by the administrator.

That's not true, it only allows attackers who gain physical access to your computer to remove security added to the machine by the administrator. You still need to be logged in as administrator (in which case, prompting for a password is cute, because you don't need a password to steal or modify all the admin's files, etc.).

Remember this change only affects local active admin users, it doesn't affect unprivileged users, they still need to enter the password.

I think the solution here is allow users to install flapaks with --user without any kind of password, and do the existing privilege checks for flatpak --system installs and removals.

flatpak does not require a password when uninstalling if you are a local active administrator, so that change would just add a new password prompt. :) I assume that's already how it works if you're not a local active administrator.

@hughsie
Copy link
Collaborator

hughsie commented Jun 5, 2020

I agree with you dude, but this isn't a battle I really want to fight right now.

@mcatanzaro
Copy link
Contributor Author

I agree with you dude, but this isn't a battle I really want to fight right now.

Tell you what, @Conan-Kudo can merge it, and you can point Fedora-related fingers at us. :)

SUSE maintains its own polkit polices and deletes upstream polkit rules files, so this change won't affect them at all, they don't install this file at all.

@DimStar77
Copy link
Collaborator

SUSE maintains its own polkit polices and deletes upstream polkit rules files, so this change won't affect them at all, they don't install this file at all.

That's because group 'wheel' is not used on a default openSUSE installation (it might be present, but no pre-configuration done for it)

@Conan-Kudo Conan-Kudo closed this May 20, 2021
@Conan-Kudo Conan-Kudo deleted the branch PackageKit:master May 20, 2021 20:54
@mcatanzaro
Copy link
Contributor Author

Hi @Conan-Kudo, can we reconsider this please?

@hughsie
Copy link
Collaborator

hughsie commented Jun 19, 2021

@mcatanzaro did you talk to Red Hat security?

@mcatanzaro
Copy link
Contributor Author

No. We don't need Product Security to tell us how passwords work. The consequences of this change are obvious: an attacker with physical access to your computer can now uninstall software if you walk away from your computer while it is unlocked. The benefit of preventing that is not worth the usability cost.

If you want a formal decision from the Workstation WG, we can do that next week. I don't think it's necessary though. The current policy of allowing installation but not removal is obviously asymmetrical.

@Conan-Kudo
Copy link
Member

I'm okay with reconsidering this, but apparently I can't reopen the PR...

@hughsie
Copy link
Collaborator

hughsie commented Jun 19, 2021

We don't need Product Security to tell us how passwords work

We did have to get permission when we introduced the feature all those years ago.

The current policy of allowing installation but not removal is obviously asymmetrical

Sure, by design! Fedora has a policy where you can't lower the security state of the system by installing trusted software -- the same isn't obviously true when removing system software that is a dep of something else. This isn't something I want to enable upstream; as it's then me that gets dinged when someone notices you can remove selinux-policy-targeted "without admin approval" and then we get an article in the Register...

@mcatanzaro
Copy link
Contributor Author

We'll probably do this downstream then. I don't really care what the Register thinks. Honestly we might want to have a JS rule that globally removes password prompts for any polkit rule that uses auth_admin_keep....

@hughsie
Copy link
Collaborator

hughsie commented Jun 19, 2021

We'll probably do this downstream then

On your head be it. From what it's worth, this is exactly the thing we're supposed to ask the security team to assess, and FESCo to approve.

@mcatanzaro
Copy link
Contributor Author

https://pagure.io/fedora-workstation/issue/233

We might do a change proposal (which requires FESCo approval) for a wider-scoped change that would affect all auth_admin_keep rules. But that seems like overkill to change one setting. :)

@mcatanzaro
Copy link
Contributor Author

https://pagure.io/fedora-workstation/issue/233

This change was approved today. Neal suggested it could go upstream rather than downstream. I've opened a new PR because it doesn't seem possible to reuse this old one: #492.

@ximion
Copy link
Collaborator

ximion commented Jun 22, 2021

FWIW, at Debian/Ubuntu we require a password for both installations and removals, especially because due to package conflicts the former can trigger the latter, and there is no rule that package installations don't change the security status of the system. You may install a vulnerable app afterall and exploit that later. Only cache refreshes are safe.
I would actually like if upstream PackageKit had the strictest security settings possible, and downstream projects would weaken them with explicit patches / config flags, so it's extremely obvious what the distribution policy is (and new distros using PK get the most secure setting by default which they can later lower, if they want to).

@mcatanzaro
Copy link
Contributor Author

Seriously, the threat model here requires secret agents (or some other malicious local attacker with physical access to the device). That's all you're protecting against. I'm very skeptical that this is something most Debian/Ubuntu users need to worry about.

That said, this still doesn't affect Debian/Ubuntu because (a) it's a JS rule, and Debian's old polkit fork doesn't support JS rules afaik, and (b) the "wheel" group is hardcoded, but Debian uses "sudo" group instead.

@mcatanzaro
Copy link
Contributor Author

secret agents

To be clear, I mean FBI/KGB/Mossad secret agents. Or some other entity that is willing to spy on you, wait for you to take a coffee break while forgetting to lock your desktop, and then... open GNOME Software and start uninstalling packages? Is that really what the spies would do if they get access to your desktop?

This only affects active local admin users. Reentering your password only reconfirms that you are still you.

@hughsie
Copy link
Collaborator

hughsie commented Jun 22, 2021

I'm not so worried about the malicious user attack here. I'm worried about the user accidentally blowing their foot off and being amazed that they were not even asked to authorize such a destructive action. This doesn't just affect gnome-software, it's also for pkcon too.

@mcatanzaro
Copy link
Contributor Author

BTW there is a new PR here: #492

This one is obsolete because I deleted my original PackageKit fork ages ago, and had to create a new fork. Apparently the originating fork of the PR cannot be changed.

I'm not so worried about the malicious user attack here. I'm worried about the user accidentally blowing their foot off and being amazed that they were not even asked to authorize such a destructive action. This doesn't just affect gnome-software, it's also for pkcon too.

I don't think we should use password prompts for this....

@ximion
Copy link
Collaborator

ximion commented Jun 22, 2021

@hughsie

I'm not so worried about the malicious user attack here. I'm worried about the user accidentally blowing their foot off and being amazed that they were not even asked to authorize such a destructive action.

...or children just clicking stuff while a computer is unattended. Or people in a shared workspace being able to mess up other people's devices far too easily when unattended (seen with Windows at my lab quite a few times). Also, people use tools like AnyDesk a lot where I work, and that thing doesn't count as "remote" in PolicyKit, so someone with access there could do quite a lot...
I see quite a lot of damage potential here for just a tiny convenience gain for users...

@mcatanzaro
Copy link
Contributor Author

Then go the other way and add the password prompt when installing packages. It should at least be parallel.

@mcatanzaro
Copy link
Contributor Author

@mcatanzaro
Copy link
Contributor Author

FWIW this change is still carried downstream in Fedora, in case anyone is interested in revisiting.

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

Successfully merging this pull request may close these issues.

None yet

5 participants