search

coreos / rpm-ostree

⚛📦 Hybrid image/package system with atomic upgrades and package layering
https://coreos.github.io/rpm-ostree
Other
843 stars 191 forks source link

multiple polkit prompts during rpm-ostree operation #1375

Open dustymabe opened 6 years ago

dustymabe commented 6 years ago

Host system details

[dustymabe@dhcp137-98 ~]$ rpm -q rpm-ostree ostree
rpm-ostree-2018.4-1.fc28.x86_64
ostree-2018.5-1.fc28.x86_64
[dustymabe@dhcp137-98 ~]$ sudo rpm-ostree status
[sudo] password for dustymabe:                                                                                                                                                                                                                                                 
Sorry, try again.       
[sudo] password for dustymabe:
State: idle; auto updates enabled (check; last run 21h ago)
Deployments:             
● ostree://unifiedrepo:fedora/28/x86_64/atomic-host                                                 
                   Version: 28.20180515.1 (2018-05-15 16:32:35) 
                BaseCommit: a29367c58417c28e2bd8306c1f438b934df79eba13706e078fe8564d9e0eb32b
              GPGSignature: Valid signature by 128CF232A9371991C8A65695E08E7E629DB62FB1
           LayeredPackages: aria2 git git-annex mosh pciutils tig vim weechat
                    Pinned: yes  

  ostree://fedora-atomic-27:fedora/27/x86_64/atomic-host 
                   Version: 27.122 (2018-04-18 23:34:24)
                BaseCommit: 931ebb3941fc49af706ac5a90ad3b5a493be4ae35e85721dabbfd966b1ecbf99
              GPGSignature: Valid signature by 860E19B0AFA800A1751881A6F55E7430F5282EE4
           LayeredPackages: aria2 git git-annex mosh pciutils tig vim weechat
                    Pinned: yes                                                                                                                                                                                                                                                

  ostree://fedora-atomic-27:fedora/27/x86_64/atomic-host
                   Version: 27.81 (2018-02-12 17:50:48)
                BaseCommit: b25bde0109441817f912ece57ca1fc39efc60e6cef4a7a23ad9de51b1f36b742
              GPGSignature: Valid signature by 860E19B0AFA800A1751881A6F55E7430F5282EE4
           LayeredPackages: aria2 git git-annex mosh pciutils tig vim weechat

Available update:                  
           Diff: 4 upgraded

Expected vs actual behavior

I get two polkit prompts for password when running a rebase --uninstall pkg operation. I'd expect typing the password one time would be enough. 

[dustymabe@dhcp137-98 ~]$ rpm-ostree rebase fedora/28/x86_64/updates/atomic-host --uninstall weechat 
==== AUTHENTICATING FOR org.projectatomic.rpmostree1.rebase ====
Authentication is required to switch to a different base OS
Authenticating as: Dusty Mabe (dustymabe)
Password: 
==== AUTHENTICATION COMPLETE ====
==== AUTHENTICATING FOR org.projectatomic.rpmostree1.install-uninstall-packages ====
Authentication is required to install and remove software
Authenticating as: Dusty Mabe (dustymabe)
Password: 
==== AUTHENTICATION COMPLETE ====

Updating metadata for 'fedora': [=============] 100%                                                                                                                                                                                                            Checking out trrpm-md repo 'fedora'; generated: 2018-04-25 04:27:32
dustymabe commented 5 years ago

this seems either easy or medium difficulty... marking as easy and we can move to medium if it needs to be adjusted

jlebon commented 5 years ago

I get two polkit prompts for password when running a rebase --uninstall pkg operation.

