Hi Andy,
Thanks for your helpful comments! See my reply below, and some questions
as well.
On 5/12/2017 3:28 PM, Andy Lutomirski wrote:
[resending due to some kind of kernel.org glitch -- sorry if anyone
gets duplicates]
On Thu, May 11, 2017 at 5:32 PM, Huang, Kai <kai.huang@xxxxxxxxxxxxxxx> wrote:
My current patch is based on this assumption. For KVM guest, naturally, we
will write the cached value to real MSRs when vcpu is scheduled in. For
host, SGX driver should write its own value to MSRs when it performs EINIT
for LE.
This seems unnecessarily slow (perhaps *extremely* slow) to me. I
would propose a totally different solution:
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.
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.
FWIW, I think that KVM will, in the long run, want to trap EINIT for
other reasons: someone is going to want to implement policy for what
enclaves are allowed that applies to guests as well as the host.
I am not very convinced why "what enclaves are allowed" in host would
apply to guest. Can you elaborate? I mean in general virtualization just
focus emulating hardware behavior. If a native machine is able to run
any LE, the virtual machine should be able to as well (of course, with
guest's IA32_FEATURE_CONTROL[bit 17] set).
Also, some day Intel may fix its architectural design flaw [1] by
allowing EINIT to personalize the enclave's keying, and, if it's done
by a new argument to EINIT instead of an MSR, KVM will have to trap
EINIT to handle it.
Looks this flaw is not the same issue as above (host enclave policy
applies to guest)?
One argument against this approach is KVM guest should never have impact on
host side, meaning host should not be aware of such MSR change
As a somewhat generic comment, I don't like this approach to KVM
development. KVM mucks with lots of important architectural control
registers, and, in all too many cases, it tries to do so independently
of the other arch/x86 code. This ends up causing all kinds of grief.
Can't KVM and the real x86 arch code cooperate for real? The host and
the KVM code are in arch/x86 in the same source tree.
Currently on host SGX driver, which is pretty much self-contained,
implements all SGX related staff.
2. KVM should restore MSRs after changing for guest.
No, this is IMO silly. Don't restore it on each exit, in the user
return hook, or anywhere else. Just make sure the host knows it was
changed.
Another thing is, not quite related to selecting which approach above, and
either we choose approach 1 or approach 2, KVM still suffers the performance
loss of writing (and/or reading) to IA32_SGXLEPUBKEYHASHn MSRs, either when
vcpu scheduled in or during each VMEXIT/VMENTRY. Given the fact that the
IA32_SGXLEPUBKEYHASHn will only be used by EINIT, We can actually do some
optimization by trapping EINIT from guest and only update MSRs in EINIT
VMEXIT.
Yep.
But trapping ENCLS requires either 1) KVM to run ENCLS on hebalf of guest,
in which case we have to reconstruct and remap guest's ENCLS parameters and
skip the ENCLS for guest; 2) using MTF to let guest to run ENCLS again,
while still trapping ENCLS.
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?
If necessary for some reason, trap EINIT when the SGXLEPUBKEYKASH is
wrong and then clear the exit flag once the MSRs are in sync. You'll
need to be careful to avoid races in which the host's value leaks into
the guest. I think you'll find that this is more complicated, less
flexible, and less performant than just handling ENCLS[EINIT] directly
in the host.
Sorry I don't quite follow this part. Why would host's value leaks into
guest? I suppose the *value* means host's IA32_SGXLEPUBKEYHASHn? guest's
MSR read/write is always trapped and emulated by KVM.
[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?
Thanks,
-Kai
