mbox series

[v2,0/3] x86/sgx: eextend ioctl

Message ID da7ae1e7-59b8-63db-a9f1-607b4e529639@fortanix.com (mailing list archive)
Headers show
Series x86/sgx: eextend ioctl | expand

Message

Raoul Strackx April 12, 2021, 8:59 a.m. UTC
Creation of an SGX enclave consists of three steps. First, a new enclave
environment is created by the ECREATE leaf function. Some enclave settings
are specified at this step by passing an SGX Enclave Control Structure
(SECS) that contains the enclave MRENCLAVE, MRSIGNER, etc. This
instruction also starts a cryptographic log of the enclave being built.
(This log should eventually result in the MRENCLAVE.) Second, pages are
added to the enclave. The EADD leaf function copies 4KB data to an empty
EPC page. The cryptographic log records (among other properties) the
location and access rights of the page being added. It _does not_ include
an entry of the page content. When the enclave writer wishes to ensure the
content of (a part of) the enclave page as well, she must use the EEXTEND
leaf function. Extending the enclave cryptographic log can only be done
per 256 bytes. Extending the log with a full 4K page thus requires 16
invocations of the EEXTEND leaf function. It is however up to the enclave
developer to decide if and how enclave memory is added to the 
cryptographic log. EEXTEND functions may be issued only for relevant parts
of an enclave page, may happen only after all pages have been added, and
so on. Finally, the enclave is finalized by the EINIT leaf function. Any
new invocations of the EADD or EEXTEND leaf functions will result in a
fault. With EINIT a number of checks are performed as well. The 
cryptographic hash of the final cryptographic log is compared to the
MRENCLAVE field of the SECS structure passed to the ECREATE leaf function
(see step one). The signature (MRSIGNER) over this MRENCLAVE is verified
as well. When all checks pass, the enclave loading is complete and it
enters the executable state.

The SGX driver currently only supports extending the cryptographic log as
part of the EADD leaf function and _must_ cover complete 4K pages.
Enclaves not constructed within these constraints, currently cannot be
loaded on the Linux platform. Trying to do so will result in a different
cryptographic log; the MRENCLAVE specified at enclave creation time will
not match the cryptographic log kept by the processor and EINIT will fail.
This poses practical problems:
- The current driver does not fully support all possible SGXv1 enclaves.
  It creates a separation between enclaves that run everywhere and
  enclaves that run everywhere, except on Linux. This includes enclaves
  already in use on other systems today.
- It limits optimizations loaders are able to perform. For example, by
  only measuring relevant parts of enclave pages, load time can be
  minimized.

This patch set adds a new ioctl to enable userspace to execute EEXTEND
leaf functions per 256 bytes of enclave memory. With this patch in place,
Linux will be able to build all valid SGXv1 enclaves.

See additional discussion at:
https://lore.kernel.org/linux-sgx/20200220221038.GA26618@linux.intel.com/
T/#m93597f53d354201e72e26d93a968f167fcdf5930


Raoul Strackx (3):
  x86/sgx: Adding eextend ioctl
  x86/sgx: Fix compatibility issue with OPENSSL < 1.1.0
  x86/sgx: eextend ioctl selftest

 arch/x86/include/uapi/asm/sgx.h         | 11 +++++
 arch/x86/kernel/cpu/sgx/ioctl.c         | 81 ++++++++++++++++++++++++++++-----
 tools/testing/selftests/sgx/defines.h   |  1 +
 tools/testing/selftests/sgx/load.c      | 57 +++++++++++++++++++----
 tools/testing/selftests/sgx/main.h      |  1 +
 tools/testing/selftests/sgx/sigstruct.c | 43 ++++++++---------
 6 files changed, 154 insertions(+), 40 deletions(-)

Comments

Dave Hansen April 12, 2021, 3:36 p.m. UTC | #1
On 4/12/21 1:59 AM, Raoul Strackx wrote:
> This patch set adds a new ioctl to enable userspace to execute EEXTEND
> leaf functions per 256 bytes of enclave memory. With this patch in place,
> Linux will be able to build all valid SGXv1 enclaves.

This didn't cover why we need a *NEW* ABI for this instead of relaxing
the page alignment rules in the existing one.
Jethro Beekman April 12, 2021, 3:58 p.m. UTC | #2
On 2021-04-12 17:36, Dave Hansen wrote:
> On 4/12/21 1:59 AM, Raoul Strackx wrote:
>> This patch set adds a new ioctl to enable userspace to execute EEXTEND
>> leaf functions per 256 bytes of enclave memory. With this patch in place,
>> Linux will be able to build all valid SGXv1 enclaves.
> 
> This didn't cover why we need a *NEW* ABI for this instead of relaxing
> the page alignment rules in the existing one.
> 

In executing the ECREATE, EADD, EEXTEND, EINIT sequence, you currently have 2 options for EADD/EEXTEND using the SGX_IOC_ENCLAVE_ADD_PAGES ioctl:
- execute EADD on any address
- execute EADD on any address followed by 16× EEXTEND for that address span

Could you be more specific on how you're suggesting that the current ioctl is modified to in addition support the following?
- execute EEXTEND on any address

--
Jethro Beekman | Fortanix
Dave Hansen April 12, 2021, 4:40 p.m. UTC | #3
On 4/12/21 8:58 AM, Jethro Beekman wrote:
> On 2021-04-12 17:36, Dave Hansen wrote:
>> On 4/12/21 1:59 AM, Raoul Strackx wrote:
>>> This patch set adds a new ioctl to enable userspace to execute EEXTEND
>>> leaf functions per 256 bytes of enclave memory. With this patch in place,
>>> Linux will be able to build all valid SGXv1 enclaves.
>> This didn't cover why we need a *NEW* ABI for this instead of relaxing
>> the page alignment rules in the existing one.
>>
> In executing the ECREATE, EADD, EEXTEND, EINIT sequence, you currently have 2 options for EADD/EEXTEND using the SGX_IOC_ENCLAVE_ADD_PAGES ioctl:
> - execute EADD on any address
> - execute EADD on any address followed by 16× EEXTEND for that address span

I think you forgot a key piece of the explanation here.  The choice as
to whether you just EADD or EADD+16xEEXTEND is governed by the addition
of the: SGX_PAGE_MEASURE flag.

> Could you be more specific on how you're suggesting that the current ioctl is modified to in addition support the following?
> - execute EEXTEND on any address

I'm still not convinced you *NEED* EEXTEND on arbitrary addresses.

Right now, we have (roughly):

	 ioctl(ADD_PAGES, ptr, PAGE_SIZE, MEASURE)

which translates in the kernel to:

	__eadd(ptr, epc)
	if (flags & MEASURE) {
		for (i = 0; i < PAGE_SIZE/256; i++)
			__eextend(epc + i*256);
	}

Instead, we could allow add_arg.src and add_arg.offset to be
non-page-aligned.  Then, we still do the same __eadd(), but modify the
__eextend() loop to only cover the actual range referred to by 'add_arg'.

The downside is that you only get a single range of measured data per
page.  Let's say a 'X' means measured (EEXTEND'ed) and '_' means not.
You could have patterns like:

	XXXXXXXXXXXXXXXX
or
	XXXXXXXXXXXXXXX_
or
	____XXXXXXXXXXXX

but not:

	_X_X_X_X_X_X_X_X
or
	_XXXXXXXXXXXXXX_


Is that a problem?
Jethro Beekman April 12, 2021, 4:41 p.m. UTC | #4
On 2021-04-12 18:40, Dave Hansen wrote:
> On 4/12/21 8:58 AM, Jethro Beekman wrote:
>> On 2021-04-12 17:36, Dave Hansen wrote:
>>> On 4/12/21 1:59 AM, Raoul Strackx wrote:
>>>> This patch set adds a new ioctl to enable userspace to execute EEXTEND
>>>> leaf functions per 256 bytes of enclave memory. With this patch in place,
>>>> Linux will be able to build all valid SGXv1 enclaves.
>>> This didn't cover why we need a *NEW* ABI for this instead of relaxing
>>> the page alignment rules in the existing one.
>>>
>> In executing the ECREATE, EADD, EEXTEND, EINIT sequence, you currently have 2 options for EADD/EEXTEND using the SGX_IOC_ENCLAVE_ADD_PAGES ioctl:
>> - execute EADD on any address
>> - execute EADD on any address followed by 16× EEXTEND for that address span
> 
> I think you forgot a key piece of the explanation here.  The choice as
> to whether you just EADD or EADD+16xEEXTEND is governed by the addition
> of the: SGX_PAGE_MEASURE flag.
> 
>> Could you be more specific on how you're suggesting that the current ioctl is modified to in addition support the following?
>> - execute EEXTEND on any address
> 
> I'm still not convinced you *NEED* EEXTEND on arbitrary addresses.
> 
> Right now, we have (roughly):
> 
> 	 ioctl(ADD_PAGES, ptr, PAGE_SIZE, MEASURE)
> 
> which translates in the kernel to:
> 
> 	__eadd(ptr, epc)
> 	if (flags & MEASURE) {
> 		for (i = 0; i < PAGE_SIZE/256; i++)
> 			__eextend(epc + i*256);
> 	}
> 
> Instead, we could allow add_arg.src and add_arg.offset to be
> non-page-aligned.  Then, we still do the same __eadd(), but modify the
> __eextend() loop to only cover the actual range referred to by 'add_arg'.
> 
> The downside is that you only get a single range of measured data per
> page.  Let's say a 'X' means measured (EEXTEND'ed) and '_' means not.
> You could have patterns like:
> 
> 	XXXXXXXXXXXXXXXX
> or
> 	XXXXXXXXXXXXXXX_
> or
> 	____XXXXXXXXXXXX
> 
> but not:
> 
> 	_X_X_X_X_X_X_X_X
> or
> 	_XXXXXXXXXXXXXX_
> 
> 
> Is that a problem?
> 

Yes this still doesn't let one execute all possible ECREATE, EADD, EEXTEND, EINIT sequences.

--
Jethro Beekman | Fortanix
Dave Hansen April 12, 2021, 4:47 p.m. UTC | #5
On 4/12/21 9:41 AM, Jethro Beekman wrote:
> Yes this still doesn't let one execute all possible ECREATE, EADD, EEXTEND, EINIT sequences.

OK, so we're going in circles now.

I don't believe we necessarily *WANT* or need Linux to support "all
possible ECREATE, EADD, EEXTEND, EINIT sequences".  Yet, it's what is
being used to justify this series without any other justification.

It's going to be a different story if you bring me a real enclave that
*REALLY* wants to do this for good reasons.
Jethro Beekman April 12, 2021, 5:01 p.m. UTC | #6
On 2021-04-12 18:47, Dave Hansen wrote:
> On 4/12/21 9:41 AM, Jethro Beekman wrote:
>> Yes this still doesn't let one execute all possible ECREATE, EADD, EEXTEND, EINIT sequences.
> 
> OK, so we're going in circles now.
> 
> I don't believe we necessarily *WANT* or need Linux to support "all
> possible ECREATE, EADD, EEXTEND, EINIT sequences".  Yet, it's what is
> being used to justify this series without any other justification.
> 
> It's going to be a different story if you bring me a real enclave that
> *REALLY* wants to do this for good reasons.
> 

It's still not clear to me what your motivations are for trying to keep Linux incompatible with the rest of the world.

--
Jethro Beekman | Fortanix
Jarkko Sakkinen April 14, 2021, 10:52 a.m. UTC | #7
On Mon, Apr 12, 2021 at 10:59:56AM +0200, Raoul Strackx wrote:
> Creation of an SGX enclave consists of three steps. First, a new enclave
> environment is created by the ECREATE leaf function. Some enclave settings
> are specified at this step by passing an SGX Enclave Control Structure
> (SECS) that contains the enclave MRENCLAVE, MRSIGNER, etc. This
> instruction also starts a cryptographic log of the enclave being built.
> (This log should eventually result in the MRENCLAVE.) Second, pages are
> added to the enclave. The EADD leaf function copies 4KB data to an empty
> EPC page. The cryptographic log records (among other properties) the
> location and access rights of the page being added. It _does not_ include
> an entry of the page content. When the enclave writer wishes to ensure the
> content of (a part of) the enclave page as well, she must use the EEXTEND
> leaf function. Extending the enclave cryptographic log can only be done
> per 256 bytes. Extending the log with a full 4K page thus requires 16
> invocations of the EEXTEND leaf function. It is however up to the enclave
> developer to decide if and how enclave memory is added to the 
> cryptographic log. EEXTEND functions may be issued only for relevant parts
> of an enclave page, may happen only after all pages have been added, and
> so on. Finally, the enclave is finalized by the EINIT leaf function. Any
> new invocations of the EADD or EEXTEND leaf functions will result in a
> fault. With EINIT a number of checks are performed as well. The 
> cryptographic hash of the final cryptographic log is compared to the
> MRENCLAVE field of the SECS structure passed to the ECREATE leaf function
> (see step one). The signature (MRSIGNER) over this MRENCLAVE is verified
> as well. When all checks pass, the enclave loading is complete and it
> enters the executable state.

Who do you expect to read this paragraph, seriously?

