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

[PAL/Linux-SGX] Fix Trusted Files degenerating to Allowed Files on fork #1796

Merged
merged 1 commit into from
Mar 7, 2024
Merged
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
5 changes: 4 additions & 1 deletion common/include/arch/x86_64/cpu.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,10 @@ static inline void wrfsbase(uint64_t addr) {
:: "D"(addr) : "memory");
}

static inline noreturn void die_or_inf_loop(void) {
/* This function must be breakable for debugging and GDB integration scripts (e.g. see
* `fork_and_access_file.gdb` in LibOS regression tests). */
__attribute__((__noinline__)) __attribute__((unused))
static noreturn void die_or_inf_loop(void) {
__asm__ volatile (
"1: \n"
"ud2 \n"
Expand Down
46 changes: 46 additions & 0 deletions libos/test/regression/fork_and_access_file.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/* SPDX-License-Identifier: LGPL-3.0-or-later */
/* Copyright (C) 2024 Intel Corporation */

#include <err.h>
#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/wait.h>
#include <unistd.h>

#include "common.h"
#include "rw_file.h"

#define FILENAME "fork_and_access_file_testfile"
#define MAX_BUF_SIZE 256

char g_parent_buf[MAX_BUF_SIZE];
char g_child_buf[MAX_BUF_SIZE];

int main(void) {
int fd = CHECK(open(FILENAME, O_RDONLY));

ssize_t parent_read_ret = CHECK(posix_fd_read(fd, g_parent_buf, sizeof(g_parent_buf)));
CHECK(lseek(fd, 0, SEEK_SET));

pid_t p = CHECK(fork());
if (p == 0) {
ssize_t child_read_ret = CHECK(posix_fd_read(fd, g_child_buf, sizeof(g_child_buf)));
if (child_read_ret != parent_read_ret ||
memcmp(g_child_buf, g_parent_buf, child_read_ret)) {
errx(1, "child read data different from what parent read");
}
exit(0);
}

int status = 0;
CHECK(wait(&status));
if (!WIFEXITED(status) || WEXITSTATUS(status) != 0)
errx(1, "child died with status: %#x", status);

CHECK(close(fd));
puts("TEST OK");
return 0;
}
32 changes: 32 additions & 0 deletions libos/test/regression/fork_and_access_file.gdb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
set breakpoint pending on
set pagination off
set backtrace past-main on

# We want to check what happens in the child process after fork()
set follow-fork-mode child

# Cannot detach after fork because of some bug in SGX version of GDB (GDB would segfault)
set detach-on-fork off

tbreak fork
commands
echo BREAK ON FORK\n

shell echo "WRITING NEW CONTENT IN FORK_AND_ACCESS_FILE_TESTFILE" > fork_and_access_file_testfile

tbreak die_or_inf_loop
commands
echo EXITING GDB WITH A GRAMINE ERROR\n
quit
end

tbreak exit
commands
echo EXITING GDB WITHOUT A GRAMINE ERROR\n
quit
end

continue
end

run
20 changes: 20 additions & 0 deletions libos/test/regression/fork_and_access_file.manifest.template
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
loader.entrypoint = "file:{{ gramine.libos }}"
libos.entrypoint = "{{ entrypoint }}"

loader.env.LD_LIBRARY_PATH = "/lib"

fs.mounts = [
{ path = "/lib", uri = "file:{{ gramine.runtimedir(libc) }}" },
{ path = "/{{ entrypoint }}", uri = "file:{{ binary_dir }}/{{ entrypoint }}" },
]

sgx.max_threads = {{ '1' if env.get('EDMM', '0') == '1' else '16' }}
sgx.debug = true
sgx.edmm_enable = {{ 'true' if env.get('EDMM', '0') == '1' else 'false' }}

sgx.trusted_files = [
"file:{{ gramine.libos }}",
"file:{{ gramine.runtimedir(libc) }}/",
"file:{{ binary_dir }}/{{ entrypoint }}",
"file:fork_and_access_file_testfile",
]
1 change: 1 addition & 0 deletions libos/test/regression/fork_and_access_file_testfile
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fork_and_access_file_testfile
1 change: 1 addition & 0 deletions libos/test/regression/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ tests = {
'file_size': {},
'flock_lock': {},
'fopen_cornercases': {},
'fork_and_access_file': {},
'fork_and_exec': {},
'fp_multithread': {
'c_args': '-fno-builtin', # see comment in the test's source
Expand Down
22 changes: 22 additions & 0 deletions libos/test/regression/test_libos.py
Original file line number Diff line number Diff line change
Expand Up @@ -1328,6 +1328,28 @@ def test_010_regs_x86_64(self):
xmm0_result = self.find('XMM0 result', stdout)
self.assertEqual(xmm0_result, '$4 = 0x4000400040004000')

@unittest.skipUnless(HAS_SGX, 'Trusted files bug was SGX-specific')
def test_020_gdb_fork_and_access_file_bug(self):
# To run this test manually, use:
# GDB=1 GDB_SCRIPT=fork_and_access_file.gdb gramine-sgx fork_and_access_file
#
# This test checks that the bug of trusted files was fixed. The bug effectively degenerated
# opened trusted files to allowed files after fork. This test starts a program that forks
# and then reads the trusted file. The GDB script stops the program after fork, modifies the
# trusted file, and then lets the program continue execution. The child process must see the
# modified trusted file, and Gramine's verification logic must fail the whole program.
try:
stdout, _ = self.run_gdb(['fork_and_access_file'], 'fork_and_access_file.gdb')
self.assertIn('BREAK ON FORK', stdout)
self.assertIn('EXITING GDB WITH A GRAMINE ERROR', stdout)
# below message must NOT be printed; it means Gramine didn't fail but the program itself
self.assertNotIn('EXITING GDB WITHOUT A GRAMINE ERROR', stdout)
# below message from program must NOT be printed; Gramine must fail before it
self.assertNotIn('child read data different from what parent read', stdout)
finally:
# restore the trusted file contents (modified by the GDB script in this test)
with open('fork_and_access_file_testfile', 'w') as f:
f.write('fork_and_access_file_testfile')

class TC_80_Socket(RegressionTestCase):
def test_000_getsockopt(self):
Expand Down
1 change: 1 addition & 0 deletions libos/test/regression/tests.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ manifests = [
"file_size",
"flock_lock",
"fopen_cornercases",
"fork_and_access_file",
"fork_and_exec",
"fork_disallowed",
"fp_multithread",
Expand Down
1 change: 1 addition & 0 deletions libos/test/regression/tests_musl.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ manifests = [
"file_size",
"flock_lock",
"fopen_cornercases",
"fork_and_access_file",
"fork_and_exec",
"fork_disallowed",
"fp_multithread",
Expand Down
69 changes: 58 additions & 11 deletions pal/src/host/linux-sgx/pal_files.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,42 @@
* GBs in size, and a pread OCALL could fail with -ENOMEM, so we cap to reasonably small size) */
#define MAX_READ_SIZE (PRESET_PAGESIZE * 1024 * 32)

void fixup_file_handle_after_deserialization(PAL_HANDLE handle) {
int ret;

assert(handle->hdr.type == PAL_TYPE_FILE);
assert(!handle->file.chunk_hashes);
assert(!handle->file.umem);
assert(handle->file.realpath);

if (!handle->file.trusted) {
/* unknown (if file check policy allows) or encrypted or allowed file, no need to fix */
return;
}

struct trusted_file* tf = get_trusted_or_allowed_file(handle->file.realpath);
if (!tf || tf->allowed) {
log_error("cannot find checkpointed trusted file '%s' in manifest", handle->file.realpath);
die_or_inf_loop();
}

tf->size = handle->file.size; /* tf size is required for load_trusted_or_allowed_file() below */

sgx_chunk_hash_t* chunk_hashes;
uint64_t file_size;
void* umem;
ret = load_trusted_or_allowed_file(tf, handle, /*create=*/false, &chunk_hashes, &file_size,
&umem);
if (ret < 0) {
log_error("cannot load checkpointed trusted file '%s'", handle->file.realpath);
die_or_inf_loop();
}

assert(file_size == handle->file.size);
handle->file.chunk_hashes = chunk_hashes;
handle->file.umem = umem;
}

static int file_open(PAL_HANDLE* handle, const char* type, const char* uri,
enum pal_access pal_access, pal_share_flags_t pal_share,
enum pal_create_mode pal_create, pal_stream_options_t pal_options) {
Expand Down Expand Up @@ -122,6 +158,7 @@ static int file_open(PAL_HANDLE* handle, const char* type, const char* uri,
hdl->file.chunk_hashes = chunk_hashes;
hdl->file.size = file_size;
hdl->file.umem = umem;
hdl->file.trusted = !tf->allowed;

*handle = hdl;
return 0;
Expand All @@ -135,9 +172,9 @@ static int file_open(PAL_HANDLE* handle, const char* type, const char* uri,

static int64_t file_read(PAL_HANDLE handle, uint64_t offset, uint64_t count, void* buffer) {
int64_t ret;
sgx_chunk_hash_t* chunk_hashes = handle->file.chunk_hashes;

if (!chunk_hashes) {
if (!handle->file.trusted) {
assert(!handle->file.chunk_hashes);
if (handle->file.seekable) {
ret = ocall_pread(handle->file.fd, buffer, count, offset);
} else {
Expand All @@ -147,16 +184,19 @@ static int64_t file_read(PAL_HANDLE handle, uint64_t offset, uint64_t count, voi
}

/* case of trusted file: already mmaped in umem, copy from there and verify hash */
assert(handle->file.chunk_hashes);

if (offset >= handle->file.size)
return 0;

off_t end = MIN(offset + count, handle->file.size);
off_t aligned_offset = ALIGN_DOWN(offset, TRUSTED_CHUNK_SIZE);
off_t aligned_end = ALIGN_UP(end, TRUSTED_CHUNK_SIZE);

assert(handle->file.size && handle->file.umem);
ret = copy_and_verify_trusted_file(handle->file.realpath, buffer, handle->file.umem,
aligned_offset, aligned_end, offset, end, chunk_hashes,
handle->file.size);
aligned_offset, aligned_end, offset, end,
handle->file.chunk_hashes, handle->file.size);
if (ret < 0)
return ret;

Expand All @@ -165,9 +205,9 @@ static int64_t file_read(PAL_HANDLE handle, uint64_t offset, uint64_t count, voi

static int64_t file_write(PAL_HANDLE handle, uint64_t offset, uint64_t count, const void* buffer) {
int64_t ret;
sgx_chunk_hash_t* chunk_hashes = handle->file.chunk_hashes;

if (!chunk_hashes) {
if (!handle->file.trusted) {
assert(!handle->file.chunk_hashes);
if (handle->file.seekable) {
ret = ocall_pwrite(handle->file.fd, buffer, count, offset);
} else {
Expand All @@ -177,15 +217,18 @@ static int64_t file_write(PAL_HANDLE handle, uint64_t offset, uint64_t count, co
}

/* case of trusted file: disallow writing completely */
assert(handle->file.chunk_hashes);
log_warning("Writing to a trusted file (%s) is disallowed!", handle->file.realpath);
return -PAL_ERROR_DENIED;
}

static void file_destroy(PAL_HANDLE handle) {
assert(handle->hdr.type == PAL_TYPE_FILE);

if (handle->file.chunk_hashes && handle->file.size) {
if (handle->file.trusted && handle->file.size) {
/* case of trusted file: the whole file was mmapped in untrusted memory */
assert(handle->file.chunk_hashes);
assert(handle->file.umem);
ocall_munmap_untrusted(handle->file.umem, handle->file.size);
}

Expand Down Expand Up @@ -249,10 +292,11 @@ static int file_map(PAL_HANDLE handle, void* addr, pal_prot_flags_t prot, uint64
#endif
}

sgx_chunk_hash_t* chunk_hashes = handle->file.chunk_hashes;
if (chunk_hashes) {
if (handle->file.trusted) {
/* case of trusted file: already mmaped in umem, copy from there into enclave memory and
* verify hashes along the way */
assert(handle->file.chunk_hashes);

off_t end = MIN(offset + size, handle->file.size);
size_t bytes_filled;
if ((off_t)offset >= end) {
Expand All @@ -271,9 +315,10 @@ static int file_map(PAL_HANDLE handle, void* addr, pal_prot_flags_t prot, uint64
goto out;
}

assert(handle->file.size && handle->file.umem);
ret = copy_and_verify_trusted_file(handle->file.realpath, addr, handle->file.umem,
aligned_offset, aligned_end, offset, end, chunk_hashes,
handle->file.size);
aligned_offset, aligned_end, offset, end,
handle->file.chunk_hashes, handle->file.size);
if (ret < 0) {
log_error("file_map - copy & verify on trusted file: %s", pal_strerror(ret));
goto out;
Expand All @@ -288,6 +333,8 @@ static int file_map(PAL_HANDLE handle, void* addr, pal_prot_flags_t prot, uint64
}
} else {
/* case of allowed file: simply read from underlying file descriptor into enclave memory */
assert(!handle->file.chunk_hashes);

size_t bytes_read = 0;
while (bytes_read < size) {
size_t read_size = MIN(size - bytes_read, MAX_READ_SIZE);
Expand Down
3 changes: 2 additions & 1 deletion pal/src/host/linux-sgx/pal_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ typedef struct {
bool seekable; /* regular files are seekable, FIFO pipes are not */
/* below fields are used only for trusted files */
sgx_chunk_hash_t* chunk_hashes; /* array of hashes of file chunks */
void* umem; /* valid only when chunk_hashes != NULL */
void* umem; /* valid only when chunk_hashes != NULL and size > 0 */
bool trusted; /* is this a Trusted File? */
} file;

struct {
Expand Down
1 change: 1 addition & 0 deletions pal/src/host/linux-sgx/pal_linux.h
Original file line number Diff line number Diff line change
Expand Up @@ -208,5 +208,6 @@ int _PalStreamSecureWrite(LIB_SSL_CONTEXT* ssl_ctx, const uint8_t* buf, size_t l
int _PalStreamSecureSave(LIB_SSL_CONTEXT* ssl_ctx, const uint8_t** obuf, size_t* olen);

void fixup_socket_handle_after_deserialization(PAL_HANDLE handle);
void fixup_file_handle_after_deserialization(PAL_HANDLE handle);

#endif /* IN_ENCLAVE */
4 changes: 3 additions & 1 deletion pal/src/host/linux-sgx/pal_streams.c
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,9 @@ static int handle_deserialize(PAL_HANDLE* handle, const void* data, size_t size,
free(hdl);
return -PAL_ERROR_NOMEM;
}
hdl->file.chunk_hashes = hdl->file.umem = NULL;
hdl->file.chunk_hashes = hdl->file.umem = NULL; /* set up in below fixup function */
hdl->file.fd = host_fd; /* correct host FD must be set for below fixup function */
fixup_file_handle_after_deserialization(hdl);
break;
}
case PAL_TYPE_DIR: {
Expand Down