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

Improve temporary directory handling #336

Merged
merged 6 commits into from
Feb 16, 2023
Merged

Improve temporary directory handling #336

merged 6 commits into from
Feb 16, 2023

Conversation

apyrgio
Copy link
Contributor

@apyrgio apyrgio commented Feb 8, 2023

This PR fixes several issues that have to do with, or are tangent to, temporary directory handling in Dangerzone. Briefly, this PR fixes the following issues:

  • Stale artifacts remaining after the conversion.
  • Reliably mounting a file in a container.

Fixes #157
Fixes #260
Fixes #317
Fixes #335

dangerzone/cli.py Outdated Show resolved Hide resolved
dangerzone/isolation_provider/container.py Outdated Show resolved Hide resolved
@apyrgio apyrgio force-pushed the 335-mount branch 2 times, most recently from ddf9f5a to 303749d Compare February 15, 2023 21:41
@apyrgio
Copy link
Contributor Author

apyrgio commented Feb 15, 2023

I have updated this branch with the changes we discussed. The commits that are of more interest are the following:

If you're ok with those, I can squash them and merge.

Copy link
Contributor

@deeplow deeplow left a comment

Choose a reason for hiding this comment

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

Only a minor comment. Other than that the code LGTM.

dangerzone/util.py Outdated Show resolved Hide resolved
Do not store temporary directories in the Dangerzone's config directory.
There are two reasons for that:

1. They are ephemeral, and they need a temporary place to be stored,
   preferably RAM-backed.
2. We need to set them while running our CI tests.
Run each CLI command in a separate config/cache dir, to avoid leaks
between tests. Moreover, this way we are able to check the contents of
the config/cache dirs for a single CLI run.
Do not leave stale temporary directories when conversion fails
unexpectedly. Instead, wrap the conversion operation in a context
manager that wipes the temporary dir afterwards.

Fixes #317
Take SELinux labels into account when mounting a file to the Dangerzone
container. Use the `:Z` flag (which is a no-op in non-SELinux systems)
to clear the existing SELinux label for a file, and apply one that
matches the container's.

Refs #335
Copy input files in a temporary dir before mounting them, thereby
changing their permissions, without affecting the original files. This
way, we can avoid cases where a file is accessible to the user only due
to a supplemental user group, which does not work for containers.

Fixes #157
Fixes #260
Fixes #335
@apyrgio apyrgio merged commit b94d071 into main Feb 16, 2023
@deeplow deeplow added this to the 0.4.1 milestone Feb 21, 2023
@deeplow deeplow deleted the 335-mount branch May 11, 2023 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants