From c63d8001bcf8c66bd1d63d058781c9ca77b2be77 Mon Sep 17 00:00:00 2001 From: Colin Date: Fri, 20 Dec 2024 09:49:23 +0000 Subject: [PATCH] bunpen: pasta: wait for pasta to be ready before executing the user program --- pkgs/by-name/bunpen/restrict/ns/pasta.ha | 66 ++++++++++++++++++------ 1 file changed, 49 insertions(+), 17 deletions(-) diff --git a/pkgs/by-name/bunpen/restrict/ns/pasta.ha b/pkgs/by-name/bunpen/restrict/ns/pasta.ha index c8ff3f4e3..2cbec137e 100644 --- a/pkgs/by-name/bunpen/restrict/ns/pasta.ha +++ b/pkgs/by-name/bunpen/restrict/ns/pasta.ha @@ -19,32 +19,37 @@ fn setup_pasta(net: restrict::net_subset) void = { // ordering: // 1. fork into (P, C1) // 2. child C1: enter netns, signal parent P. - // 3. parent P: fork and spawn `pasta --pid /proc/fd/$N` (C2) + // 3. parent P: fork and spawn `pasta --pid /dev/fd/$N` (C2) // then dumbly wait on child (C2) until completion // 4. child C1:: wait for `pasta` to signal readiness on fd `$N` // // after that, C1 can continue on & exec into the user code - // - // TODO: this currently lacks the synchronization described above. - // it still generally _works_, but possible that some applications are flaky let (pipe_parent_rd, pipe_child_wr) = unix::pipe()!; + let (pipe_child_rd, pipe_parent_wr) = unix::pipe()!; log::printfln("[namespace/pasta]: forking: parent will launch pasta while child will exec user code"); match (fork_and_die_with_parent()) { case let child_pid: os::exec::process => - // close the side of the pipe that's not ours: + // close the pipe ends which aren't ours io::close(pipe_child_wr)!; + io::close(pipe_child_rd)!; // wait for the child to signal that it's ready for us to attach pasta. io::readall(pipe_parent_rd, &[0u8])!; errors::ext::check("setup_pasta: attach", attach_pasta(net, child_pid)); + + // let the child know its environment is configured + io::write(pipe_parent_wr, [1])!; + errors::ext::check("setup_pasta: wait", wait_and_propagate(child_pid)); - // cleanup: we're done with the pipe + // cleanup: we're done with the pipes + io::close(pipe_parent_wr)!; io::close(pipe_parent_rd)!; case void => - // close the side of the pipe that's not ours: + // close the pipe ends which aren't ours + io::close(pipe_parent_wr)!; io::close(pipe_parent_rd)!; errors::ext::check("namespace: unshare net", rt::ext::unshare(rt::ext::clone_flag::NEWNET)); @@ -59,11 +64,12 @@ fn setup_pasta(net: restrict::net_subset) void = { // let the parent know we're ready for pasta to attach to us io::write(pipe_child_wr, [1])!; - // TODO: race condition here, where the child immediately continues on even - // though pasta hasn't created the device. + // wait for the parent to attach pasta + io::readall(pipe_child_rd, &[0u8])!; - // cleanup: we're done with the pipe + // cleanup: we're done with the pipes io::close(pipe_child_wr)!; + io::close(pipe_child_rd)!; case let e: (os::exec::error | rt::errno) => errors::ext::check("setup_pasta: fork", e); @@ -72,9 +78,39 @@ fn setup_pasta(net: restrict::net_subset) void = { // spawn pasta as a separate process, and have it attach to the netns of the given pid. fn attach_pasta(net: restrict::net_subset, child: os::exec::process) (void | os::exec::error | rt::errno) = { + // pipe is used by child pasta process to notify parent process once ready + let (pipe_parent_rd, pipe_child_wr) = unix::pipe(unix::pipe_flag::NOCLOEXEC)!; return match (fork_and_die_with_parent()?) { - case let pasta_pid: os::exec::process => yield void; + case let pasta_pid: os::exec::process => + // close the side of the pipe that's not ours: + io::close(pipe_child_wr)!; + + // wait for pasta to signal readiness. + // it does this by writing its pid (followed by newline) to a file. + let pasta_pid: [32]u8 = [0...]; + io::read(pipe_parent_rd, &pasta_pid)!; + + log::printfln( + "[namespace/pasta] pasta signalled readiness as pid {}: continuing", + strings::fromutf8_unsafe(pasta_pid), + ); + + // cleanup: we're done with the pipe + io::close(pipe_parent_rd)!; + + yield void; case void => + // close the side of the pipe that's not ours: + io::close(pipe_parent_rd)!; + + // move the notification pipe to be one of the special fd's which *don't* + // get closed on exec (e.g. /dev/fd/0, stdin). + // this *shouldn't* be necessary: the pipe was created NOCLOEXEC; + // but maybe pasta is doing some security measure like clearing all open fd's on exec. + rt::dup2(pipe_child_wr, 0)!; + io::close(pipe_child_wr)!; + let notify_fd_path = "/dev/fd/0"; + // pasta needs permissions to create a device in the netns (it apparently // won't raise those caps itself). TODO: reduce these resources! let res = restrict::resources { @@ -85,13 +121,9 @@ fn attach_pasta(net: restrict::net_subset, child: os::exec::process) (void | os: let netns_path = fmt::asprintf("/proc/{}/ns/net", child: int); defer free(netns_path); - // let notify_fd_path = fmt::asprintf("/proc/self/fd/{}", notify_fd); - // log::printfln("[namespace/pasta] notify {}", notify_fd_path); - // defer free(notify_fd_path); - - // TODO: append dns argument to `pasta`. let pasta_args = [ "pasta", + "--quiet", //< don't print MAC/DHCP/DNS values (TODO: enable these for BUNPEN_DEBUG>=1) "--ipv4-only", // pasta `up`s `lo` regardless of this flag; `--config-net` just tells // it to assign an IP and routes to the new device it creates @@ -108,7 +140,7 @@ fn attach_pasta(net: restrict::net_subset, child: os::exec::process) (void | os: "--netns-only", // pidstr, "--netns", netns_path, - // "--pid", notify_fd_path, + "--pid", notify_fd_path, ]; log::printfln("[namespace/pasta]: invoking pasta: {}", strings::join(" ", pasta_args...)); yield rt::ext::execvpe_or_default(