-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
libstore: fix sandboxed builds on macOS #11031
Conversation
I’ve been testing builds and I think expunging I got this error after failing to build Nix after having earlier done some testing involving
Not sure what’s going on there, but it wasn’t fatal at least. |
Oh, it actually is fatal for the Nix build. I’ll look into it… |
I can’t really reproduce that issue properly and it was coupled with other issues that seem like they couldn’t possibly be related to my changes here, so I’m going to chalk it up to gremlins. |
After the fix for CVE-2024-38531, this was only removing the nested build directory, rather than the top‐level temporary directory. Fixes: 1d3696f
The recent fix for CVE-2024-38531 broke the sandbox on macOS completely. As it’s not practical to use `chroot(2)` on macOS, the build takes place in the main filesystem tree, and the world‐unreadable wrapper directory prevents the build from accessing its `$TMPDIR` at all. The macOS sandbox probably shouldn’t be treated as any kind of a security boundary in its current state, but this specific vulnerability wasn’t possible to exploit on macOS anyway, as creating `set{u,g}id` binaries is blocked by sandbox policy. Locking down the build sandbox further may be a good idea in future, but it already has significant compatibility issues. For now, restore the previous status quo on macOS. Thanks to @alois31 for helping me come to a better understanding of the vulnerability. Fixes: 1d3696f Closes: NixOS#11002
ea0a677
to
af2e114
Compare
Force‐pushed to simply restore the previous status quo on macOS in order to maintain compatibility, after further discussion with @alois31 convinced me that the vulnerability isn’t relevant on macOS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- tested on aarch64-darwin:
- single-user
- nix-daemon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if topTmpDir
could be confused for NIX_BUILD_TOP
, but that's very a minor concern for a fix PR.
Thank you @emilazy! |
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin 2.18-maintenance
git worktree add -d .worktree/backport-11031-to-2.18-maintenance origin/2.18-maintenance
cd .worktree/backport-11031-to-2.18-maintenance
git switch --create backport-11031-to-2.18-maintenance
git cherry-pick -x 76e4adfaac3083056e79b518ccc197a7645a0f2d af2e1142b17dadf24a0b66d8973033dce6efa32b |
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin 2.19-maintenance
git worktree add -d .worktree/backport-11031-to-2.19-maintenance origin/2.19-maintenance
cd .worktree/backport-11031-to-2.19-maintenance
git switch --create backport-11031-to-2.19-maintenance
git cherry-pick -x 76e4adfaac3083056e79b518ccc197a7645a0f2d af2e1142b17dadf24a0b66d8973033dce6efa32b |
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin 2.20-maintenance
git worktree add -d .worktree/backport-11031-to-2.20-maintenance origin/2.20-maintenance
cd .worktree/backport-11031-to-2.20-maintenance
git switch --create backport-11031-to-2.20-maintenance
git cherry-pick -x 76e4adfaac3083056e79b518ccc197a7645a0f2d af2e1142b17dadf24a0b66d8973033dce6efa32b |
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin 2.21-maintenance
git worktree add -d .worktree/backport-11031-to-2.21-maintenance origin/2.21-maintenance
cd .worktree/backport-11031-to-2.21-maintenance
git switch --create backport-11031-to-2.21-maintenance
git cherry-pick -x 76e4adfaac3083056e79b518ccc197a7645a0f2d af2e1142b17dadf24a0b66d8973033dce6efa32b |
Successfully created backport PR for |
Successfully created backport PR for |
Thank you for the merge! I can do manual backports for the other branches if it’ll be possible to get releases out. If you can think of a better name for the variable I could rename it in the backports to save additional work? |
Ah, I see you already manually handled the backports; thank you! |
Motivation
Sandboxed builds are completely broken on macOS on every release branch right now.
Context
The recent fix for CVE-2024-38531 broke the sandbox on macOS completely. As it’s not practical to use
chroot(2)
on macOS, the build takes place in the main filesystem tree, and the world‐unreadable wrapper directory prevents the build from accessing its$TMPDIR
at all.The macOS sandbox probably shouldn’t be treated as any kind of a security boundary in its current state, but this specific vulnerability wasn’t possible to exploit on macOS anyway, as creating
set{u,g}id
binaries is blocked by sandbox policy.Locking down the build sandbox further may be a good idea in future, but it already has significant compatibility issues. For now, restore the previous status quo on macOS.
Thanks to @alois31 for helping me come to a better understanding of the vulnerability.
Fixes: 1d3696f
Closes: #11002
Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.