fixup! Single-process-lifetime rollback protection for protected files (WIP)

g2flyer · g2flyer · commit 6193a19ae84d · 2024-05-06T08:21:20.000-07:00
Signed-off-by: g2flyer &lt;michael.steiner@intel.com&gt;
diff --git a/common/src/protected_files/protected_files.c b/common/src/protected_files/protected_files.c
@@ -1133,7 +1133,7 @@ static void ipf_delete_cache(pf_context_t* pf) {
     }
 }
 
-static bool ipf_close(pf_context_t* pf) {
+static bool ipf_close(pf_context_t* pf, pf_mac_t* closing_root_gmac) {
     bool retval = true;
 
     if (pf->file_status != PF_STATUS_SUCCESS) {
@@ -1146,6 +1146,10 @@ static bool ipf_close(pf_context_t* pf) {
         }
     }
 
+    if (closing_root_gmac != NULL) {
+        memcpy(*closing_root_gmac, pf->file_metadata.plain_part.metadata_gmac, sizeof(pf_mac_t));
+    }
+
     // omeg: fs close is done by Gramine handler
     pf->file_status = PF_STATUS_UNINITIALIZED;
 
@@ -1192,7 +1196,7 @@ pf_status_t pf_open(pf_handle_t handle, const char* path, uint64_t underlying_si
     pf_status_t status;
     *context = ipf_open(path, mode, create, handle, underlying_size, key, &status);
     if (opening_root_gmac != NULL) {
-        memcpy(opening_root_gmac, (*context)->file_metadata.plain_part.metadata_gmac,
+        memcpy(*opening_root_gmac, (*context)->file_metadata.plain_part.metadata_gmac,
                sizeof(pf_mac_t));
     }
     return status;
@@ -1202,13 +1206,10 @@ pf_status_t pf_close(pf_context_t* pf, pf_mac_t* closing_root_gmac) {
     if (!g_initialized)
         return PF_STATUS_UNINITIALIZED;
 
-    if (ipf_close(pf)) {
+    if (ipf_close(pf, closing_root_gmac)) {
         free(pf);
         return PF_STATUS_SUCCESS;
     }
-    if (closing_root_gmac != NULL) {
-        memcpy(closing_root_gmac, pf->file_metadata.plain_part.metadata_gmac, sizeof(pf_mac_t));
-    }
 
     pf_status_t ret = pf->last_error;
     free(pf);
@@ -1292,7 +1293,7 @@ pf_status_t pf_rename(pf_context_t* pf, const char* new_path, pf_mac_t* new_root
     if (!ipf_internal_flush(pf))
         return pf->last_error;
     if (new_root_gmac != NULL) {
-        memcpy(new_root_gmac, pf->file_metadata.plain_part.metadata_gmac, sizeof(pf_mac_t));
+        memcpy(*new_root_gmac, pf->file_metadata.plain_part.metadata_gmac, sizeof(pf_mac_t));
     }
 
     return PF_STATUS_SUCCESS;
diff --git a/common/src/protected_files/protected_files.h b/common/src/protected_files/protected_files.h
@@ -26,6 +26,11 @@ typedef uint8_t pf_mac_t[PF_MAC_SIZE];
 typedef uint8_t pf_key_t[PF_KEY_SIZE];
 typedef uint8_t pf_keyid_t[32]; /* key derivation material */
 
+// convenience macros to print out some mac fingerprint: printf( "some text " MAC_PRINTF_PATTERN "
+// yet other text", MAC_PRINTF_ARGS(mac) );
+#define MAC_PRINTF_PATTERN   "0x%02x%02x%02x%02x..."
+#define MAC_PRINTF_ARGS(mac) (mac)[0], (mac)[1], (mac)[2], (mac)[3]
+
 typedef enum _pf_status_t {
     PF_STATUS_SUCCESS              = 0,
     PF_STATUS_UNKNOWN_ERROR        = -1,
diff --git a/libos/include/libos_fs_encrypted.h b/libos/include/libos_fs_encrypted.h
@@ -48,7 +48,7 @@ typedef enum {
  * them before and what the last seen root-hash is.  This is necessary to provide rollback
  */
 struct libos_encrypted_volume_state_map {
-    char* uri;  // assumptions: all paths canonicalized, symlinks are resolved & no hard links
+    char* norm_path;  // assumptions: all paths canonicalized, symlinks are resolved & no hard links
     libos_encrypted_file_state_t state;
     pf_mac_t last_seen_root_gmac;
     UT_hash_handle hh;
@@ -79,6 +79,7 @@ struct libos_encrypted_volume {
 struct libos_encrypted_file {
     size_t use_count;
     char* uri;
+    char* norm_path;  // normalized path of of uri
     struct libos_encrypted_volume* volume;
 
     /* `pf` and `pal_handle` are non-null as long as `use_count` is greater than 0 */
diff --git a/libos/src/fs/libos_fs_encrypted.c b/libos/src/fs/libos_fs_encrypted.c
@@ -164,7 +164,6 @@ static int encrypted_file_internal_open(struct libos_encrypted_file* enc, PAL_HA
     assert(!enc->pf);
 
     int ret;
-    char* normpath = NULL;
 
     if (!pal_handle) {
         enum pal_create_mode create_mode = create ? PAL_CREATE_ALWAYS : PAL_CREATE_NEVER;
@@ -185,21 +184,6 @@ static int encrypted_file_internal_open(struct libos_encrypted_file* enc, PAL_HA
     }
     size_t size = pal_attr.pending_size;
 
-    assert(strstartswith(enc->uri, URI_PREFIX_FILE));
-    const char* path = enc->uri + static_strlen(URI_PREFIX_FILE);
-
-    size_t normpath_size = strlen(path) + 1;
-    normpath = malloc(normpath_size);
-    if (!normpath) {
-        ret = -ENOMEM;
-        goto out;
-    }
-
-    if (!get_norm_path(path, normpath, &normpath_size)) {
-        ret = -EINVAL;
-        goto out;
-    }
-
     pf_context_t* pf;
     lock(&g_keys_lock);
     if (!enc->volume->key->is_set) {
@@ -209,8 +193,9 @@ static int encrypted_file_internal_open(struct libos_encrypted_file* enc, PAL_HA
         goto out;
     }
     pf_mac_t opening_root_gmac;
-    pf_status_t pfs = pf_open(pal_handle, normpath, size, PF_FILE_MODE_READ | PF_FILE_MODE_WRITE,
-                              create, &enc->volume->key->pf_key, &opening_root_gmac, &pf);
+    pf_status_t pfs =
+        pf_open(pal_handle, enc->norm_path, size, PF_FILE_MODE_READ | PF_FILE_MODE_WRITE, create,
+                &enc->volume->key->pf_key, &opening_root_gmac, &pf);
     unlock(&g_keys_lock);
     if (PF_FAILURE(pfs)) {
         log_warning("pf_open failed: %s", pf_strerror(pfs));
@@ -219,17 +204,26 @@ static int encrypted_file_internal_open(struct libos_encrypted_file* enc, PAL_HA
     }
     // rollback protection
     struct libos_encrypted_volume_state_map* file_state = NULL;
+    log_debug("file '%s' opened with MAC=" MAC_PRINTF_PATTERN, enc->norm_path,
+              MAC_PRINTF_ARGS(opening_root_gmac));  // TODO (MST): remove me eventually?
     lock(&(enc->volume->files_state_map_lock));
     // - check current state
     if ((!create) && (enc->volume->protection_mode != PF_ENCLAVE_LIFE_RB_PROTECTION_NONE)) {
-        HASH_FIND_STR(enc->volume->files_state_map, normpath, file_state);
+        HASH_FIND_STR(enc->volume->files_state_map, enc->norm_path, file_state);
         if (file_state) {
-            if ((file_state->state != PF_FILE_CLOSED) ||
-                (memcmp(file_state->last_seen_root_gmac, opening_root_gmac, sizeof(pf_mac_t)) !=
-                 0)) {
+            /*  TODO (MST): figure out what to test. Seems our test-cases, e.g., in open_close do
+             * allow for scenarios such as two concurrent open writeable fds for same file which i
+             * thought would be illegal
+             *        (file_state->state != PF_FILE_CLOSED) ||
+             */
+            if (memcmp(file_state->last_seen_root_gmac, opening_root_gmac, sizeof(pf_mac_t)) != 0) {
                 log_warning(
-                    "file '%s' was seen before but in different inconsistent (rolled-back?) state",
-                    normpath);
+                    "file '%s' was seen before but in different inconsistent (rolled-back?) "
+                    "state, expected MAC=" MAC_PRINTF_PATTERN
+                    " but file had "
+                    "MAC=" MAC_PRINTF_PATTERN,
+                    enc->norm_path, MAC_PRINTF_ARGS(file_state->last_seen_root_gmac),
+                    MAC_PRINTF_ARGS(opening_root_gmac));
                 pf_set_corrupted(pf);
                 ret = -EACCES;
                 goto out_unlock_map;
@@ -238,24 +232,24 @@ static int encrypted_file_internal_open(struct libos_encrypted_file* enc, PAL_HA
             if (enc->volume->protection_mode == PF_ENCLAVE_LIFE_RB_PROTECTION_STRICT) {
                 log_warning(
                     "file '%s' was not seen before, not allowing strict rollback protection",
-                    normpath);
+                    enc->norm_path);
                 pf_set_corrupted(pf);
                 ret = -EACCES;
                 goto out_unlock_map;
             }
         }
     }
     // - uodate map with new state
-    if (!file_state) {
+    if (file_state == NULL) {
         file_state = malloc(sizeof(struct libos_encrypted_volume_state_map));
-        if (!file_state) {
+        if (file_state == NULL) {
             ret = -ENOMEM;
             goto out_unlock_map;
         }
-        file_state->uri = normpath;
+        file_state->norm_path = enc->norm_path;
         memcpy(file_state->last_seen_root_gmac, opening_root_gmac, sizeof(pf_mac_t));
-        HASH_ADD_KEYPTR(hh, enc->volume->files_state_map, file_state->uri, strlen(file_state->uri),
-                        file_state);
+        HASH_ADD_KEYPTR(hh, enc->volume->files_state_map, file_state->norm_path,
+                        strlen(file_state->norm_path), file_state);
     }
     file_state->state = (create ? PF_FILE_IN_USE_NEW : PF_FILE_IN_USE_EXISTING);
 
@@ -267,7 +261,6 @@ static int encrypted_file_internal_open(struct libos_encrypted_file* enc, PAL_HA
     unlock(&(enc->volume->files_state_map_lock));
 out:
     if (ret < 0) {
-        free(normpath);
         PalObjectDestroy(pal_handle);
     }
     return ret;
@@ -296,10 +289,12 @@ static void encrypted_file_internal_close(struct libos_encrypted_file* enc) {
 
     pf_mac_t closing_root_gmac;
     pf_status_t pfs = pf_close(enc->pf, &closing_root_gmac);
-    struct libos_encrypted_volume_state_map* file_state = NULL;
+    log_debug("file '%s' closed with MAC=" MAC_PRINTF_PATTERN, enc->norm_path,
+              MAC_PRINTF_ARGS(closing_root_gmac));  // TODO (MST): remove me eventually?
     lock(&(enc->volume->files_state_map_lock));
-    HASH_FIND_STR(enc->volume->files_state_map, enc->uri, file_state);
-    // TODO (MST): error handling if filestate is not found
+    struct libos_encrypted_volume_state_map* file_state = NULL;
+    HASH_FIND_STR(enc->volume->files_state_map, enc->norm_path, file_state);
+    assert(file_state != NULL);
     if (PF_FAILURE(pfs)) {
         log_warning("pf_close failed: %s", pf_strerror(pfs));
         file_state->state = PF_FILE_ERROR;
@@ -515,17 +510,47 @@ static int encrypted_file_alloc(const char* uri, struct libos_encrypted_volume*
     if (!enc)
         return -ENOMEM;
 
+    int ret;
+    enc->uri       = NULL;
+    enc->norm_path = NULL;
+
     enc->uri = strdup(uri);
     if (!enc->uri) {
-        free(enc);
-        return -ENOMEM;
+        ret = -ENOMEM;
+        goto err;
     }
+
+    assert(strstartswith(enc->uri, URI_PREFIX_FILE));
+    const char* path = enc->uri + static_strlen(URI_PREFIX_FILE);
+
+    size_t norm_path_size = strlen(path) + 1;
+    enc->norm_path        = malloc(norm_path_size);
+    if (!enc->norm_path) {
+        ret = -ENOMEM;
+        goto err;
+    }
+
+    if (!get_norm_path(path, enc->norm_path, &norm_path_size)) {
+        ret = -EINVAL;
+        goto err;
+    }
+
     enc->volume     = volume;
     enc->use_count = 0;
     enc->pf = NULL;
     enc->pal_handle = NULL;
     *out_enc = enc;
     return 0;
+
+err:
+    if (enc) {
+        if (enc->uri)
+            free(enc->uri);
+        if (enc->norm_path)
+            free(enc->norm_path);
+        free(enc);
+    }
+    return ret;
 }
 
 int encrypted_file_open(const char* uri, struct libos_encrypted_volume* volume,
@@ -568,6 +593,7 @@ void encrypted_file_destroy(struct libos_encrypted_file* enc) {
     assert(!enc->pf);
     assert(!enc->pal_handle);
     free(enc->uri);
+    // do _not_ free enc->norm_path as this is still used in file_state_map!
     free(enc);
 }
 
@@ -679,32 +705,30 @@ int encrypted_file_rename(struct libos_encrypted_file* enc, const char* new_uri)
     assert(enc->pf);
 
     int ret;
-    char* new_normpath = NULL;
+    char* new_norm_path = NULL;
+    char* old_norm_path = enc->norm_path;
 
     char* new_uri_copy = strdup(new_uri);
     if (!new_uri_copy)
         return -ENOMEM;
 
-    assert(strstartswith(enc->uri, URI_PREFIX_FILE));
-    const char* old_path = enc->uri + static_strlen(URI_PREFIX_FILE);
-
     assert(strstartswith(new_uri, URI_PREFIX_FILE));
     const char* new_path = new_uri + static_strlen(URI_PREFIX_FILE);
 
-    size_t new_normpath_size = strlen(new_path) + 1;
-    new_normpath = malloc(new_normpath_size);
-    if (!new_normpath) {
+    size_t new_norm_path_size = strlen(new_path) + 1;
+    new_norm_path             = malloc(new_norm_path_size);
+    if (!new_norm_path) {
         ret = -ENOMEM;
         goto out;
     }
 
-    if (!get_norm_path(new_path, new_normpath, &new_normpath_size)) {
+    if (!get_norm_path(new_path, new_norm_path, &new_norm_path_size)) {
         ret = -EINVAL;
         goto out;
     }
 
     pf_mac_t new_root_gmac;
-    pf_status_t pfs = pf_rename(enc->pf, new_normpath, &new_root_gmac);
+    pf_status_t pfs = pf_rename(enc->pf, new_norm_path, &new_root_gmac);
     if (PF_FAILURE(pfs)) {
         log_warning("pf_rename failed: %s", pf_strerror(pfs));
         ret = -EACCES;
@@ -716,33 +740,65 @@ int encrypted_file_rename(struct libos_encrypted_file* enc, const char* new_uri)
         log_warning("PalStreamChangeName failed: %s", pal_strerror(ret));
 
         /* We failed to rename the file. Try to restore the name in header. */
-        pfs = pf_rename(enc->pf, old_path, &new_root_gmac);
+        pfs = pf_rename(enc->pf, old_norm_path, &new_root_gmac);
         if (PF_FAILURE(pfs)) {
             log_warning("pf_rename (during cleanup) failed, the file might be unusable: %s",
                         pf_strerror(pfs));
         }
-
+        old_norm_path = NULL;  // don't free it later ...
         ret = pal_to_unix_errno(ret);
         goto out;
     }
-    // TODO (MST): everything worked fine, so
-    // - get state for old_path
-    //   abort if it doesn't exist and we are in strict protection_mode
-    // - add new_path together with new_root_gmac and copy state from old_path entry (should be
-    //   either PF_FILE_IN_USE_EXISTING, PF_FILE_IN_USE_NEW or PF_FILE_CLOSED)
-    // - remove old_path from map & set its state to PF_FILE_DELETED.
+    log_debug("file '%s' renamed to '%s' with MAC=" MAC_PRINTF_PATTERN, old_norm_path,
+              new_norm_path,
+              MAC_PRINTF_ARGS(new_root_gmac));  // TODO (MST): remove me eventually?
+    lock(&(enc->volume->files_state_map_lock));
+    struct libos_encrypted_volume_state_map* old_file_state = NULL;
+    HASH_FIND_STR(enc->volume->files_state_map, old_norm_path, old_file_state);
+    assert(old_file_state != NULL);
+    struct libos_encrypted_volume_state_map* new_file_state = NULL;
+    HASH_FIND_STR(enc->volume->files_state_map, new_norm_path, new_file_state);
+    if (new_file_state == NULL) {
+        new_file_state = malloc(sizeof(struct libos_encrypted_volume_state_map));
+        if (new_file_state == NULL) {
+            ret = -ENOMEM;
+            goto out;
+        }
+        new_file_state->norm_path = new_norm_path;
+        HASH_ADD_KEYPTR(hh, enc->volume->files_state_map, new_file_state->norm_path,
+                        strlen(new_file_state->norm_path), new_file_state);
+    } else {
+        free(new_file_state->norm_path);  // should be same but free old one to simplify below
+        new_file_state->norm_path = new_norm_path;
+    }
+    new_file_state->state = old_file_state->state;
+    memcpy(new_file_state->last_seen_root_gmac, new_root_gmac, sizeof(pf_mac_t));
+    old_file_state->state = PF_FILE_DELETED;
+    memset(old_file_state->last_seen_root_gmac, 0, sizeof(pf_mac_t));
+    unlock(&(enc->volume->files_state_map_lock));
 
     free(enc->uri);
-    enc->uri = new_uri_copy;
+    enc->uri       = new_uri_copy;
     new_uri_copy = NULL;
+    enc->norm_path = new_norm_path;
+    new_norm_path  = NULL;
+
     ret = 0;
 
 out:
-    // TODO (MST): in case of error
-    // - set state for old_path to PF_FILE_ERROR (even when restore rename worked? or just update
-    //   hash in that case?) Also correspondingly set pf->file_status =
-    //   PF_STATUS_CORRUPTED?)
-    free(new_normpath);
+    if (ret) {
+        // store in file state map fact that we could not rename file properly
+        if (!locked(&(enc->volume->files_state_map_lock)))  // for OOM case from above!
+            lock(&(enc->volume->files_state_map_lock));
+        if (old_file_state == NULL)  // we might already have it!
+            HASH_FIND_STR(enc->volume->files_state_map, old_norm_path, old_file_state);
+        assert(old_file_state != NULL);
+        old_file_state->state = PF_FILE_ERROR;
+        pf_set_corrupted(enc->pf);
+        unlock(&(enc->volume->files_state_map_lock));
+    }
+    free(old_norm_path);
+    free(new_norm_path);
     free(new_uri_copy);
     return ret;
 }
