Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User-space separation #815

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,14 @@ sgx-lkl: ${THIRD_PARTY_LIB_DEVICE_MAPPER} ${THIRD_PARTY_LIB_EXT2FS} ${THIRD_PART
$(SGXLKL_LIB_TARGET): $(SGXLKL_BUILD_VARIANT)

# Generate the RSA key and sign the libsgxlkl.so
$(BUILD_DIR)/$(SGXLKL_LIB_TARGET_SIGNED): $(SGXLKL_LIB_TARGET)
$(BUILD_DIR)/$(SGXLKL_LIB_TARGET_SIGNED): $(SGXLKL_LIB_TARGET) $(SGXLKL_USER_LIB_TARGET)
@echo "openssl genrsa -out private.pem -3 3072"
@openssl genrsa -out $(BUILD_DIR)/private.pem -3 3072
@echo "oesign sign -e $(SGXLKL_LIB_TARGET) -c config/eeid-params.conf -k private.pem"
@$(OE_OESIGN_TOOL_PATH)/oesign sign -e $(BUILD_DIR)/$(SGXLKL_LIB_TARGET) -c $(OESIGN_CONFIG_PATH)/eeid-params.conf -k $(BUILD_DIR)/private.pem
$(OE_OESIGN_TOOL_PATH)/oesign sign -e "$(BUILD_DIR)/$(SGXLKL_LIB_TARGET):$(BUILD_DIR)/$(SGXLKL_USER_LIB_TARGET)" -c $(OESIGN_CONFIG_PATH)/eeid-params.conf -k $(BUILD_DIR)/private.pem

$(SGXLKL_USER_LIB_TARGET):
$(MAKE) -C user

# Create a link named build to appropiate build directory.
create-build-link:
Expand All @@ -150,6 +153,7 @@ install-git-pre-commit-hook: scripts/pre-commit

install:
mkdir -p ${PREFIX}/bin ${PREFIX}/lib ${PREFIX}/lib/gdb $(PREFIX)/lib/gdb/openenclave ${PREFIX}/share ${PREFIX}/share/schemas ${PREFIX}/tools
cp $(BUILD_DIR)/$(SGXLKL_USER_LIB_TARGET) $(PREFIX)/lib
cp $(BUILD_DIR)/$(SGXLKL_LIB_TARGET_SIGNED) $(PREFIX)/lib
cp $(BUILD_DIR)/$(SGXLKL_RUN_TARGET) $(PREFIX)/bin
cp $(TOOLS)/sgx-lkl-java $(PREFIX)/bin
Expand Down
1 change: 1 addition & 0 deletions config.mak
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ $(info $$SGXLKL_ROOT = [${SGXLKL_ROOT}])

SGXLKL_RUN_TARGET ?= sgx-lkl-run-oe
SGXLKL_LIB_TARGET ?= libsgxlkl.so
SGXLKL_USER_LIB_TARGET ?= libsgxlkl-user.so
SGXLKL_LIB_TARGET_SIGNED ?= libsgxlkl.so.signed
SGXLKL_STATIC_LIB ?= libsgxlkl.a

Expand Down
2 changes: 1 addition & 1 deletion sgx-lkl-musl
Submodule sgx-lkl-musl updated 2 files
+5 −1 Makefile
+11 −6 ldso/dynlink.c
82 changes: 78 additions & 4 deletions src/enclave/enclave_init.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#include "enclave/wireguard_util.h"
#include "shared/env.h"

#include "../../user/userargs.h"

extern struct mpmcq __scheduler_queue;

_Noreturn void __dls3(elf64_stack_t* conf, void* tos);
Expand Down Expand Up @@ -89,8 +91,60 @@ static void init_wireguard()
wgu_list_devices();
}

static int startmain(void* args)
static long _lkl_syscall_wrapper(long no, long* params)
{
//sgxlkl_warn("syscall begin: no=%u\n", no);
long ret = lkl_syscall(no, params);
//sgxlkl_warn("syscall end: ret=%u\n", ret);
return ret;
}

Comment on lines +94 to +101
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like dead code that was left in during debugging.

Suggested change
static long _lkl_syscall_wrapper(long no, long* params)
{
//sgxlkl_warn("syscall begin: no=%u\n", no);
long ret = lkl_syscall(no, params);
//sgxlkl_warn("syscall end: ret=%u\n", ret);
return ret;
}

