session-helper: Relax directory permission for Toolbox containers #2916
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Toolbox [1] containers, like Flatpak containers, need to keep some configuration files synchronized with the host. eg., /etc/localtime, /etc/resolv.conf, etc.. The easiest way to keep them updated is to bind mount the entire /etc from the host at some location inside the container, and then replace the container's copy of the files with symbolic links to the bind mounted copies. However, this doesn't work when the host's copy of a configuration file is itself a symbolic link to a location outside /etc. eg., /etc/localtime, which points somewhere inside /usr/share/zoneinfo. In that case the host needs to be actively monitored via inotify(7) and an updated copy of the file needs to maintained in a separate directory, which can be bind mounted into the container. This monitoring infrastructure is already present in Flatpak and it makes sense to leverage it for Toolbox containers too. However, Flatpak containers have only one user, the current one logged into the host, whereas Toolbox containers have a longer list of users like the host itself. Therefore, if the directory getting bind mounted (ie., $XDG_RUNTIME_DIR/.flatpak-helper/monitor) isn't world readable, then some users inside the container won't be able to read the configuration file. eg., this will fail: $ sudo -u operator cat /run/host/monitor/localtime Relaxing the directory permissions on the host shouldn't pose a security risk. They are copies of originals that are world readable anyway. [1] https://github.com/debarshiray/toolbox
|
Can one of the admins verify this patch?
|
debarshiray
added a commit
to containers/toolbox
that referenced
this pull request
May 22, 2019
The permission of $XDG_RUNTIME_DIR/.flatpak-helper/monitor needs to be relaxed [1] for this work in some corner cases. However, it already works for the vast majority of cases when used as $USER or root inside the toolbox container. [1] flatpak/flatpak#2916 https://github.com/debarshiray/toolbox/issues/70
|
Makes sense to me. @rh-atomic-bot r+ |
|
📌 Commit 38918be has been approved by |
|
☀️ Test successful - status-papr |
alexlarsson
pushed a commit
to alexlarsson/flatpak
that referenced
this pull request
Sep 19, 2019
Toolbox [1] containers, like Flatpak containers, need to keep some configuration files synchronized with the host. eg., /etc/localtime, /etc/resolv.conf, etc.. The easiest way to keep them updated is to bind mount the entire /etc from the host at some location inside the container, and then replace the container's copy of the files with symbolic links to the bind mounted copies. However, this doesn't work when the host's copy of a configuration file is itself a symbolic link to a location outside /etc. eg., /etc/localtime, which points somewhere inside /usr/share/zoneinfo. In that case the host needs to be actively monitored via inotify(7) and an updated copy of the file needs to maintained in a separate directory, which can be bind mounted into the container. This monitoring infrastructure is already present in Flatpak and it makes sense to leverage it for Toolbox containers too. However, Flatpak containers have only one user, the current one logged into the host, whereas Toolbox containers have a longer list of users like the host itself. Therefore, if the directory getting bind mounted (ie., $XDG_RUNTIME_DIR/.flatpak-helper/monitor) isn't world readable, then some users inside the container won't be able to read the configuration file. eg., this will fail: $ sudo -u operator cat /run/host/monitor/localtime Relaxing the directory permissions on the host shouldn't pose a security risk. They are copies of originals that are world readable anyway. [1] https://github.com/debarshiray/toolbox Closes: flatpak#2916 Approved by: alexlarsson (cherry picked from commit bf96de9)
rh-atomic-bot
pushed a commit
that referenced
this pull request
Sep 19, 2019
Toolbox [1] containers, like Flatpak containers, need to keep some configuration files synchronized with the host. eg., /etc/localtime, /etc/resolv.conf, etc.. The easiest way to keep them updated is to bind mount the entire /etc from the host at some location inside the container, and then replace the container's copy of the files with symbolic links to the bind mounted copies. However, this doesn't work when the host's copy of a configuration file is itself a symbolic link to a location outside /etc. eg., /etc/localtime, which points somewhere inside /usr/share/zoneinfo. In that case the host needs to be actively monitored via inotify(7) and an updated copy of the file needs to maintained in a separate directory, which can be bind mounted into the container. This monitoring infrastructure is already present in Flatpak and it makes sense to leverage it for Toolbox containers too. However, Flatpak containers have only one user, the current one logged into the host, whereas Toolbox containers have a longer list of users like the host itself. Therefore, if the directory getting bind mounted (ie., $XDG_RUNTIME_DIR/.flatpak-helper/monitor) isn't world readable, then some users inside the container won't be able to read the configuration file. eg., this will fail: $ sudo -u operator cat /run/host/monitor/localtime Relaxing the directory permissions on the host shouldn't pose a security risk. They are copies of originals that are world readable anyway. [1] https://github.com/debarshiray/toolbox Closes: #2916 Approved by: alexlarsson (cherry picked from commit bf96de9) Closes: #3115 Approved by: alexlarsson
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Toolbox [1] containers, like Flatpak containers, need to keep some
configuration files synchronized with the host. eg., /etc/localtime,
/etc/resolv.conf, etc.. The easiest way to keep them updated is to bind
mount the entire /etc from the host at some location inside the
container, and then replace the container's copy of the files with
symbolic links to the bind mounted copies.
However, this doesn't work when the host's copy of a configuration file
is itself a symbolic link to a location outside /etc. eg.,
/etc/localtime, which points somewhere inside /usr/share/zoneinfo. In
that case the host needs to be actively monitored via inotify(7) and
an updated copy of the file needs to maintained in a separate
directory, which can be bind mounted into the container.
This monitoring infrastructure is already present in Flatpak and it
makes sense to leverage it for Toolbox containers too.
However, Flatpak containers have only one user, the current one logged
into the host, whereas Toolbox containers have a longer list of users
like the host itself. Therefore, if the directory getting bind mounted
(ie., $XDG_RUNTIME_DIR/.flatpak-helper/monitor) isn't world readable,
then some users inside the container won't be able to read the
configuration file. eg., this will fail:
$ sudo -u operator cat /run/host/monitor/localtime
Relaxing the directory permissions on the host shouldn't pose a
security risk. They are copies of originals that are world readable
anyway.
[1] https://github.com/debarshiray/toolbox