From 00e101086a82ebb37839ffead4d048c70b9444d3 Mon Sep 17 00:00:00 2001 From: g2flyer Date: Thu, 22 Aug 2024 11:14:48 -0700 Subject: [PATCH] [LibOS] Make handling of corruption more consistent (WIP) Signed-off-by: g2flyer --- libos/src/fs/libos_fs_encrypted.c | 111 ++++++++++++---------- libos/test/fs/meson.build | 1 + libos/test/fs/pf_tamper.manifest.template | 28 ++++++ libos/test/fs/test_enc.py | 15 ++- libos/test/fs/tests.toml | 1 + 5 files changed, 105 insertions(+), 51 deletions(-) create mode 100644 libos/test/fs/pf_tamper.manifest.template diff --git a/libos/src/fs/libos_fs_encrypted.c b/libos/src/fs/libos_fs_encrypted.c index 4cbc835d3c..ad7a186537 100644 --- a/libos/src/fs/libos_fs_encrypted.c +++ b/libos/src/fs/libos_fs_encrypted.c @@ -19,7 +19,6 @@ static LISTP_TYPE(libos_encrypted_files_key) g_keys = LISTP_INIT; /* Protects the `g_keys` list, but also individual keys, since they can be updated */ static struct libos_lock g_keys_lock; - static LISTP_TYPE(libos_encrypted_volume) g_volumes = LISTP_INIT; /* Protects the `g_volumes` list. */ @@ -277,69 +276,78 @@ static int encrypted_file_internal_open(struct libos_encrypted_file* enc, PAL_HA ret = -EACCES; goto out; } + libos_encrypted_file_state_t new_state_in_map = PF_FILE_STATE_ACTIVE; pf_mac_t opening_root_mac; pf_status_t pfs = pf_open(pal_handle, norm_path, size, PF_FILE_MODE_READ | PF_FILE_MODE_WRITE, create, &enc->volume->key->pf_key, &opening_root_mac, &pf); unlock(&g_keys_lock); if (PF_FAILURE(pfs)) { - log_warning("pf_open failed: %s", pf_strerror(pfs)); ret = -EACCES; - goto out; + if (pfs != PF_STATUS_CORRUPTED) { + log_warning("pf_open failed: %s", pf_strerror(pfs)); + goto out; + } + log_error("pf_open of file '%s' encountered corrupted state during open", norm_path); + new_state_in_map = PF_FILE_STATE_ERROR; } /* rollback protection */ - struct libos_encrypted_volume_state_map* file_state = NULL; log_debug("file '%s' opened with MAC=" MAC_PRINTF_PATTERN, norm_path, MAC_PRINTF_ARGS(opening_root_mac)); // TODO (MST): remove me eventually? + struct libos_encrypted_volume_state_map* file_state = NULL; lock(&(enc->volume->files_state_map_lock)); /* - get current state */ HASH_FIND_STR(enc->volume->files_state_map, norm_path, file_state); - /* - check current state */ - if (create) { - if (file_state && (file_state->state != PF_FILE_STATE_DELETED)) { - log_error("newly created file '%s' is in state %s", norm_path, - file_state_to_string(file_state->state)); - if (enc->volume->protection_mode != PF_ENCLAVE_LIFE_RB_PROTECTION_NONE) { - pf_set_corrupted(pf); - ret = -EEXIST; - goto out_unlock_map; - } - } - } else { - if (file_state) { - if ((file_state->state == PF_FILE_STATE_ERROR) || - (file_state->state == PF_FILE_STATE_DELETED)) { - log_error("file '%s' was seen before but in %s state", norm_path, + if (new_state_in_map != PF_FILE_STATE_ERROR) { + /* - check current state */ + if (create) { + if (file_state && (file_state->state != PF_FILE_STATE_DELETED)) { + // Note: with create=true we want to open without overwriting, so only valid state + // for an existing map entry is if the file was known to be deleted. + log_error("newly created file '%s' is in state %s", norm_path, file_state_to_string(file_state->state)); if (enc->volume->protection_mode != PF_ENCLAVE_LIFE_RB_PROTECTION_NONE) { - pf_set_corrupted(pf); - ret = -EACCES; - goto out_unlock_map; - } - } - if (memcmp(file_state->last_seen_root_mac, opening_root_mac, sizeof(pf_mac_t)) != 0) { - log_error( - "file '%s' was seen before but in different inconsistent (rolled-back?) " - "state, expected MAC=" MAC_PRINTF_PATTERN - " but file had " - "MAC=" MAC_PRINTF_PATTERN, - norm_path, MAC_PRINTF_ARGS(file_state->last_seen_root_mac), - MAC_PRINTF_ARGS(opening_root_mac)); - if (enc->volume->protection_mode != PF_ENCLAVE_LIFE_RB_PROTECTION_NONE) { - pf_set_corrupted(pf); - ret = -EACCES; - goto out_unlock_map; + pf_close(pf, NULL); + ret = -EEXIST; + new_state_in_map = PF_FILE_STATE_ERROR; } } } else { - if (enc->volume->protection_mode == PF_ENCLAVE_LIFE_RB_PROTECTION_STRICT) { - log_error( - "file '%s' was not seen before which is not allowed with strict rollback " - "protection mode", - norm_path); - pf_set_corrupted(pf); - ret = -EACCES; - goto out_unlock_map; + if (file_state) { + if ((file_state->state == PF_FILE_STATE_ERROR) || + (file_state->state == PF_FILE_STATE_DELETED)) { + log_error("file '%s' was seen before but in %s state", norm_path, + file_state_to_string(file_state->state)); + if (enc->volume->protection_mode != PF_ENCLAVE_LIFE_RB_PROTECTION_NONE) { + pf_close(pf, NULL); + ret = -EACCES; + new_state_in_map = PF_FILE_STATE_ERROR; + } + } else if (memcmp(file_state->last_seen_root_mac, opening_root_mac, + sizeof(pf_mac_t)) != 0) { + log_error( + "file '%s' was seen before but in different inconsistent (rolled-back?) " + "state, expected MAC=" MAC_PRINTF_PATTERN + " but file had " + "MAC=" MAC_PRINTF_PATTERN, + norm_path, MAC_PRINTF_ARGS(file_state->last_seen_root_mac), + MAC_PRINTF_ARGS(opening_root_mac)); + if (enc->volume->protection_mode != PF_ENCLAVE_LIFE_RB_PROTECTION_NONE) { + pf_close(pf, NULL); + ret = -EACCES; + new_state_in_map = PF_FILE_STATE_ERROR; + } + } + } else { + if (enc->volume->protection_mode == PF_ENCLAVE_LIFE_RB_PROTECTION_STRICT) { + log_error( + "file '%s' was not seen before which is not allowed with strict rollback " + "protection mode", + norm_path); + pf_close(pf, NULL); + ret = -EACCES; + new_state_in_map = PF_FILE_STATE_ERROR; + } } } } @@ -354,15 +362,20 @@ static int encrypted_file_internal_open(struct libos_encrypted_file* enc, PAL_HA norm_path = NULL; /* to prevent freeing it */ HASH_ADD_KEYPTR(hh, enc->volume->files_state_map, file_state->norm_path, strlen(file_state->norm_path), file_state); + log_debug( + "updated file protection map with file '%s', state '%s' and MAC=" MAC_PRINTF_PATTERN, + file_state->norm_path, file_state_to_string(file_state->state), + MAC_PRINTF_ARGS(file_state->last_seen_root_mac)); } /* we do below unconditionally as we might recreate a deleted file or overwrite an existing * one */ memcpy(file_state->last_seen_root_mac, opening_root_mac, sizeof(pf_mac_t)); - file_state->state = PF_FILE_STATE_ACTIVE; + file_state->state = new_state_in_map; - enc->pf = pf; - enc->pal_handle = pal_handle; - ret = 0; + if (ret == 0) { + enc->pf = pf; + enc->pal_handle = pal_handle; + } out_unlock_map: unlock(&(enc->volume->files_state_map_lock)); diff --git a/libos/test/fs/meson.build b/libos/test/fs/meson.build index 8fef3e38f0..8bffd67155 100644 --- a/libos/test/fs/meson.build +++ b/libos/test/fs/meson.build @@ -43,6 +43,7 @@ tests = { 'open_close': {}, 'open_flags': {}, 'pf_rollback': {}, + 'pf_tamper': {}, 'read_write': {}, 'read_write_mmap': {}, 'seek_tell': {}, diff --git a/libos/test/fs/pf_tamper.manifest.template b/libos/test/fs/pf_tamper.manifest.template new file mode 100644 index 0000000000..7028233a0c --- /dev/null +++ b/libos/test/fs/pf_tamper.manifest.template @@ -0,0 +1,28 @@ +loader.entrypoint = "file:{{ gramine.libos }}" +loader.log_level ="trace" # DEBUG +libos.entrypoint = "{{ entrypoint }}" + +loader.env.LD_LIBRARY_PATH = "/lib:{{ arch_libdir }}:/usr/{{ arch_libdir }}" +loader.insecure__use_cmdline_argv = true + +fs.mounts = [ + { path = "/lib", uri = "file:{{ gramine.runtimedir(libc) }}" }, + { path = "/{{ entrypoint }}", uri = "file:{{ binary_dir }}/{{ entrypoint }}" }, + { path = "/bin", uri = "file:/bin" }, + + { type = "encrypted", protection_mode = "non-strict", path = "/tmp/enc_output", uri = "file:tmp/enc_output" }, +] + +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 }}", +] + +# See the `keys.c` test. +fs.insecure__keys.default = "ffeeddccbbaa99887766554433221100" diff --git a/libos/test/fs/test_enc.py b/libos/test/fs/test_enc.py index 2e9a3dc1f2..9f75149fdf 100644 --- a/libos/test/fs/test_enc.py +++ b/libos/test/fs/test_enc.py @@ -200,13 +200,24 @@ def test_500_invalid(self): # decryption of invalid file must fail with -1 (wrapped to 255) self.assertEqual(exc.returncode, 255) else: - print('[!] Fail: successfully decrypted file: ' + name) + print('[!] Fail: successfully decrypted file with cipher utility: ' + name) failed = True + + # test decryption as part of reading file in program running with gramine + stdout, stderr = self.run_binary(['pf_tamper', input_path]) + try: + self.assertIn('ERROR: ', stdout) + # TODO: check also that we updated map in trace/stderr? + # DEBUG: self.assertIn('truncate(' + path_1 + ') to ' + str(size_out) + ' OK', stdout) + except: + print('[!] Fail: successfully decrypted file with gramine: ' + name) + failed = True + if failed: self.fail() + # checks rollback protection def test_600_gdb_pf_rollback(self): - # This test checks rollback protection. # To run this test manually, encrypt a (contained in ) with the # default key from manifest and use: # GDB=1 GDB_TTY=1 GDB_SCRIPT=pf_rollback.gdb gramine-[sgx|direct] pf_rollback diff --git a/libos/test/fs/tests.toml b/libos/test/fs/tests.toml index 4e4991e994..811e57203f 100644 --- a/libos/test/fs/tests.toml +++ b/libos/test/fs/tests.toml @@ -14,6 +14,7 @@ manifests = [ "open_close", "open_flags", "pf_rollback", + "pf_tamper", "read_write", "read_write_mmap", "seek_tell",