diff --git a/libos/src/fs/chroot/encrypted.c b/libos/src/fs/chroot/encrypted.c index f241d986c5..94a03be134 100644 --- a/libos/src/fs/chroot/encrypted.c +++ b/libos/src/fs/chroot/encrypted.c @@ -12,9 +12,15 @@ * key for files. Multiple mounts can use the same key. The list of keys is managed in * `libos_fs_encrypted.c`. * - * - Inodes (`libos_inode`, for regular files) hold a `libos_encrypted_file` object. This object - * lives as long as the inode, but is kept *open* only as long as there are `libos_handle` objects - * corresponding to it. We use `encrypted_file_{get,put}` operations to maintain that invariant. + * - Inodes (`libos_inode`, for regular files) hold a `libos_encrypted_file` object in `inode->data` + * field. This object lives as long as the inode, but is kept *open* only as long as there are + * `libos_handle` objects corresponding to it. We use `encrypted_file_{get,put}` operations to + * maintain that invariant. + * + * It is possible that inode has no corresponding `libos_encrypted_file` object, i.e. + * `inode->data` is NULL. This happens if the encrypted file is corrupted; in this case we want to + * allow at least some operations on the file (currently only `unlink()`), but all other + * operations return -EACCES. * * An open `libos_encrypted_file` object keeps an open PAL handle and associated data * (`pf_context_t`), so that operations (read, write, truncate...) can be performed on the file. @@ -152,18 +158,25 @@ static int chroot_encrypted_lookup(struct libos_dentry* dent) { struct libos_encrypted_files_key* key = dent->mount->data; ret = encrypted_file_open(uri, key, &enc); if (ret < 0) { - goto out; - } - - ret = encrypted_file_get_size(enc, &size); - encrypted_file_put(enc); - - if (ret < 0) { - encrypted_file_destroy(enc); - goto out; + if (ret == -EACCES) { + /* allow the inode to be created even if the underlying encrypted file is corrupted; + * this is useful for unlinking a corrupted file */ + inode->data = NULL; + } else { + goto out; + } + } else { + ret = encrypted_file_get_size(enc, &size); + encrypted_file_put(enc); + + if (ret < 0) { + encrypted_file_destroy(enc); + goto out; + } + + inode->data = enc; + inode->size = size; } - inode->data = enc; - inode->size = size; } dent->inode = inode; get_inode(inode); @@ -185,6 +198,8 @@ static int chroot_encrypted_open(struct libos_handle* hdl, struct libos_dentry* if (dent->inode->type == S_IFREG) { struct libos_encrypted_file* enc = dent->inode->data; + if (!enc) + return -EACCES; lock(&dent->inode->lock); ret = encrypted_file_get(enc); @@ -325,6 +340,10 @@ static int chroot_encrypted_rename(struct libos_dentry* old, struct libos_dentry lock(&old->inode->lock); struct libos_encrypted_file* enc = old->inode->data; + if (!enc) { + ret = -EACCES; + goto out; + } ret = encrypted_file_get(enc); if (ret < 0) @@ -342,6 +361,9 @@ static int chroot_encrypted_chmod(struct libos_dentry* dent, mode_t perm) { assert(locked(&g_dcache_lock)); assert(dent->inode); + if (!dent->inode->data) + return -EACCES; + char* uri = NULL; int ret = chroot_dentry_uri(dent, dent->inode->type, &uri); @@ -374,6 +396,8 @@ static int chroot_encrypted_fchmod(struct libos_handle* hdl, mode_t perm) { assert(hdl->inode); struct libos_encrypted_file* enc = hdl->inode->data; + assert(enc); + mode_t host_perm = HOST_PERM(perm); PAL_STREAM_ATTR attr = {.share_flags = host_perm}; int ret = PalStreamAttributesSetByHandle(enc->pal_handle, &attr); @@ -389,6 +413,7 @@ static int chroot_encrypted_flush(struct libos_handle* hdl) { return 0; struct libos_encrypted_file* enc = hdl->inode->data; + assert(enc); /* If there are any MAP_SHARED mappings for the file, this will write data to `enc` */ int ret = msync_handle(hdl); @@ -408,6 +433,7 @@ static int chroot_encrypted_close(struct libos_handle* hdl) { return 0; struct libos_encrypted_file* enc = hdl->inode->data; + assert(enc); lock(&hdl->inode->lock); encrypted_file_put(enc); @@ -425,6 +451,8 @@ static ssize_t chroot_encrypted_read(struct libos_handle* hdl, void* buf, size_t } struct libos_encrypted_file* enc = hdl->inode->data; + assert(enc); + size_t actual_count; lock(&hdl->inode->lock); @@ -447,6 +475,8 @@ static ssize_t chroot_encrypted_write(struct libos_handle* hdl, const void* buf, } struct libos_encrypted_file* enc = hdl->inode->data; + assert(enc); + size_t actual_count; lock(&hdl->inode->lock); @@ -485,6 +515,7 @@ static int chroot_encrypted_truncate(struct libos_handle* hdl, file_off_t size) int ret; struct libos_encrypted_file* enc = hdl->inode->data; + assert(enc); lock(&hdl->inode->lock); ret = encrypted_file_set_size(enc, size); @@ -495,6 +526,16 @@ static int chroot_encrypted_truncate(struct libos_handle* hdl, file_off_t size) return ret; } +static int chroot_encrypted_stat(struct libos_dentry* dent, struct stat* buf) { + assert(locked(&g_dcache_lock)); + assert(dent->inode); + + if (!dent->inode->data) + return -EACCES; + + return generic_inode_stat(dent, buf); +} + struct libos_fs_ops chroot_encrypted_fs_ops = { .mount = &chroot_encrypted_mount, .flush = &chroot_encrypted_flush, @@ -517,7 +558,7 @@ struct libos_d_ops chroot_encrypted_d_ops = { .lookup = &chroot_encrypted_lookup, .creat = &chroot_encrypted_creat, .mkdir = &chroot_encrypted_mkdir, - .stat = &generic_inode_stat, + .stat = &chroot_encrypted_stat, .readdir = &chroot_readdir, /* same as in `chroot` filesystem */ .unlink = &chroot_encrypted_unlink, .rename = &chroot_encrypted_rename, diff --git a/libos/test/regression/.gitignore b/libos/test/regression/.gitignore index f9dfc946d5..0a366a38e3 100644 --- a/libos/test/regression/.gitignore +++ b/libos/test/regression/.gitignore @@ -3,6 +3,6 @@ /trusted_testfile /tmp/* -!/tmp/.dummy +!/tmp/.gitkeep /tmp_enc/* -!/tmp_enc/.dummy +!/tmp_enc/.gitkeep diff --git a/libos/test/regression/Makefile b/libos/test/regression/Makefile index 3c51c6d23e..b5181a1372 100644 --- a/libos/test/regression/Makefile +++ b/libos/test/regression/Makefile @@ -17,5 +17,5 @@ clean: testfile \ trusted_testfile \ asm/x86_64/__pycache__ \ - tmp/* \ - tmp_enc/* + tmp/* + find tmp_enc/ -not -path '*/.*' -type f -delete diff --git a/libos/test/regression/manifest.template b/libos/test/regression/manifest.template index 00c6c0d926..ae5f6cda89 100644 --- a/libos/test/regression/manifest.template +++ b/libos/test/regression/manifest.template @@ -20,8 +20,8 @@ fs.mounts = [ { type = "tmpfs", path = "/mnt/tmpfs" }, { type = "encrypted", path = "/tmp_enc", uri = "file:tmp_enc", key_name = "my_custom_key" }, - { type = "encrypted", path = "/encrypted_file_mrenclave.dat", uri = "file:encrypted_file_mrenclave.dat", key_name = "_sgx_mrenclave" }, - { type = "encrypted", path = "/encrypted_file_mrsigner.dat", uri = "file:encrypted_file_mrsigner.dat", key_name = "_sgx_mrsigner" }, + { type = "encrypted", path = "/tmp_enc/mrenclaves", uri = "file:tmp_enc/mrenclaves", key_name = "_sgx_mrenclave" }, + { type = "encrypted", path = "/tmp_enc/mrsigners", uri = "file:tmp_enc/mrsigners", key_name = "_sgx_mrsigner" }, ] sgx.max_threads = {{ '1' if env.get('EDMM', '0') == '1' else '16' }} diff --git a/libos/test/regression/sealed_file.c b/libos/test/regression/sealed_file.c index bceda29530..93c51e8b58 100644 --- a/libos/test/regression/sealed_file.c +++ b/libos/test/regression/sealed_file.c @@ -19,8 +19,10 @@ int main(int argc, char** argv) { int ret; ssize_t bytes; - if (argc != 2) - errx(EXIT_FAILURE, "Usage: %s ", argv[0]); + if (argc != 3 || (strcmp(argv[2], "unlink") && strcmp(argv[2], "nounlink"))) { + errx(EXIT_FAILURE, "Usage: %s unlink|nounlink", + argv[0]); + } ret = access(argv[1], F_OK); if (ret < 0) { @@ -35,10 +37,26 @@ int main(int argc, char** argv) { if (bytes != SECRETSTRING_LEN) errx(EXIT_FAILURE, "Wrote %ld instead of expected %ld", bytes, SECRETSTRING_LEN); - printf("CREATION OK\n"); + puts("CREATION OK"); return 0; } - err(EXIT_FAILURE, "access failed"); + if (errno != EACCES || strcmp(argv[2], "unlink") != 0) { + /* access() can legitimately return EACCES if we're testing the "modified-MRENCLAVE + * app wants to delete the previous-MRENCLAVE-sealed file" corner case */ + err(EXIT_FAILURE, "access failed"); + } + } + + /* at this point, the file exists (either created by above or already existed on storage) */ + + if (strcmp(argv[2], "unlink") == 0) { + /* verify that removing the file always works, even with a mismatching MRENCLAVE */ + ret = unlink(argv[1]); + if (ret < 0) + err(EXIT_FAILURE, "unlink failed"); + + puts("UNLINK OK"); + return 0; } char buf[SECRETSTRING_LEN]; @@ -58,9 +76,9 @@ int main(int argc, char** argv) { /* The build system adds MODIFE_MRENCLAVE macro to produce a slightly different executable (due * to the below different string), which in turn produces a different MRENCLAVE SGX measurement. * This trick is to test `_sgx_mrsigner` functionality. */ - printf("READING FROM MODIFIED ENCLAVE OK\n"); + puts("READING FROM MODIFIED ENCLAVE OK"); #else - printf("READING OK\n"); + puts("READING OK"); #endif return 0; } diff --git a/libos/test/regression/test_libos.py b/libos/test/regression/test_libos.py index 606e4c8985..4678322a7c 100644 --- a/libos/test/regression/test_libos.py +++ b/libos/test/regression/test_libos.py @@ -807,10 +807,10 @@ def test_052_mmap_file_backed_trusted(self): @unittest.skipUnless(HAS_SGX, 'Sealed (protected) files are only available with SGX') def test_053_mmap_file_backed_protected(self): # create the protected file - pf_path = 'encrypted_file_mrsigner.dat' + pf_path = 'tmp_enc/mrsigners/mmap_file_backed.dat' if os.path.exists(pf_path): os.remove(pf_path) - stdout, _ = self.run_binary(['sealed_file', pf_path]) + stdout, _ = self.run_binary(['sealed_file', pf_path, 'nounlink']) self.assertIn('CREATION OK', stdout) try: @@ -1214,39 +1214,39 @@ def test_040_sysfs(self): @unittest.skipUnless(HAS_SGX, 'Sealed (protected) files are only available with SGX') def test_050_sealed_file_mrenclave(self): - pf_path = 'encrypted_file_mrenclave.dat' + pf_path = 'tmp_enc/mrenclaves/test_050.dat' if os.path.exists(pf_path): os.remove(pf_path) - stdout, _ = self.run_binary(['sealed_file', pf_path]) + stdout, _ = self.run_binary(['sealed_file', pf_path, 'nounlink']) self.assertIn('CREATION OK', stdout) - stdout, _ = self.run_binary(['sealed_file', pf_path]) + stdout, _ = self.run_binary(['sealed_file', pf_path, 'nounlink']) self.assertIn('READING OK', stdout) @unittest.skipUnless(HAS_SGX, 'Sealed (protected) files are only available with SGX') def test_051_sealed_file_mrsigner(self): - pf_path = 'encrypted_file_mrsigner.dat' + pf_path = 'tmp_enc/mrsigners/test_051.dat' if os.path.exists(pf_path): os.remove(pf_path) - stdout, _ = self.run_binary(['sealed_file', pf_path]) + stdout, _ = self.run_binary(['sealed_file', pf_path, 'nounlink']) self.assertIn('CREATION OK', stdout) - stdout, _ = self.run_binary(['sealed_file_mod', pf_path]) + stdout, _ = self.run_binary(['sealed_file_mod', pf_path, 'nounlink']) self.assertIn('READING FROM MODIFIED ENCLAVE OK', stdout) @unittest.skipUnless(HAS_SGX, 'Sealed (protected) files are only available with SGX') def test_052_sealed_file_mrenclave_bad(self): - pf_path = 'encrypted_file_mrenclave.dat' + pf_path = 'tmp_enc/mrenclaves/test_052.dat' # Negative test: Seal MRENCLAVE-bound file in one enclave -> opening this file in # another enclave (with different MRENCLAVE) should fail if os.path.exists(pf_path): os.remove(pf_path) - stdout, _ = self.run_binary(['sealed_file', pf_path]) + stdout, _ = self.run_binary(['sealed_file', pf_path, 'nounlink']) self.assertIn('CREATION OK', stdout) try: - self.run_binary(['sealed_file_mod', pf_path]) + self.run_binary(['sealed_file_mod', pf_path, 'nounlink']) self.fail('expected to return nonzero') except subprocess.CalledProcessError as e: self.assertEqual(e.returncode, 1) @@ -1254,6 +1254,19 @@ def test_052_sealed_file_mrenclave_bad(self): self.assertNotIn('READING FROM MODIFIED ENCLAVE OK', stdout) self.assertIn('Permission denied', stdout) + @unittest.skipUnless(HAS_SGX, 'Sealed (protected) files are only available with SGX') + def test_053_sealed_file_mrenclave_unlink(self): + pf_path = 'tmp_enc/mrenclaves/test_053.dat' + # Unlinking test: Seal MRENCLAVE-bound file in one enclave -> unlinking this file in + # another enclave (with different MRENCLAVE) should still work + if os.path.exists(pf_path): + os.remove(pf_path) + + stdout, _ = self.run_binary(['sealed_file', pf_path, 'nounlink']) + self.assertIn('CREATION OK', stdout) + stdout, _ = self.run_binary(['sealed_file_mod', pf_path, 'unlink']) + self.assertIn('UNLINK OK', stdout) + def test_060_synthetic(self): stdout, _ = self.run_binary(['synthetic']) self.assertIn("TEST OK", stdout) diff --git a/libos/test/regression/tmp/.dummy b/libos/test/regression/tmp/.gitkeep similarity index 100% rename from libos/test/regression/tmp/.dummy rename to libos/test/regression/tmp/.gitkeep diff --git a/libos/test/regression/tmp_enc/.dummy b/libos/test/regression/tmp_enc/.gitkeep similarity index 100% rename from libos/test/regression/tmp_enc/.dummy rename to libos/test/regression/tmp_enc/.gitkeep diff --git a/libos/test/regression/tmp_enc/mrenclaves/.gitkeep b/libos/test/regression/tmp_enc/mrenclaves/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/libos/test/regression/tmp_enc/mrsigners/.gitkeep b/libos/test/regression/tmp_enc/mrsigners/.gitkeep new file mode 100644 index 0000000000..e69de29bb2