static void _enter_user_space(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine for now, but we should be setting up the stack in the kernel and using a clone syscall to launch the thread on a pre-prepared stack using the libc entry point.

int argc,
char** argv,
void* stack,
size_t num_ethreads,
struct timespec clock_res[4])
{
extern void* __oe_get_isolated_image_entry_point(void);
extern const void* __oe_get_isolated_image_base();
typedef int (*sgxlkl_user_enter_proc_t)(void* args, size_t size);
sgxlkl_userargs_t args;
sgxlkl_user_enter_proc_t proc;

memset(&args, 0, sizeof(args));

if (!(proc = __oe_get_isolated_image_entry_point()))
sgxlkl_fail("failed to obtain user space entry point");

args.ua_lkl_syscall = _lkl_syscall_wrapper;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above comment about dead code.

Suggested change
args.ua_lkl_syscall = _lkl_syscall_wrapper;
args.ua_lkl_syscall = lkl_syscall;

args.ua_sgxlkl_warn = sgxlkl_warn;
args.ua_sgxlkl_error = sgxlkl_error;
args.ua_sgxlkl_fail = sgxlkl_fail;
args.ua_enclave_mmap = enclave_mmap;
args.argc = argc;
args.argv = argv;
args.stack = stack;
args.elf64_hdr = (const void*)__oe_get_isolated_image_base();
args.num_ethreads = num_ethreads;
args.sw_debug_mode = sgxlkl_in_sw_debug_mode();
memcpy(args.clock_res, clock_res, sizeof(args.clock_res));

(*proc)(&args, sizeof(args));
}

typedef struct startmain_args
{
int argc;
char** argv;
struct timespec clock_res[8];
}
startmain_args_t;

static int startmain(void* args_)
{
startmain_args_t* args = args_;

__init_libc(sgxlkl_enclave_state.elf64_stack.envp,
sgxlkl_enclave_state.elf64_stack.argv[0]);
__libc_start_init();
Expand All @@ -114,9 +168,23 @@ static int startmain(void* args)
init_wireguard();
find_and_mount_disks();

/* Change to 0 to run application within the kernel image */
#if 1
/* Enter the isolated image */
_enter_user_space(
args->argc,
args->argv,
&sgxlkl_enclave_state.elf64_stack,
sgxlkl_enclave_state.config->ethreads,
args->clock_res);
#else
/* Launch stage 3 dynamic linker, passing in top of stack to overwrite.
* The dynamic linker will then load the application proper; here goes! */
__dls3(&sgxlkl_enclave_state.elf64_stack, __builtin_frame_address(0));
(void)_enter_user_space;
(void)args;
#endif
return 0;
}

int __libc_init_enclave(int argc, char** argv)
Expand Down Expand Up @@ -156,7 +224,7 @@ int __libc_init_enclave(int argc, char** argv)
max_lthreads = next_power_of_2(max_lthreads);

newmpmcq(&__scheduler_queue, max_lthreads, 0);

init_ethread_tp();

size_t espins = cfg->espins;
Expand All @@ -166,9 +234,15 @@ int __libc_init_enclave(int argc, char** argv)
SGXLKL_VERBOSE("calling _lthread_sched_init()\n");
_lthread_sched_init(cfg->stacksize);

if (lthread_create(&lt, NULL, startmain, NULL) != 0)
/* Run startmain() in a new lthread */
{
sgxlkl_fail("Failed to create lthread for startmain()\n");
static startmain_args_t args;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my understanding, is this(creating code blocks with parantheses) a standard C feature or a gcc extension?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also wondering if this was intentional, and if yes what was the rationale behind it.

args.argc = argc;
args.argv = argv;
memcpy(args.clock_res, tmp, sizeof(args.clock_res));

if (lthread_create(&lt, NULL, startmain, &args) != 0)
sgxlkl_fail("Failed to create lthread for startmain()\n");
}

lthread_run();
Expand Down
49 changes: 42 additions & 7 deletions src/main-oe/sgxlkl_run_oe.c
Original file line number Diff line number Diff line change
Expand Up @@ -704,10 +704,10 @@ int getopt_sgxlkl(int argc, char* argv[], struct option long_options[])
return -1;
}

