Re: [RFC PATCH v1 00/18] x86: Secure Memory Encryption (AMD)

[Paolo Bonzini]

On 02/05/2016 20:31, Andy Lutomirski wrote:
> And did the SEV implementation remember to encrypt the guest register
> state?  Because, if not, everything of importance will leak out
> through the VMCB and/or GPRs.

No, it doesn't.  And SEV is very limited unless you paravirtualize
everything.

For example, the hypervisor needs to read some instruction bytes from
memory, and instruction bytes are always encrypted (15.34.5 in the APM).
 So you're pretty much restricted to IN/OUT operations (not even
INS/OUTS) on emulated (non-assigned) devices, paravirtualized MSRs, and
hypercalls.  These are the only operations that connect the guest and
the hypervisor, where the vmexit doesn't have the need to e.g. walk
guest page tables (also always encrypted).  It possibly can be made to
work once the guest boots, and a modern UEFI firmware probably can cope
with it too just like a kernel can, but you need to ensure that your
hardware has no memory BARs for example.  And I/O port space is not very
abundant.

Even in order to emulate I/O ports or RDMSR/WRMSR or process hypercalls,
the hypervisor needs to read the GPRs.  The VMCB doesn't store guest
GPRs, not even on SEV-enabled processors.  Accordingly, the hypervisor
has access to the guest GPRs on every exit.

In general, SEV provides mitigation only.  Even if the hypervisor cannot
write known plaintext directly to memory, an accomplice virtual machine
can e.g. use the network to spray the attacked VM's memory.  At least
it's not as easy as "disable NX under the guest's feet and redirect RIP"
(pte.nx is reserved if efer.nxe=0, all you get is a #PF).  But the
hypervisor can still disable SMEP and SMAP, it can use hardware
breakpoints to leak information through the registers, and it can do all
the other attacks you mentioned.  If AMD had rdrand/rdseed, it could
replace the output with not so random values, and so on.

It's surely better than nothing, but "encryption that really is nothing
more than mitigation" is pretty weird.  I'm waiting for cloud vendors to
sell this as the best thing since sliced bread, when in reality it's
just mitigation.  I wonder how wise it is to merge SEV in its current
state---and since security is not my specialty I am definitely looking
for advice on this.

Paolo

ps: I'm now reminded of this patch:

    commit dab429a798a8ab3377136e09dda55ea75a41648d
    Author: David Kaplan <David.Kaplan@xxxxxxx>
    Date:   Mon Mar 2 13:43:37 2015 -0600

    kvm: svm: make wbinvd faster

    No need to re-decode WBINVD since we know what it is from the
    intercept.

    Signed-off-by: David Kaplan <David.Kaplan@xxxxxxx>
    [extracted from larger unlrelated patch, forward ported,
     tested,style cleanup]
    Signed-off-by: Joel Schopp <joel.schopp@xxxxxxx>
    Reviewed-by: Radim KrÄmÃÅ <rkrcmar@xxxxxxxxxx>
    Signed-off-by: Marcelo Tosatti <mtosatti@xxxxxxxxxx>

and I wonder if the larger unlrelated patch had anything to do with SEV!