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/[email protected]/
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
SGXv1 enclaves can be created by an ECREATE, followed by any number of
EADD and EEXTEND functions. It is finalized by an EINIT. The SGX enclave
vendor defines the order of these invocations when the enclave is being
developed, and cannot be changed later on. Currently enclave measurements
can only be extended per 4K measurements immediately after the page has
been added by the EADD instruction. This commit adds a new ioctl to
execute the EEXTEND leaf function per 256 bytes of enclave memory. In
combination with the SGX_IOC_ENCLAVE_ADD_PAGES ioctl (without the
SGX_PAGE_MEASURE flag), this enables the driver to load all SGXv1
compatible enclaves.
Signed-off-by: Raoul Strackx <[email protected]>
---
arch/x86/include/uapi/asm/sgx.h | 11 ++++++
arch/x86/kernel/cpu/sgx/ioctl.c | 81 +++++++++++++++++++++++++++++++++++------
2 files changed, 81 insertions(+), 11 deletions(-)
diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
index 9034f30..121ca5f 100644
--- a/arch/x86/include/uapi/asm/sgx.h
+++ b/arch/x86/include/uapi/asm/sgx.h
@@ -27,6 +27,8 @@ enum sgx_page_flags {
_IOW(SGX_MAGIC, 0x02, struct sgx_enclave_init)
#define SGX_IOC_ENCLAVE_PROVISION \
_IOW(SGX_MAGIC, 0x03, struct sgx_enclave_provision)
+#define SGX_IOC_ENCLAVE_EXTEND \
+ _IOW(SGX_MAGIC, 0x04, struct sgx_enclave_extend)
/**
* struct sgx_enclave_create - parameter structure for the
@@ -57,6 +59,15 @@ struct sgx_enclave_add_pages {
};
/**
+ * struct sgx_enclave_extend - parameter structure for the
+ * %SGX_IOC_ENCLAVE_MEASURE ioctl
+ * @offset: offset of the data from the start address for the data
+ */
+struct sgx_enclave_extend {
+ __u64 offset;
+};
+
+/**
* struct sgx_enclave_init - parameter structure for the
* %SGX_IOC_ENCLAVE_INIT ioctl
* @sigstruct: address for the SIGSTRUCT data
diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index 90a5caf..69521e9 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -261,20 +261,20 @@ static int __sgx_encl_add_page(struct sgx_encl *encl,
return ret ? -EIO : 0;
}
-/*
- * If the caller requires measurement of the page as a proof for the content,
- * use EEXTEND to add a measurement for 256 bytes of the page. Repeat this
- * operation until the entire page is measured."
- */
-static int __sgx_encl_extend(struct sgx_encl *encl,
- struct sgx_epc_page *epc_page)
+static int __sgx_encl_extend_chunk(struct sgx_encl *encl,
+ void *chunk, unsigned long size)
{
unsigned long offset;
int ret;
+ void *secs_addr;
- for (offset = 0; offset < PAGE_SIZE; offset += SGX_EEXTEND_BLOCK_SIZE) {
- ret = __eextend(sgx_get_epc_virt_addr(encl->secs.epc_page),
- sgx_get_epc_virt_addr(epc_page) + offset);
+ if (!size || !IS_ALIGNED(size, SGX_EEXTEND_BLOCK_SIZE))
+ return -EINVAL;
+
+ secs_addr = sgx_get_epc_virt_addr(encl->secs.epc_page);
+ for (offset = 0; offset < size; offset += SGX_EEXTEND_BLOCK_SIZE) {
+ ret = __eextend(secs_addr,
+ chunk + offset);
if (ret) {
if (encls_failed(ret))
ENCLS_WARN(ret, "EEXTEND");
@@ -286,6 +286,19 @@ static int __sgx_encl_extend(struct sgx_encl *encl,
return 0;
}
+/*
+ * If the caller requires measurement of the page as a proof for the content,
+ * use EEXTEND to add a measurement for 256 bytes of the page. Repeat this
+ * operation until the entire page is measured."
+ */
+static int __sgx_encl_extend_page(struct sgx_encl *encl,
+ struct sgx_epc_page *epc_page)
+{
+ void *chunk = sgx_get_epc_virt_addr(epc_page);
+
+ return __sgx_encl_extend_chunk(encl, chunk, PAGE_SIZE);
+}
+
static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long src,
unsigned long offset, struct sgx_secinfo *secinfo,
unsigned long flags)
@@ -346,7 +359,7 @@ static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long src,
encl->secs_child_cnt++;
if (flags & SGX_PAGE_MEASURE) {
- ret = __sgx_encl_extend(encl, epc_page);
+ ret = __sgx_encl_extend_page(encl, epc_page);
if (ret)
goto err_out;
}
@@ -466,6 +479,49 @@ static long sgx_ioc_enclave_add_pages(struct sgx_encl *encl, void __user *arg)
return ret;
}
+static long sgx_ioc_enclave_extend(struct sgx_encl *encl, void __user *user_arg)
+{
+ struct sgx_enclave_extend arg;
+ struct sgx_encl_page *encl_page;
+ void *chunk;
+ long ret = 0;
+
+ if (!test_bit(SGX_ENCL_CREATED, &encl->flags) ||
+ test_bit(SGX_ENCL_INITIALIZED, &encl->flags))
+ return -EINVAL;
+
+ if (copy_from_user(&arg, user_arg, sizeof(arg)))
+ return -EFAULT;
+
+ if (!arg.offset || !IS_ALIGNED(arg.offset, SGX_EEXTEND_BLOCK_SIZE)) {
+ pr_info("offset not a multiple of 256: %llu\n", arg.offset);
+ return -EINVAL;
+ }
+
+ encl_page = xa_load(&encl->page_array, PFN_DOWN(encl->base + arg.offset));
+
+ if (!encl_page) {
+ pr_info("enc page not found\n");
+ return -EFAULT;
+ }
+
+ mmap_read_lock(current->mm);
+ mutex_lock(&encl->lock);
+ sgx_unmark_page_reclaimable(encl_page->epc_page);
+
+ chunk = sgx_get_epc_virt_addr(encl_page->epc_page) + (arg.offset & (PAGE_SIZE - 1));
+
+ if (__sgx_encl_extend_chunk(encl, chunk, SGX_EEXTEND_BLOCK_SIZE)) {
+ pr_info("extend returned an error\n");
+ ret = -EFAULT;
+ }
+
+ sgx_mark_page_reclaimable(encl_page->epc_page);
+ mutex_unlock(&encl->lock);
+ mmap_read_unlock(current->mm);
+ return ret;
+}
+
static int __sgx_get_key_hash(struct crypto_shash *tfm, const void *modulus,
void *hash)
{
@@ -706,6 +762,9 @@ long sgx_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
case SGX_IOC_ENCLAVE_PROVISION:
ret = sgx_ioc_enclave_provision(encl, (void __user *)arg);
break;
+ case SGX_IOC_ENCLAVE_EXTEND:
+ ret = sgx_ioc_enclave_extend(encl, (void __user *)arg);
+ break;
default:
ret = -ENOIOCTLCMD;
break;
--
2.7.4
In order to test the new eextend ioctl, the SGX selftest is modified to
only partially measure the last page of segments. Most segments are larger
than 4k, so the MEASURE flag for SGX_IOC_ENCLAVE_ADD_PAGE is still being
tested.
Signed-off-by: Raoul Strackx <[email protected]>
---
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 | 38 +++++++++++-----------
4 files changed, 69 insertions(+), 28 deletions(-)
diff --git a/tools/testing/selftests/sgx/defines.h b/tools/testing/selftests/sgx/defines.h
index 592c1cc..c09550d 100644
--- a/tools/testing/selftests/sgx/defines.h
+++ b/tools/testing/selftests/sgx/defines.h
@@ -10,6 +10,7 @@
#define PAGE_SIZE 4096
#define PAGE_MASK (~(PAGE_SIZE - 1))
+#define SGX_EEXTEND_BLOCK_SIZE 256
#define __aligned(x) __attribute__((__aligned__(x)))
#define __packed __attribute__((packed))
diff --git a/tools/testing/selftests/sgx/load.c b/tools/testing/selftests/sgx/load.c
index 9d43b75..39484fc 100644
--- a/tools/testing/selftests/sgx/load.c
+++ b/tools/testing/selftests/sgx/load.c
@@ -102,28 +102,69 @@ static bool encl_ioc_create(struct encl *encl)
static bool encl_ioc_add_pages(struct encl *encl, struct encl_segment *seg)
{
struct sgx_enclave_add_pages ioc;
+ struct sgx_enclave_extend ioc_extend;
struct sgx_secinfo secinfo;
+ uint64_t size_full_pages;
+ uint64_t size;
+ uint64_t chunk_offset;
int rc;
+ size_full_pages = size_fit(seg->size, SGX_EEXTEND_BLOCK_SIZE) & PAGE_MASK;
+ size = size_fit(seg->size, SGX_EEXTEND_BLOCK_SIZE);
+
memset(&secinfo, 0, sizeof(secinfo));
secinfo.flags = seg->flags;
+ // Add and extend full pages
ioc.src = (uint64_t)encl->src + seg->offset;
ioc.offset = seg->offset;
- ioc.length = seg->size;
ioc.secinfo = (unsigned long)&secinfo;
+ ioc.length = size_full_pages;
ioc.flags = SGX_PAGE_MEASURE;
- rc = ioctl(encl->fd, SGX_IOC_ENCLAVE_ADD_PAGES, &ioc);
- if (rc < 0) {
- fprintf(stderr, "SGX_IOC_ENCLAVE_ADD_PAGES failed: errno=%d.\n",
- errno);
- return false;
+ if (ioc.length > 0) {
+ rc = ioctl(encl->fd, SGX_IOC_ENCLAVE_ADD_PAGES, &ioc);
+ if (rc < 0) {
+ fprintf(stderr, "SGX_IOC_ENCLAVE_ADD_PAGES failed: errno=%d.\n",
+ errno);
+ return false;
+ }
+ }
+
+ if (size_full_pages < size) {
+ // Add last, partly measured page
+ ioc.offset = seg->offset + size_full_pages;
+ ioc.length = 0x1000;
+ ioc.flags = 0;
+
+ rc = ioctl(encl->fd, SGX_IOC_ENCLAVE_ADD_PAGES, &ioc);
+ if (rc < 0) {
+ fprintf(stderr, "SGX_IOC_ENCLAVE_ADD_PAGES failed: errno=%d.\n",
+ errno);
+ return false;
+ }
+
+ // extend chunks
+ for (chunk_offset = 0; chunk_offset < size - size_full_pages;
+ chunk_offset += SGX_EEXTEND_BLOCK_SIZE) {
+ ioc_extend.offset = seg->offset + size_full_pages + chunk_offset;
+ rc = ioctl(encl->fd, SGX_IOC_ENCLAVE_EXTEND, &ioc_extend);
+ if (rc < 0) {
+ fprintf(stderr, "SGX_IOC_ENCLAVE_EXTEND failed: errno=%d.\n",
+ errno);
+ return false;
+ }
+ }
}
return true;
}
+uint64_t size_fit(uint64_t size_in_bytes, uint64_t block_size)
+{
+ return (size_in_bytes + block_size - 1) & (~(block_size - 1));
+}
+
bool encl_load(const char *path, struct encl *encl)
{
Elf64_Phdr *phdr_tbl;
@@ -197,7 +238,7 @@ bool encl_load(const char *path, struct encl *encl)
}
seg->offset = (phdr->p_offset & PAGE_MASK) - src_offset;
- seg->size = (phdr->p_filesz + PAGE_SIZE - 1) & PAGE_MASK;
+ seg->size = phdr->p_filesz;
printf("0x%016lx 0x%016lx 0x%02x\n", seg->offset, seg->size,
seg->prot);
@@ -209,7 +250,7 @@ bool encl_load(const char *path, struct encl *encl)
encl->src = encl->bin + src_offset;
encl->src_size = encl->segment_tbl[j - 1].offset +
- encl->segment_tbl[j - 1].size;
+ size_fit(encl->segment_tbl[j - 1].size, PAGE_SIZE);
for (encl->encl_size = 4096; encl->encl_size < encl->src_size; )
encl->encl_size <<= 1;
diff --git a/tools/testing/selftests/sgx/main.h b/tools/testing/selftests/sgx/main.h
index 67211a7..9d63bda 100644
--- a/tools/testing/selftests/sgx/main.h
+++ b/tools/testing/selftests/sgx/main.h
@@ -34,6 +34,7 @@ void encl_delete(struct encl *ctx);
bool encl_load(const char *path, struct encl *encl);
bool encl_measure(struct encl *encl);
bool encl_build(struct encl *encl);
+uint64_t size_fit(uint64_t size_in_bytes, uint64_t block_size);
int sgx_call_vdso(void *rdi, void *rsi, long rdx, u32 function, void *r8, void *r9,
struct sgx_enclave_run *run);
diff --git a/tools/testing/selftests/sgx/sigstruct.c b/tools/testing/selftests/sgx/sigstruct.c
index aac9cbc..eba7c86 100644
--- a/tools/testing/selftests/sgx/sigstruct.c
+++ b/tools/testing/selftests/sgx/sigstruct.c
@@ -260,28 +260,25 @@ static bool mrenclave_eextend(EVP_MD_CTX *ctx, uint64_t offset,
const uint8_t *data)
{
struct mreextend mreextend;
- int i;
- for (i = 0; i < 0x1000; i += 0x100) {
- memset(&mreextend, 0, sizeof(mreextend));
- mreextend.tag = MREEXTEND;
- mreextend.offset = offset + i;
+ memset(&mreextend, 0, sizeof(mreextend));
+ mreextend.tag = MREEXTEND;
+ mreextend.offset = offset;
- if (!mrenclave_update(ctx, &mreextend))
- return false;
+ if (!mrenclave_update(ctx, &mreextend))
+ return false;
- if (!mrenclave_update(ctx, &data[i + 0x00]))
- return false;
+ if (!mrenclave_update(ctx, &data[0x00]))
+ return false;
- if (!mrenclave_update(ctx, &data[i + 0x40]))
- return false;
+ if (!mrenclave_update(ctx, &data[0x40]))
+ return false;
- if (!mrenclave_update(ctx, &data[i + 0x80]))
- return false;
+ if (!mrenclave_update(ctx, &data[0x80]))
+ return false;
- if (!mrenclave_update(ctx, &data[i + 0xC0]))
- return false;
- }
+ if (!mrenclave_update(ctx, &data[0xC0]))
+ return false;
return true;
}
@@ -289,12 +286,13 @@ static bool mrenclave_eextend(EVP_MD_CTX *ctx, uint64_t offset,
static bool mrenclave_segment(EVP_MD_CTX *ctx, struct encl *encl,
struct encl_segment *seg)
{
- uint64_t end = seg->offset + seg->size;
+ uint64_t end = seg->offset + size_fit(seg->size, SGX_EEXTEND_BLOCK_SIZE);
uint64_t offset;
- for (offset = seg->offset; offset < end; offset += PAGE_SIZE) {
- if (!mrenclave_eadd(ctx, offset, seg->flags))
- return false;
+ for (offset = seg->offset; offset < end; offset += SGX_EEXTEND_BLOCK_SIZE) {
+ if (offset % PAGE_SIZE == 0)
+ if (!mrenclave_eadd(ctx, offset, seg->flags))
+ return false;
if (!mrenclave_eextend(ctx, offset, encl->src + offset))
return false;
--
2.7.4
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.
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
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.
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?
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
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
The `RSA_get0_key` function only got introduced in OpenSSL 1.1.0. This
makes compilation fail with older versions.
Signed-off-by: Raoul Strackx <[email protected]>
---
tools/testing/selftests/sgx/sigstruct.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/sgx/sigstruct.c b/tools/testing/selftests/sgx/sigstruct.c
index dee7a3d..aac9cbc 100644
--- a/tools/testing/selftests/sgx/sigstruct.c
+++ b/tools/testing/selftests/sgx/sigstruct.c
@@ -128,8 +128,11 @@ static bool check_crypto_errors(void)
static inline const BIGNUM *get_modulus(RSA *key)
{
const BIGNUM *n;
-
+#if OPENSSL_VERSION_NUMBER >= 0x10100000L
RSA_get0_key(key, &n, NULL, NULL);
+#else
+ n = key->n;
+#endif
return n;
}
--
2.7.4
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
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/[email protected]/
> 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
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/[email protected]/
>> 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
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