> The SGX driver currently only supports extending the cryptographic log as
> part of the EADD leaf function and _must_ cover complete 4K pages.
> Enclaves not constructed within these constraints, currently cannot be
> loaded on the Linux platform. Trying to do so will result in a different
> cryptographic log; the MRENCLAVE specified at enclave creation time will
> not match the cryptographic log kept by the processor and EINIT will fail.
> This poses practical problems:
> - The current driver does not fully support all possible SGXv1 enclaves.
>   It creates a separation between enclaves that run everywhere and
>   enclaves that run everywhere, except on Linux. This includes enclaves
>   already in use on other systems today.
> - It limits optimizations loaders are able to perform. For example, by
>   only measuring relevant parts of enclave pages, load time can be
>   minimized.
> 
> This patch set adds a new ioctl to enable userspace to execute EEXTEND
> leaf functions per 256 bytes of enclave memory. With this patch in place,
> Linux will be able to build all valid SGXv1 enclaves.
> 
> See additional discussion at:
> https://lore.kernel.org/linux-sgx/20200220221038.GA26618@linux.intel.com/
> T/#m93597f53d354201e72e26d93a968f167fcdf5930
> 
> 
> Raoul Strackx (3):
>   x86/sgx: Adding eextend ioctl
>   x86/sgx: Fix compatibility issue with OPENSSL < 1.1.0
>   x86/sgx: eextend ioctl selftest
> 
>  arch/x86/include/uapi/asm/sgx.h         | 11 +++++
>  arch/x86/kernel/cpu/sgx/ioctl.c         | 81 ++++++++++++++++++++++++++++-----
>  tools/testing/selftests/sgx/defines.h   |  1 +
>  tools/testing/selftests/sgx/load.c      | 57 +++++++++++++++++++----
>  tools/testing/selftests/sgx/main.h      |  1 +
>  tools/testing/selftests/sgx/sigstruct.c | 43 ++++++++---------
>  6 files changed, 154 insertions(+), 40 deletions(-)
> 
> -- 
> 2.7.4
> 
> 

/Jarkko
Jethro Beekman April 14, 2021, 11:01 a.m. UTC | #8
On 2021-04-14 12:52, Jarkko Sakkinen wrote:
> On Mon, Apr 12, 2021 at 10:59:56AM +0200, Raoul Strackx wrote:
>> Creation of an SGX enclave consists of three steps. First, a new enclave
>> environment is created by the ECREATE leaf function. Some enclave settings
>> are specified at this step by passing an SGX Enclave Control Structure
>> (SECS) that contains the enclave MRENCLAVE, MRSIGNER, etc. This
>> instruction also starts a cryptographic log of the enclave being built.
>> (This log should eventually result in the MRENCLAVE.) Second, pages are
>> added to the enclave. The EADD leaf function copies 4KB data to an empty
>> EPC page. The cryptographic log records (among other properties) the
>> location and access rights of the page being added. It _does not_ include
>> an entry of the page content. When the enclave writer wishes to ensure the
>> content of (a part of) the enclave page as well, she must use the EEXTEND
>> leaf function. Extending the enclave cryptographic log can only be done
>> per 256 bytes. Extending the log with a full 4K page thus requires 16
>> invocations of the EEXTEND leaf function. It is however up to the enclave
>> developer to decide if and how enclave memory is added to the 
>> cryptographic log. EEXTEND functions may be issued only for relevant parts
>> of an enclave page, may happen only after all pages have been added, and
>> so on. Finally, the enclave is finalized by the EINIT leaf function. Any
>> new invocations of the EADD or EEXTEND leaf functions will result in a
>> fault. With EINIT a number of checks are performed as well. The 
>> cryptographic hash of the final cryptographic log is compared to the
>> MRENCLAVE field of the SECS structure passed to the ECREATE leaf function
>> (see step one). The signature (MRSIGNER) over this MRENCLAVE is verified
>> as well. When all checks pass, the enclave loading is complete and it
>> enters the executable state.
> 
> Who do you expect to read this paragraph, seriously?

What do you mean? There was a request for more architectural details in the cover letter.

