Re: [PATCH V2 16/32] x86/sgx: Support restricting of enclave page permissions

[Reinette Chatre]

Hi Jarkko,

On 3/1/2022 5:42 AM, Jarkko Sakkinen wrote:
>> With EACCEPTCOPY (kudos to Mark S. for reminding me of this version of
>> EACCEPT @ chat.enarx.dev) it is possible to make R and RX pages but
>> obviously new RX pages are now out of the picture:
>>
>>
>> 	/*
>> 	 * Adding a regular page that is architecturally allowed to only
>> 	 * be created with RW permissions.
>> 	 * TBD: Interface with user space policy to support max permissions
>> 	 * of RWX.
>> 	 */
>> 	prot = PROT_READ | PROT_WRITE;
>> 	encl_page->vm_run_prot_bits = calc_vm_prot_bits(prot, 0);
>> 	encl_page->vm_max_prot_bits = encl_page->vm_run_prot_bits;
>>
>> If that TBD is left out to the final version the page augmentation has a
>> risk of a API bottleneck, and that risk can realize then also in the page
>> permission ioctls.
>>
>> I.e. now any review comment is based on not fully known territory, we have
>> one known unknown, and some unknown unknowns from unpredictable effect to
>> future API changes.

The plan to complete the "TBD" in the above snippet was to follow this work
with user policy integration at this location. On a high level the plan was
for this to look something like:


 	/*
 	 * Adding a regular page that is architecturally allowed to only
 	 * be created with RW permissions.
 	 * Interface with user space policy to support max permissions
 	 * of RWX.
 	 */
 	prot = PROT_READ | PROT_WRITE;
 	encl_page->vm_run_prot_bits = calc_vm_prot_bits(prot, 0);

        if (user space policy allows RWX on dynamically added pages)
	 	encl_page->vm_max_prot_bits = calc_vm_prot_bits(PROT_READ | PROT_WRITE | PROT_EXEC, 0);
	else
		encl_page->vm_max_prot_bits = calc_vm_prot_bits(PROT_READ | PROT_WRITE, 0);

The work that follows this series aimed to do the integration with user
space policy.

> I think the best way to move forward would be to do EAUG's explicitly with
> an ioctl that could also include secinfo for permissions. Then you can
> easily do the rest with EACCEPTCOPY inside the enclave.

SGX_IOC_ENCLAVE_ADD_PAGES already exists and could possibly be used for
this purpose. It already includes SECINFO which may also be useful if
needing to later support EAUG of PT_SS* pages.

How this could work is user space calls SGX_IOC_ENCLAVE_ADD_PAGES
after enclave initialization on any memory region within the enclave where
pages are planned to be added dynamically. This ioctl() calls EAUG to add the
new pages with RW permissions and their vm_max_prot_bits can be set to the
permissions found in the included SECINFO. This will support later EACCEPTCOPY
as well as SGX_IOC_ENCLAVE_RELAX_PERMISSIONS

The big question is whether communicating user policy after enclave initialization
via the SECINFO within SGX_IOC_ENCLAVE_ADD_PAGES is acceptable to all? I would
appreciate a confirmation on this direction considering the significant history
behind this topic.
 
> Putting EAUG to the #PF handler and implicitly call it just too flakky and
> hard to make deterministic for e.g. JIT compiler in our use case (not to
> mention that JIT is not possible at all because inability to do RX pages).

In this series this is indeed not possible because it lacks the user policy
integration. JIT will be possible after user policy integration.

Reinette