[LibOS] Support special keys (MRENCLAVE, MRSIGNER) in encrypted fs

[pwmarcz]

Description of the changes

Key names beginning with _ are considered special, and PAL is queried for these keys. This is used for SGX MRENCLAVE and MRSIGNER keys.

If a key is not supported by the current PAL, mounting will still succeed (but files will not be readable). This way, manifests using these PAL-host-specific keys can still be loaded when using other PAL hosts.

(This design is as discussed in #371).

How to test this PR?

The existing tests for MRENCLAVE and MRSIGNER keys now also check the new implementation.

---

This change is 

[dimakuv]

Reviewed 11 of 11 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL), "fixup! " found in commit messages' one-liners (waiting on @pwmarcz)

---

LibOS/shim/src/fs/shim_fs_encrypted.c, line 390 at r1 (raw file):

        } else if (ret == -PAL_ERROR_NOTIMPLEMENTED) {
            log_warning("Special key \"%s\" is not supported by current PAL. Mounts using this key "
                        "will not work.", name);

I think this doesn't deserve a warning level. Maybe log_debug? Otherwise it will be rather annoying for users.

---

Pal/include/pal/pal.h, line 764 at r1 (raw file):

 * remote attestation and secret provisioning, before the user application starts.
 */
int DkSetProtectedFilesKey(const char* pf_key_hex);

Can you add either here, or somewhere in GitHub (your meta-issue?) that this is deprecated and should be removed as part of the "move away from PFs in PAL", and that it should be removed in Gramine v1.4.

Or maybe I forgot how you planned to support the "old manifest syntax"? Did you plan to emulate the old manifest syntax with your new LibOS implementation, or did you plan for these two to live together until v1.4?

---

Pal/src/host/Linux-SGX/db_misc.c, line 632 at r1 (raw file):

    sgx_key_128bit_t sgx_key;

    if (*key_size < sizeof(key))

Wait what? Shouldn't it be sizeof(sgx_key)?

[pwmarcz]

Reviewable status: 9 of 11 files reviewed, 3 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL), "fixup! " found in commit messages' one-liners (waiting on @dimakuv)

---

LibOS/shim/src/fs/shim_fs_encrypted.c, line 390 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

I think this doesn't deserve a warning level. Maybe log_debug? Otherwise it will be rather annoying for users.

Yeah, on some manifests this message will appear every time you're running gramine-direct, that's annoying. Changed to log_debug.

---

Pal/include/pal/pal.h, line 764 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Can you add either here, or somewhere in GitHub (your meta-issue?) that this is deprecated and should be removed as part of the "move away from PFs in PAL", and that it should be removed in Gramine v1.4.

Or maybe I forgot how you planned to support the "old manifest syntax"? Did you plan to emulate the old manifest syntax with your new LibOS implementation, or did you plan for these two to live together until v1.4?

Actually, no, the plan is to remove this soon, and just support the old syntax (sgx.protected_files) in the new feature.

That's quite a lot of code to remove, so I don't want to add TODOs everywhere. I added a comment in the issue: #371 (comment)

---

Pal/src/host/Linux-SGX/db_misc.c, line 632 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Wait what? Shouldn't it be sizeof(sgx_key)?

My mistake. Fixed.

[dimakuv]

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required), "fixup! " found in commit messages' one-liners

[dimakuv]

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved