On 5/13/2017 6:48 AM, Christopherson, Sean J wrote:
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 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.