> 
>> The SGX driver currently only supports extending the cryptographic log as
>> part of the EADD leaf function and _must_ cover complete 4K pages.
>> Enclaves not constructed within these constraints, currently cannot be
>> loaded on the Linux platform. Trying to do so will result in a different
>> cryptographic log; the MRENCLAVE specified at enclave creation time will
>> not match the cryptographic log kept by the processor and EINIT will fail.
>> This poses practical problems:
>> - The current driver does not fully support all possible SGXv1 enclaves.
>>   It creates a separation between enclaves that run everywhere and
>>   enclaves that run everywhere, except on Linux. This includes enclaves
>>   already in use on other systems today.
>> - It limits optimizations loaders are able to perform. For example, by
>>   only measuring relevant parts of enclave pages, load time can be
>>   minimized.
>>
>> This patch set adds a new ioctl to enable userspace to execute EEXTEND
>> leaf functions per 256 bytes of enclave memory. With this patch in place,
>> Linux will be able to build all valid SGXv1 enclaves.
>>
>> See additional discussion at:
>> https://lore.kernel.org/linux-sgx/20200220221038.GA26618@linux.intel.com/
>> T/#m93597f53d354201e72e26d93a968f167fcdf5930
>>
>>
>> Raoul Strackx (3):
>>   x86/sgx: Adding eextend ioctl
>>   x86/sgx: Fix compatibility issue with OPENSSL < 1.1.0
>>   x86/sgx: eextend ioctl selftest
>>
>>  arch/x86/include/uapi/asm/sgx.h         | 11 +++++
>>  arch/x86/kernel/cpu/sgx/ioctl.c         | 81 ++++++++++++++++++++++++++++-----
>>  tools/testing/selftests/sgx/defines.h   |  1 +
>>  tools/testing/selftests/sgx/load.c      | 57 +++++++++++++++++++----
>>  tools/testing/selftests/sgx/main.h      |  1 +
>>  tools/testing/selftests/sgx/sigstruct.c | 43 ++++++++---------
>>  6 files changed, 154 insertions(+), 40 deletions(-)
>>
>> -- 
>> 2.7.4
>>
>>
> 
> /Jarkko
> 

--
Jethro Beekman | Fortanix
Jarkko Sakkinen April 14, 2021, 11:07 a.m. UTC | #9
On Mon, Apr 12, 2021 at 07:01:39PM +0200, Jethro Beekman wrote:
> On 2021-04-12 18:47, Dave Hansen wrote:
> > On 4/12/21 9:41 AM, Jethro Beekman wrote:
> >> Yes this still doesn't let one execute all possible ECREATE, EADD, EEXTEND, EINIT sequences.
> > 
> > OK, so we're going in circles now.
> > 
> > I don't believe we necessarily *WANT* or need Linux to support "all
> > possible ECREATE, EADD, EEXTEND, EINIT sequences".  Yet, it's what is
> > being used to justify this series without any other justification.
> > 
> > It's going to be a different story if you bring me a real enclave that
> > *REALLY* wants to do this for good reasons.
> > 
> 
> It's still not clear to me what your motivations are for trying to keep Linux incompatible with the rest of the world.

What specifically are you referring as the "rest of the world"?

That would be mean that there is reviewable workload "out there".

/Jarkko
Jarkko Sakkinen April 16, 2021, 1:08 p.m. UTC | #10
On Wed, Apr 14, 2021 at 01:01:02PM +0200, Jethro Beekman wrote:
> On 2021-04-14 12:52, Jarkko Sakkinen wrote:
> > On Mon, Apr 12, 2021 at 10:59:56AM +0200, Raoul Strackx wrote:
> >> Creation of an SGX enclave consists of three steps. First, a new enclave
> >> environment is created by the ECREATE leaf function. Some enclave settings
> >> are specified at this step by passing an SGX Enclave Control Structure
> >> (SECS) that contains the enclave MRENCLAVE, MRSIGNER, etc. This
> >> instruction also starts a cryptographic log of the enclave being built.
> >> (This log should eventually result in the MRENCLAVE.) Second, pages are
> >> added to the enclave. The EADD leaf function copies 4KB data to an empty
> >> EPC page. The cryptographic log records (among other properties) the
> >> location and access rights of the page being added. It _does not_ include
> >> an entry of the page content. When the enclave writer wishes to ensure the
> >> content of (a part of) the enclave page as well, she must use the EEXTEND
> >> leaf function. Extending the enclave cryptographic log can only be done
> >> per 256 bytes. Extending the log with a full 4K page thus requires 16
> >> invocations of the EEXTEND leaf function. It is however up to the enclave
> >> developer to decide if and how enclave memory is added to the 
> >> cryptographic log. EEXTEND functions may be issued only for relevant parts
> >> of an enclave page, may happen only after all pages have been added, and
> >> so on. Finally, the enclave is finalized by the EINIT leaf function. Any
> >> new invocations of the EADD or EEXTEND leaf functions will result in a
> >> fault. With EINIT a number of checks are performed as well. The 
> >> cryptographic hash of the final cryptographic log is compared to the
> >> MRENCLAVE field of the SECS structure passed to the ECREATE leaf function
> >> (see step one). The signature (MRSIGNER) over this MRENCLAVE is verified
> >> as well. When all checks pass, the enclave loading is complete and it
> >> enters the executable state.
> > 
> > Who do you expect to read this paragraph, seriously?
> 
> What do you mean? There was a request for more architectural details in the cover letter.

So you are saying that it is well structured text and not a brain dump?

/Jarkko