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.26] backport fix for CVE-2024-11218 #5931

Merged

Conversation

dashea
Copy link

@dashea dashea commented Jan 22, 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.26 branch.

How to verify it

Which issue(s) this PR fixes:

RHEL-67609

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 <[email protected]>
Signed-off-by: David Shea <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
Signed-off-by: David Shea <[email protected]>
Add a way to pass a "set the SELinux contexts" labels to
MountWithOptions.

Signed-off-by: Nalin Dahyabhai <[email protected]>
Signed-off-by: David Shea <[email protected]>
@openshift-ci openshift-ci bot added the kind/bug Categorizes issue or PR as related to a bug. label Jan 22, 2025
@nalind
Copy link
Member

nalind commented Jan 22, 2025

This branch predates the splitup in pkg/overlay, so I don't think we'll need 44e2788

@nalind
Copy link
Member

nalind commented Jan 22, 2025

/retitle [release-1.26] backport fix for CVE-2024-11218

@openshift-ci openshift-ci bot changed the title Backport fix for CVE-2024-11218 [release-1.26] backport fix for CVE-2024-11218 Jan 22, 2025
@nalind
Copy link
Member

nalind commented Jan 22, 2025

/approve

Copy link
Contributor

openshift-ci bot commented Jan 22, 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

@dashea dashea force-pushed the dshea-1.26-CVE-2024-11218 branch 2 times, most recently from ce3be90 to 8673136 Compare January 22, 2025 15:31
@nalind
Copy link
Member

nalind commented Jan 22, 2025

I think we're missing an equivalent for 7320f49#diff-88267013462361b7dd8d8bde4c038ce82c090a9c72936d14bcd6c368aa91323bR526-R530 from 8673136

@dashea dashea force-pushed the dshea-1.26-CVE-2024-11218 branch from 8673136 to 2fee799 Compare January 22, 2025 16:20
@dashea
Copy link
Author

dashea commented Jan 22, 2025

I think we're missing an equivalent for 7320f49#diff-88267013462361b7dd8d8bde4c038ce82c090a9c72936d14bcd6c368aa91323bR526-R530 from 8673136

Thanks, added

internal/parse/parse.go Outdated Show resolved Hide resolved
run_linux.go Show resolved Hide resolved
run_linux.go Show resolved Hide resolved
run_linux.go Show resolved Hide resolved
internal/parse/parse.go Outdated Show resolved Hide resolved
@dashea dashea force-pushed the dshea-1.26-CVE-2024-11218 branch 2 times, most recently from d70c3ab to 8b8b588 Compare January 22, 2025 19:06
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 <[email protected]>
Signed-off-by: David Shea <[email protected]>
@dashea dashea force-pushed the dshea-1.26-CVE-2024-11218 branch from 990c324 to 6475964 Compare January 22, 2025 21:03
nalind and others added 3 commits January 23, 2025 11:34
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 <[email protected]>
Signed-off-by: David Shea <[email protected]>
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 <[email protected]>
Signed-off-by: David Shea <[email protected]>
Append to the lock list instead of replacing it.

Signed-off-by: David Shea <[email protected]>
@dashea dashea force-pushed the dshea-1.26-CVE-2024-11218 branch from 6475964 to 138ad69 Compare January 23, 2025 16:34
@dashea
Copy link
Author

dashea commented Jan 23, 2025

Found the problem with the bud-git-context test: I was not returning the new intermediate directory in one of the paths through TempDirForURL.

Signed-off-by: David Shea <[email protected]>
@dashea dashea force-pushed the dshea-1.26-CVE-2024-11218 branch from 6ff8b71 to a313689 Compare January 23, 2025 22:17
@nalind
Copy link
Member

nalind commented Jan 24, 2025

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Jan 24, 2025
@openshift-merge-bot openshift-merge-bot bot merged commit a4ab900 into containers:release-1.26 Jan 24, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved kind/bug Categorizes issue or PR as related to a bug. lgtm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants