[common] Refactor Protected Files, part 4

This commit refactors PF code without changing functionality (part 4 in
a series of commits). In particular, this commit re-orders variables,
struct definitions and fields, typedefs, and macros for readability.

Signed-off-by: Dmitrii Kuvaiskii <[email protected]>

common/src/protected_files/protected_files.c

@@ -151,7 +151,7 @@ static bool ipf_import_metadata_key(pf_context_t* pf, bool restore, pf_key_t* ou
     kdf_input_t buf = {0};
     pf_status_t status;
 
-    buf.index = 1;
+    buf.counter = 1;
     if (!strcpy_static(buf.label, METADATA_KEY_NAME, MAX_LABEL_SIZE))
         return false;
 
@@ -560,7 +560,7 @@ static file_node_t* ipf_get_data_node(pf_context_t* pf, uint64_t offset) {
     }
 
     // even if we didn't get the required data_node, we might have read other nodes in the process
-    while (lruc_size(pf->cache) > MAX_PAGES_IN_CACHE) {
+    while (lruc_size(pf->cache) > MAX_NODES_IN_CACHE) {
         void* data = lruc_get_last(pf->cache);
         assert(data != NULL);
         // for production -

common/src/protected_files/protected_files.h

@@ -12,6 +12,8 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#define PF_NODE_SIZE 4096U
+
 /*! Size of the AES-GCM encryption key */
 #define PF_KEY_SIZE 16
 
@@ -21,10 +23,13 @@
 /*! Size of MAC fields */
 #define PF_MAC_SIZE 16
 
+/*! Size of the nonce used in KDF (Key Derivation Function) */
+#define PF_NONCE_SIZE 32
+
 typedef uint8_t pf_iv_t[PF_IV_SIZE];
 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 */
+typedef uint8_t pf_keyid_t[PF_NONCE_SIZE];
 
 typedef enum _pf_status_t {
     PF_STATUS_SUCCESS              = 0,
@@ -50,8 +55,6 @@ typedef enum _pf_status_t {
 #define PF_SUCCESS(status) ((status) == PF_STATUS_SUCCESS)
 #define PF_FAILURE(status) ((status) != PF_STATUS_SUCCESS)
 
-#define PF_NODE_SIZE 4096U
-
 /*! PF open modes */
 typedef enum _pf_file_mode_t {
     PF_FILE_MODE_READ  = 1,
@@ -61,6 +64,9 @@ typedef enum _pf_file_mode_t {
 /*! Opaque file handle type, interpreted by callbacks as necessary */
 typedef void* pf_handle_t;
 
+/*! Context representing an open protected file */
+typedef struct pf_context pf_context_t;
+
 /*!
  * \brief File read callback.
  *
@@ -192,9 +198,6 @@ void pf_set_callbacks(pf_read_f read_f, pf_write_f write_f, pf_fsync_f fsync_f,
                       pf_aes_gcm_decrypt_f aes_gcm_decrypt_f, pf_random_f random_f,
                       pf_debug_f debug_f);
 
-/*! Context representing an open protected file */
-typedef struct pf_context pf_context_t;
-
 /* Public API */
 
 /*!

common/src/protected_files/protected_files_format.h

@@ -6,6 +6,8 @@
 
 #pragma once
 
+#pragma pack(push, 1)
+
 #ifdef USE_STDLIB
 #include <assert.h>
 #else
@@ -22,10 +24,30 @@
 
 #define METADATA_KEY_NAME "SGX-PROTECTED-FS-METADATA-KEY"
 #define MAX_LABEL_SIZE    64
-
 static_assert(sizeof(METADATA_KEY_NAME) <= MAX_LABEL_SIZE, "label too long");
 
-#pragma pack(push, 1)
+#define PATH_MAX_SIZE (260 + 512)
+
+#define MD_USER_DATA_SIZE (PF_NODE_SIZE * 3 / 4)
+static_assert(MD_USER_DATA_SIZE == 3072, "bad struct size");
+
+#define MAX_NODES_IN_CACHE 48
+
+enum {
+    FILE_MHT_NODE_TYPE  = 1,
+    FILE_DATA_NODE_TYPE = 2,
+};
+
+typedef struct _data_node_crypto {
+    pf_key_t key;
+    pf_mac_t gmac;
+} gcm_crypto_data_t;
+
+// for PF_NODE_SIZE == 4096, we have 96 attached data nodes and 32 mht child nodes
+#define ATTACHED_DATA_NODES_COUNT ((PF_NODE_SIZE / sizeof(gcm_crypto_data_t)) * 3 / 4)
+#define CHILD_MHT_NODES_COUNT ((PF_NODE_SIZE / sizeof(gcm_crypto_data_t)) * 1 / 4)
+static_assert(ATTACHED_DATA_NODES_COUNT == 96, "ATTACHED_DATA_NODES_COUNT");
+static_assert(CHILD_MHT_NODES_COUNT == 32, "CHILD_MHT_NODES_COUNT");
 
 typedef struct _metadata_plain {
     uint64_t   file_id;
@@ -35,13 +57,6 @@ typedef struct _metadata_plain {
     pf_mac_t   metadata_gmac; /* GCM mac */
 } metadata_plain_t;
 
-#define PATH_MAX_SIZE (260 + 512)
-
-// these are all defined as relative to node size, so we can decrease node size in tests
-// and have deeper tree
-#define MD_USER_DATA_SIZE (PF_NODE_SIZE * 3 / 4) // 3072
-static_assert(MD_USER_DATA_SIZE == 3072, "bad struct size");
-
 typedef struct _metadata_encrypted {
     char     path[PATH_MAX_SIZE];
     uint64_t size;
@@ -60,74 +75,50 @@ typedef struct _metadata_node {
     metadata_encrypted_blob_t encrypted_part;
     metadata_padding_t        padding;
 } metadata_node_t;
-
 static_assert(sizeof(metadata_node_t) == PF_NODE_SIZE, "sizeof(metadata_node_t)");
 
-typedef struct _data_node_crypto {
-    pf_key_t key;
-    pf_mac_t gmac;
-} gcm_crypto_data_t;
-
-// for PF_NODE_SIZE == 4096, we have 96 attached data nodes and 32 mht child nodes
-// for PF_NODE_SIZE == 2048, we have 48 attached data nodes and 16 mht child nodes
-// for PF_NODE_SIZE == 1024, we have 24 attached data nodes and 8 mht child nodes
-// 3/4 of the node size is dedicated to data nodes
-#define ATTACHED_DATA_NODES_COUNT ((PF_NODE_SIZE / sizeof(gcm_crypto_data_t)) * 3 / 4)
-static_assert(ATTACHED_DATA_NODES_COUNT == 96, "ATTACHED_DATA_NODES_COUNT");
-// 1/4 of the node size is dedicated to child mht nodes
-#define CHILD_MHT_NODES_COUNT ((PF_NODE_SIZE / sizeof(gcm_crypto_data_t)) * 1 / 4)
-static_assert(CHILD_MHT_NODES_COUNT == 32, "CHILD_MHT_NODES_COUNT");
-
 typedef struct _mht_node {
     gcm_crypto_data_t data_nodes_crypto[ATTACHED_DATA_NODES_COUNT];
     gcm_crypto_data_t mht_nodes_crypto[CHILD_MHT_NODES_COUNT];
 } mht_node_t;
-
 static_assert(sizeof(mht_node_t) == PF_NODE_SIZE, "sizeof(mht_node_t)");
 
 typedef struct _data_node {
     uint8_t data[PF_NODE_SIZE];
 } data_node_t;
-
 static_assert(sizeof(data_node_t) == PF_NODE_SIZE, "sizeof(data_node_t)");
 
 typedef struct _encrypted_node {
     uint8_t cipher[PF_NODE_SIZE];
 } encrypted_node_t;
-
 static_assert(sizeof(encrypted_node_t) == PF_NODE_SIZE, "sizeof(encrypted_node_t)");
 
-#define MAX_PAGES_IN_CACHE 48
-
-enum {
-    FILE_MHT_NODE_TYPE  = 1,
-    FILE_DATA_NODE_TYPE = 2,
-};
-
-// make sure these are the same size
-static_assert(sizeof(mht_node_t) == sizeof(data_node_t),
-              "sizeof(mht_node_t) == sizeof(data_node_t)");
+static_assert(sizeof(mht_node_t) == sizeof(data_node_t), "sizes of MHT and data nodes differ");
 
+// Data struct that wraps the 4KB encrypted-node buffer (bounce buffer) and the corresponding 4KB
+// decrypted-data buffer (plain buffer), plus additional fields. This data struct is used for both
+// Data and MHT nodes (but not for Metadata node).
 typedef struct _file_node {
     uint8_t type;
-    uint64_t node_number;
-    struct _file_node* parent;
     bool need_writing;
-    struct {
-        uint64_t physical_node_number;
-        encrypted_node_t encrypted; // the actual data from the disk
-    };
-    union { // decrypted data
+    struct _file_node* parent;
+
+    uint64_t node_number;
+    uint64_t physical_node_number;
+
+    encrypted_node_t encrypted; // encrypted data from storage (bounce buffer)
+    union {                     // decrypted data, supposed to be stored in private memory
         mht_node_t mht;
         data_node_t data;
     } decrypted;
 } file_node_t;
 
+// input materials for the KDF construction of NIST-SP800-108
 typedef struct {
-    uint32_t index;
+    uint32_t counter;           // always "1"
     char label[MAX_LABEL_SIZE]; // must be NULL terminated
-    pf_keyid_t nonce;
-    uint32_t output_len; // in bits
+    pf_keyid_t nonce;           // nonce for key derivation from KDK, stored in metadata node
+    uint32_t output_len;        // in bits; always 128
 } kdf_input_t;
 
 #pragma pack(pop)

common/src/protected_files/protected_files_internal.h

@@ -13,16 +13,21 @@
 #include "protected_files_format.h"
 
 struct pf_context {
-    metadata_node_t file_metadata; // actual data from disk's meta data node
-    pf_status_t last_error;
-    metadata_encrypted_t encrypted_part_plain; // encrypted part of metadata node, decrypted
-    file_node_t root_mht; // the root of the mht is always needed (for files bigger than 3KB)
-    pf_handle_t file;
-    pf_file_mode_t mode;
-    bool need_writing;
-    pf_status_t file_status;
-    pf_key_t user_kdk_key;
-    lruc_context_t* cache;
+    pf_handle_t file; // opaque file handle (e.g. PAL handle) used by callbacks
+    pf_file_mode_t mode; // read-only, write-only or read-write
+    bool need_writing; // whether file was modified and thus needs writing to storage
+
+    pf_status_t file_status; // PF_STATUS_SUCCESS, PF_STATUS_CRYPTO_ERROR, etc.
+    pf_status_t last_error;  // FIXME: unclear why this is needed
+
+    pf_key_t user_kdk_key; // KDK installed by user of PF (e.g. from Gramine manifest)
+
+    metadata_node_t file_metadata; // plaintext and encrypted metadata from storage (bounce buffer)
+    metadata_encrypted_t encrypted_part_plain; // contains file path, size, etc.
+
+    file_node_t root_mht; // root MHT node is needed for files bigger than 3KB
+
+    lruc_context_t* cache; // up to MAX_NODES_IN_CACHE nodes are cached for each file
 #ifdef DEBUG
     char* debug_buffer; // buffer for debug output
 #endif

tools/sgx/pf_tamper/pf_tamper.c

@@ -62,7 +62,7 @@ static void derive_main_key(const pf_key_t* kdk, const pf_keyid_t* key_id, pf_ke
     kdf_input_t buf = {0};
     pf_status_t status;
 
-    buf.index = 1;
+    buf.counter = 1;
     strncpy(buf.label, METADATA_KEY_NAME, MAX_LABEL_SIZE);
     COPY_ARRAY(buf.nonce, *key_id);
     buf.output_len = 0x80;