Use securejoin.SecureJoin when forming userns paths #2134
Conversation
We need to read /etc/passwd and /etc/group in the container to get an idea of how many UIDs and GIDs we need to allocate for a user namespace when `--userns=auto` is specified. We were forming paths for these using filepath.Join, which is not safe for paths within a container, resulting in this CVE allowing crafted symlinks in the container to access paths on the host instead. Addresses CVE-2024-9676 Signed-off-by: Matt Heon <[email protected]>
mheon
force-pushed
the
fix_CVE-2024-9676
branch
from
fba61fc to
491d72e
Compare
| //go:build linux | ||
|
|
There was a problem hiding this comment.
Just to make sure does this still build the freebsd build in podman?
Another thing if we now that this will only ever be used on linux we can rename userns.go to userns_linux.go which I think is a bit better, also for the test userns_linux_test.go
There was a problem hiding this comment.
Should be fine, FreeBSD doesn't support user namespaces last I checked.
There was a problem hiding this comment.
|
Cross builds: So I think we're fine |
|
LGTM |
|
@rhatdan PTAL and merge |
|
/lgtm Three reviewers should be enough :) |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon, mtrmac 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 |
|
/cherry-pick release-1.55 |
|
@mheon: #2134 failed to apply on top of branch "release-1.55": In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
|
||
| // Securely open (read-only) a file in a container mount. | ||
| func secureOpen(containerMount, file string) (*os.File, error) { | ||
| tmpFile, err := securejoin.OpenInRoot(containerMount, file) |
There was a problem hiding this comment.
@mheon, would it be possible to use the older API here?
Why? Using filepath-securejoin 0.3.x would bring the internal use of the slices package, which requires Go 1.21 or newer. Which is something we don't have for when building images for older releases of OpenShift.
|
I like having the new version on main, but replacing the new API with a
securejoin and os.Open for older versions should be fine
…On Wed, Oct 16, 2024 at 03:11 Krzysztof Wilczyński ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In userns.go
<#2134 (comment)>:
> @@ -309,3 +331,14 @@ func getAutoUserNSIDMappings(
gidMap := append(availableGIDs.zip(requestedContainerGIDs), additionalGIDMappings...)
return uidMap, gidMap, nil
}
+
+// Securely open (read-only) a file in a container mount.
+func secureOpen(containerMount, file string) (*os.File, error) {
+ tmpFile, err := securejoin.OpenInRoot(containerMount, file)
@mheon <https://github.com/mheon>, would it be possible to use the older
API here?
Why? Using filepath-securejoin *3.x* would bring the internal use of the
slices package, which requires Go 1.21 or newer. Which is something we
don't have for when building images for older releases of OpenShift.
—
Reply to this email directly, view it on GitHub
<#2134 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB3AOCFQ5NF7HTMZPVAPAX3Z3YGTLAVCNFSM6AAAAABP5KIU2GVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGNZRGQYDIMZXGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@mheon, sounds good! I will made the necessary changes and then ask you for a quick review, to make sure everything checks out. |
We need to read /etc/passwd and /etc/group in the container to get an idea of how many UIDs and GIDs we need to allocate for a user namespace when
--userns=autois specified. We were forming paths for these using filepath.Join, which is not safe for paths within a container, resulting in this CVE allowing crafted symlinks in the container to access paths on the host instead.Addresses CVE-2024-9676