Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

[GSC] Allow in-kernel Intel SGX driver; improve GSC testing #2165

Merged
merged 1 commit into from
Feb 23, 2021

Conversation

dimakuv
Copy link

@dimakuv dimakuv commented Feb 17, 2021

Description of the changes

Previously, GSC required to use an out-of-tree SGX driver (either the legacy one or the DCAP one) but could not support the in-kernel SGX driver (merged into Linux 5.11). This PR allows to specify the in-kernel SGX driver in the configuration file config.yaml for GSC.

This PR also fixes a couple tiny bugs in the testing Makefile.

How to test this PR?

GSC in Jenkins must succeed. Manual testing on Linux 5.11 with the in-kernel SGX driver is in progress.


This change is Reviewable

Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 4 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: Intel) (waiting on @dimakuv)


Tools/gsc/templates/entrypoint.manifest.template, line 29 at r1 (raw file):

# TODO: provide a switch `--insecure_envs` similar to `--insecure_args` above.
# The issue is documented at https://github.com/oscarlab/graphene/issues/1520.
loader.insecure__use_host_env = 1

Hmm, do I understand correctly, that merging this PR will make GSC temporarily insecure? If that's the case, then I'd rather wait for this fix and squash it here.

Copy link
Author

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 4 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: Intel) (waiting on @mkow)


Tools/gsc/templates/entrypoint.manifest.template, line 29 at r1 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

Hmm, do I understand correctly, that merging this PR will make GSC temporarily insecure? If that's the case, then I'd rather wait for this fix and squash it here.

OK, let me revert the second commit then. Your understanding is correct, it makes GSC temporarily insecure (due to forwarded env vars).

Copy link
Author

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 3 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: Intel) (waiting on @mkow)


Tools/gsc/templates/entrypoint.manifest.template, line 29 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

OK, let me revert the second commit then. Your understanding is correct, it makes GSC temporarily insecure (due to forwarded env vars).

Done.

mkow
mkow previously approved these changes Feb 18, 2021
Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3 of 4 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required)

Copy link
Author

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 3 of 4 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), "fixup! " found in commit messages' one-liners


Tools/gsc/finalize_manifest.py, line 35 at r3 (raw file):

                            f'|^{cwd}/('
                                r'.*\.manifest'
                                r'|finalize_manifest\.py)$')

FYI: This is simply not needed anymore, now that we always use the absolute paths for GSC-related files.


Tools/gsc/finalize_manifest.py, line 88 at r3 (raw file):

    env.globals.update({'library_paths': generate_library_paths(), 'env_path': os.getenv('PATH')})

    manifest = '/entrypoint.manifest'

FYI: I missed to change this to absolute path (from the incorrect relative path). This should have gone into already-merged PR #2151, but I failed to notice that. So I added it in this PR.

mkow
mkow previously approved these changes Feb 19, 2021
Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@dimakuv dimakuv force-pushed the dimakuv/gsc-allow-inkernel-sgx-driver branch from 5562abd to a134193 Compare February 23, 2021 07:37
mkow
mkow previously approved these changes Feb 23, 2021
Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Previously, GSC required to use an out-of-tree SGX driver (either the
legacy one or the DCAP one) but could not support the in-kernel SGX
driver (merged into Linux 5.11). This commit allows to specify the
in-kernel SGX driver in the configuration file config.yaml for GSC. This
commit also fixes a couple tiny bugs in the testing Makefile.

Signed-off-by: Dmitrii Kuvaiskii <[email protected]>
@mkow mkow force-pushed the dimakuv/gsc-allow-inkernel-sgx-driver branch from a134193 to 544a775 Compare February 23, 2021 11:36
Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@mkow
Copy link
Member

mkow commented Feb 23, 2021

Jenkins, retest Jenkins please (openssl failed with unable to write 'random state', disk full maybe?)

@mkow
Copy link
Member

mkow commented Feb 23, 2021

Jenkins, retest Jenkins please (apps.LTP.kill11 failed, known issue)

@dimakuv dimakuv merged commit 544a775 into master Feb 23, 2021
@dimakuv dimakuv deleted the dimakuv/gsc-allow-inkernel-sgx-driver branch February 23, 2021 16:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants