Andy Lutomirski <luto@xxxxxxxxxx> wrote: > On Thu, May 11, 2017 at 9:56 PM, Huang, Kai <kai.huang@xxxxxxxxxxxxxxx> wrote: > > I am not sure whether the cost of writing to 4 MSRs would be *extremely* > > slow, as when vcpu is schedule in, KVM is already doing vmcs_load, writing > > to several MSRs, etc. > > I'm speculating that these MSRs may be rather unoptimized and hence > unusualy slow. > Good speculation :) We've been told to expect that writing the hash MSRs will be at least 2.5x slower than normal MSRs. > > > >> > >> Have a percpu variable that stores the current SGXLEPUBKEYHASH along > >> with whatever lock is needed (probably just a mutex). Users of EINIT > >> will take the mutex, compare the percpu variable to the desired value, > >> and, if it's different, do WRMSR and update the percpu variable. > >> > >> KVM will implement writes to SGXLEPUBKEYHASH by updating its in-memory > >> state but *not* changing the MSRs. KVM will trap and emulate EINIT to > >> support the same handling as the host. There is no action required at > >> all on KVM guest entry and exit. > > > > > > This is doable, but SGX driver needs to do those things and expose > > interfaces for KVM to use. In terms of the percpu data, it is nice to have, > > but I am not sure whether it is mandatory, as IMO EINIT is not even in > > performance critical path. We can simply read old value from MSRs out and > > compare whether the old equals to the new. > > I think the SGX driver should probably live in arch/x86, and the > interface could be a simple percpu variable that is exported (from the > main kernel image, not from a module). > Agreed, this would make life easier for future SGX code that can't be self-contained in the driver, e.g. EPC cgroup. Future architectural enhancements might also require tighter integration with the kernel. > >> > >> I would advocate for the former approach. (But you can't remap the > >> parameters due to TOCTOU issues, locking, etc. Just copy them. I > >> don't see why this is any more complicated than emulating any other > >> instruction that accesses memory.) > > > > > > No you cannot just copy. Because all address in guest's ENCLS parameters are > > guest's virtual address, we cannot use them to execute ENCLS in KVM. If any > > guest virtual addresses is used in ENCLS parameters, for example, > > PAGEINFO.SECS, PAGEINFO.SECINFO/PCMD, etc, you have to remap them to KVM's > > virtual address. > > > > Btw, what is TOCTOU issue? would you also elaborate locking issue? > > I was partially mis-remembering how this worked. It looks like > SIGSTRUCT and EINITTOKEN could be copied but SECS would have to be > mapped. If KVM applied some policy to the launchable enclaves, it > would want to make sure that it only looks at fields that are copied > to make sure that the enclave that gets launched is the one it > verified. The locking issue I'm imagining is that the SECS (or > whatever else might be mapped) doesn't disappear and get reused for > something else while it's mapped in the host. Presumably KVM has an > existing mechanism for this, but maybe SECS is special because it's > not quite normal memory IIRC. > Mapping the SECS in the host should not be an issue, AFAIK there aren't any restrictions on the VA passed to EINIT as long as it resolves to a SECS page in the EPCM, e.g. the SGX driver maps the SECS for EINIT with an arbitrary VA. I don't think emulating EINIT introduces any TOCTOU race conditions that wouldn't already exist. Evicting the SECS or modifying the page tables on a different thread while executing EINIT is either a guest kernel bug or bizarre behavior that the guest can already handle. Similarly, KVM would need special handling for evicting a guest's SECS, regardless of EINIT emulation. > >> [1] Guests that steal sealed data from each other or from the host can > >> manipulate that data without compromising the hypervisor by simply > >> loading the same enclave that its rightful owner would use. If you're > >> trying to use SGX to protect your crypto credentials so that, if > >> stolen, they can't be used outside the guest, I would consider this to > >> be a major flaw. It breaks the security model in a multi-tenant cloud > >> situation. I've complained about it before. > >> > > > > Looks potentially only guest's IA32_SGXLEPUBKEYHASHn may be leaked? In this > > case even it is leaked looks we cannot dig anything out just the hash value? > > Not sure what you mean. Are you asking about the lack of guest > personalization? > > Concretely, imagine I write an enclave that seals my TLS client > certificate's private key and offers an API to sign TLS certificate > requests with it. This way, if my system is compromised, an attacker > can use the certificate only so long as they have access to my > machine. If I kick them out or if they merely get the ability to read > the sealed data but not to execute code, the private key should still > be safe. But, if this system is a VM guest, the attacker could run > the exact same enclave on another guest on the same physical CPU and > sign using my key. Whoops! I know this issue has been raised internally as well, but I don't know the status of the situation. I'll follow up and provide any information I can.
