-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
libpod: mount safely subpaths #17992
libpod: mount safely subpaths #17992
Conversation
b0fb2e6
to
38b9036
Compare
} | ||
}() | ||
|
||
fdPath := fmt.Sprintf("/proc/%d/fd/%d", os.Getpid(), f.Fd()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I understand it correctly that the trick to avoid a TOCTOU is by using the FD here? The path may change but the FD will continue pointing to the same file?
Whether I understood it correctly or not, I think that a comment would be nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the trick is that once it is opened, it cannot become a symlink anymore. So if we open, validate it and then use it, we are safe. I'll add a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent, thanks!
Clever idea, btw
38b9036
to
cd748fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, vrothberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
cd748fb
to
32d6622
Compare
Nice work. One nit, but not worth fixing unless you need to repush. /lgtm |
32d6622
to
8a88390
Compare
add a function to securely mount a subpath inside a volume. We cannot trust that the subpath is safe since it is beneath a volume that could be controlled by a separate container. To avoid TOCTOU races between when we check the subpath and when the OCI runtime mounts it, we open the subpath, validate it, bind mount to a temporary directory and use it instead of the original path. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
8a88390
to
4d56292
Compare
/lgtm |
/hold cancel |
add a function to securely mount a subpath inside a volume. We cannot trust that the subpath is safe since it is beneath a volume that could be controlled by a separate container. To avoid TOCTOU races between when we check the subpath and when the OCI runtime mounts it, we open the subpath, validate it, bind mount to a temporary directory and use it instead of the original path.
Does this PR introduce a user-facing change?