[LibOS] Make handling of corruption more consistent (WIP)

Signed-off-by: g2flyer <[email protected]>

Author: g2flyer

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,14 +276,19 @@ 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 */
@@ -294,53 +298,58 @@ static int encrypted_file_internal_open(struct libos_encrypted_file* enc, PAL_HA
     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;
+                    ret              = -EEXIST;
+                    new_state_in_map = PF_FILE_STATE_ERROR;
                 }
             }
-            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) {
+        } 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,
+                              file_state_to_string(file_state->state));
+                    if (enc->volume->protection_mode != PF_ENCLAVE_LIFE_RB_PROTECTION_NONE) {
+                        pf_set_corrupted(pf);
+                        ret              = -EACCES;
+                        new_state_in_map = PF_FILE_STATE_ERROR;
+                    }
+                }
+                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;
+                        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;
+                    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_set_corrupted(pf);
-                ret = -EACCES;
-                goto out_unlock_map;
-            }
         }
     }
     /* - uodate map with new state */
@@ -354,11 +363,15 @@ 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,
+            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;

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': {},

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"

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 <input_file> (contained in <work_dir>) with the 
         # default key from manifest and use:
         #    GDB=1 GDB_TTY=1 GDB_SCRIPT=pf_rollback.gdb gramine-[sgx|direct] pf_rollback <work_dir> <input_file>

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",