Re: [intel-sgx-kernel-dev] [PATCH 08/10] kvm: vmx: add guest's IA32_SGXLEPUBKEYHASHn runtime switch support

["Huang, Kai" <kai.huang@linux.intel.com>]

On 6/12/2017 8:36 PM, Jarkko Sakkinen wrote:
> On Mon, Jun 12, 2017 at 10:45:07AM +1200, Huang, Kai wrote:
>>>> But it may still be better if SGX driver can provide function like:
>>>>
>>>>       int sgx_validate_sigstruct(struct sigstruct *sig);
>>>>
>>>> for KVM to call, in case driver is changed (ex, to only allows LEs from some
>>>> particular ones to run), but this is not necessary now. KVM changes can be
>>>> done later when driver make the changes.
>>>>
>>>> Andy,
>>>>
>>>> Am I understanding correctly? Does this make sense to you?
>>>>
>>>> Thanks,
>>>> -Kai
>>>
>>> Nope. I don't even understand the *beginnings* what that function would
>>> do. I don't understand what the validation means here and what VMM would
>>> do if that functions reports "success".
>>
>> The validation means either the sigstruct->modulus or
>> SHA256(sigstruct->modulus) should be in a 'approved white-list' maintained
>> by kernel (which I know doesn't exist now, but Andy some kind suggested we
>> may or should have, in the future I guess), otherwise the function returns
>> error to indicate the LE from guest is "unapproved by host kernel/driver".
>>
>> Andy, would you explain here?
> 
> That can be considered but I still have zero idea what this function is
> and what its relation to whitelist would be.

The relation is this function only returns success when 
sigstruct->modulus (or SHA256(sigstruct->modulus)) matches the value in 
the 'white-list'...

VMM does nothing if the function returns success, it continues to run 
EINIT on behalf of guest, and inject the result to guest (either success 
or failure, VMM doesn't care the result but need to report the result to 
guest to reflect HW behavior).

If this function returns error, VMM needs to report some error code to 
guest. VMM even doesn't need to run EINIT anymore.

But IMO we don't have add this function now. If your driver choose to 
have such 'white-list', we can do this in the future.

> 
>>> How that would work on a system where MSRs cannot be changed?
>>
>> This is simple, we simply won't allow guest to choose its own
>> IA32_SGXLEPUBKEYHASHn by specifying 'lehash' value in Qemu parameter when
>> creating the guest.
> 
> Why not? You could have virtual MSRs and ask host LE to generate token
> if they match to modulus.

The guest has its own LE running inside, and guest's LE will generate 
token for enclaves in guest. The host will not generate token for guest 
in any circumstances, because this is totally guest's behavior.

Virtualization is only about to emulate hardware's behavior, but not to 
assume, or depend on, or change guest's SW behavior. We are not trying 
to make sure EINIT will run successfully in guest, instead, we are 
trying to make sure EINIT will be just the same behavior as it runs on 
physical machine -- either success or failure, according to guest's 
sigstruct and token.

If EINIT in guest is supposed to fail (ex, incorrect token is generated 
by guest's LE), KVM will need to inject the supposed error to guest, to 
reflect the real hardware behavior. One more example, if KVM even 
chooses not to trap EINIT, how can you provide token generated by host 
LE to guest? You simply cannot. Host LE will never generate token for 
guest, in any circumstance.

> 
>> To elaborate, currently in my design Qemu has below new parameters to
>> support SGX:
>>
>> 	# qemu-system-x86_64 -sgx, epc=<size>,lehash=<sha-256 hash>,lewr
>>
>> The 'epc=<size>' specifies guest's EPC size obviously, lehash specifies
>> guest's initial IA32_SGXLEPUBKEYHASHn (similar to the value configured in
>> BIOS for real machine), and 'lewr' specifies whether guest's
>> IA32_SGXLEPUBKEYHASHn can be changed by OS at runtime. The 'lehash' and
>> 'lewr' are optional.
>>
>> If MSRs cannot be changed on physical machine, then we will fail to create
>> guest if either 'lehash' or 'lewr' is specified when creating the guest.
>>
>>>
>>> In that kind of system the host OS must generate EINITTOKEN for the LE
>>> running on inside the guest and maintain completely virtualized MSR
>>> values for the guest.
>>
>> The host OS will not generate EINITTOKEN for guest in any circumstances, as
>> EINITTOKEN will always be from guest's EINIT instruction. KVM traps EINIT
>> from guest and gets both SIGSTRUCT and EINITTOKEN from the EINIT leaf,
>> update MSRs, and run EINIT on behalf of guest.
> 
> Seriously sounds like a stupid constraint or I'm not getting something
> (which also might be the case). If you anyway trap EINIT, you could
> create a special case for guest LE.

This is not constraint, but KVM has to emulate hardware correctly. For 
this part please see my explanation above.

And let me explain the purpose of trapping EINIT again here.

When guest is about to run EINIT, if guest's SHA256(sigstruct->modulus) 
matches guest's virtual IA32_SGXLEPUBKEYHASHn (and if others are 
correctly populated in sigstruct and token as well), KVM needs to make 
sure that EINIT will run successfully in guest, even physical 
IA32_SGXLEPUBKEYHASHn are not equal to guest's virtual MSRs at this 
particular time. This is because given the same condition, the EINIT 
will run successfully on physical machine. KVM needs to emulate the 
right HW behavior.

How to make sure guest's EINIT will run successfully in this case? We 
need to update guest's virtual MSRs to physical MSRs before guest runs 
EINIT. The whole purpose for KVM to trap EINIT from guest, is to 
guarantee that before EINIT, KVM can update guest's virtual MSRs to 
physical MSRs.

Like I said before, KVM even doesn't need to trap EINIT for this 
purpose, ex, KVM can update guest's virtual MSRs to real MSRs when vcpu 
is scheduled in (which is exactly what I did in this patch, btw), but 
this is considered to be performance unfriendly, so Andy, Sean suggested 
we should do this when trapping EINIT from guest.

Another problem of updating MSRs when vcpu is scheduled in is, this will 
break SGX driver if SGX driver does some performance optimization in 
terms of updating MSRs, such as keeping per_cpu variables for MSR 
current value, and only updating MSRs if per_cpu value is not equal to 
the value you want to update. Because of this, Andy, Sean also suggested 
that SGX driver can provide a function to handle updating MSRs and run 
EINIT together with mutex protected, and KVM simply calls that function 
(of course KVM will provide the sigstruct and token by trapping EINIT 
from guest). This can cover MSRs updating and EINIT for both host and 
KVM guests. IMO this is a good idea as well, and you should consider to 
do in this way.

Thanks,
-Kai

> 
> /Jarkko
>