Re: [Qemu-devel] [RFC PATCH v1 10/22] sev: add SEV debug decrypt command

[Michael S. Tsirkin]

On Wed, Sep 14, 2016 at 04:06:33PM +0100, Daniel P. Berrange wrote:
> On Wed, Sep 14, 2016 at 05:48:17PM +0300, Michael S. Tsirkin wrote:
> > On Wed, Sep 14, 2016 at 03:15:07PM +0100, Daniel P. Berrange wrote:
> > > On Wed, Sep 14, 2016 at 04:50:51PM +0300, Michael S. Tsirkin wrote:
> > > > On Wed, Sep 14, 2016 at 02:37:49PM +0100, Daniel P. Berrange wrote:
> > > > > On Wed, Sep 14, 2016 at 04:32:44PM +0300, Michael S. Tsirkin wrote:
> > > > > > On Wed, Sep 14, 2016 at 02:23:14PM +0100, Daniel P. Berrange wrote:
> > > > > > > On Wed, Sep 14, 2016 at 03:07:58PM +0200, Paolo Bonzini wrote:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > On 14/09/2016 15:05, Michael S. Tsirkin wrote:
> > > > > > > > > I assumed that with debug on, memory is still encrypted but 
> > > > > > > > > the
> > > > > > > > > hypervisor can break encryption, and as the cover letter 
> > > > > > > > > states, the
> > > > > > > > > hypervisor is assumed benign. If true I don't see a need to
> > > > > > > > > give users more rope.
> > > > > > > > 
> > > > > > > > The hypervisor is assumed benign but vulnerable.
> > > > > > > > 
> > > > > > > > So, if somebody breaks the hypervisor, you would like to make 
> > > > > > > > it as hard
> > > > > > > > as possible for the attacker to do evil stuff to the guests.  
> > > > > > > > If the
> > > > > > > > attacker can just ask the secure processor "decrypt some memory 
> > > > > > > > for me",
> > > > > > > > then the encryption is effectively broken.
> > > > > > > 
> > > > > > > So there's going to be a tradeoff here between use of SEV and use 
> > > > > > > of
> > > > > > > certain other features. eg, it seems that if you're using SEV, 
> > > > > > > then
> > > > > > > any concept of creating & analysing guest core dumps from the host
> > > > > > > is out.
> > > > > > 
> > > > > > I don't see why - as long as we don't trigger dumps, there's no 
> > > > > > leak :)
> > > > > 
> > > > > If the facility to trigger dumps is available, then the memory
> > > > > encryption feature of SEV is as useful as a chocolate teapot,
> > > > > as the would be attacker can simply trigger a dump
> > > > 
> > > > If attacker can trigger things, IOW execute code in hypervisor,
> > > > then encrypting memory is not useful anyway.
> > > 
> > > The presentation at KVM forum claimed it *would* protect against
> > > this, and that things like core dump of unencrypted memory would
> > > not be permitted, so there's a disconnect between that preso and
> > > what you're saying.
> > > 
> > > Regards,
> > > Daniel
> > 
> > You mean presentation claimed protection against leaks to a malicious
> > active attacker within a hypervisor?  I guess the presentation covers
> > more than this patchset does then.  And the disconnect would be with
> > what the patchset cover letter says, not just with what I say.  Clearly
> > encrypting memory is not enough to protect against a malicious
> > hypervisor. E.g. just running info cpus is enough to leak information
> > from guest.
> 
> It was explicit about the fact that the host admin would not have any
> way to get access to the full contents of guest memory, without the
> guest admin granting it. Only those non-encrypted pages used for I/O
> transfer between host & guest would be accessible.
> 
> Regards,
> Daniel

If you like, that's the vision. I'd rather discuss the patchset in
question though. It encrypts all memory but this does not protect against
all attackers, only passive ones. If you disable debugging,
it seems to additionally reduce the amount of information that can be
leaked to an active attacker in the hypervisor at one go.

Paolo seems to think it's useful, but it's a far cry from a deal
breaker, and your email just makes me worry that it has been oversold to
the point where everyone will start disabling debugging everywhere in
production and claim that otherwise it's a security problem.  IMO a much
better in-tree documentation is needed so people know what they are
getting in return.

Attestation seems mostly unrelated. The whitepaper says
        With this attestation, a guest owner can ensure that the hypervisor did
        not interfere with the initialization of SEV before transmitting
        confidential information to the guest.
which seems to imply an active attacker that is able to interfere
with the hypervisor during guest initialization but not afterwards.
So I have no idea why that's useful at the moment - I suspect
it's part of the future vision when there are protections
against all active attackers in place, but for now it seems to extend the
firmware/software interface unnecessarily.

> -- 
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-             http://virt-manager.org :|
> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|