[Pal/Linux-SGX] Fix renaming issue with protected files

LibOS/shim/src/fs/chroot/fs.c

@@ -969,7 +969,7 @@ static int chroot_rename(struct shim_dentry* old, struct shim_dentry* new) {
     }
 
     PAL_HANDLE pal_hdl = NULL;
-    ret = DkStreamOpen(qstrgetstr(&old_data->host_uri), 0, 0, 0, 0, &pal_hdl);
+    ret = DkStreamOpen(qstrgetstr(&old_data->host_uri), 0, 0, 0, PAL_OPTION_RENAME, &pal_hdl);
     if (ret < 0) {
         return pal_to_unix_errno(ret);
     }

LibOS/shim/test/regression/.gitignore

@@ -73,6 +73,7 @@
 /multi_pthread
 /multi_pthread_exitless
 /openmp
+/pf_rename
 /pipe
 /pipe_nonblocking
 /pipe_ocloexec
@@ -107,7 +108,9 @@
 /sysfs_common
 /tcp_ipv6_v6only
 /tcp_msg_peek
-/tmp
 /udp
 /unix
 /vfork_and_exec
+
+/pftmp
+/tmp

LibOS/shim/test/regression/Makefile

@@ -60,6 +60,7 @@ c_executables = \
 	mprotect_prot_growsdown \
 	multi_pthread \
 	openmp \
+	pf_rename \
 	pipe \
 	pipe_nonblocking \
 	pipe_ocloexec \

LibOS/shim/test/regression/manifest.template

@@ -38,3 +38,8 @@ sgx.trusted_files = [
   "file:{{ entrypoint }}",
   "file:exec_victim",
 ]
+
+sgx.protected_files_key = "ffeeddccbbaa99887766554433221100"
+sgx.protected_files = [
+  "file:pftmp/"
+]

LibOS/shim/test/regression/pf_rename.c

@@ -0,0 +1,79 @@
+/* Protected file renaming. Renaming a file without closing its handle. */
+
+#include <assert.h>
+#include <err.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#define PF_FOO_PATH "pftmp/foo.txt"
+#define PF_BAR_PATH "pftmp/bar.txt"
+#define BUF_SIZE    15
+
+int main(void) {
+    char buf[BUF_SIZE];
+    char input_text[] = "Hello, world!";
+    int fd = open(PF_FOO_PATH, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
+    if (fd < 0)
+        err(1, "File creation failed");
+
+    int size = strlen(input_text) + 1;
+    char* str = input_text;
+    while (size > 0) {
+        ssize_t n = write(fd, str, size);
+        if (n == -1) {
+            close(fd);
+            err(1, "Writing to file failed");
+        }
+        assert(n <= size);
+        size -= n;
+        str += n;
+    }
+
+    int ret = close(fd);
+    if (ret < 0)
+        err(1, "Cannot close file");
+
+    ret = rename(PF_FOO_PATH, PF_BAR_PATH);
+    if (ret < 0) {
+        err(1, "Rename failed");
+    }
+
+    /* Open the renamed file */
+    fd = open(PF_BAR_PATH, O_RDONLY);
+    if (fd < 0)
+        err(1, "Cannot open renamed file");
+
+    size_t pos = 0;
+    do {
+        ssize_t n = read(fd, &buf[pos], BUF_SIZE - pos);
+        if (n == -1)
+            err(1, "Reading from renamed file failed");
+        if (n == 0) {
+            if (size > 0) {
+                warnx("read less bytes than expected");
+                return -1;
+            }
+            break;
+        }
+        pos += n;
+    } while (pos < BUF_SIZE);
+
+    close(fd);
+    if (ret < 0)
+        err(1, "Cannot close file");
+
+    buf[sizeof(buf) - 1] = '\0';
+
+    /* Check if the renamed file's contents are same as original text */
+    if (strncmp(input_text, buf, sizeof(input_text)))
+        err(1, "Renamed file content mismatching");
+
+    printf("TEST OK\n");
+
+    return 0;
+}
+

LibOS/shim/test/regression/test_libos.py

@@ -830,6 +830,13 @@ def test_040_sysfs(self):
             self.assertIn(f'{node}/hugepages/hugepages-2048kB/nr_hugepages: file', lines)
             self.assertIn(f'{node}/hugepages/hugepages-1048576kB/nr_hugepages: file', lines)
 
+    def test_080_pf_rename(self):
+        for path in ['pftmp/foo.txt', 'pftmp/bar.txt']:
+            if os.path.exists(path):
+                os.remove(path)
+        os.makedirs('pftmp/', exist_ok=True)
+        stdout, _ = self.run_binary(['pf_rename'])
+        self.assertIn('TEST OK', stdout)
 
 class TC_50_GDB(RegressionTestCase):
     def setUp(self):

Pal/include/host/Linux-common/pal_flags_conv.h

@@ -57,7 +57,7 @@ static inline int PAL_CREATE_TO_LINUX_OPEN(int create) {
 }
 
 static inline int PAL_OPTION_TO_LINUX_OPEN(int options) {
-    assert(WITHIN_MASK(options, PAL_OPTION_CLOEXEC | PAL_OPTION_NONBLOCK));
+    assert(WITHIN_MASK(options, PAL_OPTION_CLOEXEC | PAL_OPTION_NONBLOCK | PAL_OPTION_RENAME));
     return (options & PAL_OPTION_CLOEXEC  ? O_CLOEXEC  : 0) |
            (options & PAL_OPTION_NONBLOCK ? O_NONBLOCK : 0);
 }

Pal/include/pal/pal.h

@@ -297,8 +297,9 @@ enum PAL_OPTION {
     PAL_OPTION_CLOEXEC       = 1,
     PAL_OPTION_EFD_SEMAPHORE = 2, /*!< specific to `eventfd` syscall */
     PAL_OPTION_NONBLOCK      = 4,
+    PAL_OPTION_RENAME        = 8, /*!<specific to `rename` syscall */
 
-    PAL_OPTION_MASK          = 7,
+    PAL_OPTION_MASK          = 15,
 };
 
 #define WITHIN_MASK(val, mask) (((val) | (mask)) == (mask))

Pal/src/host/Linux-SGX/db_files.c

@@ -122,6 +122,15 @@ static int file_open(PAL_HANDLE* handle, const char* type, const char* uri, int
             pf_mode = PF_FILE_MODE_READ | PF_FILE_MODE_WRITE;
         }
 
+        /* The file is being opened for renaming. We will need to update the metadata in the file,
+         * so open with RDWR mode with necessary share permissions.
+         */
+        if (pal_options & PAL_OPTION_RENAME) {
+            pal_share = PAL_SHARE_OWNER_R | PAL_SHARE_OWNER_W;
+            pf_mode = PF_FILE_MODE_READ | PF_FILE_MODE_WRITE;
+            flags |= O_RDWR;
+        }
+
         if ((pf_mode & PF_FILE_MODE_WRITE) && pf->writable_fd >= 0) {
             log_warning("file_open(%s): disallowing concurrent writable handle",
                         hdl->file.realpath);
@@ -792,9 +801,55 @@ static int file_rename(PAL_HANDLE handle, const char* type, const char* uri) {
     if (!tmp)
         return -PAL_ERROR_NOMEM;
 
+
+    struct protected_file* pf = find_protected_file_handle(handle);
+
+    /* TODO: Handle the case of renaming a file that has a file handle already open, so that the
+     * file operations work on both the handles properly
+     */
+    if (pf) {
+        size_t uri_size = strlen(uri) + 1;
+        char* new_path = (char*)calloc(1, uri_size);
+
+        if (!new_path) {
+            free(tmp);
+            return -PAL_ERROR_NOMEM;
+        }
+
+        if (get_norm_path(uri, new_path, &uri_size) < 0) {
+            log_warning("Could not normalize path (%s)", uri);
+            free(tmp);
+            free(new_path);
+            return -PAL_ERROR_DENIED;
+        }
+
+        if (!get_protected_file(new_path)) {
+            log_warning("New path is disallowed for protected files (%s)", new_path);
+            free(tmp);
+            free(new_path);
+            return -PAL_ERROR_DENIED;
+        }
+
+        /* update the metadata of the protected file */
+        pf_status_t pf_ret = pf_rename(pf->context, new_path);
+
+        free(new_path);
+
+        if (PF_FAILURE(pf_ret)) {
+            log_warning("pf_rename failed: %s", pf_strerror(pf_ret));
+            free(tmp);
+            return -PAL_ERROR_DENIED;
+        }
+    }
+
     int ret = ocall_rename(handle->file.realpath, uri);
     if (ret < 0) {
         free(tmp);
+        /* restore the original file name in pf metadata */
+        pf_status_t pf_ret = pf_rename(pf->context, handle->file.realpath);
+        if (PF_FAILURE(pf_ret)) {
+            log_warning("Rename failed: %s, the file might be unusable.", pf_strerror(pf_ret));
+        }
         return unix_to_pal_error(ret);
     }
 

Pal/src/host/Linux-SGX/protected-files/protected_files.c

@@ -372,6 +372,15 @@ static bool ipf_init_new_file(pf_context_t* pf, const char* path) {
     return true;
 }
 
+static bool ipf_rename_file(pf_context_t* pf, const char* new_path) {
+    memset(&pf->encrypted_part_plain.path, 0, sizeof(pf->encrypted_part_plain.path));
+    memcpy(pf->encrypted_part_plain.path, new_path, strlen(new_path) + 1);
+
+    pf->need_writing = true;
+
+    return true;
+}
+
 static bool ipf_close(pf_context_t* pf) {
     void* data;
     bool retval = true;
@@ -1320,6 +1329,31 @@ pf_status_t pf_flush(pf_context_t* pf) {
     return PF_STATUS_SUCCESS;
 }
 
+pf_status_t pf_rename(pf_context_t* pf, const char* new_path) {
+    if (!g_initialized)
+        return PF_STATUS_UNINITIALIZED;
+
+    if (!pf) {
+        DEBUG_PF("pf_rename(PF): PF not initialized\n");
+        return PF_STATUS_UNKNOWN_ERROR;
+    }
+
+    if (!new_path)
+        return PF_STATUS_INVALID_PATH;
+
+    if (strlen(new_path) > PATH_MAX_SIZE - 1) {
+        return PF_STATUS_PATH_TOO_LONG;
+    }
+
+    if (!ipf_rename_file(pf, new_path))
+        return pf->last_error;
+
+    if (!ipf_internal_flush(pf))
+        return pf->last_error;
+
+    return PF_STATUS_SUCCESS;
+}
+
 pf_status_t pf_get_handle(pf_context_t* pf, pf_handle_t* handle) {
     if (!g_initialized)
         return PF_STATUS_UNINITIALIZED;

Pal/src/host/Linux-SGX/protected-files/protected_files.h

@@ -275,4 +275,16 @@ pf_status_t pf_get_handle(pf_context_t* pf, pf_handle_t* handle);
  */
 pf_status_t pf_flush(pf_context_t* pf);
 
+/*!
+ * \brief Update the path in the metadata during a rename
+ *
+ * \param [in] pf PF context
+ * \param [in] new_path Renamed path
+ * \return PF status
+ * \details For protected files, the file name including the path is stored in the encrypted
+ *          metadata which is verified against the actual path during open. So, during a rename
+ *          we need to update the metadata with the new path.
+ */
+pf_status_t pf_rename(pf_context_t* pf, const char* new_path);
+
 #endif /* PROTECTED_FILES_H_ */