[common] Refactor Protected Files, part 5

[dimakuv]

Description of the changes

This commit refactors PF code without changing functionality (part 5 in a series of commits). In particular, this commit renames variables, struct definitions, struct fields and functions for readability. This is the last commit in this series.

Closes #1841.

Note that this PR is based on #1869.

How to test this PR?

N/A

---

This change is 

[dimakuv]

Reviewable status: 0 of 5 files reviewed, all discussions resolved, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel)

---

common/src/protected_files/protected_files.c line 188 at r1 (raw file):

}

static bool ipf_generate_nonced_metadata_key(pf_context_t* pf, pf_key_t* output) {

FYI: I'm not particularly happy with this name (the "nonced" part, which should be read as "generate the key dervied from the nonce")

[woju]

Jenkins, test this please

[woju]

Jenkins, test this please

[woju]

Jenkins, test this please

[woju]

Jenkins, test this please

[woju]

Jenkins, test this please

[mkow]

Reviewed 3 of 5 files at r1, all commit messages.
Reviewable status: 3 of 5 files reviewed, 6 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)

---

common/src/protected_files/protected_files_format.h line 63 at r1 (raw file):

typedef struct _metadata_decrypted_header {
    char     file_path[PATH_MAX_SIZE];
    uint64_t file_size;

I assume this is the size after decryption? Not the encrypted file (storing which wouldn't make much sense).
Could you add a short comment here clarifying that?

---

common/src/protected_files/protected_files_internal.h line 28 at r1 (raw file):

    metadata_decrypted_header_t metadata_decrypted_header; // contains file path, size, etc.

    file_node_t root_mht_node;     // root MHT node is always needed (for files bigger than 3KB)

Please undo the comment change (mistake when rebasing?)

Code quote:

// root MHT node is always needed (for files bigger than 3KB)

---

tools/sgx/pf_tamper/pf_tamper.c line 83 at r1 (raw file):

 * - Node 0: metadata (metadata_node_t)
 *   - metadata_plaintext_header_t
 *   - metadata_decrypted_header_t (may include MD_USER_DATA_SIZE bytes of data)

I'd mark somehow that it's not that struct which is in the file, it's the encryption of it.

Suggestion:

enc(metadata_decrypted_header_t) (may include MD_USER_DATA_SIZE bytes of data)

---

tools/sgx/pf_tamper/pf_tamper.c line 131 at r1 (raw file):

    DBG("metadata_plaintext_header_t.metadata_key_nonce     : 0x%04lx (0x%04lx)\n",
        offsetof(metadata_plaintext_header_t, metadata_key_nonce),
        FIELD_SIZEOF(metadata_plaintext_header_t, metadata_key_nonce));

copy-paste fail? what happened to mac?

---

tools/sgx/pf_tamper/pf_tamper.c line 164 at r1 (raw file):

    truncate_file("trunc_meta_plain_5", FIELD_TRUNCATED(metadata_plaintext_header_t,
                metadata_key_nonce));
    truncate_file("trunc_meta_plain_6", offsetof(metadata_plaintext_header_t, metadata_key_nonce));

another copy-paste fail?

---

tools/sgx/pf_tamper/pf_tamper.c line 164 at r1 (raw file):

    truncate_file("trunc_meta_plain_5", FIELD_TRUNCATED(metadata_plaintext_header_t,
                metadata_key_nonce));
    truncate_file("trunc_meta_plain_6", offsetof(metadata_plaintext_header_t, metadata_key_nonce));

This PR is super-tedious to review and you've made multiple copy-paste/find-and-replace errors :/ Would it be possible to split it further into smaller PRs, with less renames at the same time? It would clean up the diff a bit. Finding all those errors manually will take hours :/

[dimakuv]

Reviewable status: 3 of 5 files reviewed, 6 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow)

---

common/src/protected_files/protected_files_format.h line 63 at r1 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

I assume this is the size after decryption? Not the encrypted file (storing which wouldn't make much sense).
Could you add a short comment here clarifying that?

Done, in a new PR

---

common/src/protected_files/protected_files_internal.h line 28 at r1 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

Please undo the comment change (mistake when rebasing?)

Done, in a new PR

---

tools/sgx/pf_tamper/pf_tamper.c line 83 at r1 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

I'd mark somehow that it's not that struct which is in the file, it's the encryption of it.

Will be done later

---

tools/sgx/pf_tamper/pf_tamper.c line 131 at r1 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

copy-paste fail? what happened to mac?

Yeah, copy-paste fail

---

tools/sgx/pf_tamper/pf_tamper.c line 164 at r1 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

another copy-paste fail?

Yeah

---

tools/sgx/pf_tamper/pf_tamper.c line 164 at r1 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

This PR is super-tedious to review and you've made multiple copy-paste/find-and-replace errors :/ Would it be possible to split it further into smaller PRs, with less renames at the same time? It would clean up the diff a bit. Finding all those errors manually will take hours :/

Done: #1899

I will split the rest of the changes (that are all accumulated in this super-tedious PR) in another 3-4 smaller PRs. The first one in the rest of the series is #1899.

In the next PRs, I plan to introduce renames one at a time (well, I will group those that are easy to review together).

[dimakuv]

Superseded by #1899