diff --git a/common/include/arch/x86_64/cpu.h b/common/include/arch/x86_64/cpu.h index 52a912b55f..2c250bcace 100644 --- a/common/include/arch/x86_64/cpu.h +++ b/common/include/arch/x86_64/cpu.h @@ -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" diff --git a/libos/test/regression/fork_and_access_file.c b/libos/test/regression/fork_and_access_file.c new file mode 100644 index 0000000000..8b711e2a32 --- /dev/null +++ b/libos/test/regression/fork_and_access_file.c @@ -0,0 +1,46 @@ +/* SPDX-License-Identifier: LGPL-3.0-or-later */ +/* Copyright (C) 2024 Intel Corporation */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#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; +} diff --git a/libos/test/regression/fork_and_access_file.gdb b/libos/test/regression/fork_and_access_file.gdb new file mode 100644 index 0000000000..93e58bdff0 --- /dev/null +++ b/libos/test/regression/fork_and_access_file.gdb @@ -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 diff --git a/libos/test/regression/fork_and_access_file.manifest.template b/libos/test/regression/fork_and_access_file.manifest.template new file mode 100644 index 0000000000..490ca9633a --- /dev/null +++ b/libos/test/regression/fork_and_access_file.manifest.template @@ -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", +] diff --git a/libos/test/regression/fork_and_access_file_testfile b/libos/test/regression/fork_and_access_file_testfile new file mode 100644 index 0000000000..90a0c5bc9e --- /dev/null +++ b/libos/test/regression/fork_and_access_file_testfile @@ -0,0 +1 @@ +fork_and_access_file_testfile \ No newline at end of file diff --git a/libos/test/regression/meson.build b/libos/test/regression/meson.build index 9374a47a14..26ea4a2a10 100644 --- a/libos/test/regression/meson.build +++ b/libos/test/regression/meson.build @@ -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 diff --git a/libos/test/regression/test_libos.py b/libos/test/regression/test_libos.py index 1c2d5adb99..fb19c238ff 100644 --- a/libos/test/regression/test_libos.py +++ b/libos/test/regression/test_libos.py @@ -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): diff --git a/libos/test/regression/tests.toml b/libos/test/regression/tests.toml index 76eb73c68d..1b7c54b043 100644 --- a/libos/test/regression/tests.toml +++ b/libos/test/regression/tests.toml @@ -39,6 +39,7 @@ manifests = [ "file_size", "flock_lock", "fopen_cornercases", + "fork_and_access_file", "fork_and_exec", "fork_disallowed", "fp_multithread", diff --git a/libos/test/regression/tests_musl.toml b/libos/test/regression/tests_musl.toml index 925cf7f60b..50e60551d0 100644 --- a/libos/test/regression/tests_musl.toml +++ b/libos/test/regression/tests_musl.toml @@ -41,6 +41,7 @@ manifests = [ "file_size", "flock_lock", "fopen_cornercases", + "fork_and_access_file", "fork_and_exec", "fork_disallowed", "fp_multithread", diff --git a/pal/src/host/linux-sgx/pal_files.c b/pal/src/host/linux-sgx/pal_files.c index adb00868e6..7f6a01b95b 100644 --- a/pal/src/host/linux-sgx/pal_files.c +++ b/pal/src/host/linux-sgx/pal_files.c @@ -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) { @@ -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; @@ -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 { @@ -147,6 +184,8 @@ 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; @@ -154,9 +193,10 @@ static int64_t file_read(PAL_HANDLE handle, uint64_t offset, uint64_t count, voi 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; @@ -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 { @@ -177,6 +217,7 @@ 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; } @@ -184,8 +225,10 @@ static int64_t file_write(PAL_HANDLE handle, uint64_t offset, uint64_t count, co 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); } @@ -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) { @@ -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; @@ -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); diff --git a/pal/src/host/linux-sgx/pal_host.h b/pal/src/host/linux-sgx/pal_host.h index c530c6f689..846e8e52a4 100644 --- a/pal/src/host/linux-sgx/pal_host.h +++ b/pal/src/host/linux-sgx/pal_host.h @@ -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 { diff --git a/pal/src/host/linux-sgx/pal_linux.h b/pal/src/host/linux-sgx/pal_linux.h index 509c17441e..701f69a438 100644 --- a/pal/src/host/linux-sgx/pal_linux.h +++ b/pal/src/host/linux-sgx/pal_linux.h @@ -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 */ diff --git a/pal/src/host/linux-sgx/pal_streams.c b/pal/src/host/linux-sgx/pal_streams.c index ee947da203..9dff810493 100644 --- a/pal/src/host/linux-sgx/pal_streams.c +++ b/pal/src/host/linux-sgx/pal_streams.c @@ -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: {