Add sandboxed building for FreeBSD using jails#9968
Add sandboxed building for FreeBSD using jails#9968Ericson2314 merged 2 commits intoNixOS:masterfrom
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2024-02-12-nix-team-meeting-minutes-123/39775/1 |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This is the utility changes from #9968, which were easier to rebase first.
This is the utility changes from #9968, which were easier to rebase first. I (@Ericson2314) didn't write this code; I just rebased it. Co-Authored-By: Artemis Tosini <me@artem.ist> Co-Authored-By: Audrey Dutcher <audrey@rhelmot.io>
6e711ce to
d1a4478
Compare
|
I have redone the rebase, fixed a couple of terrible bugs, added a few new features (made the chroot cleanup a lot more bulletproof, made ^C work). Should be ready. |
|
From dogfooding, this refactor seems to have introduced some issues related to the cleanup stuff. |
The <() process substitution syntax doesn't work for this one testcase in bash for FreeBSD. The exact reason for this is unknown, possibly to do with pipe vs file vs fifo EOF behavior. The prior behavior was this test hanging forever, with no children of the bash process. Change-Id: I71822a4b9dea6059b34300568256c5b7848109ac (cherry picked from commit ae628d4)
New FreeBSD sandboxes are based on jails and chroots. They provide fairly similar capabilities to sandboxes on Linux and allow for pure builds of FreeBSD nixpkgs. Although it would also be possble to use jails for Linux emulation, that is not supported with this commit. Change-Id: I619e1e34c56de7aaa64a38408210a410bb13adba Co-Authored-By: Artemis Tosini <me@artem.ist> Co-Authored-By: John Ericson <John.Ericson@Obsidian.Systems>
|
Just kiddinggggggggggggg I was dogfooding the wrong branch. This works for me now. |
xokdvium
left a comment
There was a problem hiding this comment.
Sounds wonderful. Excited for this! I do have some nitpicks, but I'll not block this - I can do a follow-up with minor cleanup after the merge (once I dogfood this a bit too).
| PathFmt(user.home), | ||
| PathFmt(user.shell))); |
There was a problem hiding this comment.
Hm, this doesn't seem right since PathFmt is meant for debug messages and adds quotes around the path. This should probably be .native() to just give you a string instead.
| struct stat stat_buf; | ||
| if (stat(i.second.source.c_str(), &stat_buf) < 0) { | ||
| throw SysError("stat"); | ||
| } |
There was a problem hiding this comment.
We have a nix::stat helper nowadays.
| struct stat stat_buf; | ||
| if (stat(i.second.source.c_str(), &stat_buf) < 0) { | ||
| throw SysError("stat"); | ||
| } | ||
|
|
||
| // mount points must exist and be the right type | ||
| if (S_ISDIR(stat_buf.st_mode)) { | ||
| createDirs(path); | ||
| } else { | ||
| createDirs(path.parent_path()); | ||
| writeFile(path, ""); | ||
| } |
There was a problem hiding this comment.
Do we want to follow symlinks in stat? Shouldn't this be lstat at least? Or maybe open(..., O_NOFOLLOW | O_PATH) + fstat?
There was a problem hiding this comment.
This is getting the stat of files to be nullfs mounted into the build sandbox, so the only time we would ever see a symlink is... if there were a derivation which realized to just a symlink? Is that even possible? I don't think that nullfs can bind symlinks like that, so I would probably want to check if it's a symlink upfront and if so implement it with another symlink rather than a mount.
There was a problem hiding this comment.
if there were a derivation which realized to just a symlink? Is that even possible?
Yes, we do it all the time.
There was a problem hiding this comment.
Following symlinks is actually very bad for security. Any derivation can build a store object pointing to /etc/verysecretblahblah and the priviledged daemon would resolve it and put in the build sandbox. Not good
| if (i.second.source == "/proc") { | ||
| continue; // backwards compatibility | ||
| } |
There was a problem hiding this comment.
This can be yeeted right, since freebsd doesn't have procfs (I'm no freebsd guru though).
There was a problem hiding this comment.
FreeBSD does in fact have a procfs, though it is deprecated.
There was a problem hiding this comment.
Yeah but this back-compat is for nix's config itself, which we don't need to carry over from the linux builder.
| PathFmt(user.home), | ||
| PathFmt(user.shell))); |
There was a problem hiding this comment.
| PathFmt(user.home), | |
| PathFmt(user.shell))); | |
| user.home.native(), | |
| user.shell.native())); |
| for (size_t i = 0; i < users.size(); i++) { | ||
| auto user = users[i]; |
There was a problem hiding this comment.
This should also work:
| for (size_t i = 0; i < users.size(); i++) { | |
| auto user = users[i]; | |
| for (const auto& [i, user]: enumerate(users)) { |
|
Let's indeed fix it after because this is a two year old PR that I had held up for so many things :) (I do agree with all those build comments, though) |
|
@rhelmot anyway, I'm doing some cleanup here and will put up a PR shortly once I make sure that this stuff still works after all my changes. |
Add sandboxed building for FreeBSD using jails
Motivation
Build isolation is good! In Linux, this is accomplished with namespaces (containers). The equivalent technology on FreeBSD is jails.
Context
This is part of my ongoing project to make FreeBSD a first class citizen in the nix world.
This was a fairly simple patch, just needed to add parallel implementations for all the sandboxed Linux build pieces.
The most fragile part of this implementation is the fact that there is a lot of global state that gets set up in order to construct the jail - the chroot dir in the nix store, the nullfs mounts (the FreeBSD equivalent of a bind mount), and the jail ID itself. Lots of steps have been taken to make sure these all get cleaned up, both at the end of the build and at the start of any rebuilds. It seems to be resilient to interruption.
This has been live-fire tested with my fork of nixpkgs for FreeBSD. It is able to build the stdenv without issue.
Please squash-merge this PR! It includes some changes that were later reverted, which don’t belong in this repository but instead in the FreeBSD ports repository.
Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.