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

[Cole Robinson]

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.

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.

That bug was filed in may of last year. When it was filed, virt-manager
was still using the CPU model from 'virsh capabilities' as the default.
With spectre stuff that is very bad for sure, because once we set
Opteron_G5 it's there forever in the VM config, even after the host is
updated to possibly expose more CPU flags or whatever, the VM always
sees the old feature set.

However now we are defaulting to host-model for new enough qemu+libvirt,
including in RHEL7 it seems. The default VM CPU config is as safe as the
host CPU allows. So maybe the original motivation for the bug isn't
really as valid any more because we use a better default.

So I kind of suspect that the original urgency around the bug was due to
virt-manager potentially defaulting to the insecure model=Opteron_G5

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?
A warning label that appears like:

"The following host security features are not supported by the selected
CPU: ipbp, ...

Use 'Copy host CPU' for maximum security'"

And a virt-install printed warning to that effect too. Maybe we keep the
--cpu secure=on CLI option but we keep it off by default. Basically if
people are going to deviate from the default and specify a manual cpu
name I don't think we need to bend over backwards to protect them in
that case, just tell them 'dont do that' or if they have valid poweruser
usecases then they have all the tools at their disposal to make it work,
at least on the cli.

Thanks,
Cole