On Thu, Jun 8, 2017 at 4:47 PM, Huang, Kai <kai.huang@xxxxxxxxxxxxxxx> wrote:
>
>
> On 6/9/2017 12:31 AM, Jarkko Sakkinen wrote:
>>
>> On Wed, Jun 07, 2017 at 08:52:42AM +1200, Huang, Kai wrote:
>>>
>>>
>>>
>>> On 5/18/2017 7:45 PM, Huang, Kai wrote:
>>>>
>>>>
>>>>
>>>> On 5/17/2017 12:09 PM, Andy Lutomirski wrote:
>>>>>
>>>>> On Mon, May 15, 2017 at 5:48 PM, Huang, Kai
>>>>> <kai.huang@xxxxxxxxxxxxxxx> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 5/12/2017 6:11 PM, Andy Lutomirski 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.
>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> 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).
>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> 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).
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> I strongly disagree. I can imagine two classes of sensible policies
>>>>>>> for launch control:
>>>>>>>
>>>>>>> 1. Allow everything. This seems quite sensible to me.
>>>>>>>
>>>>>>> 2. Allow some things, and make sure that VMs have at least as
>>>>>>> restrictive a policy as host root has. After all, what's the point
>>>>>>> of
>>>>>>> restricting enclaves in the host if host code can simply spawn a
>>>>>>> little VM to run otherwise-disallowed enclaves?
>>>>>>
>>>>>>
>>>>>>
>>>>>> What's the current SGX driver launch control policy? Yes allow
>>>>>> everything
>>>>>> works for KVM so lets skip this. Are we going to support
>>>>>> allowing several
>>>>>> LEs, or just allowing one single LE? I know Jarkko is doing in-kernel
>>>>>> LE
>>>>>> staff but I don't know details.
>>>>>>
>>>>>> I am trying to find a way that we can both not break host launch
>>>>>> control
>>>>>> policy, and be consistent to HW behavior (from guest's view).
>>>>>> Currently we
>>>>>> can create a KVM guest with runtime change to
>>>>>> IA32_SGXLEPUBKEYHASHn either
>>>>>> enabled or disabled. I introduced an Qemu parameter 'lewr' for
>>>>>> this purpose.
>>>>>> Actually I introduced below Qemu SGX parameters for creating guest:
>>>>>>
>>>>>> -sgx epc=<size>,lehash='SHA-256 hash',lewr
>>>>>>
>>>>>> where 'epc' specifies guest's EPC size, lehash specifies
>>>>>> (initial) value of
>>>>>> guest's IA32_SGXLEPUBKEYHASHn, and 'lewr' specifies whether
>>>>>> guest is allowed
>>>>>> to change guest's IA32_SGXLEPUBKEYHASHn at runtime.
>>>>>>
>>>>>> If host only allows one single LE to run, KVM can add a restrict
>>>>>> that only
>>>>>> allows to create KVM guest with runtime change to
>>>>>> IA32_SGXLEPUBKEYHASHn
>>>>>> disabled, so that only host allowed (single) hash can be used by
>>>>>> guest. From
>>>>>> guest's view, it simply has IA32_FEATURE_CONTROL[bit17] cleared and
>>>>>> has
>>>>>> IA32_SGXLEPUBKEYHASHn with default value to be host allowed
>>>>>> (single) hash.
>>>>>>
>>>>>> If host allows several LEs (not but everything), and if we
>>>>>> create guest with
>>>>>> 'lewr', then the behavior is not consistent with HW behavior, as from
>>>>>> guest's hardware's point of view, we can actually run any LE but
>>>>>> we have to
>>>>>> tell guest that you are only allowed to change
>>>>>> IA32_SGXLEPUBKEYHASHn to some
>>>>>> specific values. One compromise solution is we don't allow to
>>>>>> create guest
>>>>>> with 'lewr' specified, and at the meantime, only allow to create
>>>>>> guest with
>>>>>> host approved hashes specified in 'lehash'. This will make
>>>>>> guest's behavior
>>>>>> consistent to HW behavior but only allows guest to run one LE (which
>>>>>> is
>>>>>> specified by 'lehash' when guest is created).
>>>>>
>>>>>
>>>>> I'm not sure I entirely agree for a couple reasons.
>>>>>
>>>>> 1. I wouldn't be surprised if the kernel ends up implementing a policy
>>>>> in which it checks all enclaves (not just LEs) for acceptability. In
>>>>> fact, if the kernel sticks with the "no LE at all or just
>>>>> kernel-internal LE", then checking enclaves directly against some
>>>>> admin- or distro-provided signer list seems reasonable. This type of
>>>>> policy can't be forwarded to a guest by restricting allowed LE
>>>>> signers. But this is mostly speculation since AFAIK no one has
>>>>> seriously proposed any particular policy support and the plan was to
>>>>> not have this for the initial implementation.
>>>>>
>>>>> 2. While matching hardware behavior is nice in principle, there
>>>>> doesn't seem to be useful hardware behavior to match here. If the
>>>>> host had a list of five allowed LE signers, how exactly would it
>>>>> restrict the MSRs? They're not written atomically, so you can't
>>>>> directly tell what's being written.
>>>>
>>>>
>>>> In this case I actually plan to just allow creating guest with guest's
>>>> IA32_SGXLEPUBKEYHASHn disabled (without 'lewr' specified). If 'lewr' is
>>>> specified, creating guest will fail. And we only allow creating guest
>>>> with host allowed hash values (with 'lehash=hash-value'), and if
>>>> 'hash-value' specified by 'lehash' is not allowed by host, we also fail
>>>> to create guest.
>>>>
>>>> We can only allow creating guest with 'lewr' specified when host allows
>>>> anything.
>>>>
>>>> But in this way, we are restricting guest OS's ability to run LE, as
>>>> only one LE, that is specified by 'lehash' parameter, can be run. But I
>>>> think this won't hurt much, as multiple guests still are able to run
>>>> different LEs?
>>>>
>>>> Also, the only way to fail an MSR
>>>>>
>>>>> write is to send #GP, and Windows (and even Linux) may not expect
>>>>> that. Linux doesn't panic due to #GP on MSR writes these days, but
>>>>> you still get a big fat warning. I wouldn't be at all surprised if
>>>>> Windows BSODs.
>>>>
>>>>
>>>> We cannot allow writing some particular value to MSRs successfully,
>>>> while injecting #GP when writing other values to the same MSRs. So #GP
>>>> is not option.
>>>>
>>>> ENCLS[EINIT], on the other hand, returns an actual
>>>>>
>>>>> error code. I'm not sure that a sensible error code exists
>>>>> ("SGX_HYPERVISOR_SAID_NO?", perhaps),
>>>>
>>>>
>>>> Looks no such error code exists. And we cannot return such error code to
>>>> guest as such error code is only supposed to be valid when ENCLS is run
>>>> in hypervisor.
>>>>
>>>> but SGX_INVALID_EINITTOKEN seems
>>>>>
>>>>> to mean, more or less, "the CPU thinks you're not authorized to do
>>>>> this", so forcing that error code could be entirely reasonable.
>>>>>
>>>>> If the host policy is to allow a list of LE signers, you could return
>>>>> SGX_INVALID_EINITTOKEN if the guest tries to EINIT an LE that isn't in
>>>>> the list.
>>>>
>>>>
>>>> But this would be inconsistent with HW behavior. If the hash value in
>>>> guest's IA32_SGXLEPUBKEYHASHn is matched with the one passed by EINIT,
>>>> EINIT is not supposed to return SGX_INVALID_EINITTOKEN.
>>>>
>>>> I think from VMM's perspective, emulating HW behavior to be consistent
>>>> with real HW behavior is very important.
>>>>
>>>> Paolo, would you provide your comments?
>>>
>>>
>>> Hi all,
>>>
>>> This has been quite for a while and I'd like to start discussion again.
>>> Jarkko told me that currently he only supports one LE in SGX driver, but
>>> I
>>> am not sure whether he is going to extend in the future or not. I think
>>> this
>>> might also depend on requirements from customers.
>>>
>>> Andy,
>>>
>>> If we only support one LE in driver, then we can only support the same LE
>>> for all KVM guests, according to your comments that host kernel launch
>>> control policy should also apply to KVM guests? WOuld you comments more?
>>>
>>> Jarkko,
>>>
>>> Could you help to clarify the whole launch control policy in host side so
>>> that we can have a better understanding together?
>>>
>>> Thanks,
>>> -Kai
>>
>>
>> So. I have pass through LE. It creates EINITTOKEN for anything. Couldn't
>> VMM keep virtual values for MSRs and ask host side LE create token when
>> it needs to?
>
>
> Hi Jarkko,
>
> Thanks for replying. VMM doesn't need driver to generate EINITTOKEN. The
> EINITTOKEN is from guest too, upon VMM traps EINIT.
>
> I think Andy's comments is, if host SGX driver only allows someone's LE to
> run (ex, LE from Intel, Redhat, etc...), then SGX driver should also govern
> LEs from KVM guests as well, by checking SIGSTRUCT from KVM guest (the
> SIGSTRUCT is provided by KVM by trapping guest's EINIT).
>
> In my understanding, although you only allows one LE in kernel, but you
> won't limit who's LE can be run (basically kernel can run LE signed by
> anyone, but just one LE when kernel is running), so I don't see there is any
> limitation to KVM guests here.
>
> 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?
My understanding is that the kernel will not (at least initially)
allow users to supply an LE. The kernel will handle launches all by
itself. How it does so is an implementation detail, and it seems to
me that it will cause compatibility issues if guests have to use the
host's LE.
sgx_validate_sigstruct(...) sounds like it could be a good approach to me.
>
> Thanks,
> -Kai
>
>>
>> /Jarkko
>>
>
