From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Huang, Kai" Subject: Re: [intel-sgx-kernel-dev] [PATCH 08/10] kvm: vmx: add guest's IA32_SGXLEPUBKEYHASHn runtime switch support Date: Tue, 16 May 2017 13:22:52 +1200 Message-ID: <933546f5-40f2-e4ce-1e4a-37dbf38ea053@linux.intel.com> References: <20170508052434.3627-1-kai.huang@linux.intel.com> <20170508052434.3627-9-kai.huang@linux.intel.com> <58dcdb2d-6894-b0a3-8d6f-2ab752fd6d22@linux.intel.com> <6ab7ec4e-e0fa-af47-11b2-f26edcb088fb@linux.intel.com> <37306EFA9975BE469F115FDE982C075B9B706869@ORSMSX108.amr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm list , Radim Krcmar , "Cohen, Haim" , "intel-sgx-kernel-dev@lists.01.org" , Paolo Bonzini To: "Christopherson, Sean J" , 'Andy Lutomirski' Return-path: Received: from mga14.intel.com ([192.55.52.115]:61771 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751134AbdEPBXB (ORCPT ); Mon, 15 May 2017 21:23:01 -0400 In-Reply-To: <37306EFA9975BE469F115FDE982C075B9B706869@ORSMSX108.amr.corp.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On 5/13/2017 6:48 AM, Christopherson, Sean J wrote: > Andy Lutomirski wrote: >> On Thu, May 11, 2017 at 9:56 PM, Huang, Kai 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 think this is better as well. In this way we can leverage SGX code more easily. Some SGX detection code can be done in arch/x86/ as well, so that other code can access, ex, SGX capabilities, quickly. Another thing is actually SDK says SGX CPUID is per-thread, and we should not assume SGX CPUID will report the same info on all processors. I think it's better to check this as well. Moving SGX detection to identify_cpu can make this work more easily. Thanks, -Kai > > >>>> >>>> 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. >