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

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

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));

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