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

Previously, Trusted Files feature had the following bug: after fork, the
metadata of currently-opened-in-parent-process TFs (SHA256 hashes for
each chunk of the file) was not available in the child SGX enclave. This
effectively degenerated all currently-opened TFs into Allowed Files, and
thus the child enclave lost integrity guarantees in these TFs. An
attacker could substitute the TF contents on the storage with arbitrary
code/data, and the child enclave would not notice this.

This bug was exposed only in the child process (child SGX enclave) and
only in these scenarios:
- fork without execve: the child continues reading from one of the file
  descriptors pointing to a trusted file opened before `fork()`;
- fork with execve: the child reads from an inherited file descriptor
  pointing to a trusted file.

This commit also adds a test to verify this bug is fixed.

Signed-off-by: Dmitrii Kuvaiskii <dmitrii.kuvaiskii@intel.com>

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"

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 <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;
+}

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

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",
+]

libos/test/regression/fork_and_access_file_testfile

@@ -0,0 +1 @@
+fork_and_access_file_testfile

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

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):

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",

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",

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,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;
 
@@ -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,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);
     }
 
@@ -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);

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 {

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 */

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: {