keystone-enclave / keystone-sdk Goto Github PK

Currently, we don't have userland debugging in the busybear setup we are using.

Once we do, not being able to attach to an enclave application for debugging will become a more serious problem.

Any debugging features are a serious potential security vulnerability, and need to be treated carefully.

If we want to allow debugging, the sm will have to allow enclave regions that are not PMP protected, and we'll need a way to ensure that the attestation report is either invalid or otherwise clearly indicates this.

I'm opening this up to discussion as to the features we need, and how we should support them.

When creating and destroying an enclave multiple times in the same binary, it crashes. This occurs after the 36th iteration.

Via Kaz on mailing list:

I've built and tested keystone on HiFive board using glibc-2.28 instead of glibc-2.26.
I'm doing it to get Debian Sid environment on the top of keystone.
Almost everything works fine for both keystone-hifive-unleashed and keystone dev branch.
A tiny issue found with glibc-2.28 is that tests and demo abort at near the end of the host code with
"free(): double free detected in tcache 2".
With the attached patch against sdk, they work well for me again.

patch:

diff --git a/lib/host/src/keystone.cpp b/lib/host/src/keystone.cpp
index 1dde700..408e8d8 100644
--- a/lib/host/src/keystone.cpp
+++ b/lib/host/src/keystone.cpp
@@ -16,8 +16,14 @@ Keystone::Keystone()
 
 Keystone::~Keystone()
 {
-  delete runtimeFile;
-  delete enclaveFile;
+  if(runtimeFile)
+  {
+    delete runtimeFile;
+  }
+  if(enclaveFile)
+  {
+    delete enclaveFile;
+  }
   destroy();
 }
 
