On 5/13/2017 6:48 AM, Christopherson, Sean J wrote:
Andy Lutomirski <luto(a)kernel.org> wrote:
> On Thu, May 11, 2017 at 9:56 PM, Huang, Kai <kai.huang(a)linux.intel.com> 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.
>>> 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
>>>  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
> 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