This is because rpm-ostree considers this two actions: a rebase and an uninstall. You may be authorized for only one of those. (You can see the action you're authenticating for is different for each prompt). E.g. on a corporate laptop, doing an upgrade might be allowed for unprivileged users, but not override remove.

More background in https://github.com/projectatomic/rpm-ostree/pull/825#discussion_r122029573.

OTOH, I understand this makes things more annoying for Silverblue/AH admins. Hmm, maybe we could ship a rule to allow all actions for users in wheel?

jlebon commented 5 years ago

OTOH, I understand this makes things more annoying for Silverblue/AH admins. Hmm, maybe we could ship a rule to allow all actions for users in wheel?

Example here: https://wiki.archlinux.org/index.php/Polkit#Globally

dustymabe commented 5 years ago

This is because rpm-ostree considers this two actions: a rebase and an uninstall. You may be authorized for only one of those. (You can see the action you're authenticating for is different for each prompt).

yep. I noticed that, was just thinking we should be able to batch them somehow and say if the user isn't authenticated for all operations then fail. i.e. all or nothing

cgwalters commented 5 years ago

PolicyKit supports an "imply" annotation, see the docs specifically

The org.freedesktop.policykit.imply annotation (its value is a string containing a space separated list of action identifiers) can be used to define meta actions. The way it works is that if a subject is authorized for an action with this annotation, then it is also authorized for any action specified by the annotation. A typical use of this annotation is when defining an UI shell with a single lock button that should unlock multiple actions from distinct mechanisms.

The idea here would be that given multiple requests, the daemon checks for the "most powerful" one first, and that implies weaker ones.

jlebon commented 5 years ago

the daemon checks for the "most powerful" one first, and that implies weaker ones

Yeah, I think we could do this for some things (e.g. rebase implies deploy implies upgrade), but I'm not sure if there's an obvious intuitive way to order every possible action that way.

OTOH, I understand this makes things more annoying for Silverblue/AH admins. Hmm, maybe we could ship a rule to allow all actions for users in wheel?

Actually, we do already do this for Silverblue:

[root@lux ~]# cat /usr/share/polkit-1/rules.d/org.projectatomic.rpmostree1.rules
polkit.addRule(function(action, subject) {
    if ((action.id == "org.projectatomic.rpmostree1.install-uninstall-packages" ||
         action.id == "org.projectatomic.rpmostree1.install-local-packages" ||
         action.id == "org.projectatomic.rpmostree1.override" ||
         action.id == "org.projectatomic.rpmostree1.deploy" ||
         action.id == "org.projectatomic.rpmostree1.upgrade" ||
         action.id == "org.projectatomic.rpmostree1.rebase" ||
         action.id == "org.projectatomic.rpmostree1.rollback" ||
         action.id == "org.projectatomic.rpmostree1.bootconfig" ||
         action.id == "org.projectatomic.rpmostree1.reload-daemon" ||
         action.id == "org.projectatomic.rpmostree1.cancel" ||
         action.id == "org.projectatomic.rpmostree1.cleanup" ||
         action.id == "org.projectatomic.rpmostree1.repo-refresh" ||
         action.id == "org.projectatomic.rpmostree1.client-management") &&
        subject.active == true &&
        subject.local == true &&
        subject.isInGroup("wheel")) {
            return polkit.Result.YES;
    }
});

This comes from:

https://src.fedoraproject.org/rpms/fedora-release/blob/99da61a0bb09e0be9845fe4bff119a0269af1baa/f/org.projectatomic.rpmostree1.rules https://src.fedoraproject.org/rpms/fedora-release/blob/06189b18f52650a9a6cb8d26afe5952dc3872409/f/fedora-release.spec#_345

So I think Dusty's hitting this because we're not shipping it in FAH?

https://src.fedoraproject.org/rpms/fedora-release/blob/06189b18f52650a9a6cb8d26afe5952dc3872409/f/fedora-release.spec#_318

@r4f4 Want to submit a patch for that? :)

r4f4 commented 5 years ago

@jlebon Yep, leave it to me.

r4f4 commented 5 years ago

Pull Request

jlebon commented 5 years ago

@dustymabe was still getting prompted even with @r4f4's PR above. Copy/pasting reply:

16:01:05 < jlebon> dustymabe: ahh OK, so I think the issue there is that connecting through the serial console does not constitute a "local" user
16:01:17 < jlebon> that's the `subject.local == true` part
16:01:42 < jlebon> the same applies to SSH
16:02:11 < jlebon> the rules were initially written for GNOME Software, which clearly runs within a user's local session
16:03:01 < jlebon> so, it doesn't help much for AH/FCOS since the primary mode of access is SSH there

Though I'm not sure if it's worth dropping the local == true bit for FCOS or what the security implications of that are beyond "if someone can SSH as a wheel user, then they can run all the rpm-ostree commands!".

jlebon commented 5 years ago

Related: https://github.com/systemd/systemd/issues/8582

dustymabe commented 5 years ago

going back to the original problem...

the daemon checks for the "most powerful" one first, and that implies weaker ones

Yeah, I think we could do this for some things (e.g. rebase implies deploy implies upgrade), but I'm not sure if there's an obvious intuitive way to order every possible action that way.

could we just group actions into read/write and have one auth required for "write" commands, i.e. all write commands would imply all other write commands.

sgallagher commented 5 years ago

(Deleted my previous message. It was intended for https://src.fedoraproject.org/rpms/fedora-release/pull-request/61)