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.27] Backport fix for CVE-2024-11218 #5946

Merged

Conversation

dashea
Copy link

@dashea dashea commented Jan 27, 2025

What type of PR is this?

/kind bug

What this PR does / why we need it:

Backport the changes for GHSA-5vpc-35f4-r8w6 to the 1.27 branch.

How to verify it

Which issue(s) this PR fixes:

RHEL-67598

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

Allow cache mounts (RUN --mount=type=cache) to refer to other stages or
additional build contexts.

Update the build-check-cve-2024-9675 integration test to use different
directories for its main build context and the additional build context
that it uses for its final run.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Signed-off-by: David Shea <dshea@redhat.com>
Add a package that lets us open a directory in a chroot, pass its
descriptor up, and then bind mount that directory to a specified
location.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Add a helper that uses the new internal/open package to bind mount a
location inside of a chroot direct to a new temporary location, for
ensuring that the latter is not bind-mounted from outside of the chroot.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Add a ForceMount flag to pkg/overlay.Options that forces mounting the
overlay filesystem and returning a bind mount to it instead of trying to
leave that for later in cases where we're able to have the kernel do it.

This is mainly for the sake of callers that want to do more things with
the mounted overlay filesystem before passing them to the (presumably)
OCI runtime.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Signed-off-by: David Shea <dshea@redhat.com>
Add a way to pass a "set the SELinux contexts" labels to
MountWithOptions.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Signed-off-by: David Shea <dshea@redhat.com>
Copy link
Member

@nalind nalind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with changes.

@nalind
Copy link
Member

nalind commented Jan 29, 2025

/approve

@TomSweeneyRedHat
Copy link
Member

Other than @nalind 's comments, LGTM

nalind and others added 4 commits January 29, 2025 15:13
When handling RUN --mount=type=bind, where the mount is read-write,
instead of a simple bind mount, create an overlay mount with an upper
directory that will be discarded after the overlay mount is unmounted.
This brings us in line with the expected behavior, wherein writes to
bind mounts should be discarded.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Signed-off-by: David Shea <dshea@redhat.com>
Ensure that the temporary directory that we create is never itself the
top-level directory of the content that we're downloading, in case it's
an archive which includes a "." with weird permissions.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Signed-off-by: David Shea <dshea@redhat.com>
Fix a time-of-check/time-of-use error when mounting type=bind and
type=cache directories that use a "src" flag.  A hostile writer could
use a concurrently-running stage or build to replace that "src" location
between the point when we had resolved possible symbolic links and when
runc/crun/whatever actually went to create the bind mount
(CVE-2024-11218).

Stop ignoring the "src" option for cache mounts when there's no "from"
option.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Signed-off-by: David Shea <dshea@redhat.com>
Append to the lock list instead of replacing it.

Signed-off-by: David Shea <dshea@redhat.com>
@dashea dashea force-pushed the dshea-1.27-CVE-2024-11218 branch from 8bc1301 to 2a43238 Compare January 29, 2025 20:13
@nalind
Copy link
Member

nalind commented Jan 29, 2025

It's almost certainly a race condition somewhere else, but 2a43238 is consistently causing the "bud-multiple-platform-values" integration test to fail on my test system. Can anyone confirm that it's not universally observed?

@TomSweeneyRedHat
Copy link
Member

LGTM

@TomSweeneyRedHat
Copy link
Member

@nalind can you take a second look please?

@nalind
Copy link
Member

nalind commented Jan 30, 2025

Same code, same test, passes today. I guess it's a flake.
/lgtm

@openshift-ci openshift-ci bot added the lgtm label Jan 30, 2025
@dashea dashea requested a review from nalind January 30, 2025 18:23
Copy link
Contributor

openshift-ci bot commented Jan 30, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dashea, nalind

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

@nalind
Copy link
Member

nalind commented Jan 30, 2025

/override "Total Success"

Copy link
Contributor

openshift-ci bot commented Jan 30, 2025

@nalind: Overrode contexts on behalf of nalind: Total Success

In response to this:

/override "Total Success"

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.

@openshift-merge-bot openshift-merge-bot bot merged commit 85dabc8 into containers:release-1.27 Jan 30, 2025
4 checks passed
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.

3 participants