[PAL] Enforce that device files are in the list of sgx.allowed_files

Previously, device files (like `dev:/dev/zero`) were mounted in Gramine
via the FS-mounts syntax in manifest:

    fs.mounts = [ {path = "/dev/zero", uri = "dev:/dev/zero"} ]

However, devices are pass-through; Gramine doesn't add any protections
to the app-to-device communication. Therefore, a careless usage of
`write(device_fd)` may lead to data leaks and `read(device_fd)` may
expose app's vulnerable parsing logic to the attacker or lead to
trusting untrusted data.

To make it more pronounced that Gramine doesn't provide any security
guarantees to the device communication, this commit forces to specify
devices as `sgx.allowed_files`. This is similar to how Gramine forces to
specify device IOCTLs as `sys.allowed_ioctls`. **This is a breaking
change!**

At the PAL level, this requires normalizing the device pathname and
finding it in the list of allowed files. The device pathname must also
be checkpointed and restored in the child processes. Finally, this
commit also introduces previously-missing device operations like
`unlink` (delete) and `ftruncate` (setlength).

Co-authored-by: Li, Xun <xun.li@intel.com>
Signed-off-by: Li, Xun <xun.li@intel.com>
Signed-off-by: Dmitrii Kuvaiskii <dmitrii.kuvaiskii@intel.com>

Documentation/manifest-syntax.rst

@@ -878,6 +878,8 @@ of SGX sealing should use these masks. In particular, these masks allow to
 specify a subset of enclave/machine attributes to be used in sealing key
 derivation. Moreover, these masks themselves are used in sealing key derivation.
 
+.. _allowed_files:
+
 Allowed files
 ^^^^^^^^^^^^^
 
@@ -888,16 +890,22 @@ Allowed files
       "[URI]",
     ]
 
-This syntax specifies the files that are allowed to be created or loaded into
-the enclave unconditionally. In other words, allowed files can be opened for
-reading/writing and can be created if they do not exist already. Allowed files
-are not cryptographically hashed and are thus not protected.
+This syntax specifies the files/directories (with the ``file:`` prefix) and
+devices (with the ``dev:`` prefix) that are allowed to be created or opened in
+the enclave unconditionally. In other words, allowed files, directories and
+devices can be opened for reading/writing and can be created if they do not
+exist already. Allowed files are not cryptographically hashed and are thus not
+protected.
 
 .. warning::
    It is insecure to allow files containing code or critical information;
    developers must not allow files blindly! Instead, use trusted or encrypted
    files.
 
+   Similarly, communication with allowed devices is pass-through and thus
+   potentially insecure by itself. It is the responsibility of the app developer
+   to correctly communicate with devices, with security implications in mind.
+
 Trusted files
 ^^^^^^^^^^^^^
 
@@ -1008,16 +1016,17 @@ File check policy
     (Default: "strict")
 
 This syntax specifies the file check policy, determining the behavior of
-authentication when opening files. By default, only files explicitly listed as
-``trusted_files`` or ``allowed_files`` declared in the manifest are allowed for
-access.
-
-If the file check policy is ``allow_all_but_log``, all files other than trusted
-and allowed are allowed for access, and Gramine emits a warning message for
-every such file. Effectively, this policy operates on all unknown files as if
-they were listed as ``allowed_files``. (However, this policy still does not
-allow writing/creating files specified as trusted.) This policy is a convenient
-way to determine the set of files that the ported application uses.
+authentication when opening files or devices. By default, only files explicitly
+listed as ``trusted_files`` and files or devices explicitly listed as
+``allowed_files`` are allowed for access.
+
+If the file check policy is ``allow_all_but_log``, all files and devices other
+than trusted and allowed are allowed for access, and Gramine emits a warning
+message for every such file/device. Effectively, this policy operates on all
+unknown files and devices as if they were listed as ``allowed_files``. (However,
+this policy still does not allow writing/creating files specified as trusted.)
+This policy is a convenient way to determine the set of files that the ported
+application uses.
 
 Attestation and quotes
 ^^^^^^^^^^^^^^^^^^^^^^

common/include/api.h

@@ -275,6 +275,7 @@ char* alloc_substr(const char* start, size_t len);
 char* alloc_concat(const char* a, size_t a_len, const char* b, size_t b_len);
 char* alloc_concat3(const char* a, size_t a_len, const char* b, size_t b_len,
                     const char* c, size_t c_len);
+void* alloc_and_copy(const void* src, size_t size);
 
 /* Libc memory allocation functions */
 void* malloc(size_t size);
@@ -385,6 +386,7 @@ int buf_flush(struct print_buf* buf);
 #define URI_PREFIX_EVENTFD  URI_TYPE_EVENTFD URI_PREFIX_SEPARATOR
 #define URI_PREFIX_FILE     URI_TYPE_FILE URI_PREFIX_SEPARATOR
 
+#define URI_PREFIX_DEV_LEN  (static_strlen(URI_PREFIX_DEV))
 #define URI_PREFIX_FILE_LEN (static_strlen(URI_PREFIX_FILE))
 
 #define TIME_US_IN_S 1000000ul

common/src/string/util.c

@@ -40,3 +40,11 @@ char* alloc_concat3(const char* a, size_t a_len, const char* b, size_t b_len,
     buf[a_len + b_len + c_len] = '\0';
     return buf;
 }
+
+void* alloc_and_copy(const void* src, size_t size) {
+    void* dst = malloc(size);
+    if (!dst)
+        return NULL;
+    memcpy(dst, src, size);
+    return dst;
+}

libos/test/regression/device_ioctl.manifest.template

@@ -17,6 +17,10 @@ sgx.trusted_files = [
   "file:{{ binary_dir }}/{{ entrypoint }}",
 ]
 
+sgx.allowed_files = [
+  "dev:/dev/gramine_test_dev",
+]
+
 sys.ioctl_structs.gramine_test_dev_ioctl_write = [
     { size = 8, direction = "out", name = "buf_size" },    # buf_size
     { ptr = [ {size = "buf_size", direction = "out"} ] },  # buf

libos/test/regression/device_ioctl_fail.manifest.template

@@ -19,6 +19,10 @@ sgx.trusted_files = [
   "file:{{ binary_dir }}/{{ entrypoint }}",
 ]
 
+sgx.allowed_files = [
+  "dev:/dev/gramine_test_dev",
+]
+
 sys.allowed_ioctls = [
   # no allowed IOCTLs to force test failure
 ]

libos/test/regression/device_ioctl_parse_fail.manifest.template

@@ -17,6 +17,10 @@ sgx.trusted_files = [
   "file:{{ binary_dir }}/{{ entrypoint }}",
 ]
 
+sgx.allowed_files = [
+  "dev:/dev/gramine_test_dev",
+]
+
 # Below IOCTL structs are intentionally modified to test IOCTL parsing (i.e. they don't really make
 # sense). Each IOCTL struct has a single error/limitation that should fail Gramine's IOCTL parser.
 

libos/test/regression/device_passthrough.manifest.template

@@ -17,3 +17,7 @@ sgx.trusted_files = [
   "file:{{ gramine.runtimedir(libc) }}/",
   "file:{{ binary_dir }}/{{ entrypoint }}",
 ]
+
+sgx.allowed_files = [
+  "dev:/dev/zero",
+]

pal/src/host/linux-sgx/enclave_framework.c

@@ -405,11 +405,25 @@ static LISTP_TYPE(trusted_file) g_trusted_file_list = LISTP_INIT;
 static spinlock_t g_trusted_file_lock = INIT_SPINLOCK_UNLOCKED;
 static int g_file_check_policy = FILE_CHECK_POLICY_STRICT;
 
+static void find_path_in_uri(const char* uri, size_t uri_len, const char** out_path,
+                             size_t* out_path_len) {
+    if (strstartswith(uri, URI_PREFIX_FILE)) {
+        *out_path = uri + URI_PREFIX_FILE_LEN;
+        *out_path_len = uri_len - URI_PREFIX_FILE_LEN;
+        return;
+    }
+
+    assert(strstartswith(uri, URI_PREFIX_DEV));
+    *out_path = uri + URI_PREFIX_DEV_LEN;
+    *out_path_len = uri_len - URI_PREFIX_DEV_LEN;
+}
+
 /* assumes `path` is normalized */
 static bool path_is_equal_or_subpath(const struct trusted_file* tf, const char* path,
                                      size_t path_len) {
-    const char* tf_path = tf->uri + URI_PREFIX_FILE_LEN;
-    size_t tf_path_len  = tf->uri_len - URI_PREFIX_FILE_LEN;
+    const char* tf_path;
+    size_t tf_path_len;
+    find_path_in_uri(tf->uri, tf->uri_len, &tf_path, &tf_path_len);
 
     if (tf_path_len > path_len || memcmp(tf_path, path, tf_path_len)) {
         /* tf path is not a prefix of `path` */
@@ -447,8 +461,9 @@ struct trusted_file* get_trusted_or_allowed_file(const char* path) {
             }
         } else {
             /* trusted files: must be exactly the same URI */
-            const char* tf_path = tmp->uri + URI_PREFIX_FILE_LEN;
-            size_t tf_path_len  = tmp->uri_len - URI_PREFIX_FILE_LEN;
+            const char* tf_path;
+            size_t tf_path_len;
+            find_path_in_uri(tmp->uri, tmp->uri_len, &tf_path, &tf_path_len);
             if (tf_path_len == path_len && !memcmp(tf_path, path, path_len + 1)) {
                 tf = tmp;
                 break;
@@ -769,9 +784,16 @@ static int register_file(const char* uri, const char* hash_str, bool check_dupli
 static int normalize_and_register_file(const char* uri, const char* hash_str) {
     int ret;
 
-    if (!strstartswith(uri, URI_PREFIX_FILE)) {
-        log_error("Invalid URI [%s]: Trusted/allowed files must start with 'file:'", uri);
-        return -PAL_ERROR_INVAL;
+    if (hash_str) {
+        if (!strstartswith(uri, URI_PREFIX_FILE)) {
+            log_error("Invalid URI [%s]: Trusted files must start with 'file:'", uri);
+            return -PAL_ERROR_INVAL;
+        }
+    } else {
+        if (!strstartswith(uri, URI_PREFIX_FILE) && !strstartswith(uri, URI_PREFIX_DEV)) {
+            log_error("Invalid URI [%s]: Allowed files must start with 'file:' or 'dev:'", uri);
+            return -PAL_ERROR_INVAL;
+        }
     }
 
     const size_t norm_uri_size = strlen(uri) + 1;
@@ -780,10 +802,20 @@ static int normalize_and_register_file(const char* uri, const char* hash_str) {
         return -PAL_ERROR_NOMEM;
     }
 
-    memcpy(norm_uri, URI_PREFIX_FILE, URI_PREFIX_FILE_LEN);
-    size_t norm_path_size = norm_uri_size - URI_PREFIX_FILE_LEN;
-    if (!get_norm_path(uri + URI_PREFIX_FILE_LEN, norm_uri + URI_PREFIX_FILE_LEN,
-                       &norm_path_size)) {
+    const char* uri_prefix;
+    size_t uri_prefix_len;
+    if (strstartswith(uri, URI_PREFIX_FILE)) {
+        uri_prefix = URI_PREFIX_FILE;
+        uri_prefix_len = URI_PREFIX_FILE_LEN;
+    } else {
+        assert(strstartswith(uri, URI_PREFIX_DEV));
+        uri_prefix = URI_PREFIX_DEV;
+        uri_prefix_len = URI_PREFIX_DEV_LEN;
+    }
+
+    memcpy(norm_uri, uri_prefix, uri_prefix_len);
+    size_t norm_path_size = norm_uri_size - uri_prefix_len;
+    if (!get_norm_path(uri + uri_prefix_len, norm_uri + uri_prefix_len, &norm_path_size)) {
         log_error("Path (%s) normalization failed", uri);
         ret = -PAL_ERROR_INVAL;
         goto out;

pal/src/host/linux-sgx/pal_devices.c

@@ -11,13 +11,15 @@
  */
 
 #include "api.h"
+#include "enclave_tf.h"
 #include "ioctls.h"
 #include "pal.h"
 #include "pal_error.h"
 #include "pal_flags_conv.h"
 #include "pal_internal.h"
 #include "pal_linux.h"
 #include "pal_linux_error.h"
+#include "path_utils.h"
 #include "perm.h"
 #include "toml.h"
 #include "toml_utils.h"
@@ -26,6 +28,8 @@ static int dev_open(PAL_HANDLE* handle, const char* type, const char* uri, enum
                     pal_share_flags_t share, enum pal_create_mode create,
                     pal_stream_options_t options) {
     int ret;
+    char* normpath = NULL;
+
     assert(create != PAL_CREATE_IGNORED);
 
     assert(WITHIN_MASK(share,   PAL_SHARE_MASK));
@@ -53,6 +57,32 @@ static int dev_open(PAL_HANDLE* handle, const char* type, const char* uri, enum
     }
     hdl->dev.fd = ret;
 
+    size_t normpath_size = strlen(uri) + 1;
+    normpath = malloc(normpath_size);
+    if (!normpath) {
+        ret = -PAL_ERROR_NOMEM;
+        goto fail;
+    }
+    ret = get_norm_path(uri, normpath, &normpath_size);
+    if (ret < 0) {
+        log_warning("Could not normalize path (%s): %s", uri, pal_strerror(ret));
+        ret = -PAL_ERROR_DENIED;
+        goto fail;
+    }
+    hdl->dev.realpath = normpath;
+
+    struct trusted_file* tf = get_trusted_or_allowed_file(hdl->dev.realpath);
+    if (!tf || !tf->allowed) {
+        if (get_file_check_policy() != FILE_CHECK_POLICY_ALLOW_ALL_BUT_LOG) {
+            log_warning("Disallowing access to device '%s'; device is not allowed.",
+                        hdl->dev.realpath);
+            ret = -PAL_ERROR_DENIED;
+            goto fail;
+        }
+        log_warning("Allowing access to unknown device '%s' due to file_check_policy settings.",
+                    hdl->dev.realpath);
+    }
+
     if (access == PAL_ACCESS_RDONLY) {
         hdl->flags |= PAL_HANDLE_FD_READABLE;
     } else if (access == PAL_ACCESS_WRONLY) {
@@ -66,6 +96,7 @@ static int dev_open(PAL_HANDLE* handle, const char* type, const char* uri, enum
     return 0;
 fail:
     free(hdl);
+    free(normpath);
     return ret;
 }
 
@@ -107,6 +138,23 @@ static void dev_destroy(PAL_HANDLE handle) {
     free(handle);
 }
 
+static int dev_delete(PAL_HANDLE handle, enum pal_delete_mode delete_mode) {
+    assert(handle->hdr.type == PAL_TYPE_DEV);
+
+    if (delete_mode != PAL_DELETE_ALL)
+        return -PAL_ERROR_INVAL;
+
+    int ret = ocall_delete(handle->dev.realpath);
+    return ret < 0 ? unix_to_pal_error(ret) : ret;
+}
+
+static int64_t dev_setlength(PAL_HANDLE handle, uint64_t length) {
+    assert(handle->hdr.type == PAL_TYPE_DEV);
+
+    int ret = ocall_ftruncate(handle->dev.fd, length);
+    return ret < 0 ? unix_to_pal_error(ret) : (int64_t)length;
+}
+
 static int dev_flush(PAL_HANDLE handle) {
     assert(handle->hdr.type == PAL_TYPE_DEV);
 
@@ -158,6 +206,8 @@ struct handle_ops g_dev_ops = {
     .read           = &dev_read,
     .write          = &dev_write,
     .destroy        = &dev_destroy,
+    .delete         = &dev_delete,
+    .setlength      = &dev_setlength,
     .flush          = &dev_flush,
     .attrquery      = &dev_attrquery,
     .attrquerybyhdl = &dev_attrquerybyhdl,

pal/src/host/linux-sgx/pal_host.h

@@ -78,6 +78,7 @@ typedef struct {
 
         struct {
             PAL_IDX fd;
+            char* realpath;
             bool nonblocking;
         } dev;