/* Determines path of libsgxlkl.so.signed */
void get_signed_libsgxlkl_path(char* path_buf, size_t len)
/* Determines path of the given library */
void find_lib(const char* libname, char* path_buf, size_t len)
{
/* Look for libsgxlkl.so.signed in:
/* Look for library in:
* 1. .
* 2. ../lib
* 3. /lib
Expand Down Expand Up @@ -756,7 +756,7 @@ void get_signed_libsgxlkl_path(char* path_buf, size_t len)
"%.*s/%s",
(int)base_len,
base,
"libsgxlkl.so.signed") < max_len)
libname) < max_len)
{
// If accessible, path found.
if (!access(path_buf, R_OK))
Expand All @@ -767,7 +767,19 @@ void get_signed_libsgxlkl_path(char* path_buf, size_t len)
base += strspn(base, ":");
}

sgxlkl_host_fail("Unable to locate libsgxlkl.so.signed\n");
sgxlkl_host_fail("Unable to locate %s\n", libname);
}

/* Determines path of libsgxlkl.so.signed */
void get_signed_libsgxlkl_path(char* path_buf, size_t len)
{
find_lib("libsgxlkl.so.signed", path_buf, len);
}

/* Determines path of libsgxlkl-user */
void get_libsgxlkl_user_path(char* path_buf, size_t len)
{
find_lib("libsgxlkl-user.so", path_buf, len);
Copy link
Contributor

@vtikoo vtikoo Aug 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my understanding, is this shared object not signed like the kernel counterpart?

}

void mk_clock_res_string(int clock)
Expand Down Expand Up @@ -1358,6 +1370,7 @@ void* enclave_init(ethread_args_t* args)
/* Creates an SGX-LKL enclave with enclave configuration in the EEID. */
void _create_enclave(
char* libsgxlkl,
char* libsgxlkl_user,
uint32_t oe_flags,
oe_enclave_t** oe_enclave)
{
Expand All @@ -1367,6 +1380,7 @@ void _create_enclave(

char* buffer = NULL;
size_t buffer_size = 0;
char path[PATH_MAX];

serialize_enclave_config(
&sgxlkl_host_state.enclave_config, &buffer, &buffer_size);
Expand All @@ -1391,8 +1405,15 @@ void _create_enclave(

setting.u.eeid = eeid;

// Format the follwing path <libsgxlkl>:<libsgxlkl_user>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Format the follwing path <libsgxlkl>:<libsgxlkl_user>
// Format the following path <libsgxlkl>:<libsgxlkl_user>

if (snprintf(path, sizeof(path), "%s:%s", libsgxlkl, libsgxlkl_user)
>= sizeof(path))
{
sgxlkl_host_fail("path overflow: %s:%s\n", libsgxlkl, libsgxlkl_user);
}

result = oe_create_sgxlkl_enclave(
libsgxlkl, OE_ENCLAVE_TYPE_SGX, oe_flags, &setting, 1, oe_enclave);
path, OE_ENCLAVE_TYPE_SGX, oe_flags, &setting, 1, oe_enclave);

free(eeid);

Expand Down Expand Up @@ -1682,6 +1703,7 @@ int main(int argc, char* argv[], char* envp[])
char* host_config_path = NULL;
char* enclave_config_path = NULL;
char libsgxlkl[PATH_MAX];
char libsgxlkl_user[PATH_MAX];
// const sgxlkl_host_config_t* hconf = &host_state.config;
const sgxlkl_enclave_config_t* econf = &sgxlkl_host_state.enclave_config;
char* root_hd = NULL;
Expand All @@ -1696,6 +1718,7 @@ int main(int argc, char* argv[], char* envp[])
cpu_set_t set;
void* return_value;
bool enclave_image_provided = false;
bool isolated_image_provided = false;

oe_enclave_t* oe_enclave = NULL;
uint32_t oe_flags = 0;
Expand All @@ -1722,6 +1745,7 @@ int main(int argc, char* argv[], char* envp[])
{"help-tls", no_argument, 0, 't'},
{"help", no_argument, 0, 'h'},
{"enclave-image", required_argument, 0, 'e'},
{"isolated-image", required_argument, 0, 'i'},
{"host-config", required_argument, 0, 'H'},
{"enclave-config", required_argument, 0, 'c'},
{0, 0, 0, 0}};
Expand All @@ -1746,6 +1770,10 @@ int main(int argc, char* argv[], char* envp[])
enclave_image_provided = true;
strcpy(libsgxlkl, optarg);
break;
case 'i':
enclave_image_provided = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
enclave_image_provided = true;
isolated_image_provided = true;

strcpy(libsgxlkl_user, optarg);
break;
case 'v':
version();
exit(EXIT_SUCCESS);
Expand Down Expand Up @@ -1863,6 +1891,13 @@ int main(int argc, char* argv[], char* envp[])
}
sgxlkl_host_verbose_raw("result=%s\n", libsgxlkl);

sgxlkl_host_verbose("get_libsgxlkl_user_path... ");
if (!isolated_image_provided)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will always evaluate to true, I don't see where isolated_image_provided is being set again after initialization.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe should happen on L1774?

{
get_libsgxlkl_user_path(libsgxlkl_user, PATH_MAX);
}
sgxlkl_host_verbose_raw("result=%s\n", libsgxlkl_user);

parse_cpu_affinity_params(
sgxlkl_host_state.config.ethreads_affinity,
&ethreads_cores,
Expand Down Expand Up @@ -1903,7 +1938,7 @@ int main(int argc, char* argv[], char* envp[])

/* Enclave creation */
sgxlkl_host_verbose("oe_create_enclave...\n");
_create_enclave(libsgxlkl, oe_flags, &oe_enclave);
_create_enclave(libsgxlkl, libsgxlkl_user, oe_flags, &oe_enclave);

/* Perform host interface initialization */
sgxlkl_host_interface_initialization();
Expand Down
51 changes: 51 additions & 0 deletions user/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
TOP = $(abspath ..)

-include $(TOP)/sgx-lkl-musl/muslobjs.mak

TARGET = $(TOP)/build_musl/libsgxlkl-user.so

CFLAGS = -m64 -g -O3 -g -fPIC -Werror -std=c99 -nostdinc -ffreestanding -fexcess-precision=standard -frounding-math -Wa,--noexecstack -D_XOPEN_SOURCE=700 -pipe -fomit-frame-pointer -fno-unwind-tables -fno-asynchronous-unwind-tables -ffunction-sections -fdata-sections -Werror=implicit-function-declaration -Werror=implicit-int -Werror=pointer-sign -D__USE_GNU -DDL_NOMMU_SUPPORT=1

INCLUDES =
INCLUDES += -I$(TOP)/build_musl/sgx-lkl-musl/include
INCLUDES += -I$(TOP)/build_musl/openenclave/include
INCLUDES += -I$(TOP)/build_musl/config
INCLUDES += -I$(TOP)/sgx-lkl-musl/src/internal
INCLUDES += -I$(TOP)/sgx-lkl-musl/arch/x86_64
INCLUDES += -I$(TOP)/src/include

DYNAMIC_LIST = $(TOP)/sgx-lkl-musl/dynamic.list

LDFLAGS1 = -Wl,--sort-section,alignment -Wl,--sort-common -Wl,--gc-sections -Wl,--hash-style=both -Wl,--no-undefined -Wl,--exclude-libs=ALL -Wl,--dynamic-list=$(DYNAMIC_LIST) -nostdlib -nodefaultlibs -nostartfiles
LDFLAGS1 += -Wl,-esgxlkl_user_enter

LDFLAGS2 = -Wl,-Bstatic -Wl,-Bsymbolic -Wl,--export-dynamic -Wl,-pie -Wl,--build-id -Wl,-z,noexecstack -Wl,-z,now

LDFLAGS = $(LDFLAGS1) $(LDFLAGS2)

LDFLAGS += -lgcc

SOURCES = $(wildcard *.c)

ifndef MUSL_OBJECTS
$(error "please run $(TOP)/sgx-lkl-musl/Makefile first")
endif

LOCAL_OBJECTS = $(SOURCES:.c=.o)
OBJECTS = $(LOCAL_OBJECTS) $(MUSL_OBJECTS)

all: $(OBJECTS)
@ $(CC) -o $(TARGET) $(OBJECTS) $(LDFLAGS)
@ echo "Created $(TARGET)"

%.o: %.c
$(CC) -c $(CFLAGS) $(INCLUDES) -o $@ $<

clean:
rm -f $(TARGET) $(LOCAL_OBJECTS)

muslobjs:
echo $(MUSL_OBJECTS)

nm:
nm $(TARGET) | grep dls
42 changes: 42 additions & 0 deletions user/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
libsgxlkl-user.so
=================

This directory builds **libsgxlkl-user.so**: the user-space image that hosts
the C runtime (crt), which contains the dynamic program loader (ldso) and the
the C library (libc).

SGX-LKL builds two ELF images that are loaded into the enclave.

- libsgxlkl.so (the kernel-space image)
- libsgxlkl-user.so (the user-space image)

Both are passed to the Open Enclave **create_enclave()** function in the
**path** argument, which has the form:

"<enclave-elf>:<extra-elf>"

For example:

"libsgxlkl.so:libsgxlkl-user.so"

Open Enclave loads the two images into distinct ELF memory regions. This
effectively isolates the symbols of the two images. The kernel enters the
user-space image through its entry point given by its ELF header, passing
state information and callbacks (through a C structure). The user-space
image calls back into the kernel via these callbacks.

The main functions of the user-space image are to

- Initialize the C library
- Load the application program and any shared libraries
- Start executing the application program

During its execution, the program calls C library functions that may initiate
syscalls. These syscalls are forwarded to the kernel-space image for handling.

This directory provides the following sources:

- userargs.h - defines the struct passed by the kernel to the entry point.
- enter.c - contains the entry point (**sgxlkl_user_enter()**).
- stubs.c - contains stubs that invoke callback functions.

Loading