On Wed, Sep 14, 2016 at 07:10:50PM +0300, Michael S. Tsirkin wrote: [...] > > > > >> Frankly I don't understand why do you need to mess with boot at all. > > > > >> Quoting the cover letter: > > > > >> > > > > >> SEV is designed to protect guest VMs from a benign but > > > > >> vulnerable > > > > >> (i.e. not fully malicious) hypervisor. In particular, it > > > > >> reduces the > > > > >> attack > > > > >> surface of guest VMs and can prevent certain types of VM-escape > > > > >> bugs > > > > >> (e.g. hypervisor read-anywhere) from being used to steal guest > > > > >> data. > > > > >> > > > > >> it seems highly unlikely that any secret data is used during boot. > > > > >> So just let guest boot normally, and encrypt afterwards. > > > > > > > > > > After boot seems too late for the attestation part (see Section > > > > > 1.3.1: Launch in the spec), unless you can ensure the memory > > > > > contents will always be exactly what the guest owner expects > > > > > after every boot. > > > > > > > > And the attestation is what lets the guest check that the memory > > > > contents are indeed what the guest owner expects. > > > > > > > > Paolo > > > > > > So the cover letter says hypervisor is benign, and then people turn > > > around and start discussing guest owner checking memory as if hypervisor > > > is malicious and might load something unexpected there. Makes no sense > > > to me. > > > > Cover letter says "a benign but vulnerable (i.e. not fully > > malicious) hypervisor". The hypervisor might be compromised from > > the very beginning, but even a compromised hypervisor shouldn't > > be able to provide an attestation that it has encrypted the > > memory. > > You seem to argue that this patch does protect against malicious > hypervisors. Is this so?
I am just assuming that the whole attestation system is there to protect against compromised hypervisors. > > > > > > > I suggest we just drop this attestation thing in v1. Try to merge > > > something minimal that actually works first. > > > > As far as I can see from the spec, attestation is part of the > > encryption process (the Launch event). I don't see how this could > > be even dropped. > > > > One may argue to drop the usefulness of the attestation by doing > > it very late. But I don't really see the point of doing it: are > > there any users that would want to use SEV with a useless > > attestation process? > > This is what the cover letter says: protecting against passive > adversaries: if the adversary can read all hypervisor memory but nothing > else, you can stop some information leaks to that adversary. I believe it tries to protect against additional attacks. To be sure about it, I need to see the talk and read the specs more carefully. (Maybe it's easier to simply wait for the patchset author describe their thread model.) > > It sounds like adding dead code that nobody > > would use until attestation is done properly. > > All I am saying is let's assume guests will ignore the measurement > result for now. Judging by e.g. the whitepaper that I read, > attestation is designed to protect against a malicious hypervisor, so > it's part of a future vision, not the current patchset. I'm not sure about "future vision, not the current patchset" part. I expect the current patchset to be effective against additional attacks, but I agree this need to be more clearly described in the patchset description/justification. -- Eduardo