This discussion originated in vagrant-libvirt review in bug 1168333. Since we do not ship the polkit rule enabled by default anymore, we should find some better way how to make this better for vagrant users. --- Additional comment from Miloslav Trmač on 2015-01-26 22:35:37 CET --- (In reply to Vít Ondruch from comment #15) > > > vagrant-libvirt.pkla > > > > This was replaced by 10-vagrant-libvirt.rules (although I am still not > > convinced we should ship this file, since there are security concerns such > > as bug 957300). > > Actually, lets see what is polkit's maintainer point of view. In principle, I think using groups (or even better, groups-of-groups, for “role” groups containing groups of users) to manage permissions is a good model; getting the exact role design is rather tricky (e.g. for VMs it could likely make sense to have a role that allows starting/stopping/restarting VMs and a separate one for more general administration) but that is something the various upstreams are probably best equipped to design, at least for roles that don’t span more roles. AFAICS 10-vagrant-libvirt.rules differs from the above in a significant way, though: it is in a package _different from_ libvirt that overrides rules that affect _all_ of libvirt. I strongly think that this should not be happening. * If an author of a polkit-using service designs some default rules and group membership rules, that is fine (upstream documentation currently prohibits this, but that should be changed, bug #956005). * If a site administrator wants to override defaults in their own polkit rules RPM, that is obviously fine. * Changing the system policy by installing an apparently unrelated RPM is _not_ good. It might make sense for vagrant-libvirt to install rules that apply only to vagrant VMs (I don’t know, I haven’t looked into vagraint-libvirt details at all at the moment). But if you are effectively trying to implement the libvirt-general RFE from bug #957300, that really needs to happen in the general libvirt RPM, not as a side-effect of installing (not even choosing to use?) a particular backend. --- Additional comment from Miloslav Trmač on 2015-01-26 22:39:58 CET --- (In reply to Miloslav Trmač from comment #16) > (or even better, groups-of-groups, for > “role” groups containing groups of users) (And no, Linux can not do groups-of-groups outside of specific setups like sssd.) > It might make sense for vagrant-libvirt to install rules that apply only to > vagrant VMs (I don’t know, I haven’t looked into vagraint-libvirt details at > all at the moment). This might have been drowned in the rest of the text. What exactly is the use case and your desired policy design? I have only seen what (I think) the policy file does (and I think it shouldn’t do that); perhaps the right path really is to do bug #957300, perhaps libvirt-vagrant should be installing some other rule file; perhaps I just don’t understand how libvirt with this package installed works. --- Additional comment from Michael Adam on 2015-01-27 01:34:07 CET --- --- Additional comment from Vít Ondruch on 2015-01-28 10:02:15 CET --- After discussion with mitr, I am going to drop the polkit rule for now. The policy is too big hammer ATM. We need something more tailored. Unfortunately, I am not libvirt nor polkit expert. This should be probably solved in similar manner to bug 957300. --- Additional comment from James (purpleidea) on 2015-01-28 11:50:52 CET --- (In reply to Vít Ondruch from comment #19) > After discussion with mitr, I am going to drop the polkit rule for now. The > policy is too big hammer ATM. We need something more tailored. > Unfortunately, I am not libvirt nor polkit expert. This should be probably > solved in similar manner to bug 957300. > I'm not sure entirely how this relates to the other bug, but not including the polkit file makes this package quite useless... Every user will have to add it themselves. At the bare minimum, please include that polkit file in /usr/share/vagrant/ or something, so the user can at least run: sudo cp -a ... /etc/ ... I'd love to hear alternate solutions :) Thanks, James --- Additional comment from James (purpleidea) on 2015-01-28 11:58:30 CET --- In parallel one solution could be to use the libvirt qemu:///session instead of system, but it's not clear how vagrant would be then able to manipulate networks... If someone wants to help hack on that, I'm down to write some code too, but I'm not sure about what networking changes would need to be done. See: https://github.com/pradels/vagrant-libvirt/issues/272 --- Additional comment from Vít Ondruch on 2015-01-28 13:54:10 CET --- (In reply to James (purpleidea) from comment #22) James, would you mind to open separate BZ ticket for this? I am going to simple remove the file for the moment, but we can add it later to allow users to cp it into appropriate place. Also, I was suggested to explore either pkexec probably console helper (e.g. something along the lines mock is done). Also, libvirt has a lot of polkit rules already define, but I was not able to trigger any of them. Not sure it that is due to old version of rubygem-ruby-libvirt --- Additional comment from Miloslav Trmač on 2015-01-28 13:57:37 CET --- (In reply to James (purpleidea) from comment #22) > (In reply to Vít Ondruch from comment #19) > > After discussion with mitr, I am going to drop the polkit rule for now. The > > policy is too big hammer ATM. We need something more tailored. > > Unfortunately, I am not libvirt nor polkit expert. This should be probably > > solved in similar manner to bug 957300. > > > > I'm not sure entirely how this relates to the other bug Including the file we are talking about, pretty much verbatim (except perhaps using a different group name), in one of the main libvirt packages, would be a resolution to bug 957300. (This does not automatically mean that this is precisely the right thing to do, but if this is to be done anywhere it should be in the main libvirt srpm). --- Additional comment from James (purpleidea) on 2015-01-28 14:04:25 CET --- (In reply to Vít Ondruch from comment #25) > (In reply to James (purpleidea) from comment #22) > James, would you mind to open separate BZ ticket for this? I am going to > simple remove the file for the moment, but we can add it later to allow > users to cp it into appropriate place. lol, no way. it will take me less time to write the patch :) send me a link to the packaging git repo, and I'll include a patch if you'd like one :) --- Additional comment from Michael Adam on 2015-01-28 14:10:11 CET --- (In reply to James (purpleidea) from comment #22) > (In reply to Vít Ondruch from comment #19) > > After discussion with mitr, I am going to drop the polkit rule for now. The > > policy is too big hammer ATM. We need something more tailored. > > Unfortunately, I am not libvirt nor polkit expert. This should be probably > > solved in similar manner to bug 957300. > > > > I'm not sure entirely how this relates to the other bug, but not including > the polkit file makes this package quite useless... > Every user will have to add it themselves. Well, I think we are in search for a better solution. The main concern raised with the vagrant-specific solution was as far as my understanding goes that while it is introduced by vagrant and for the vagrant group, the effect is for all of libvirt. The bug 957300 to my understanding propose to introduce a group libvirt that has the dersired polkit rule installed. That would mean in turn that in order to use vagrant-libirt without all the nasty password prompts, you would have to be a member of the libvirt group. I think this would be more than ok. Michael --- Additional comment from Michael Adam on 2015-01-28 14:17:19 CET --- (In reply to James (purpleidea) from comment #27) > (In reply to Vít Ondruch from comment #25) > > (In reply to James (purpleidea) from comment #22) > > James, would you mind to open separate BZ ticket for this? I am going to > > simple remove the file for the moment, but we can add it later to allow > > users to cp it into appropriate place. > > lol, no way. it will take me less time to write the patch :) send me a link > to the packaging git repo, and I'll include a patch if you'd like one :) I don't think that we should rush something in that is not a thought though to the end. We want to have a proper solution. And our discussion goes towards that... :) Cheers - Michael --- Additional comment from James (purpleidea) on 2015-01-28 14:21:36 CET --- (In reply to Michael Adam from comment #29) > (In reply to James (purpleidea) from comment #27) > > (In reply to Vít Ondruch from comment #25) > > > (In reply to James (purpleidea) from comment #22) > > > James, would you mind to open separate BZ ticket for this? I am going to > > > simple remove the file for the moment, but we can add it later to allow > > > users to cp it into appropriate place. > > > > lol, no way. it will take me less time to write the patch :) send me a link > > to the packaging git repo, and I'll include a patch if you'd like one :) > > I don't think that we should rush something in that is > not a thought though to the end. > > We want to have a proper solution. > And our discussion goes towards that... :) > > Cheers - Michael I agree... I thought the BZ request was if we should include the .polkit file in /usr/share/doc/ or something or not, to convenience the user if they want to use root and copy it in. No discussion needed there :) For the "best" proper solution (which might not be possible) see my comment#23.
This bug appears to have been reported against 'rawhide' during the Fedora 22 development cycle. Changing version to '22'. More information and reason for this action is here: https://fedoraproject.org/wiki/Fedora_Program_Management/HouseKeeping/Fedora22
As an aside, this bug could become moot if this issue is fixed instead: https://github.com/pradels/vagrant-libvirt/issues/272 I highly recommend trying to do this :) Cheers, James
Note that upstream libvirt now ships a polkit rule that means any user in the 'libvirt' group can avoid password prompts: https://bugzilla.redhat.com/show_bug.cgi?id=957300 I plan on backporting this to f22 as well. So that will help here but it won't work out of the box. Though making vagrant work with qemu:///session is definitely good too...
From a system security perspective, having passwordless vagrant off by default makes sense, but it would be good if there was a standard utility whereby a user that *already* has root access can give themselves permanent permission to run vagrant without being asked for their password all the time. Developer desktops are not servers :) I'm not sure it makes sense for such a utility to be Vagrant specific though. The kind of UX I am thinking of here would be something like: $ sudo dnf install vagrant-libvirt $ accessctl grant vagrant $ vagrant up <enjoy your passwordless vagrant!> So users would remain protected by default, but devs that wanted to avoid the password prompts would have an easy way to turn them off. Make it plugin based and we may also be able to have things like: $ accessctl grant mock $ accessctl grant docker Other subcommands would be "revoke" and "list". That way developers wouldn't need to care whether the backend implementation of the permissions management was "add yourself to this group" or "add this polkit policy" or something else entirely, they'd just need to know to grant themselves the appropriate permission through accessctl.
(In reply to Nick Coghlan from comment #4) > From a system security perspective, having passwordless vagrant off by I think the best solution would be to have someone work on: https://github.com/pradels/vagrant-libvirt/issues/272
(In reply to Nick Coghlan from comment #4) > From a system security perspective, having passwordless vagrant off by > default makes sense, but it would be good if there was a standard utility > whereby a user that *already* has root access can give themselves permanent > permission to run vagrant without being asked for their password all the > time. Yeah, I would prefer that standard utility to be “add yourself to a group”. Now that IPA and sssd supports groups-of-groups, we can have user-defined groups (departments, ”$site administrators” and the like) for arbitrary grouping, and application-defined role groups (”those allowed to run vagrant”) for permission checking: the role groups can contain both individual users and user-defined groups. Groups are a well-understood mechanism and if we don’t really need a different mechanism, I would prefer to not add a new parallel not-quite-a-group mechanism. And libvirt/vagrant is already using this design, which is great.
In the meantime, I and phracek, started to work on Vagrant DevAssistant[0] that can easily set this up! This way we can also keep adding other setups like effortless NFS and such. [0] https://dapi.devassistant.org/dap/vagrant/
The polkit rules here are fundamentally broken from a Fedora security perspective (and the security policy of many other linux based operating systems as well): http://pkgs.fedoraproject.org/cgit/vagrant-libvirt.git/tree/10-vagrant-libvirt.rules Those who can call the libvirt API can execute whatever they want as root and hose the system. Using the above linked rule to grant access to the vagrant group to unconditionally and without authentication, audit or logging to escalate to root privileges is a broken default. Note that we recently fixed a similar hole by having a docker group that allowed unconditional and unlogged escalation to root. It's fine if people want to have this vagrant group on their own systems, but these polkit rules should not get merged into Fedora as is. From a security perspective, it is essentially the same as running your login session as root. It's even worse that unconditional nopasswd access via sudo, since that's logged and audited. What vagrant-libvirt can and should do if it wants to use the qemu://system is one or both of: * Support use via sudo correctly. One should be able to 'sudo vagrant up' ... much like one can 'sudo docker run ...' * Open a per tty connection to libvirtd using a background process. The privilege escalation prompt will happen once per tty (ie: terminal). Alternatively, vagrant could use qemu://session ... although that still has some privilege escalation issues with regard to networking: https://github.com/pradels/vagrant-libvirt/issues/272 I'm all for fixing this, but it should be fixed correctly, not by voiding default system security.
(In reply to Stef Walter from comment #8) > > Those who can call the libvirt API can execute whatever they want as root > and hose the system. Using the above linked rule to grant access to the > vagrant group to unconditionally and without authentication, audit or > logging to escalate to root privileges is a broken default. If this is true (and I believe you) then it's a serious issue, and we can't include the polkit rule by default. To enrich my brain, is there a POC or some example you can refer me to, so I can see how this is possible? > What vagrant-libvirt can and should do if it wants to use the qemu://system > is one or both of: > > * Support use via sudo correctly. One should be able to 'sudo vagrant up' The sudo path is problematic, in particular if vagrant or omv makes a log file or similar, since it would have root perms in the users dir. > Alternatively, vagrant could use qemu://session ... although that still has > some privilege escalation issues with regard to networking: > > https://github.com/pradels/vagrant-libvirt/issues/272 Agreed... I'm not sure how the networking problem can be solved, but logically it must be possible, since a network could be virtual and run in user space, with only a pre-existing network used to route publicly.
(In reply to James (purpleidea) from comment #9) > (In reply to Stef Walter from comment #8) > > > > Those who can call the libvirt API can execute whatever they want as root > > and hose the system. Using the above linked rule to grant access to the > > vagrant group to unconditionally and without authentication, audit or > > logging to escalate to root privileges is a broken default. > > If this is true (and I believe you) then it's a serious issue, and we can't > include the polkit rule by default. To enrich my brain, is there a POC or > some example you can refer me to, so I can see how this is possible? The most obvious is in libvirt-lxc, where you can specify the supervisor. Search for <init> on this page: https://libvirt.org/formatdomain.html But there are lots of places where libvirt executes scripts or executables. 'git grep virCommandRun' will get you started. Most importantly though, the libvirt developers will tell you the same thing: https://bugzilla.redhat.com/show_bug.cgi?id=890291#c16
(In reply to Stef Walter from comment #10) > (In reply to James (purpleidea) from comment #9) > > (In reply to Stef Walter from comment #8) > > > > > > Those who can call the libvirt API can execute whatever they want as root > > > and hose the system. Using the above linked rule to grant access to the > > > vagrant group to unconditionally and without authentication, audit or > > > logging to escalate to root privileges is a broken default. > > > > If this is true (and I believe you) then it's a serious issue, and we can't > > include the polkit rule by default. To enrich my brain, is there a POC or > > some example you can refer me to, so I can see how this is possible? > > The most obvious is in libvirt-lxc, where you can specify the supervisor. > Search for <init> on this page: > > https://libvirt.org/formatdomain.html > > But there are lots of places where libvirt executes scripts or executables. > 'git grep virCommandRun' will get you started. > > Most importantly though, the libvirt developers will tell you the same thing: > > https://bugzilla.redhat.com/show_bug.cgi?id=890291#c16 Very interesting, thanks. TIL another way to get root ;) At least it's clear to me that https://github.com/pradels/vagrant-libvirt/issues/272 and figuring out how to keep networking happy is the way to go.
Please note that since libvirt-1.2.9.3-2.fc21 [1], there is shipped PolKit rule, which is essentially exactly the same rule we are currently shipping in vagrant-libvirt. The only difference is that libvirt requires user to be added into libvirt group instead of vagrant group. I'm considering to drop the rule and the vagrant group from vagrant-libvirt entirely and suggest users to use the libvirt group instead. Of course this would apply just to F24+, to there is approximately 6 months to update documentation, etc. There should not be any backward compatibility concern. Of course support for the user session [2] is the preferred solution. [1] http://pkgs.fedoraproject.org/cgit/libvirt.git/commit/?h=f21&id=b59373e03c2da66d7aabce2c6a9b09e193435f0b [2] https://github.com/pradels/vagrant-libvirt/issues/272
Is the libvirt group root equivalent?
This is the upstream polkit .rules file: https://github.com/libvirt/libvirt/blob/master/daemon/libvirt.rules
So then the question is whether having libvirt qemu+kvm:///system access is root equivalent or not.
(In reply to Stef Walter from comment #15) > So then the question is whether having libvirt qemu+kvm:///system access is > root equivalent or not. Yes it is root equivalent. Everything you mention about libvirt in comment #8 is correct. Though libvirt does have audit rules so things aren't completely unlogged. But this is the first I've heard about polkit rules of that nature being a bad idea from a security logging perspective... can you file a libvirt bug so we can discuss there?
(In reply to Cole Robinson from comment #16) > can you file a libvirt bug so we can discuss there? And link it here please :)
Filed bug: https://bugzilla.redhat.com/show_bug.cgi?id=1284447
Fedora 22 changed to end-of-life (EOL) status on 2016-07-19. Fedora 22 is no longer maintained, which means that it will not receive any further security or bug fix updates. As a result we are closing this bug. If you can reproduce this bug against a currently maintained version of Fedora please feel free to reopen this bug against that version. If you are unable to reopen this bug, please file a new report against the current release. If you experience problems, please add a comment to this bug. Thank you for reporting this bug and we are sorry it could not be fixed.
Libvirt now ships polkit ruiles, but this should be better IMO.
This bug appears to have been reported against 'rawhide' during the Fedora 25 development cycle. Changing version to '25'.
This message is a reminder that Fedora 25 is nearing its end of life. Approximately 4 (four) weeks from now Fedora will stop maintaining and issuing updates for Fedora 25. It is Fedora's policy to close all bug reports from releases that are no longer maintained. At that time this bug will be closed as EOL if it remains open with a Fedora 'version' of '25'. Package Maintainer: If you wish for this bug to remain open because you plan to fix it in a currently maintained version, simply change the 'version' to a later Fedora version. Thank you for reporting this issue and we are sorry that we were not able to fix it before Fedora 25 is end of life. If you would still like to see this bug fixed and are able to reproduce it against a later version of Fedora, you are encouraged change the 'version' to a later Fedora version prior this bug is closed as described in the policy above. Although we aim to fix as many bugs as possible during every release's lifetime, sometimes those efforts are overtaken by events. Often a more recent Fedora release includes newer upstream software that fixes bugs or makes them obsolete.
No change here, except that the upstream ticket about qemu://session was rejected :/ https://github.com/vagrant-libvirt/vagrant-libvirt/issues/272#issuecomment-214398425
This bug appears to have been reported against 'rawhide' during the Fedora 28 development cycle. Changing version to '28'.
There seems to be developement using QEMU session: https://github.com/vagrant-libvirt/vagrant-libvirt/pull/868 Not sure how to proceed though, as it's not used by default. Also requires `qemu-bridge-helper`[0] is required, additionally and some root-powered features in Vagrant might not work. [0] https://wiki.qemu.org/Features/HelperNetworking
(In reply to Pavel Valena from comment #25) Nice. Once this is released, it seems it will be enough to change this [1] line to make the qemu:///session default. The question is if we want to. It will probably need some testing. [1] https://github.com/vagrant-libvirt/vagrant-libvirt/blob/e696032debe485b4af346aa75569c55c5fadf903/lib/vagrant-libvirt/config.rb#L654
This is now available in Rawhide as part of https://fedoraproject.org/wiki/Changes/Vagrant_2.2_with_QEMU_Session