@@ -73,6 +79,8 @@ keystone_status_t Keystone::init(const char* eapppath, const char* runtimepath,
   /* Files were consumed by driver and copied into epm, no longer needed */
   delete enclaveFile;
   delete runtimeFile;
+  enclaveFile = NULL;
+  runtimeFile = NULL;
 
   if(ret) {
     ERROR("failed to create enclave - ioctl() failed: %d", ret);

The maximum number of edge calls that can be registered
is hard-coded to 10.

This isn't optimal way of handling this, so we'd change it to use a dynamic data structure.

https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/keystone-enclave-forum/FGuZMBlR1-M/ZSsiTVV7BwAJ

hello,we want to know why untrusted shared memory must be from the buddy allocator, which results in that the data transmitted at one time must be less than or equal to 4MB, and the large-size data that needs to be processed by the eapp must be split for multiple transmission.
Thank you!
hupeng

I have made a wrong assumption that the starting VA of each program header is page-aligned, which is not true.
This causes a wrong page allocation with a misaligned VA, making the enclave non-functional.

At this moment, temporary fix has been applied for iozone benchmark (#17),
which just works for cases where the misaligned program header is contained in a non-overlapping page with the previous program header (e.g., phdr0 ends at page A and phdr1 starts at page B where A != B)
This should be re-written such that the ELF loader identifies misaligned VAs and handle them correctly.

Currently there are only minor plugin needs (USE_FREEMEM), but they aren't being managed by anything.
We need to integrate plugin configuration decisions into the build process and expose it.

Not quite sure where to open this issue, I put it here - if you think it should go somewhere else feel free to close this issue and put it where it belongs.

The initial idea for this issue comes from one of our recent papers: https://jovanbulck.github.io/files/acsac20-fpu.pdf where we reported similar issues in several SGX runtimes (including MS OE and Intel SDK).

The basic problem is that the global RISC-V FPU rounding control register frm is apparently not sanitised on enclave entry. This means that an untrusted attacker can change the rounding mode outside the enclave and - depending on the setting - the enclave will give different outputs for floating point computations.

See here for a (hacky) diff that - when applied to master - demonstrates this with the hello-native QEMU example:

poc_fpu_poisoning.txt

Basically this performs a floating-point computation (2.1 * 3.4) in the enclave with different rounding modes set from the untrusted host through fesetround(). The output is something like this:

# insmod keystone-driver.ko
[   12.225487] keystone_driver: loading out-of-tree module taints kernel.
[   12.231693] keystone_enclave: keystone enclave v1.0.0
# cd hello-native/
# ./hello-native.ke
Verifying archive integrity... All good.
Uncompressing Keystone Enclave Package
Run with fesetround(FE_UPWARD)
Enclave said: 7.14000000000000056843418861
Run with fesetround(FE_DOWNWARD)
Enclave said: 7.13999999999999968025576890

This issue could be mitigated by setting a fixed frm on enclave entry, and restoring the previous frm on exit. This could probably either happen in the SM or the runtime, depending on what you think is best.

../keyedge/ocalls_eapp.h: In function 'print_string':
../keyedge/ocalls_eapp.h:34:39: error: passing argument 1 of 'strlen' discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
  size_t __0_msg_keyedge_size = strlen(msg) + 1;
                                       ^~~
In file included from ../keyedge/flatbuffers_common_reader.h:221:0,
                 from ../keyedge/ocalls_reader.h:7,
                 from ../keyedge/ocalls_builder.h:7,
                 from ../keyedge/ocalls_eapp.h:6,
                 from eapp.c:2:
/home/nisiyama/keystone/sdk/lib/app/include/string.h:9:8: note: expected 'char *' but argument is of type 'const char *'
 size_t strlen(char* str);
        ^~~~~~

I think we might have dropped the const part of const char* in strlen. Is it intentional?

Currently if you make a native eapp (don't link to libc) without an lds/etc the linker will just choose the first function in the file as the entry point (doesn't see a _start).

We need a simple standard way to specify the entry point at compilation time of the eapp. We don't want to have to coordinate with the host app.

Destroying an enclave twice causes an error when it should be safe. The code only checks if the enclave has ever been created.

Previous SDK tests are now part of the keystone platform tests.

We need SDK specific tests, probably just build tests.

Hi,
I was trying to write a Keystone enclave application. As part of my application, I need to use mprotect inside the enclave. However, since mprotect is not available inside the enclave, I wrote an ocall which takes an address, size, and permissions as arguments and executes the mprotect call from outside the enclave.
However, the call to mprotect in the ocall is failing with the error code ENOMEM. Please suggest a correction.
I am on the dev branch of Keystone.

When reading RISC-V Summit slides, I got the impression that Keystone was not built for a specific OS as long as it provides SBI to the Security Monitor.

However, I noticed in the documentation that I should also compile linux as part of the riscv-gnu-toolchain. If I fail to do so, compiling the SDK fails due to a missing header part of glibc:

riscv64-unknown-linux-gnu-gcc -Iinclude -c src/tiny-malloc.c -o src/tiny-malloc.o
In file included from /opt/riscv/sysroot/usr/include/features.h:447,
                 from /opt/riscv/sysroot/usr/include/bits/libc-header-start.h:33,
                 from /opt/riscv/sysroot/usr/include/stdint.h:26,
                 from /opt/riscv/lib/gcc/riscv64-unknown-linux-gnu/8.2.0/include/stdint.h:9,
                 from include/string.h:4,
                 from src/tiny-malloc.c:60:
/opt/riscv/sysroot/usr/include/gnu/stubs.h:23:11: fatal error: gnu/stubs-lp64d.h: No such file or directory

Can you clarify the situation if the current SDK is made purposely for Linux as host OS?

Hi, I'm developing an app on keystone now, and find there may exist some problem. Here I guess it's checking whether the shared buffer is big enough for the data(pointed by ptr, of size bytes) returned by the host, so it may be the size instead of sizeof(struct edge_data) passed to edge_call_get_offset_from_ptr for boundary check.

I've tried a bit newer version of glibc 2.28 during building keystone. Everything are built without failure, but tests fail a bit randomly. Those tests fail with segfault at memcpy in the host library function:

void PhysicalEnclaveMemory::WriteMem(vaddr_t src, vaddr_t dst, size_t size){
  vaddr_t va_dst = (vaddr_t) mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, keystone_fd, dst - start_phys_addr);
  memcpy((void *) va_dst, (void *) src, size);
}

Looking into details, mmap returns MAP_FAILED==-1 for the problematic case and memcpy of riscv-glibc 2.26 does nothing for this destination address 0xffffffffffffffff, though 2.28's memcpy segfaults with it. This WriteMem is called by Keystone::allocPage() with

    case UTM_FULL: {
       *pte = pte_create(page_addr, PTE_D | PTE_A | PTE_R | PTE_W |PTE_V);
       pMemory->WriteMem(src, (vaddr_t) page_addr << PAGE_BITS, PAGE_SIZE);
       break;

and ((vaddr_t) page_addr << PAGE_BITS) is less than start_phys_addr when the issue happens. I guess that WriteMem won't work well for UTM, because it computes the offset from start_phys_addr which may not be the base address of utm pages.
It seems that the above WriteMem isn't required in the first place, because UTM is cleared already by SM with clean_enclave_memory in create_enclave().

I want to add some code on top of hello-native example.
Basically I have 3 files - rsa.c, bignum.c and types.h. I want to use the functions from these 3 files and use them in the eapp_native.c file and host_native.cpp file.
I included "types.h" as #include file in both eapp/eapp_native.c and host/host_native.cpp. And then Inorder to include rsa.c and bignum.c, I added both these files to /eapp and /host directories and then made changes to CMakeLists.txt file.
The first 4 lines of CMakeLists.txt look like this before I made the changes

set(eapp_bin hello-native) 
set(eapp_src eapp/eapp_native.c) 
set(host_bin hello-native-runner) 
set(host_src host/host_native.cpp)

I changed the 2nd and 4th lines, they now look like

set(eapp_bin hello-native)
set(eapp_src eapp/eapp_native.c eapp/rsa.c eapp/bignum.c)
set(host_bin hello-native-runner)
set(host_src host/host_native.cpp host/rsa.c host/bignum.c)

When I tried running the code, I am still getting some errors.
My Question is - What more changes should I make in the CMakeLists.txt to get this running without any errors?

Currently uninitialized value usage in libs (and possibly elsewhere) does not result in a compile time error.
It should.

Hi guys, I am wondering whether the attenstation example is completed, I see it just let the eapp sign the data and copy the report, but there is no more steps about how to compare the signed data?

Thanks
Huaqi

These 15 pages are all used for the last level page table and can only accommodate just 30MB enclave/runtime. There is an error in comment. Especially when the malloc_zone of eapp is increased to tens of MB, LoadELF will trigger out-of-bounds access (signal 11 sigsegv error).

I want to use clock_gettime() in eapp. I add -Lrt to CFLAGS in Makefile. However, riscv64-unknown-linux-gnu-ld report error: undefined reference to "clock_gettime".