Browse Source

[Pal/Linux-SGX] Emulate reads/writes to allowed and trusted files differently

Previously, read()/write() for both trusted and allowed files were
emulated in the same manner: using mmap/memcpy/munmap. This led to
insidious data races in multi-process applications like Apache: one
process would mmap a file and access it during read/write emulation,
while another process would ftruncate the same file (this leads to
SIGBUS as per mmap man page). This happens even for allowed files which
do not require the complicated mmap/memcpy/munmap emulation (this logic
was introduced primarily for trusted-file checksum verification).

This commit separates the emulation of trusted files and allowed files.
The former still use mmap/memcpy/munmap (slightly optimized for
performance), the latter use lseek+read/write (similar to Linux PAL).
This requires new OCALL ocall_lseek(). Also, writing to trusted files
is now explicitly disallowed.
Dmitrii Kuvaiskii 4 years ago
parent
commit
c237bf6bfb

+ 75 - 48
Pal/src/host/Linux-SGX/db_files.c

@@ -79,81 +79,106 @@ static int file_open(PAL_HANDLE* handle, const char* type, const char* uri, int
         return ret;
     }
 
-    hdl->file.stubs = (PAL_PTR)stubs;
-    hdl->file.total = total;
-    *handle         = hdl;
+    hdl->file.stubs  = (PAL_PTR)stubs;
+    hdl->file.total  = total;
+    hdl->file.offset = 0;
+
+    if (hdl->file.stubs) {
+        /* case of trusted file: mmap the whole file in untrusted memory for future reads/writes */
+        ret = ocall_map_untrusted(hdl->file.fd, 0, hdl->file.total, PROT_READ, &hdl->file.umem);
+        if (IS_ERR(ret)) {
+            /* note that we don't free stubs because they are re-used in same trusted file */
+            free(hdl);
+            return unix_to_pal_error(ERRNO(ret));
+        }
+    }
+
+    *handle = hdl;
     return 0;
 }
 
 /* 'read' operation for file streams. */
 static int64_t file_read(PAL_HANDLE handle, uint64_t offset, uint64_t count, void* buffer) {
-    sgx_stub_t* stubs  = (sgx_stub_t*)handle->file.stubs;
-    unsigned int total = handle->file.total;
-    int ret;
+    int64_t ret;
+    sgx_stub_t* stubs = (sgx_stub_t*)handle->file.stubs;
+
+    if (!stubs) {
+        /* case of allowed file: emulate via lseek + read */
+        if (handle->file.offset != offset) {
+            ret = ocall_lseek(handle->file.fd, offset, SEEK_SET);
+            if (IS_ERR(ret))
+                return -PAL_ERROR_DENIED;
+            handle->file.offset = offset;
+        }
+
+        ret = ocall_read(handle->file.fd, buffer, count);
+        if (IS_ERR(ret))
+            return unix_to_pal_error(ERRNO(ret));
+
+        handle->file.offset = offset + ret;
+        return ret;
+    }
 
+    /* case of trusted file: already mmaped in umem, copy from there and verify hash */
+    uint64_t total = handle->file.total;
     if (offset >= total)
         return 0;
 
-    uint64_t end = (offset + count > total) ? total : offset + count;
-    uint64_t map_start, map_end;
+    _Static_assert((TRUSTED_STUB_SIZE & (TRUSTED_STUB_SIZE - 1)) == 0,
+                   "TRUSTED_STUB_SIZE must be a power of two");
 
-    if (stubs) {
-        map_start = offset & ~(TRUSTED_STUB_SIZE - 1);
-        map_end   = (end + TRUSTED_STUB_SIZE - 1) & ~(TRUSTED_STUB_SIZE - 1);
-        /* Don't go past the end of file with the stub map either */
-        if (map_end > total)
-            map_end = ALLOC_ALIGNUP(total);
-    } else {
-        map_start = ALLOC_ALIGNDOWN(offset);
-        map_end   = ALLOC_ALIGNUP(end);
-    }
+    uint64_t end       = (offset + count > total) ? total : offset + count;
+    uint64_t map_start = offset & ~(TRUSTED_STUB_SIZE - 1);
+    uint64_t map_end   = (end + TRUSTED_STUB_SIZE - 1) & ~(TRUSTED_STUB_SIZE - 1);
 
-    void* umem;
-    ret = ocall_map_untrusted(handle->file.fd, map_start, map_end - map_start, PROT_READ, &umem);
-    if (IS_ERR(ret))
-        return unix_to_pal_error(ERRNO(ret));
+    if (map_end > total)
+        map_end = ALLOC_ALIGNUP(total);
 
-    if (stubs) {
-        ret = copy_and_verify_trusted_file(handle->file.realpath, umem, map_start, map_end, buffer,
-                                           offset, end - offset, stubs, total);
-        if (ret < 0) {
-            ocall_unmap_untrusted(umem, map_end - map_start);
-            return ret;
-        }
-    } else {
-        memcpy(buffer, umem + (offset - map_start), end - offset);
-    }
+    ret = copy_and_verify_trusted_file(handle->file.realpath, handle->file.umem + map_start,
+            map_start, map_end, buffer, offset, end - offset, stubs, total);
+    if (ret < 0)
+        return ret;
 
-    ocall_unmap_untrusted(umem, map_end - map_start);
     return end - offset;
 }
 
 /* 'write' operation for file streams. */
 static int64_t file_write(PAL_HANDLE handle, uint64_t offset, uint64_t count, const void* buffer) {
-    uint64_t map_start = ALLOC_ALIGNDOWN(offset);
-    uint64_t map_end   = ALLOC_ALIGNUP(offset + count);
-    void* umem;
-    int ret;
+    int64_t ret;
+    sgx_stub_t* stubs = (sgx_stub_t*)handle->file.stubs;
 
-    ret = ocall_map_untrusted(handle->file.fd, map_start, map_end - map_start, PROT_WRITE, &umem);
-    if (IS_ERR(ret))
-        return unix_to_pal_error(ERRNO(ret));
+    if (!stubs) {
+        /* case of allowed file: emulate via lseek + write */
+        if (handle->file.offset != offset) {
+            ret = ocall_lseek(handle->file.fd, offset, SEEK_SET);
+            if (IS_ERR(ret))
+                return -PAL_ERROR_DENIED;
+            handle->file.offset = offset;
+        }
 
-    if (offset + count > handle->file.total) {
-        ocall_ftruncate(handle->file.fd, offset + count);
-        handle->file.total = offset + count;
-    }
+        ret = ocall_write(handle->file.fd, buffer, count);
+        if (IS_ERR(ret))
+            return unix_to_pal_error(ERRNO(ret));
 
-    memcpy(umem + offset - map_start, buffer, count);
+        handle->file.offset = offset + ret;
+        return ret;
+    }
 
-    ocall_unmap_untrusted(umem, map_end - map_start);
-    return count;
+    /* case of trusted file: disallow writing completely */
+    SGX_DBG(DBG_E, "Writing to a trusted file (%s) is disallowed!\n", handle->file.realpath);
+    return -PAL_ERROR_DENIED;
 }
 
 /* 'close' operation for file streams. In this case, it will only
-   close the file withou deleting it. */
+   close the file without deleting it. */
 static int file_close(PAL_HANDLE handle) {
     int fd = handle->file.fd;
+
+    if (handle->file.stubs) {
+        /* case of trusted file: the whole file was mmapped in untrusted memory */
+        ocall_unmap_untrusted(handle->file.umem, handle->file.total);
+    }
+
     ocall_close(fd);
 
     /* initial realpath is part of handle object and will be freed with it */
@@ -210,6 +235,8 @@ static int file_map(PAL_HANDLE handle, void** addr, int prot, uint64_t offset, u
     uint64_t map_start, map_end;
 
     if (stubs) {
+        _Static_assert((TRUSTED_STUB_SIZE & (TRUSTED_STUB_SIZE - 1)) == 0,
+                       "TRUSTED_STUB_SIZE must be a power of two");
         map_start = offset & ~(TRUSTED_STUB_SIZE - 1);
         map_end   = (end + TRUSTED_STUB_SIZE - 1) & ~(TRUSTED_STUB_SIZE - 1);
     } else {

+ 3 - 2
Pal/src/host/Linux-SGX/db_main.c

@@ -112,8 +112,9 @@ static PAL_HANDLE setup_dummy_file_handle (const char * name)
     }
     handle->file.realpath = path;
 
-    handle->file.total = 0;
-    handle->file.stubs = NULL;
+    handle->file.total  = 0;
+    handle->file.offset = 0;
+    handle->file.stubs  = NULL;
 
     return handle;
 }

+ 20 - 0
Pal/src/host/Linux-SGX/enclave_ocalls.c

@@ -431,6 +431,26 @@ int ocall_ftruncate (int fd, uint64_t length)
     return retval;
 }
 
+int ocall_lseek(int fd, uint64_t offset, int whence) {
+    int retval = 0;
+    ms_ocall_lseek_t* ms;
+
+    ms = sgx_alloc_on_ustack(sizeof(*ms));
+    if (!ms) {
+        sgx_reset_ustack();
+        return -EPERM;
+    }
+
+    ms->ms_fd     = fd;
+    ms->ms_offset = offset;
+    ms->ms_whence = whence;
+
+    retval = sgx_ocall(OCALL_LSEEK, ms);
+
+    sgx_reset_ustack();
+    return retval;
+}
+
 int ocall_mkdir (const char * pathname, unsigned short mode)
 {
     int retval = 0;

+ 2 - 0
Pal/src/host/Linux-SGX/enclave_ocalls.h

@@ -45,6 +45,8 @@ int ocall_fsync (int fd);
 
 int ocall_ftruncate (int fd, uint64_t length);
 
+int ocall_lseek(int fd, uint64_t offset, int whence);
+
 int ocall_mkdir (const char *pathname, unsigned short mode);
 
 int ocall_getdents (int fd, struct linux_dirent64 *dirp, unsigned int size);

+ 7 - 0
Pal/src/host/Linux-SGX/ocall_types.h

@@ -36,6 +36,7 @@ enum {
     OCALL_FCHMOD,
     OCALL_FSYNC,
     OCALL_FTRUNCATE,
+    OCALL_LSEEK,
     OCALL_MKDIR,
     OCALL_GETDENTS,
     OCALL_WAKE_THREAD,
@@ -145,6 +146,12 @@ typedef struct {
     uint64_t ms_length;
 } ms_ocall_ftruncate_t;
 
+typedef struct {
+    int ms_fd;
+    uint64_t ms_offset;
+    int ms_whence;
+} ms_ocall_lseek_t;
+
 typedef struct {
     const char * ms_pathname;
     unsigned short ms_mode;

+ 4 - 1
Pal/src/host/Linux-SGX/pal_host.h

@@ -96,7 +96,10 @@ typedef struct pal_handle
             PAL_IDX fd;
             PAL_STR realpath;
             PAL_NUM total;
-            PAL_PTR stubs;
+            PAL_NUM offset;
+            /* below fields are used only for trusted files */
+            PAL_PTR stubs;    /* contains hashes of file chunks */
+            PAL_PTR umem;     /* valid only when stubs != NULL */
         } file;
 
         struct {

+ 9 - 0
Pal/src/host/Linux-SGX/sgx_enclave.c

@@ -213,6 +213,14 @@ static int sgx_ocall_ftruncate(void * pms)
     return ret;
 }
 
+static int sgx_ocall_lseek(void* pms) {
+    ms_ocall_lseek_t* ms = (ms_ocall_lseek_t*)pms;
+    int ret;
+    ODEBUG(OCALL_LSEEK, ms);
+    ret = INLINE_SYSCALL(lseek, 3, ms->ms_fd, ms->ms_offset, ms->ms_whence);
+    return ret;
+}
+
 static int sgx_ocall_mkdir(void * pms)
 {
     ms_ocall_mkdir_t * ms = (ms_ocall_mkdir_t *) pms;
@@ -696,6 +704,7 @@ sgx_ocall_fn_t ocall_table[OCALL_NR] = {
         [OCALL_FCHMOD]          = sgx_ocall_fchmod,
         [OCALL_FSYNC]           = sgx_ocall_fsync,
         [OCALL_FTRUNCATE]       = sgx_ocall_ftruncate,
+        [OCALL_LSEEK]           = sgx_ocall_lseek,
         [OCALL_MKDIR]           = sgx_ocall_mkdir,
         [OCALL_GETDENTS]        = sgx_ocall_getdents,
         [OCALL_WAKE_THREAD]     = sgx_ocall_wake_thread,