[PAL] Disable attestation flows in child enclaves
Signed-off-by: Liang Ma [email protected]
Description of the changes
Only the initial process should be possible to attest remotely (otherwise various security properties break) - we need to disable doing this in children.
Reviewed 3 of 3 files at r1, all commit messages. Reviewable status: all files reviewed, 6 unresolved discussions, not enough approvals from maintainers (3 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @billionairiam)
a discussion (no related file): Generally looks correct.
But now I wonder -- do we really want this restriction? Imagine that we have a Bash script that becomes the first process, and all this Bash script does is to set up some envvars and then calls the real workload. E.g., this is the case for the Redis Docker image.
With this PR, Redis (which is the child enclave) won't be able to perform remote attestation. And asking Bash to perform remote attestation is ridiculous... @mkow @boryspoplawski I think this was the idea from ITL, could you comment on this?
-- commitsline 2 at r1: I suggest:[PAL] Disable attestation flows in child enclaves
Documentation/attestation.rstline 25 at r1 (raw file):In many cases, the user also wants :term:`Secret Provisioning` to transparently provision secret keys and other sensitive data to the remote TEE.I would move the added sentence to a separate paragraph and mark it as a
note. So like this:.. note:: In Gramine, only the initial process can perform remote attestation; child processes have no access to remote-attestation flows.
pal/src/host/linux-sgx/host_main.cline 957 at r1 (raw file):sgx_target_info_t qe_targetinfo = {0}; if (enclave->attestation_type != SGX_ATTESTATION_NONE && parent_stream_fd < 0) { /* initialize communication with Quoting Enclave only if app requests attestation */Please append the following in this comment:
... and this is the first process (child processes are forbidden to perform attestation)Don't forget to re-wrap the comment to 100-chars-per-line limit.
pal/src/host/linux-sgx/pal_main.cline 631 at r1 (raw file):} enum sgx_attestation_type attestation_type=SGX_ATTESTATION_NONE;Please add spaces around
=
pal/src/host/linux-sgx/pal_main.cline 632 at r1 (raw file):enum sgx_attestation_type attestation_type=SGX_ATTESTATION_NONE; if(parent_stream_fd < 0){Please add a comment before
ret = parse_attestation_type(...):/* allow remote attestation only if this is the first process (child processes are forbidden * to perform remote attestation) */Don't forget to re-wrap the comment to 100-chars-per-line limit.
Well, Just now my colleagues discussed these issues carefully, they think that the current implementation is no problem, so there is no need to commit this PR. I will send you a letter to illustrate our discussion details.
I'm currently closing this PR as this requirement doesn't seem to be relevant anymore.