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

[Michael S. Tsirkin]

On Wed, Sep 14, 2016 at 08:45:25PM +0200, Paolo Bonzini wrote:
> 
> 
> On 14/09/2016 20:15, Michael S. Tsirkin wrote:
> > On Wed, Sep 14, 2016 at 06:53:22PM +0200, Paolo Bonzini wrote:
> >>
> >>
> >> On 14/09/2016 17:02, Michael S. Tsirkin wrote:
> >>> If you believe there are attackers that have access to the
> >>> monitor and nothing else, then a feature to disable debugging
> >>> is a generally useful one. But once we merge sev patchset then of course
> >>> sev people disappear and it will be up to others to make it
> >>> work on non-amd CPUs.
> >>>
> >>> Another is to help merge other parts faster.  E.g.  looking at what
> >>> Daniel writes, the feature might have been over-sold so people will
> >>> disable debugging thinking this will prevent all active attacks. Thus we
> >>> now need to add good documentation so people know what they can actually
> >>> expect to get from QEMU in return for disabling debugging. Why not merge
> >>> the simple "encrypt memory part" while this documentation work is going
> >>> on?
> >>
> >> Encrypting memory makes no sense if anyone can ask to decrypt it.
> > 
> > It's not useless since the attack model here is a passive adversary
> > that can not ask anything.
> 
> Does _that attack model_ make sense then?

It seems to make sense superficially.

> Also, I don't think this is
> the attack model; limited protection against a compromised hypervisor is
> included.

Well limited protection is of a limited use :) Seriously, the point of
mitigation should be blocking classes of vulenrabilities not making
things more complex.

> If the adversary is passive and cannot ask anything is it even an
> adversary?  Why do you need encryption at all if you can't even ptrace QEMU?

The cover letter mentioned a read everything adversary.
How do you read everything? Well, you probably don't but
there could be attacks that cause kernel to leak
contents of random memory to an attacker.


> >>  And
> >> I'm not even sure how force-enabling debug r/w, which is literally a
> >> single bit set in the feature register, would make the patchset simpler.
> > 
> > It will make the *interface* simpler.
> 
> If we made debug r/w force-disabled, the interface would be just as
> simple, and the outcome more secure and more sensible.

If you don't think debugging is useful (maybe it isn't) do it for
everyone then :)

> >> If anything, as I said already, it would make the patchset simpler to
> >> force-*disable* it, since you don't need to introduce debug hooks that
> >> go through the secure processor.
> > 
> > My suggestion is to add a processor independent hook that disables
> > debugging.  Arguably this improves security in case attacker only has
> > access to the monitor.
> 
> The default is the wrong direction though for encrypted guests...
> 
> Paolo

I think this is just tying unrelated features together. Hardware vendors
always do this - they want to sell their hardware that
solves all the problems. On the software side, we should try to
push for enabling features independently, this way more
hardware can benefit.

People that do not need debugging can disable it and maybe some exploit
will be prevented. Not at all different for encryption.

-- 
MST