Do gramine-sgx-get-token automatically

[woju]

Description of the changes

There's no EINITTOKEN on upstream driver (nor DCAP), and most people use that driver (it is/will be the only packaged flavour). It's bad UX to require them to do gramine-sgx-get-token for nothing (like, 384 * \0).

This PR is the first of series of two. It changes UX (gramine-sgx-get-token gets called automatically), but doesn't change behaviour of loader (the .token file is still required).

How to test this PR?

Run gramine-sgx without doing gramine-sgx-get-token first.

---

This change is 

[oshogbo]

Reviewable status: 0 of 17 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @woju)

a discussion (no related file):
LGTM.

---

---

Documentation/manpages/gramine-sgx-get-token.rst line 15 at r1 (raw file):

Using this command is not necessary (it was previously), since the token is
fetched automatically if needed during first enclave start.

If this command is not to be used by user - maybe we can just remove this man page?
Or maybe we should add some note to remove it after 2 version.

[dimakuv]

Reviewed 17 of 17 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @woju)

a discussion (no related file):
Same should be done for:

• https://github.com/gramineproject/examples
• https://github.com/gramineproject/gsc

But only after this PR is merged and appears in the release of Gramine.

@woju Do you want to do these PRs, or should we just create corresponding GitHub issues, or should somebody else (=me?) create such PRs?

---

a discussion (no related file):
Jenkins failed.

---

---

Documentation/manpages/gramine.rst line 30 at r1 (raw file):

   If not empty, :command:`gramine-sgx` will not automatically generate
   EINITTOKEN (for out-of-tree driver), or its dummy counterpart (for upstream
   and DCAP drivers).

Why do we introduce such a variable? I guess it doesn't hurt so I'm not blocking. But also, I don't see much need for it.

---

Documentation/manpages/gramine-sgx-get-token.rst line 15 at r1 (raw file):

Previously, oshogbo (Mariusz Zaborski) wrote…

If this command is not to be used by user - maybe we can just remove this man page?
Or maybe we should add some note to remove it after 2 version.

I wouldn't remove this man page and I wouldn't remove this command. It may be useful for users who still have non-FLC SGX machines (= they need the token), and these users may want to use this script explicitly in their flows, for whatever reason.

[woju]

Reviewable status: 16 of 17 files reviewed, 4 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @dimakuv and @oshogbo)

a discussion (no related file):

Previously, oshogbo (Mariusz Zaborski) wrote…

LGTM.

so plz unblock :)

---

a discussion (no related file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Jenkins failed.

Yeah, for stdout. Should be fixed.

---

a discussion (no related file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Same should be done for:

• https://github.com/gramineproject/examples
• https://github.com/gramineproject/gsc

But only after this PR is merged and appears in the release of Gramine.

@woju Do you want to do these PRs, or should we just create corresponding GitHub issues, or should somebody else (=me?) create such PRs?

I'd prefer to make separate issues and fix when convenient. Those changes should be compatible, so if someone (gsc) is still calling gramine-sgx-get-token, it shouldn't break anything, so it's not that urgent to fix.

---

---

Documentation/manpages/gramine.rst line 30 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Why do we introduce such a variable? I guess it doesn't hurt so I'm not blocking. But also, I don't see much need for it.

As a rule, you should be able to undo any automation. For instance, automatic get-token adds latency (on the order of 2000 ms) to starting enclave (or alternatively it adds variance to enclave start time, depending on how you view it). So if someone would be actually measuring start times, I'd expect s/he'll get-token separately and set this variable as a safety device to make sure s/he's measuring what s/he meant, or fail hard.

---

Documentation/manpages/gramine-sgx-get-token.rst line 15 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

I wouldn't remove this man page and I wouldn't remove this command. It may be useful for users who still have non-FLC SGX machines (= they need the token), and these users may want to use this script explicitly in their flows, for whatever reason.

As long as the command is available, the manpage should remain.

[dimakuv]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @oshogbo)

a discussion (no related file):

Previously, woju (Wojtek Porczyk) wrote…

I'd prefer to make separate issues and fix when convenient. Those changes should be compatible, so if someone (gsc) is still calling gramine-sgx-get-token, it shouldn't break anything, so it's not that urgent to fix.

Done:

• Remove gramine-sgx-get-token Makefile target examples#46
• Remove gramine-sgx-get-token Makefile target gsc#119

---

[woju]

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Done:

• Remove gramine-sgx-get-token Makefile target examples#46
• Remove gramine-sgx-get-token Makefile target gsc#119

Thanks!

[mkow]

Reviewed 16 of 17 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @woju)

a discussion (no related file):

But only after this PR is merged and appears in the release of Gramine.

Isn't GSC master using Gramine master? We have GSC tagging for some reason :)

---

---

Documentation/manpages/gramine.rst line 30 at r1 (raw file):

Previously, woju (Wojtek Porczyk) wrote…

As a rule, you should be able to undo any automation. For instance, automatic get-token adds latency (on the order of 2000 ms) to starting enclave (or alternatively it adds variance to enclave start time, depending on how you view it). So if someone would be actually measuring start times, I'd expect s/he'll get-token separately and set this variable as a safety device to make sure s/he's measuring what s/he meant, or fail hard.

Jeeez, why get-token takes 2 seconds? It's a local operation which doesn't require any networking, this sounds completely unreasonable.
// but this is unrelated to the PR, at least not directly

---

Documentation/manpages/gramine.rst line 29 at r2 (raw file):

   If not empty, :command:`gramine-sgx` will not automatically generate
   EINITTOKEN (for out-of-tree driver), or its dummy counterpart (for upstream

In official docs we need to be more precise, because AFAIR some people call non-upstreamed DCAP driver "out-of-tree driver" and the upstreamed one "in-tree driver".
@dimakuv: What's the best name to use here?

Suggestion:

for out-of-tree EPID driver

---

Documentation/manpages/gramine-sgx-get-token.rst line 15 at r2 (raw file):

Using this command is not necessary (it was previously), since the token is
fetched automatically if needed during first enclave start.

(I think)

Suggestion:

during the first enclave start.

[dimakuv]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @mkow and @woju)

a discussion (no related file):

But only after this PR is merged and appears in the release of Gramine.

Isn't GSC master using Gramine master? We have GSC tagging for some reason :)

I see no reason to break GSC like this. If we can avoid it (by postponing by a couple weeks), I prefer avoiding it.

But yes, technically no specific reason.

---

---

Documentation/manpages/gramine.rst line 30 at r1 (raw file):

Jeeez, why get-token takes 2 seconds? It's a local operation which doesn't require any networking, this sounds completely unreasonable.

Firstly, this happens only for non-FLC systems. Secondly, iirc, producing a token (EINITTOKEN) requires launching the special Launch Enclave (LE) -- so basically we need to start an additional SGX enclave to get the token. And of course starting and then destroying an SGX enclave is slow.

---

Documentation/manpages/gramine.rst line 29 at r2 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

In official docs we need to be more precise, because AFAIR some people call non-upstreamed DCAP driver "out-of-tree driver" and the upstreamed one "in-tree driver".
@dimakuv: What's the best name to use here?

@mkow Your proposal sounds good. Adding EPID makes it unambigious. We also call it old and deprecated in our docs: https://gramine.readthedocs.io/en/latest/sgx-intro.html#linux-kernel-drivers. I also refer to it as legacy.

Anyway, Michal's proposal is good enough.

[woju]

Reviewable status: 15 of 17 files reviewed, 2 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @dimakuv and @mkow)

---

Documentation/manpages/gramine.rst line 30 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Jeeez, why get-token takes 2 seconds? It's a local operation which doesn't require any networking, this sounds completely unreasonable.

Firstly, this happens only for non-FLC systems. Secondly, iirc, producing a token (EINITTOKEN) requires launching the special Launch Enclave (LE) -- so basically we need to start an additional SGX enclave to get the token. And of course starting and then destroying an SGX enclave is slow.

Up to half of that (500-1000 ms) is Python interpreter start, which does slow things (like reading a number of small files). I think there's draft PR that RIIR's gramine-sgx-get-token exactly for this reason. Recent Python releases brought some improvements in this area, but we're talking about old driver on old kernel, which means old distro with old Python interpreters, so those improvements aren't available if we have #!/usr/bin/python3 shebang.

---

Documentation/manpages/gramine.rst line 29 at r2 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

@mkow Your proposal sounds good. Adding EPID makes it unambigious. We also call it old and deprecated in our docs: https://gramine.readthedocs.io/en/latest/sgx-intro.html#linux-kernel-drivers. I also refer to it as legacy.

Anyway, Michal's proposal is good enough.

Done. And I also need to fix this in #1094, because the note about "dummy counterpart" is not right.

---

Documentation/manpages/gramine-sgx-get-token.rst line 15 at r2 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

(I think)

Done.

[dimakuv]

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @mkow and @woju)

a discussion (no related file):
Jenkins-direct-sanitizers failed.

---

[woju]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dimakuv and @mkow)

a discussion (no related file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Jenkins-direct-sanitizers failed.

Jenkins slave is heavily swapped, I'll fix it and rerun pipeline.

---

[woju]

Jenkins, test Jenkins-Direct-Sanitizers please

[mkow]

Reviewed 2 of 2 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @dimakuv and @woju)

a discussion (no related file):
Note to myself: review commit split before merging.

---

[woju]

Jenkins, test Jenkins-SGX-18.04-apps please

[woju]

Jenkins, test Jenkins-SGX-20.04-apps please

[dimakuv]

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @woju)

[mkow]

Reviewable status: complete! all files reviewed, all discussions resolved