Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Work around TOML Python parser bug of parsing \x #101

Merged
merged 1 commit into from
Oct 13, 2022

Conversation

dimakuv
Copy link

@dimakuv dimakuv commented Oct 11, 2022

Description of the changes

The currently used TOML parser in Python is buggy and doesn't allow to parse filenames like system-systemd\x2dcryptsetup.slice. Since the only files that we detected in real world are systemd helper files (which are not used by graminized apps anyway), the current workaround is to simply skip such files when collecting sgx.trusted_files.

The bug was already reported by someone else: uiri/toml#404.

The bug was found in #100.

Fixes #100.

How to test this PR?

Try the workload in #100.

Looks like this:

./gsc build --no-cache --insecure-args gcr.io/k8s-minikube/kicbase:v0.0.35 test/generic.manifest
...
Step 26/28 : RUN chmod u+x /gramine/app_files/apploader.sh     && /usr/bin/python3 -B /gramine/app_files/finalize_manifest.py     && rm -f /gramine/app_files/finalize_manifest.py

 ---> Running in 37fb48e07174
        [from inside Docker container] File /usr/lib/systemd/system/system-systemd\x2dcryptsetup.slice contains `\x` sequence and will be skipped from `sgx.trusted_files`!
        [from inside Docker container] Found 20250 files in `/`.
        [from inside Docker container] Successfully finalized `/gramine/app_files/entrypoint.manifest`.

This change is Reviewable

The currently used TOML parser in Python is buggy and doesn't allow to
parse filenames like `system-systemd\x2dcryptsetup.slice`. Since the
only files that we detected in real world are systemd helper files
(which are not used by graminized apps anyway), the current workaround
is to simply skip such files when collecting `sgx.trusted_files`.

Signed-off-by: Dmitrii Kuvaiskii <[email protected]>
@dimakuv dimakuv force-pushed the dimakuv/workaround-toml-backslash-bug branch from 7777f21 to af6d2f4 Compare October 11, 2022 14:00
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 r1, all commit messages.
Reviewable status: all 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)

a discussion (no related file):
Please create an issue about migration to tomli in the main repo (and maybe also here? or just mention gsc in the main repo).


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: all 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)

a discussion (no related file):

Previously, mkow (Michał Kowalczyk) wrote…

Please create an issue about migration to tomli in the main repo (and maybe also here? or just mention gsc in the main repo).

Done: gramineproject/gramine#968


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: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required)

Copy link

@boryspoplawski boryspoplawski 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 r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gsc sign-image errors out with toml: Reserved escape sequence used
3 participants