[virt-tools-list] [virt-manager PATCH 0/5] CPU security features improvements

[Daniel P. Berrangé]

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 :|