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

[Michael S. Tsirkin]

On Wed, Sep 14, 2016 at 04:19:38PM +0200, Paolo Bonzini wrote:
> 
> 
> On 14/09/2016 15:48, Michael S. Tsirkin wrote:
> >> One of the bit in policy field is "debugging", if this bit is set then
> >> hypervisor can use SEV commands to decrypt a guest memory
> > 
> > That is my point. Arbitrary code execution in hypervisor means game over
> > anyway, at least with the hardware we have today.
> 
> Game is over if you assume the attacker has infinite power.  In practice
> the attacker may be limited by other security features (SELinux,
> seccomp, external firewalls, whatever), by the money and time they can
> spend on the attack.  So anything that makes things harder for the
> attacker is a security improvement.

Why don't we add assert(i == X) after each i = X in code?
This will make things harder for attackers that can skip
a single instruction only.

I think that the answer is that's because we do not believe there are
such attackers.  So defining a threat model is important.
The one that this patchset cover letter defined does not
include active attackers.

> > My suggestion is to merge the support for encrypting memory first,
> > then make extras like disabling debugging on top.
> 
> Sorry but I concur with others that this makes no sense at all.  If
> anything, it's *enabling* debugging that can be done on top.  That said...
> 
> > I can't say I understand how does guest measuring help prevent
> > leaks in any way. Looks like a separate feature - why not split it
> > out?
> 
> ... the patch series seems to be pretty small and self contained.  I
> don't see any point in splitting it further.
> 
> Paolo

One point is so we can discuss features and generalize them.

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?


-- 
MST