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

[release-1.55] backport fix for CVE-2024-9676 #2135

Conversation

mheon
Copy link
Member

@mheon mheon commented Oct 14, 2024

Also drags in a backport of #2105 to make the diff apply cleanly. I don't think there's harm in doing so.

giuseppe and others added 4 commits October 14, 2024 16:15
fix the detection for the maximum userns size from an image.

If the maximum ID used in an image is X, we need to use a user
namespace with size X+1 to include UID=X.

Closes: containers#2104

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
the alpine image defines a "nogroup":

$ podman run --rm alpine grep nogroup /etc/group
nogroup:x:65533:

ignore it as we are already doing for the "nobody" user.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
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 <mheon@redhat.com>
@mheon mheon changed the base branch from main to release-1.55 October 14, 2024 20:17
@mheon
Copy link
Member Author

mheon commented Oct 14, 2024

This takes care of release-1.55 which takes care of the current Podman and Buildah release branches

@mheon
Copy link
Member Author

mheon commented Oct 15, 2024

@nalind Any idea what's going on with lint over here? I'm seeing the same locally, but I'm pretty sure they aren't caused by this PR either.

@nalind
Copy link
Member

nalind commented Oct 15, 2024

I'm not sure why, but .cirrus.yml is specifying golang as the image for some tasks (including the linting task) and golang:1.21 for others. Changing it produces a better result for me, but not quite passing.

@mheon
Copy link
Member Author

mheon commented Oct 15, 2024

Do we have a minimum supported Go version of 1.21 for the branch? My preliminary investigation suggests 1.22 might pass.

@nalind
Copy link
Member

nalind commented Oct 15, 2024

It tends to be dictated by its consumers. Organization-wide, I think we tend to either use "the oldest currently supported golang", or "the version available on the distributions where we still need to be able to build". The buildah 1.37 branch and podman 5.2 branches appear to be using 1.21, fwiw.

@nalind
Copy link
Member

nalind commented Oct 15, 2024

/retitle [release-1.55] backport fix for CVE-2026-9676

@openshift-ci openshift-ci bot changed the title Backport fix for CVE-2026-9676 [release-1.55] backport fix for CVE-2026-9676 Oct 15, 2024
@mheon
Copy link
Member Author

mheon commented Oct 15, 2024

A lot of these seem completely erroneous. It's complaining that ContainerOptions doesn't have certain fields, for example - those are brought in by the included types.IDMappingOptions in ContainerOptions. It'd be a compile error if those were missing. I think most of them boil down to things that are allowed by the compiler, but the linter is refusing to handle.

Matches what we're compiling with.

Signed-off-by: Matt Heon <mheon@redhat.com>
@mheon
Copy link
Member Author

mheon commented Oct 15, 2024

Well, whatever I was doing locally was clearly incorrect, because 1.21 failed locally but seems to pass here. Fascinating.

@nalind
Copy link
Member

nalind commented Oct 15, 2024

LGTM

@mheon
Copy link
Member Author

mheon commented Oct 15, 2024

@containers/storage-maintainers PTAL

@kolyshkin
Copy link
Contributor

I guess there's a typo and it should be CVE-2024-9676.

@mheon
Copy link
Member Author

mheon commented Oct 15, 2024

That is indeed a typo. At least I got the identifier within the year correct.

@rhatdan
Copy link
Member

rhatdan commented Oct 15, 2024

/approve
/lgtm

Copy link
Contributor

openshift-ci bot commented Oct 15, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mheon, rhatdan

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 465d8c0 into containers:release-1.55 Oct 15, 2024
18 checks passed
@lsm5
Copy link
Member

lsm5 commented Oct 17, 2024

Need a new release cut with this and vendored into the usual places.

@mheon
Copy link
Member Author

mheon commented Oct 17, 2024

Ack, I'll start vendoring this afternoon.

@danishprakash
Copy link

It would be helpful to retitle this to CVE-2024-9676 for those trying to search for this CVE in the repo.

@mheon mheon changed the title [release-1.55] backport fix for CVE-2026-9676 [release-1.55] backport fix for CVE-2024-9676 Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants