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
