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

Reliably mount input files #335

Closed
apyrgio opened this issue Feb 8, 2023 · 1 comment · Fixed by #336
Closed

Reliably mount input files #335

apyrgio opened this issue Feb 8, 2023 · 1 comment · Fixed by #336
Labels
bug Something isn't working container P:linux
Milestone

Comments

@apyrgio
Copy link
Contributor

apyrgio commented Feb 8, 2023

The Dangerzone container runs with the same UID/GID (--userns=keep-id) as the user that runs the Dangerzone application in the host. However, this does not mean that the container can always read the files that the user can, mainly for two reasons:

  1. A file may belong to a different group:

    The file may be accessible to the user outside the container solely because the user belongs to the same Unix group as the file. We already have error reports for that (Permission denied: container can't write to /dangerzone #157). The underlying cause for this error is that Podman does not pass supplemental group information to the container by default. For more info, read this Red Had article.

  2. An SELinux policy may block access:

    The file may have an SELinux label that does not match the container's. See this Red Hat article for more details. This issue has also been reported by our users (Permission denied: container can't write to /dangerzone #157 (comment)).

A solution to the above can be the following:

  • Copy the file into a temporary directory. The copied file will then have the same UID as the user in the Dangerzone container, which is what we want. The alternative approach, leaking the user groups in the container, is not desirable, because other groups (like sudoers) could also leak
  • Label the copied file with the proper SELinux label (:Z). It's important that we change the SELinux label in the copied file, else the original file would become inaccessible even from the host.
@apyrgio apyrgio added bug Something isn't working P:linux container labels Feb 8, 2023
@apyrgio apyrgio added this to the 0.4.1 milestone Feb 8, 2023
@apyrgio apyrgio changed the title Reliable mount input files Reliably mount input files Feb 8, 2023
@apyrgio
Copy link
Contributor Author

apyrgio commented Feb 8, 2023

I don't like it, but I have to live with the idea that this feature will not have CI tests, at least for now. There are two reasons for that:

  1. Testing case 1, i.e., file accessible only to a supplemental groups requires the following:

    • A second user that will own the file, and root permissions to change the owner.
    • A second group that will own the file, and adding the user that runs the tests to the new group. This must be done before the test process starts, so that it inherits this group.
    • Read permissions for the group, no permissions for others, and root permissions to do that.

    Implementing the above is not worth it, since we will soon merge WIP: Implement Linux User Namespaces support #248, which will remove the --userns=keep-id flag, and thus all this logic.

  2. Testing case 2, i.e., mismatching SELinux label, requires an SELinux capable host system, and probably root permissions (haven't tried it actually). Again, this complexity is not worth it for a :Z mount argument.

apyrgio added a commit that referenced this issue Feb 8, 2023
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
apyrgio added a commit that referenced this issue Feb 8, 2023
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 added a commit that referenced this issue Feb 8, 2023
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
apyrgio added a commit that referenced this issue Feb 8, 2023
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 added a commit that referenced this issue Feb 15, 2023
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
apyrgio added a commit that referenced this issue Feb 15, 2023
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 added a commit that referenced this issue Feb 15, 2023
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
apyrgio added a commit that referenced this issue Feb 15, 2023
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 added a commit that referenced this issue Feb 16, 2023
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working container P:linux
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant