On Wed, Apr 03, 2019 at 07:49:48PM -0400, Cole Robinson wrote: > ccing danpb > > On 4/3/19 9:52 AM, Pavel Hrdina wrote: > > Pavel Hrdina (5): > > domcapabilities: remove recommended CPU features from security > > features > > domcapabilities: fix typo in function name > > cli: introduce CPU secure parameter > > domcapabilities: add caching of CPU security features > > virt-manager: add new checkbox to control CPU security features > > > > So the bug spawning the original work is: > > https://bugzilla.redhat.com/show_bug.cgi?id=1582667 > > Some CPU model names lack security critical CPU flags. Example > Opteron_G5 lacks ibpb. Even if the user is running on an Opteron_G5 host > CPU which has ibpb, if they select Opteron_G5 the guest doesn't see it. > > In that case, the existing patches in git will check to see if the host > supports ibpb, and if so, manually specify that <feature> in the <cpu> > config. So now use of --cpu Opteron_G5 is secure by default. > > But the problem is this creates a migration compat issue: selecting > --cpu Opteron_G5 means different things for different hosts. So these > patches basically make the 'secure' <feature> additions the default > behavior, but gives users the option to turn it off on the cli and > virt-manager UI. You are correct about migration compat in the general case, but I think it is reasonable to assume that if the user has upgraded the microcode on some hosts they will have done it on all hosts. If they have not upgraded on all hosts, I still think it is sensible to apply the security mitigations on the hosts which have been upgraded unless the user explicitly says to use the insecure mode. > Taking a step back, I don't really get what the usecase is for users to > be selecting manual cpu model names with virt-manager or virt-install. I > understand the migration compat thing, if you need to generate a > baseline CPU that works for migrating a machine across multiple hosts, > but in practice that seems like pretty advanced usage for people using > virt-manager or even virt-install, and if they are using virt-install > they have all the tools at their disposal to generate a full + secure > <cpu> config if they want. I think using a explicit CPU model is quite a reasonable thing todo. It isn't solely live migration it helps. Even save/restore where you restore the image on 2nd machine wants a portable CPU model. It may not be the out of the box common case, but I don't think we should write it off as "advanced" usage and require people to take extra steps to figure out which security flags they need to set themselves. > Anyways, since the current code requires the user to explicitly opt in > to choosing a manual CPU model name, what if instead we just warn them? For these hardware security flaws I don't think it is acceptable to merely warn the user. We should do the right thing out of the box to enable the security mitigations. The fact that virt-intsall doesn't do this for named CPU models is arguably worthy of filing a CVE against virt-install itself. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list