[1.13.1-rhel] Fix leaked dirfd #58
Merged
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
Ensure that the pipe is always closed during the error processing of StartInitialization. Also: * Fix a comment typo. * Use newContainerInit directly as there's no need for i to be an initer. * Move the comment about the behaviour of Init() directly above it, clarifying what happens for all defers. [@kolyshkin: backport to docker-1.13.1-rhel branch] Signed-off-by: Steven Hartland <steven.hartland@multiplay.co.uk> (cherry picked from commit 64aa78b) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
If we pass a file descriptor to the host filesystem while joining a container, there is a race condition where a process inside the container can ptrace(2) the joining process and stop it from closing its file descriptor to the stateDirFd. Then the process can access the *host* filesystem from that file descriptor. This was fixed in part by 5d93fed ("Set init processes as non-dumpable"), but that fix is more of a hail-mary than an actual fix for the underlying issue. To fix this, don't open or pass the stateDirFd to the init process unless we're creating a new container. A proper fix for this would be to remove the need for even passing around directory file descriptors (which are quite dangerous in the context of mount namespaces). There is still an issue with containers that have CAP_SYS_PTRACE and are using the setns(2)-style of joining a container namespace. Currently I'm not really sure how to fix it without rampant layer violation. [@kolyshkin: backport to docker-1.13.1-rhel branch] Fixes: CVE-2016-9962 Fixes: 5d93fed ("Set init processes as non-dumpable") Signed-off-by: Aleksa Sarai <asarai@suse.de> (cherry picked from commit e034ced) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
While we have significant protections in place against CVE-2016-9962, we still were holding onto a file descriptor that referenced the host filesystem. This meant that in certain scenarios it was still possible for a semi-privileged container to gain access to the host filesystem (if they had CAP_SYS_PTRACE). Instead, open the FIFO itself using a O_PATH. This allows us to reference the FIFO directly without providing the ability for directory-level access. When opening the FIFO inside the init process, open it through procfs to re-open the actual FIFO (this is currently the only supported way to open such a file descriptor). [@kolyshkin: backport to docker-1.13.1-rhel branch] Signed-off-by: Aleksa Sarai <asarai@suse.de> (cherry picked from commit 7d66aab) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Given the core issue in GHSA-xr7r-f8xq-vfvv was that we were unknowingly leaking file descriptors to "runc init", it seems prudent to make sure we proactively prevent this in the future. The solution is to simply mark all non-stdio file descriptors as O_CLOEXEC before we spawn "runc init". For libcontainer library users, this could result in unrelated files being marked as O_CLOEXEC -- however (for the same reason we are doing this for runc), for security reasons those files should've been marked as O_CLOEXEC anyway. Fixes: GHSA-xr7r-f8xq-vfvv CVE-2024-21626 Signed-off-by: Aleksa Sarai <cyphar@cyphar.com> (cherry picked from commit 9a72fd4be1fdc22148642ee8cd2adfee0bf575e5) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
|
LGTM |
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.
This is a manual backport of the following upstream PRs:
This fixes the leak of a host mount namespace opened directory file descriptor into the container. Compile and run-time tested.
Plus, there is a backport of 1 upstream commit which is part of original CVE-2024-21626 fix:
NOTE all other commits from the original fix either require newer kernel, newer golang, or are not relevant to this codebase.