From 6a0b559107ef83885405ad0942f2da60f510b5c7 Mon Sep 17 00:00:00 2001 From: Akram Date: Thu, 10 Sep 2026 20:21:07 +0400 Subject: [PATCH 1/9] build(docker): add OpenShift in-cluster build variants for supervisor and gateway Add multi-stage Dockerfiles that build the OpenShell supervisor and gateway images entirely inside an OpenShift/Buildah cluster, for environments without the upstream Nix CI pipeline that stages prebuilt binaries under deploy/docker/.build/prebuilt-binaries. Both reproduce the exact upstream artifacts by running the project's own Nix devShells in a builder stage, then assembling a runtime stage identical to the existing Dockerfile.supervisor / Dockerfile.gateway: - Dockerfile.supervisor.multistage: builds the static musl openshell-sandbox binary via the musl devShell; runtime is alpine:3.22 with nftables/iptables and COPY --chmod=0555. - Dockerfile.gateway.multistage: builds openshell-gateway via the glibc-2-28 devShell, normalizes the ELF interpreter with patchelf and asserts z3 is statically embedded; runtime is distroless cc-debian13. Each builder collapses build and cleanup into a single RUN so the Nix store never enters the committed layer, keeping the intermediate commit small and within the node's ephemeral-storage budget. The upstream Dockerfiles and CI binary pipeline are unchanged. Signed-off-by: Akram Signed-off-by: Akram --- deploy/docker/Dockerfile.gateway.multistage | 80 +++++++++++++++++++ .../docker/Dockerfile.supervisor.multistage | 72 +++++++++++++++++ 2 files changed, 152 insertions(+) create mode 100644 deploy/docker/Dockerfile.gateway.multistage create mode 100644 deploy/docker/Dockerfile.supervisor.multistage diff --git a/deploy/docker/Dockerfile.gateway.multistage b/deploy/docker/Dockerfile.gateway.multistage new file mode 100644 index 0000000000..7971a08657 --- /dev/null +++ b/deploy/docker/Dockerfile.gateway.multistage @@ -0,0 +1,80 @@ +# syntax=docker/dockerfile:1.4 +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +# In-cluster (OpenShift/Buildah) variant of Dockerfile.gateway. +# +# Upstream CI builds the GNU-linked openshell-gateway binary inside the +# project's Nix devShell with the gnu cross target (z3 and aws-lc statically +# embedded, standard ELF interpreter), and stages it under +# deploy/docker/.build/prebuilt-binaries. This multi-stage Dockerfile +# reproduces that exact artifact in a builder stage so clusters without the Nix +# CI pipeline can build the gateway image directly from source. The final stage +# is identical to Dockerfile.gateway. + +# In a multi-stage build, an ARG that feeds a `FROM` must be declared before the +# very first FROM (global pre-FROM scope). Declaring it between the two stages +# makes Buildah attach it to the builder stage and fail the second FROM with +# "no FROM statement found". +ARG GATEWAY_BASE_IMAGE=gcr.io/distroless/cc-debian13:nonroot@sha256:d97bc0a941b8d4be647dc0ee75b264ddbb772f1ac5ba690a4309c00723b23775 + +# ---- Builder: glibc base + Nix, reproduce upstream's cross build ------------- +# +# The build must run on a glibc/FHS base (like the CI runners), NOT on the +# minimal nixos/nix image: cross-compiling emits host build-scripts linked for +# x86_64-unknown-linux-gnu whose ELF interpreter is /lib64/ld-linux-x86-64.so.2, +# which the nixos/nix (Alpine, store-only) image does not provide. +FROM debian:bookworm-slim AS builder + +RUN apt-get update && apt-get install -y --no-install-recommends \ + curl xz-utils ca-certificates git && rm -rf /var/lib/apt/lists/* + +# Single-user Nix (no daemon) as root. NIX_CONFIG disables per-build users +# during the install itself (the installer reads config before our nix.conf +# exists); the written nix.conf carries that to later `nix develop` runs, plus +# flakes and the flake's cachix substituter for prebuilt toolchain/z3/aws-lc. +RUN export NIX_CONFIG="build-users-group =" && \ + mkdir -m 0755 /nix && \ + curl -L https://nixos.org/nix/install -o /tmp/nix-install.sh && \ + sh /tmp/nix-install.sh --no-daemon && \ + mkdir -p /etc/nix && \ + printf 'experimental-features = nix-command flakes\naccept-flake-config = true\nbuild-users-group =\nsandbox = false\n' > /etc/nix/nix.conf + +# Put single-user Nix on PATH directly (Buildah RUN is not a login shell, so the +# profile script is unreliable) and point it at Debian's CA bundle for HTTPS +# substituter fetches. +ENV HOME=/root \ + PATH=/root/.nix-profile/bin:/nix/var/nix/profiles/default/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin \ + NIX_SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt + +WORKDIR /build +COPY . . + +# Build the gateway exactly like CI: same devShell, same cargo command and +# target triple. The gnu cross toolchain emits a standard ELF interpreter, so +# the binary runs on distroless without post-processing. Everything runs in a +# single layer: build, stage the binary at /, then delete the Nix store and +# Cargo target so the committed builder layer stays small. +RUN nix develop -c bash -euo pipefail -c '\ + GIT_DIR=/nonexistent cargo auditable build --release \ + --target x86_64-unknown-linux-gnu \ + --package openshell-gateway --bin openshell-gateway' && \ + cp target/x86_64-unknown-linux-gnu/release/openshell-gateway /openshell-gateway && \ + cd / && rm -rf /nix /build /root/.cache /tmp/* + +# ---- Runtime: identical to deploy/docker/Dockerfile.gateway ------------------ +# +# Distroless Debian provides the glibc runtime required by the binary. +FROM ${GATEWAY_BASE_IMAGE} AS gateway + +ARG TARGETARCH + +WORKDIR /app + +COPY --from=builder /openshell-gateway /usr/local/bin/openshell-gateway + +USER 1000:1000 +EXPOSE 8080 + +ENTRYPOINT ["/usr/local/bin/openshell-gateway"] +CMD ["--bind-address", "0.0.0.0", "--port", "8080"] diff --git a/deploy/docker/Dockerfile.supervisor.multistage b/deploy/docker/Dockerfile.supervisor.multistage new file mode 100644 index 0000000000..4e6196d758 --- /dev/null +++ b/deploy/docker/Dockerfile.supervisor.multistage @@ -0,0 +1,72 @@ +# syntax=docker/dockerfile:1.4 +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +# In-cluster (OpenShift/Buildah) variant of Dockerfile.supervisor. +# +# Upstream CI produces the static musl `openshell-sandbox` binary by building +# inside the project's Nix devShell with the musl cross target, and stages it +# under deploy/docker/.build/prebuilt-binaries. This multi-stage Dockerfile +# reproduces that exact binary in a builder stage so clusters without the Nix +# CI pipeline can build the supervisor image directly from source. The final +# stage is identical to Dockerfile.supervisor. + +# ---- Builder: glibc base + Nix, reproduce upstream's cross build ------------- +# +# The build must run on a glibc/FHS base (like the CI runners), NOT on the +# minimal nixos/nix image: cross-compiling emits host build-scripts linked for +# x86_64-unknown-linux-gnu whose ELF interpreter is /lib64/ld-linux-x86-64.so.2, +# which the nixos/nix (Alpine, store-only) image does not provide. +FROM debian:bookworm-slim AS builder + +RUN apt-get update && apt-get install -y --no-install-recommends \ + curl xz-utils ca-certificates git && rm -rf /var/lib/apt/lists/* + +# Single-user Nix (no daemon) as root. NIX_CONFIG disables per-build users +# during the install itself (the installer reads config before our nix.conf +# exists); the written nix.conf carries that to later `nix develop` runs, plus +# flakes and the flake's cachix substituter for prebuilt toolchain/z3/aws-lc. +RUN export NIX_CONFIG="build-users-group =" && \ + mkdir -m 0755 /nix && \ + curl -L https://nixos.org/nix/install -o /tmp/nix-install.sh && \ + sh /tmp/nix-install.sh --no-daemon && \ + mkdir -p /etc/nix && \ + printf 'experimental-features = nix-command flakes\naccept-flake-config = true\nbuild-users-group =\nsandbox = false\n' > /etc/nix/nix.conf + +# Put single-user Nix on PATH directly (Buildah RUN is not a login shell, so the +# profile script is unreliable) and point it at Debian's CA bundle for HTTPS +# substituter fetches. +ENV HOME=/root \ + PATH=/root/.nix-profile/bin:/nix/var/nix/profiles/default/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin \ + NIX_SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt + +WORKDIR /build +COPY . . + +# Build the sandbox binary exactly like CI: same devShell, same cargo command +# and target triple. Everything runs in a single layer: build, stage the static +# binary at /, then delete the Nix store and Cargo target. The binary is static +# musl (it needs nothing from /nix at runtime), so the committed builder layer +# shrinks to the binary alone, keeping the intermediate commit fast and within +# the node's ephemeral-storage budget. +RUN nix develop -c bash -euo pipefail -c '\ + GIT_DIR=/nonexistent cargo auditable build --release \ + --target x86_64-unknown-linux-musl \ + --package openshell-sandbox --bin openshell-sandbox' && \ + cp target/x86_64-unknown-linux-musl/release/openshell-sandbox /openshell-sandbox && \ + cd / && rm -rf /nix /build /root/.cache /tmp/* + +# ---- Runtime: identical to deploy/docker/Dockerfile.supervisor --------------- +# +# Alpine supplies nftables and iptables for pod-namespace egress enforcement. +FROM alpine:3.22 AS supervisor + +ARG TARGETARCH + +RUN apk add --no-cache nftables iptables iptables-legacy + +# Keep the binary root-owned for Podman image-volume mounts and executable by +# the Kubernetes network sidecar's non-root proxy UID. +COPY --from=builder --chmod=0555 /openshell-sandbox /openshell-sandbox + +ENTRYPOINT ["/openshell-sandbox"] From 813092fe6bab5596003a71ec31ad35c90dd2a246 Mon Sep 17 00:00:00 2001 From: Akram Date: Thu, 10 Sep 2026 23:54:37 +0400 Subject: [PATCH 2/9] feat(sandbox): default to official Alpine sandbox image default_sandbox_image() now returns docker.io/library/alpine:3.22, a generic version-qualified official image, so a fresh install no longer depends on the community sandbox image catalog. All compute drivers (docker, podman, kubernetes, vm) inherit this fallback. Part of #3116. Signed-off-by: Akram Signed-off-by: Akram --- crates/openshell-core/src/image.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/openshell-core/src/image.rs b/crates/openshell-core/src/image.rs index e804afd60f..abf4ff2fa5 100644 --- a/crates/openshell-core/src/image.rs +++ b/crates/openshell-core/src/image.rs @@ -13,13 +13,14 @@ /// Override at runtime with the `OPENSHELL_COMMUNITY_REGISTRY` env var. pub const DEFAULT_COMMUNITY_REGISTRY: &str = "ghcr.io/nvidia/openshell-community/sandboxes"; -/// Return the default sandbox image reference (`{registry}/base:latest`). +/// Return the default sandbox image reference. /// /// Used by all compute drivers as the fallback image when none is specified in -/// the sandbox spec. +/// the sandbox spec. Defaults to a generic, version-qualified official Alpine +/// image so a fresh install does not depend on the community image catalog. #[must_use] pub fn default_sandbox_image() -> String { - format!("{DEFAULT_COMMUNITY_REGISTRY}/base:latest") + "docker.io/library/alpine:3.22".to_string() } /// Resolve a user-supplied image string into a fully-qualified reference. From 037cd5bb77a8a3a5e580af3d836532b8898b8f06 Mon Sep 17 00:00:00 2001 From: Akram Date: Fri, 11 Sep 2026 10:23:16 +0400 Subject: [PATCH 3/9] refactor(cli)!: remove community image resolution Remove DEFAULT_COMMUNITY_REGISTRY, resolve_community_image, and the OPENSHELL_COMMUNITY_REGISTRY override. Bare --from values are no longer expanded into the OpenShell Community registry; the CLI and TUI now pass explicit OCI image references through to the gateway unchanged. The openshell-core image module is reduced to default_sandbox_image(). BREAKING CHANGE: community sandbox shorthand names and OPENSHELL_COMMUNITY_REGISTRY are no longer supported; pass a full OCI image reference to --from. Part of #3116. Signed-off-by: Akram Signed-off-by: Akram --- crates/openshell-cli/src/main.rs | 10 +-- crates/openshell-cli/src/run.rs | 7 +- crates/openshell-core/src/image.rs | 115 +---------------------------- crates/openshell-tui/src/lib.rs | 3 +- 4 files changed, 10 insertions(+), 125 deletions(-) diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index db18795b68..9001ae088f 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1275,13 +1275,9 @@ enum SandboxCommands { #[arg(long, conflicts_with_all = ["from", "gpu", "cpu", "memory", "driver_config_json", "envs"])] template: Option, - /// Sandbox source: a community sandbox name (e.g., `ollama`), a rootfs - /// tar archive (`.tar`, `.tar.gz`, or `.tgz`), or a full container - /// image reference (e.g., `myregistry.com/img:tag`). - /// - /// Community names are resolved to - /// `ghcr.io/nvidia/openshell-community/sandboxes/:latest` - /// (override the prefix with `OPENSHELL_COMMUNITY_REGISTRY`). + /// Sandbox source: a full container image reference (e.g., + /// `docker.io/library/alpine:3.22`, `myregistry.com/img:tag`) or a + /// rootfs tar archive (`.tar`, `.tar.gz`, or `.tgz`). /// /// To use a local Dockerfile, build and tag it with the container /// engine used by your local gateway, then pass the resulting image diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index fbad0d505f..6386eb68f5 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -1234,11 +1234,8 @@ fn resolve_from(value: &str) -> Result { )); } - // Full image reference or community sandbox name — delegate to shared - // resolution in openshell-core. - Ok(ResolvedSource::Image( - openshell_core::image::resolve_community_image(value), - )) + // Explicit OCI image reference — passed through to the gateway unchanged. + Ok(ResolvedSource::Image(value.to_string())) } #[allow(clippy::case_sensitive_file_extension_comparisons)] // already lowercased diff --git a/crates/openshell-core/src/image.rs b/crates/openshell-core/src/image.rs index abf4ff2fa5..df44008e77 100644 --- a/crates/openshell-core/src/image.rs +++ b/crates/openshell-core/src/image.rs @@ -1,17 +1,11 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -//! Shared image-name resolution for community sandbox images. +//! Default sandbox image. //! -//! Both the CLI and TUI need to expand bare sandbox names (e.g. `"base"`) into -//! fully-qualified container image references. This module centralises that -//! logic so every client resolves names identically. - -/// Default registry prefix for community sandbox images. -/// -/// Bare sandbox names are expanded to `{prefix}/{name}:latest`. -/// Override at runtime with the `OPENSHELL_COMMUNITY_REGISTRY` env var. -pub const DEFAULT_COMMUNITY_REGISTRY: &str = "ghcr.io/nvidia/openshell-community/sandboxes"; +//! Provides the fallback image used by all compute drivers when a sandbox spec +//! does not specify one. User-supplied `--from` values are explicit OCI image +//! references passed through unchanged by the CLI and TUI. /// Return the default sandbox image reference. /// @@ -22,104 +16,3 @@ pub const DEFAULT_COMMUNITY_REGISTRY: &str = "ghcr.io/nvidia/openshell-community pub fn default_sandbox_image() -> String { "docker.io/library/alpine:3.22".to_string() } - -/// Resolve a user-supplied image string into a fully-qualified reference. -/// -/// Resolution rules (applied in order): -/// 1. If the value contains `/`, `:`, or `.` it is treated as a complete image -/// reference and returned as-is. -/// 2. Otherwise it is treated as a community sandbox name and expanded to -/// `{registry}/{value}:latest` where `{registry}` defaults to -/// [`DEFAULT_COMMUNITY_REGISTRY`] but can be overridden via the -/// `OPENSHELL_COMMUNITY_REGISTRY` environment variable. -/// -/// This function only handles image-name resolution. Dockerfile detection is -/// the responsibility of the caller (e.g. the CLI's `resolve_from()`). -pub fn resolve_community_image(value: &str) -> String { - // Already a fully-qualified reference. - if value.contains('/') || value.contains(':') || value.contains('.') { - return value.to_string(); - } - - // Community sandbox shorthand → expand with registry prefix. - let prefix = std::env::var("OPENSHELL_COMMUNITY_REGISTRY") - .unwrap_or_else(|_| DEFAULT_COMMUNITY_REGISTRY.to_string()); - let prefix = prefix.trim_end_matches('/'); - format!("{prefix}/{value}:latest") -} - -#[cfg(test)] -#[allow(unsafe_code)] -mod tests { - use super::*; - use std::sync::{Mutex, OnceLock}; - - fn env_lock() -> &'static Mutex<()> { - static ENV_LOCK: OnceLock> = OnceLock::new(); - ENV_LOCK.get_or_init(|| Mutex::new(())) - } - - #[test] - fn bare_name_expands_to_community_registry() { - let _guard = env_lock().lock().unwrap(); - let result = resolve_community_image("base"); - assert_eq!( - result, - "ghcr.io/nvidia/openshell-community/sandboxes/base:latest" - ); - } - - #[test] - fn bare_name_with_env_override() { - let _guard = env_lock().lock().unwrap(); - // Use a temp env override. Safety: test-only, and these env-var tests - // are not run concurrently with other tests reading the same var. - let key = "OPENSHELL_COMMUNITY_REGISTRY"; - let prev = std::env::var(key).ok(); - // SAFETY: single-threaded test context; no other thread reads this var. - unsafe { std::env::set_var(key, "my-registry.example.com/sandboxes") }; - let result = resolve_community_image("python"); - assert_eq!(result, "my-registry.example.com/sandboxes/python:latest"); - // Restore. - match prev { - Some(v) => unsafe { std::env::set_var(key, v) }, - None => unsafe { std::env::remove_var(key) }, - } - } - - #[test] - fn full_reference_with_slash_passes_through() { - let _guard = env_lock().lock().unwrap(); - let input = "ghcr.io/myorg/myimage:v1"; - assert_eq!(resolve_community_image(input), input); - } - - #[test] - fn reference_with_colon_passes_through() { - let _guard = env_lock().lock().unwrap(); - let input = "myimage:latest"; - assert_eq!(resolve_community_image(input), input); - } - - #[test] - fn reference_with_dot_passes_through() { - let _guard = env_lock().lock().unwrap(); - let input = "registry.example.com"; - assert_eq!(resolve_community_image(input), input); - } - - #[test] - fn trailing_slash_in_env_is_trimmed() { - let _guard = env_lock().lock().unwrap(); - let key = "OPENSHELL_COMMUNITY_REGISTRY"; - let prev = std::env::var(key).ok(); - // SAFETY: single-threaded test context; no other thread reads this var. - unsafe { std::env::set_var(key, "my-registry.example.com/sandboxes/") }; - let result = resolve_community_image("base"); - assert_eq!(result, "my-registry.example.com/sandboxes/base:latest"); - match prev { - Some(v) => unsafe { std::env::set_var(key, v) }, - None => unsafe { std::env::remove_var(key) }, - } - } -} diff --git a/crates/openshell-tui/src/lib.rs b/crates/openshell-tui/src/lib.rs index 174f9910d0..188c4b9668 100644 --- a/crates/openshell-tui/src/lib.rs +++ b/crates/openshell-tui/src/lib.rs @@ -1374,9 +1374,8 @@ fn spawn_create_sandbox(app: &mut App, tx: mpsc::UnboundedSender) { tokio::spawn(async move { let has_custom_image = !image.is_empty(); let template = if has_custom_image { - let resolved = openshell_core::image::resolve_community_image(&image); Some(openshell_core::proto::SandboxTemplate { - image: resolved, + image: image.clone(), ..Default::default() }) } else { From b035d6d930b123dc42da229728a53612af6eda88 Mon Sep 17 00:00:00 2001 From: Akram Date: Fri, 11 Sep 2026 10:26:56 +0400 Subject: [PATCH 4/9] refactor(policy): drop community image /app path from default policy The restrictive default policy granted read-only access to /app, a directory that only existed in the community base image. A generic Alpine default has no /app, so remove it. Landlock best-effort already ignores absent paths; this just stops advertising a community-specific layout in the default. Part of #3116. Signed-off-by: Akram Signed-off-by: Akram --- crates/openshell-policy/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index fe644c3d6a..28eeb29373 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -1282,7 +1282,6 @@ pub fn restrictive_default_policy() -> SandboxPolicy { "/lib".into(), "/proc".into(), "/dev/urandom".into(), - "/app".into(), "/etc".into(), "/var/log".into(), ], From 3375968796a5cde3543f6f0e0d38af2477384888 Mon Sep 17 00:00:00 2001 From: Akram Date: Fri, 11 Sep 2026 13:37:47 +0400 Subject: [PATCH 5/9] feat(driver): default to numeric non-root identity for USER-less images Docker and Podman cleared SANDBOX_UID/GID and forwarded the image's OCI Config.User, so the supervisor rejected any image without a USER (e.g. a plain Alpine base) with 'OCI USER is required'. With the default sandbox image now Alpine, such images must start. When the image declares no OCI USER, the Docker and Podman drivers now supply a numeric non-root identity (DEFAULT_SANDBOX_UID/GID = 1000) instead of an empty declaration, mirroring the Kubernetes and VM drivers. The supervisor's existing resolved-identity path synthesizes the passwd/group entry, so the sandbox runs non-root without the account existing in the image. Images that declare a USER keep the OCI resolution path unchanged. The identity normalization already prefers a resolved numeric pair over an empty OCI declaration (covered by driver_identity_inputs_are_mutually_exclusive_and_complete), so no supervisor logic change is needed. Part of #3116. Signed-off-by: Akram Signed-off-by: Akram --- crates/openshell-core/src/sandbox_env.rs | 12 ++++++ crates/openshell-driver-docker/src/lib.rs | 43 +++++++++++++------ .../openshell-driver-podman/src/container.rs | 43 +++++++++++++------ .../src/identity.rs | 6 ++- 4 files changed, 78 insertions(+), 26 deletions(-) diff --git a/crates/openshell-core/src/sandbox_env.rs b/crates/openshell-core/src/sandbox_env.rs index 2ce8e4b058..2f888f6fd8 100644 --- a/crates/openshell-core/src/sandbox_env.rs +++ b/crates/openshell-core/src/sandbox_env.rs @@ -224,6 +224,18 @@ pub const SANDBOX_UID: &str = "OPENSHELL_SANDBOX_UID"; /// supervisor drops privileges to a group other than the UID's primary group. pub const SANDBOX_GID: &str = "OPENSHELL_SANDBOX_GID"; +/// Default numeric UID assigned to a sandbox when the image declares no OCI +/// `USER` (e.g. a plain Alpine base). +/// +/// Local container drivers (Docker, Podman) supply this in place of an empty +/// OCI declaration so the supervisor runs the sandbox as a synthesized non-root +/// account instead of rejecting the image, matching the numeric-identity +/// behavior of the Kubernetes and VM drivers. +pub const DEFAULT_SANDBOX_UID: u32 = 1000; + +/// Default numeric GID paired with [`DEFAULT_SANDBOX_UID`]. +pub const DEFAULT_SANDBOX_GID: u32 = 1000; + /// Raw OCI `Config.User` declaration from the immutable image selected by a /// local container driver. /// diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index f3d47c0a2d..962d3e8874 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -2896,18 +2896,37 @@ fn build_environment_for_oci_user( // hostname could otherwise present a certificate for a name they control // and intercept the sandbox JWT. environment.remove(openshell_core::sandbox_env::GATEWAY_TLS_SERVER_NAME); - environment.insert( - openshell_core::sandbox_env::OCI_IMAGE_USER.to_string(), - oci_user.to_string(), - ); - environment.insert( - openshell_core::sandbox_env::SANDBOX_UID.to_string(), - String::new(), - ); - environment.insert( - openshell_core::sandbox_env::SANDBOX_GID.to_string(), - String::new(), - ); + if oci_user.is_empty() { + // The image declares no OCI USER (e.g. a plain Alpine base). Assign a + // numeric non-root identity like the Kubernetes and VM drivers so the + // supervisor synthesizes the account instead of rejecting the image. + environment.insert( + openshell_core::sandbox_env::OCI_IMAGE_USER.to_string(), + String::new(), + ); + environment.insert( + openshell_core::sandbox_env::SANDBOX_UID.to_string(), + openshell_core::sandbox_env::DEFAULT_SANDBOX_UID.to_string(), + ); + environment.insert( + openshell_core::sandbox_env::SANDBOX_GID.to_string(), + openshell_core::sandbox_env::DEFAULT_SANDBOX_GID.to_string(), + ); + } else { + // The image declares a USER; preserve the OCI resolution path. + environment.insert( + openshell_core::sandbox_env::OCI_IMAGE_USER.to_string(), + oci_user.to_string(), + ); + environment.insert( + openshell_core::sandbox_env::SANDBOX_UID.to_string(), + String::new(), + ); + environment.insert( + openshell_core::sandbox_env::SANDBOX_GID.to_string(), + String::new(), + ); + } // Gateway-minted sandbox JWT. Keep the raw bearer out of container // metadata; the supervisor reads it from this driver-owned bind mount. diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index a81ee13e1d..7b65abd57b 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -578,18 +578,37 @@ fn build_env( // hostname could otherwise present a certificate for a name they control // and intercept the sandbox JWT. env.remove(openshell_core::sandbox_env::GATEWAY_TLS_SERVER_NAME); - env.insert( - openshell_core::sandbox_env::OCI_IMAGE_USER.into(), - oci_user.to_string(), - ); - env.insert( - openshell_core::sandbox_env::SANDBOX_UID.into(), - String::new(), - ); - env.insert( - openshell_core::sandbox_env::SANDBOX_GID.into(), - String::new(), - ); + if oci_user.is_empty() { + // The image declares no OCI USER (e.g. a plain Alpine base). Assign a + // numeric non-root identity like the Kubernetes and VM drivers so the + // supervisor synthesizes the account instead of rejecting the image. + env.insert( + openshell_core::sandbox_env::OCI_IMAGE_USER.into(), + String::new(), + ); + env.insert( + openshell_core::sandbox_env::SANDBOX_UID.into(), + openshell_core::sandbox_env::DEFAULT_SANDBOX_UID.to_string(), + ); + env.insert( + openshell_core::sandbox_env::SANDBOX_GID.into(), + openshell_core::sandbox_env::DEFAULT_SANDBOX_GID.to_string(), + ); + } else { + // The image declares a USER; preserve the OCI resolution path. + env.insert( + openshell_core::sandbox_env::OCI_IMAGE_USER.into(), + oci_user.to_string(), + ); + env.insert( + openshell_core::sandbox_env::SANDBOX_UID.into(), + String::new(), + ); + env.insert( + openshell_core::sandbox_env::SANDBOX_GID.into(), + String::new(), + ); + } // 4. Gateway-minted sandbox JWT. Keep the raw bearer out of container // metadata; the supervisor reads it from a driver-owned bind mount. diff --git a/crates/openshell-supervisor-process/src/identity.rs b/crates/openshell-supervisor-process/src/identity.rs index df79a4137d..feb65ecb12 100644 --- a/crates/openshell-supervisor-process/src/identity.rs +++ b/crates/openshell-supervisor-process/src/identity.rs @@ -44,8 +44,10 @@ impl DriverIdentity { ) -> Result { // Resolved-identity drivers explicitly clear the OCI declaration so // an image-baked or user-supplied value cannot select the OCI path. - // Preserve an empty declaration when no resolved pair is present: - // Docker and Podman use that state to reject images without USER. + // Preserve an empty declaration when no resolved pair is present so a + // bare OCI path still rejects a USER-less image; container drivers now + // pair an empty declaration with a numeric default for USER-less images, + // which selects the resolved path here instead of rejecting. let oci_user = if oci_user.as_deref() == Some("") && (uid.is_some() || gid.is_some()) { None } else { From 4aef3a4f3dc9fd8e3511a074f3e4e19387d28ff2 Mon Sep 17 00:00:00 2001 From: Akram Date: Fri, 11 Sep 2026 15:57:23 +0400 Subject: [PATCH 6/9] feat(deploy): default deployment configs to the official Alpine sandbox image Update the shared gateway default_image, Helm chart values, the standalone Kubernetes manifest, and the dev gateway task scripts to use docker.io/library/alpine:3.22 instead of the community base image, consistent with default_sandbox_image(). GPU e2e image-build base is left unchanged (CUDA needs a glibc base). Part of #3116. Signed-off-by: Akram Signed-off-by: Akram --- deploy/docker/gateway.toml | 2 +- deploy/helm/openshell/values.yaml | 2 +- deploy/kube/manifests/openshell-helmchart.yaml | 2 +- tasks/scripts/gateway-docker.sh | 2 +- tasks/scripts/gateway-podman.sh | 2 +- tasks/scripts/gateway-vm.sh | 2 +- tasks/scripts/gateway.sh | 2 +- tasks/scripts/helm-k3s-local.sh | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/deploy/docker/gateway.toml b/deploy/docker/gateway.toml index 4fe84d633a..80e20cadc0 100644 --- a/deploy/docker/gateway.toml +++ b/deploy/docker/gateway.toml @@ -35,7 +35,7 @@ disable_tls = true [openshell.drivers.docker] # Default image pulled for `openshell sandbox create` without --from. -default_image = "ghcr.io/nvidia/openshell-community/sandboxes/base:latest" +default_image = "docker.io/library/alpine:3.22" # Supervisor image from which the openshell-sandbox binary is extracted on # first start. The binary is cached to XDG_DATA_HOME and reused on restart. supervisor_image = "ghcr.io/nvidia/openshell/supervisor:latest" diff --git a/deploy/helm/openshell/values.yaml b/deploy/helm/openshell/values.yaml index 453654f790..49eb1f9527 100644 --- a/deploy/helm/openshell/values.yaml +++ b/deploy/helm/openshell/values.yaml @@ -223,7 +223,7 @@ server: # `uri` key, e.g. postgresql://user:pass@host:5432/dbname. externalDbSecret: "" # -- Default sandbox image used when requests do not specify one. - sandboxImage: "ghcr.io/nvidia/openshell-community/sandboxes/base:latest" + sandboxImage: "docker.io/library/alpine:3.22" # -- Kubernetes imagePullPolicy for sandbox pods. Empty = Kubernetes default # (Always for :latest, IfNotPresent otherwise). Set to "Always" for dev # clusters so new images are picked up without manual eviction. diff --git a/deploy/kube/manifests/openshell-helmchart.yaml b/deploy/kube/manifests/openshell-helmchart.yaml index 3ca6e3b902..8fd83c2796 100644 --- a/deploy/kube/manifests/openshell-helmchart.yaml +++ b/deploy/kube/manifests/openshell-helmchart.yaml @@ -29,7 +29,7 @@ spec: tag: latest pullPolicy: __IMAGE_PULL_POLICY__ server: - sandboxImage: ghcr.io/nvidia/openshell-community/sandboxes/base:latest + sandboxImage: docker.io/library/alpine:3.22 sandboxImagePullPolicy: __SANDBOX_IMAGE_PULL_POLICY__ supervisorImage: ghcr.io/nvidia/openshell/supervisor:latest dbUrl: __DB_URL__ diff --git a/tasks/scripts/gateway-docker.sh b/tasks/scripts/gateway-docker.sh index 5a6fb62887..bf43e33498 100644 --- a/tasks/scripts/gateway-docker.sh +++ b/tasks/scripts/gateway-docker.sh @@ -28,7 +28,7 @@ PORT="${OPENSHELL_SERVER_PORT:-18080}" GATEWAY_NAME="${OPENSHELL_DOCKER_GATEWAY_NAME:-docker-dev}" STATE_DIR="${OPENSHELL_DOCKER_GATEWAY_STATE_DIR:-${ROOT}/.cache/gateway-docker}" SANDBOX_NAMESPACE="${OPENSHELL_SANDBOX_NAMESPACE:-docker-dev}" -SANDBOX_IMAGE="${OPENSHELL_SANDBOX_IMAGE:-ghcr.io/nvidia/openshell-community/sandboxes/base:latest}" +SANDBOX_IMAGE="${OPENSHELL_SANDBOX_IMAGE:-docker.io/library/alpine:3.22}" SANDBOX_IMAGE_PULL_POLICY="${OPENSHELL_SANDBOX_IMAGE_PULL_POLICY:-IfNotPresent}" LOG_LEVEL="${OPENSHELL_LOG_LEVEL:-info}" GATEWAY_BIN="${ROOT}/target/debug/openshell-gateway" diff --git a/tasks/scripts/gateway-podman.sh b/tasks/scripts/gateway-podman.sh index ab166865ef..1dbe47f92f 100644 --- a/tasks/scripts/gateway-podman.sh +++ b/tasks/scripts/gateway-podman.sh @@ -25,7 +25,7 @@ PORT="${OPENSHELL_SERVER_PORT:-18080}" GATEWAY_NAME="${OPENSHELL_PODMAN_GATEWAY_NAME:-podman-dev}" STATE_DIR="${OPENSHELL_PODMAN_GATEWAY_STATE_DIR:-${OPENSHELL_GATEWAY_STATE_DIR:-${ROOT}/.cache/gateway-podman}}" SANDBOX_NAMESPACE="${OPENSHELL_SANDBOX_NAMESPACE:-podman-dev}" -SANDBOX_IMAGE="${OPENSHELL_SANDBOX_IMAGE:-ghcr.io/nvidia/openshell-community/sandboxes/base:latest}" +SANDBOX_IMAGE="${OPENSHELL_SANDBOX_IMAGE:-docker.io/library/alpine:3.22}" SANDBOX_IMAGE_PULL_POLICY="${OPENSHELL_SANDBOX_IMAGE_PULL_POLICY:-IfNotPresent}" GRPC_ENDPOINT="${OPENSHELL_GRPC_ENDPOINT:-}" LOG_LEVEL="${OPENSHELL_LOG_LEVEL:-info}" diff --git a/tasks/scripts/gateway-vm.sh b/tasks/scripts/gateway-vm.sh index 80d723eb11..44f1cd8dfe 100755 --- a/tasks/scripts/gateway-vm.sh +++ b/tasks/scripts/gateway-vm.sh @@ -37,7 +37,7 @@ PORT="${OPENSHELL_SERVER_PORT:-18081}" GATEWAY_NAME="${OPENSHELL_VM_GATEWAY_NAME:-vm-dev}" STATE_DIR="${OPENSHELL_VM_GATEWAY_STATE_DIR:-${ROOT}/.cache/gateway-vm}" SANDBOX_NAMESPACE="${OPENSHELL_SANDBOX_NAMESPACE:-vm-dev}" -SANDBOX_IMAGE="${OPENSHELL_SANDBOX_IMAGE:-${COMMUNITY_SANDBOX_IMAGE:-ghcr.io/nvidia/openshell-community/sandboxes/base:latest}}" +SANDBOX_IMAGE="${OPENSHELL_SANDBOX_IMAGE:-${COMMUNITY_SANDBOX_IMAGE:-docker.io/library/alpine:3.22}}" VM_BOOTSTRAP_IMAGE="${OPENSHELL_VM_BOOTSTRAP_IMAGE:-}" SANDBOX_IMAGE_PULL_POLICY="${OPENSHELL_SANDBOX_IMAGE_PULL_POLICY:-IfNotPresent}" LOG_LEVEL="${OPENSHELL_LOG_LEVEL:-info}" diff --git a/tasks/scripts/gateway.sh b/tasks/scripts/gateway.sh index 906379df79..fb282dc2a0 100644 --- a/tasks/scripts/gateway.sh +++ b/tasks/scripts/gateway.sh @@ -205,7 +205,7 @@ PORT="${OPENSHELL_SERVER_PORT:-8080}" GATEWAY_NAME="${OPENSHELL_GATEWAY_NAME:-${DRIVER}-dev}" STATE_DIR="${OPENSHELL_GATEWAY_STATE_DIR:-${ROOT}/.cache/gateway-${DRIVER}}" SANDBOX_NAMESPACE="${OPENSHELL_SANDBOX_NAMESPACE:-${DRIVER}-dev}" -SANDBOX_IMAGE="${OPENSHELL_SANDBOX_IMAGE:-ghcr.io/nvidia/openshell-community/sandboxes/base:latest}" +SANDBOX_IMAGE="${OPENSHELL_SANDBOX_IMAGE:-docker.io/library/alpine:3.22}" SANDBOX_IMAGE_PULL_POLICY="${OPENSHELL_SANDBOX_IMAGE_PULL_POLICY:-IfNotPresent}" GRPC_ENDPOINT="${OPENSHELL_GRPC_ENDPOINT:-}" LOG_LEVEL="${OPENSHELL_LOG_LEVEL:-info}" diff --git a/tasks/scripts/helm-k3s-local.sh b/tasks/scripts/helm-k3s-local.sh index dc8adb9bdf..f93dbc3fb6 100755 --- a/tasks/scripts/helm-k3s-local.sh +++ b/tasks/scripts/helm-k3s-local.sh @@ -29,7 +29,7 @@ K3D_CLUSTER_NAME_MAX=32 HOST_LB_PORT="${HELM_K3S_LB_HOST_PORT:-8080}" # Preload the default community sandbox image so the first sandbox create does # not pay the full registry pull cost inside the cluster. -DEFAULT_SANDBOX_PRELOAD_IMAGE="ghcr.io/nvidia/openshell-community/sandboxes/base:latest" +DEFAULT_SANDBOX_PRELOAD_IMAGE="docker.io/library/alpine:3.22" PRELOAD_SANDBOX_IMAGE="${HELM_K3S_PRELOAD_SANDBOX_IMAGE-${DEFAULT_SANDBOX_PRELOAD_IMAGE}}" # Upstream agent-sandbox release pinned for both CRDs/controller and extensions. From fdf8b8021e794ce8d7ea577acfba3289f26baea1 Mon Sep 17 00:00:00 2001 From: Akram Date: Fri, 11 Sep 2026 17:42:09 +0400 Subject: [PATCH 7/9] feat(supervisor): add rtnetlink-based netns programming module Introduce netns/netlink.rs, which creates and tears down the sandbox network namespace, veth pair, addresses, and routes entirely in-process over route netlink (rtnetlink) plus unshare/setns/mount syscalls, with no external ip/nsenter binary. The namespace is FD-owned: a short-lived thread unshares CLONE_NEWNET and bind-mounts it onto netns_path via mount(2) (not `ip netns add`), so bare Alpine works and the nsenter-based nft path still reaches it. rtnetlink is async and confined here, driven from synchronous callers via a local current-thread runtime; namespace-scoped operations run on a dedicated setns thread. Refs #3281 Signed-off-by: Akram --- .../openshell-supervisor-process/Cargo.toml | 3 + .../src/netns/netlink.rs | 484 ++++++++++++++++++ 2 files changed, 487 insertions(+) create mode 100644 crates/openshell-supervisor-process/src/netns/netlink.rs diff --git a/crates/openshell-supervisor-process/Cargo.toml b/crates/openshell-supervisor-process/Cargo.toml index 2e2120f1d0..4fab166011 100644 --- a/crates/openshell-supervisor-process/Cargo.toml +++ b/crates/openshell-supervisor-process/Cargo.toml @@ -39,7 +39,10 @@ rustix = { workspace = true } [target.'cfg(target_os = "linux")'.dependencies] capctl = "0.2.4" +futures-util = { version = "0.3", default-features = false } landlock = "0.4" +netlink-packet-route = "0.19" +rtnetlink = "0.14" seccompiler = "0.5" socket2 = { workspace = true } tempfile = "3" diff --git a/crates/openshell-supervisor-process/src/netns/netlink.rs b/crates/openshell-supervisor-process/src/netns/netlink.rs new file mode 100644 index 0000000000..e4e50fd464 --- /dev/null +++ b/crates/openshell-supervisor-process/src/netns/netlink.rs @@ -0,0 +1,484 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! In-process network-namespace programming over route netlink. +//! +//! Replaces shelling out to `ip`/`nsenter` for sandbox netns setup. All +//! `rtnetlink` (async) work is confined here, driven from synchronous callers +//! via a local `current_thread` runtime. Host-side operations run on the +//! calling thread; namespace-scoped operations run on a dedicated OS thread +//! that `setns()` into the target namespace first. + +use std::ffi::CString; +use std::future::Future; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; +use std::os::unix::ffi::OsStrExt; +use std::os::unix::io::RawFd; +use std::path::Path; + +use futures_util::stream::TryStreamExt; +use miette::{IntoDiagnostic, Result, miette}; +use netlink_packet_route::route::{RouteAddress, RouteAttribute}; + +/// Bind-mount the calling thread's network namespace onto `target`. +/// +/// Equivalent to what `ip netns add` does internally, but via `mount(2)` so no +/// external binary is required. Must run on the thread that just `unshare`d. +fn bind_mount_current_netns(target: &Path) -> Result<()> { + let src = CString::new("/proc/thread-self/ns/net").expect("static path has no NUL"); + let tgt = CString::new(target.as_os_str().as_bytes()).into_diagnostic()?; + let fstype = CString::new("none").expect("static string has no NUL"); + // SAFETY: libc FFI; bind-mounts the netns inode onto the target file. + #[allow(unsafe_code)] + let rc = unsafe { + libc::mount( + src.as_ptr(), + tgt.as_ptr(), + fstype.as_ptr(), + libc::MS_BIND, + std::ptr::null(), + ) + }; + if rc != 0 { + return Err(miette!( + "bind-mount netns onto {} failed: {}", + target.display(), + std::io::Error::last_os_error() + )); + } + Ok(()) +} + +/// Unmount and remove the netns bind-mount file at `ns_path` (best-effort +/// unmount; propagates only the file-removal error). +fn unmount_and_remove(ns_path: &Path) -> Result<()> { + if let Ok(tgt) = CString::new(ns_path.as_os_str().as_bytes()) { + // SAFETY: libc FFI; lazy detach so a busy mount still unwinds. + #[allow(unsafe_code)] + unsafe { + libc::umount2(tgt.as_ptr(), libc::MNT_DETACH); + } + } + std::fs::remove_file(ns_path).into_diagnostic() +} + +/// Create a fresh, FD-owned network namespace named `name`. +/// +/// Runs `unshare(CLONE_NEWNET)` on a short-lived thread and bind-mounts the new +/// namespace onto `netns_path(name)` (via `mount(2)`, not `ip netns add`) so it +/// persists and stays reachable for the `nsenter`-based nft path. Returns a raw +/// fd opened on that path for the `setns` paths. The caller owns both and frees +/// them with [`destroy_netns`]. +pub fn create_netns_fd(name: &str) -> Result { + let ns_path = openshell_core::container_paths::netns_path(name); + if let Some(dir) = ns_path.parent() { + std::fs::create_dir_all(dir).into_diagnostic()?; + } + // Create the mount-target file (as `ip netns add` does). + std::fs::OpenOptions::new() + .create(true) + .write(true) + .truncate(false) + .open(&ns_path) + .into_diagnostic()?; + + // Unshare a new netns on a dedicated thread and bind-mount it onto the + // target path. `/proc/thread-self` reflects THIS thread's namespaces, + // unlike `/proc/self` which follows the thread-group leader. + let target = ns_path.clone(); + let (tx, rx) = std::sync::mpsc::channel::>(); + std::thread::spawn(move || { + let result = (|| -> Result<()> { + // SAFETY: unshare affects only this dedicated, short-lived thread. + #[allow(unsafe_code)] + if unsafe { libc::unshare(libc::CLONE_NEWNET) } != 0 { + return Err(miette!( + "unshare(CLONE_NEWNET) failed: {}", + std::io::Error::last_os_error() + )); + } + bind_mount_current_netns(&target) + })(); + let _ = tx.send(result); + }); + if let Err(e) = rx + .recv() + .map_err(|_| miette!("netns creation thread panicked"))? + { + let _ = std::fs::remove_file(&ns_path); + return Err(e); + } + + // Open a persistent fd on the bind-mounted netns for the `setns` paths. + match nix::fcntl::open( + ns_path.as_path(), + nix::fcntl::OFlag::O_RDONLY, + nix::sys::stat::Mode::empty(), + ) { + Ok(fd) => Ok(fd), + Err(e) => { + let _ = unmount_and_remove(&ns_path); + Err(e).into_diagnostic() + } + } +} + +/// Tear down a namespace created by [`create_netns_fd`]: close the fd, unmount +/// the bind mount, and remove the target file. +pub fn destroy_netns(name: &str, fd: RawFd) -> Result<()> { + // SAFETY: fd is owned by the caller and dropped here. + #[allow(unsafe_code)] + unsafe { + libc::close(fd); + } + let ns_path = openshell_core::container_paths::netns_path(name); + unmount_and_remove(&ns_path) +} + +/// Build a local current-thread runtime, open a route-netlink connection +/// scoped to the current thread's network namespace, run `f`, then tear the +/// connection task down. +fn block_on_netlink(f: impl FnOnce(rtnetlink::Handle) -> F) -> Result +where + F: Future>, +{ + let rt = tokio::runtime::Builder::new_current_thread() + .enable_io() + .build() + .into_diagnostic()?; + rt.block_on(async move { + let (connection, handle, _) = rtnetlink::new_connection().into_diagnostic()?; + let conn_task = tokio::spawn(connection); + let result = f(handle).await; + conn_task.abort(); + result + }) +} + +/// Resolve a link index by interface name in the current netns. +async fn link_index_by_name(handle: &rtnetlink::Handle, name: &str) -> Result { + let mut links = handle.link().get().match_name(name.to_string()).execute(); + let msg = links + .try_next() + .await + .into_diagnostic()? + .ok_or_else(|| miette!("link {name} not found"))?; + Ok(msg.header.index) +} + +/// Create the veth pair, move the sandbox peer into `ns_fd`, and configure the +/// host end (address + up). Runs in the caller's (host) network namespace. +pub fn setup_host_side( + veth_host: &str, + veth_sandbox: &str, + host_ip: IpAddr, + prefix: u8, + ns_fd: RawFd, +) -> Result<()> { + let veth_host = veth_host.to_string(); + let veth_sandbox = veth_sandbox.to_string(); + block_on_netlink(move |handle| async move { + // Create veth pair. + handle + .link() + .add() + .veth(veth_host.clone(), veth_sandbox.clone()) + .execute() + .await + .into_diagnostic()?; + + // Move the sandbox peer into the target namespace by fd. + let sandbox_idx = link_index_by_name(&handle, &veth_sandbox).await?; + handle + .link() + .set(sandbox_idx) + .setns_by_fd(ns_fd) + .execute() + .await + .into_diagnostic()?; + + // Configure the host end: address + up. + let host_idx = link_index_by_name(&handle, &veth_host).await?; + handle + .address() + .add(host_idx, host_ip, prefix) + .execute() + .await + .into_diagnostic()?; + handle + .link() + .set(host_idx) + .up() + .execute() + .await + .into_diagnostic()?; + Ok(()) + }) +} + +/// Run `work` on a dedicated OS thread that has `setns()`'d into `ns_fd`. +/// +/// Mirrors the existing `bind_tcp_in_netns` pattern: a short-lived thread +/// enters the network namespace and exits, so no thread-pool worker is left +/// with contaminated namespace state. +fn in_netns_thread(ns_fd: RawFd, work: W) -> Result +where + T: Send + 'static, + W: FnOnce() -> Result + Send + 'static, +{ + let (tx, rx) = std::sync::mpsc::channel::>(); + std::thread::spawn(move || { + let result = (|| -> Result { + // SAFETY: setns on a dedicated, short-lived thread. + #[allow(unsafe_code)] + if unsafe { libc::setns(ns_fd, libc::CLONE_NEWNET) } != 0 { + return Err(miette!("setns failed: {}", std::io::Error::last_os_error())); + } + work() + })(); + let _ = tx.send(result); + }); + rx.recv() + .map_err(|_| miette!("netns worker thread panicked"))? +} + +/// Configure the sandbox end inside the namespace: address, link up, loopback +/// up, and default route via the host gateway. +pub fn setup_sandbox_side( + ns_fd: RawFd, + veth_sandbox: &str, + sandbox_ip: IpAddr, + prefix: u8, + gateway: IpAddr, +) -> Result<()> { + let veth_sandbox = veth_sandbox.to_string(); + in_netns_thread(ns_fd, move || { + block_on_netlink(move |handle| async move { + let sandbox_idx = link_index_by_name(&handle, &veth_sandbox).await?; + handle + .address() + .add(sandbox_idx, sandbox_ip, prefix) + .execute() + .await + .into_diagnostic()?; + handle + .link() + .set(sandbox_idx) + .up() + .execute() + .await + .into_diagnostic()?; + + let lo_idx = link_index_by_name(&handle, "lo").await?; + handle + .link() + .set(lo_idx) + .up() + .execute() + .await + .into_diagnostic()?; + + // Default route via the host gateway. + let route = handle.route().add(); + match gateway { + IpAddr::V4(gw) => route.v4().gateway(gw).execute().await.into_diagnostic()?, + IpAddr::V6(gw) => route.v6().gateway(gw).execute().await.into_diagnostic()?, + } + Ok(()) + }) + }) +} + +/// Delete a link by name in the current (host) network namespace. Removing a +/// veth end removes its peer too. +pub fn delete_link(name: &str) -> Result<()> { + let name = name.to_string(); + block_on_netlink(move |handle| async move { + let idx = link_index_by_name(&handle, &name).await?; + handle.link().del(idx).execute().await.into_diagnostic()?; + Ok(()) + }) +} + +/// Replace a route for `cidr` with output interface `lo`, inside `ns_fd`. +/// Used to attract the synthetic IPv6 pool to the local transparent listener. +pub fn replace_route_dev_lo_in_netns(ns_fd: RawFd, cidr: ipnet::IpNet) -> Result<()> { + in_netns_thread(ns_fd, move || { + block_on_netlink(move |handle| async move { + let lo = link_index_by_name(&handle, "lo").await?; + match cidr { + ipnet::IpNet::V4(n) => handle + .route() + .add() + .v4() + .destination_prefix(n.addr(), n.prefix_len()) + .output_interface(lo) + .replace() + .execute() + .await + .into_diagnostic()?, + ipnet::IpNet::V6(n) => handle + .route() + .add() + .v6() + .destination_prefix(n.addr(), n.prefix_len()) + .output_interface(lo) + .replace() + .execute() + .await + .into_diagnostic()?, + } + Ok(()) + }) + }) +} + +/// Dump destination prefixes of all routes for one family inside `ns_fd`. A +/// missing destination attribute denotes the default route and is returned as +/// `0.0.0.0/0` or `::/0`. +pub fn dump_route_prefixes_in_netns(ns_fd: RawFd, v6: bool) -> Result> { + in_netns_thread(ns_fd, move || { + block_on_netlink(move |handle| async move { + let ip_version = if v6 { + rtnetlink::IpVersion::V6 + } else { + rtnetlink::IpVersion::V4 + }; + let mut routes = handle.route().get(ip_version).execute(); + let mut out = Vec::new(); + while let Some(route) = routes.try_next().await.into_diagnostic()? { + let prefix_len = route.header.destination_prefix_length; + let dst = route.attributes.iter().find_map(|attr| match attr { + RouteAttribute::Destination(RouteAddress::Inet(a)) => Some(IpAddr::V4(*a)), + RouteAttribute::Destination(RouteAddress::Inet6(a)) => Some(IpAddr::V6(*a)), + _ => None, + }); + let addr = dst.unwrap_or(if v6 { + IpAddr::V6(Ipv6Addr::UNSPECIFIED) + } else { + IpAddr::V4(Ipv4Addr::UNSPECIFIED) + }); + if let Ok(net) = ipnet::IpNet::new(addr, prefix_len) { + out.push(net); + } + } + Ok(out) + }) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Root-only: creating an FD-owned netns yields an fd pointing at a + /// different network namespace than the caller's. + #[test] + #[ignore = "requires root / CAP_NET_ADMIN"] + fn create_netns_fd_is_isolated() { + let self_ns = std::fs::read_link("/proc/thread-self/ns/net").unwrap(); + let fd = create_netns_fd("nl-test-iso").expect("create netns fd"); + let created_ns = std::fs::read_link(format!("/proc/self/fd/{fd}")).unwrap(); + assert_ne!( + self_ns, created_ns, + "created namespace must differ from the caller's" + ); + let _ = destroy_netns("nl-test-iso", fd); + } + + /// Root-only: host-side setup creates veth-h on the host and moves veth-s + /// out of the host namespace. + #[test] + #[ignore = "requires root / CAP_NET_ADMIN"] + fn setup_host_side_creates_and_moves() { + let ns_fd = create_netns_fd("nl-test-host").expect("netns fd"); + let host_ip: IpAddr = "10.200.0.1".parse().unwrap(); + let veth_h = "veth-h-test0001"; + let veth_s = "veth-s-test0001"; + + setup_host_side(veth_h, veth_s, host_ip, 24, ns_fd).expect("host side"); + + let host_has_h = block_on_netlink(|h| async move { + Ok(link_index_by_name(&h, "veth-h-test0001").await.is_ok()) + }) + .unwrap(); + assert!(host_has_h, "veth-h must exist on host"); + let host_has_s = block_on_netlink(|h| async move { + Ok(link_index_by_name(&h, "veth-s-test0001").await.is_ok()) + }) + .unwrap(); + assert!(!host_has_s, "veth-s must have moved into the netns"); + + let _ = delete_link(veth_h); + let _ = destroy_netns("nl-test-host", ns_fd); + } + + /// Root-only: after host setup, sandbox-side setup installs the sandbox + /// address and a default route inside the namespace. + #[test] + #[ignore = "requires root / CAP_NET_ADMIN"] + fn setup_sandbox_side_installs_addr_and_route() { + let ns_fd = create_netns_fd("nl-test-sbx").expect("netns fd"); + let host_ip: IpAddr = "10.200.0.1".parse().unwrap(); + let sandbox_ip: IpAddr = "10.200.0.2".parse().unwrap(); + let veth_h = "veth-h-test0002"; + let veth_s = "veth-s-test0002"; + + setup_host_side(veth_h, veth_s, host_ip, 24, ns_fd).expect("host side"); + setup_sandbox_side(ns_fd, veth_s, sandbox_ip, 24, host_ip).expect("sandbox side"); + + let routes = dump_route_prefixes_in_netns(ns_fd, false).expect("dump v4 routes"); + assert!( + routes.iter().any(|p| p.prefix_len() == 0), + "default route must be present in the namespace" + ); + + let _ = delete_link(veth_h); + let _ = destroy_netns("nl-test-sbx", ns_fd); + } + + /// Root-only: delete_link removes a host veth end. + #[test] + #[ignore = "requires root / CAP_NET_ADMIN"] + fn delete_link_removes_interface() { + let ns_fd = create_netns_fd("nl-test-del").expect("netns fd"); + let host_ip: IpAddr = "10.200.0.1".parse().unwrap(); + setup_host_side("veth-h-test0003", "veth-s-test0003", host_ip, 24, ns_fd) + .expect("host side"); + + delete_link("veth-h-test0003").expect("delete"); + + let still_there = block_on_netlink(|h| async move { + Ok(link_index_by_name(&h, "veth-h-test0003").await.is_ok()) + }) + .unwrap(); + assert!(!still_there, "veth-h must be gone after delete_link"); + let _ = destroy_netns("nl-test-del", ns_fd); + } + + /// Root-only: dump_route_prefixes_in_netns sees a route added on lo. + #[test] + #[ignore = "requires root / CAP_NET_ADMIN"] + fn replace_and_dump_lo_route() { + let ns_fd = create_netns_fd("nl-test-route").expect("netns fd"); + // lo must be up for a route to install. + in_netns_thread(ns_fd, || { + block_on_netlink(|h| async move { + let lo = link_index_by_name(&h, "lo").await?; + h.link().set(lo).up().execute().await.into_diagnostic()?; + Ok(()) + }) + }) + .unwrap(); + + let cidr: ipnet::IpNet = "fd00:dead:beef::/48".parse().unwrap(); + replace_route_dev_lo_in_netns(ns_fd, cidr).expect("route replace"); + + let routes = dump_route_prefixes_in_netns(ns_fd, true).expect("dump v6"); + assert!( + routes.contains(&cidr), + "installed route must appear in dump" + ); + let _ = destroy_netns("nl-test-route", ns_fd); + } +} From 69e8643f5e46f17625e9d1e1e973d5d0b66bda0e Mon Sep 17 00:00:00 2001 From: Akram Date: Fri, 11 Sep 2026 17:42:09 +0400 Subject: [PATCH 8/9] refactor(supervisor): build sandbox netns via netlink instead of ip/nsenter Rewire NetworkNamespace::create(), Drop, install_transparent_tcp_rules, and validate_synthetic_pool_routes onto the new netlink module, and change first_route_overlap to work on parsed route prefixes. Remove the now-dead run_ip/run_ip_netns/run_ip_netns_output helpers and IP_SEARCH_PATHS, and drop the iproute2 hint from the proxy-mode error. nsenter and the nft helpers stay: the nft-in-namespace path is converted separately (#3282). Refs #3281 Signed-off-by: Akram --- .../src/netns/mod.rs | 271 +++++------------- 1 file changed, 79 insertions(+), 192 deletions(-) diff --git a/crates/openshell-supervisor-process/src/netns/mod.rs b/crates/openshell-supervisor-process/src/netns/mod.rs index 2b4ea554ed..b1315b3300 100644 --- a/crates/openshell-supervisor-process/src/netns/mod.rs +++ b/crates/openshell-supervisor-process/src/netns/mod.rs @@ -7,6 +7,7 @@ //! the sandbox to the host. This ensures the sandboxed process can only //! communicate through the proxy running on the host side of the veth. +mod netlink; mod nft_ruleset; use miette::{IntoDiagnostic, Result}; @@ -26,7 +27,6 @@ const SANDBOX_IP_SUFFIX: u8 = 2; /// this listener before the bypass fence runs. pub const POLICY_DNS_PORT: u16 = 15_053; pub const TRANSPARENT_TCP_PORT: u16 = 15_001; -const IP_SEARCH_PATHS: &[&str] = &["/usr/sbin/ip", "/sbin/ip", "/usr/bin/ip", "/bin/ip"]; const NSENTER_SEARCH_PATHS: &[&str] = &[ "/usr/bin/nsenter", "/bin/nsenter", @@ -88,88 +88,25 @@ impl NetworkNamespace { .build() ); - // Create the namespace - run_ip(&["netns", "add", &name])?; - - // Create veth pair - if let Err(e) = run_ip(&[ - "link", - "add", - &veth_host, - "type", - "veth", - "peer", - "name", - &veth_sandbox, - ]) { - // Cleanup namespace on failure - let _ = run_ip(&["netns", "delete", &name]); - return Err(e); - } + // Create the FD-owned namespace, bind-mounted at netns_path via + // `mount(2)` (not `ip netns add`) so the nsenter-based nft path still + // reaches it. Returns a persistent fd for the setns paths. + let ns_fd = netlink::create_netns_fd(&name)?; - // Move sandbox veth into namespace - if let Err(e) = run_ip(&["link", "set", &veth_sandbox, "netns", &name]) { - let _ = run_ip(&["link", "delete", &veth_host]); - let _ = run_ip(&["netns", "delete", &name]); + // Host side: veth pair, move peer into the namespace, host addr + up. + if let Err(e) = netlink::setup_host_side(&veth_host, &veth_sandbox, host_ip, 24, ns_fd) { + let _ = netlink::destroy_netns(&name, ns_fd); return Err(e); } - // Configure host side - let host_cidr = format!("{host_ip}/24"); - if let Err(e) = run_ip(&["addr", "add", &host_cidr, "dev", &veth_host]) { - let _ = run_ip(&["link", "delete", &veth_host]); - let _ = run_ip(&["netns", "delete", &name]); + // Sandbox side: addr on veth-s, veth-s up, lo up, default route. + if let Err(e) = netlink::setup_sandbox_side(ns_fd, &veth_sandbox, sandbox_ip, 24, host_ip) { + let _ = netlink::delete_link(&veth_host); + let _ = netlink::destroy_netns(&name, ns_fd); return Err(e); } - if let Err(e) = run_ip(&["link", "set", &veth_host, "up"]) { - let _ = run_ip(&["link", "delete", &veth_host]); - let _ = run_ip(&["netns", "delete", &name]); - return Err(e); - } - - // Configure sandbox side (inside namespace) - let sandbox_cidr = format!("{sandbox_ip}/24"); - if let Err(e) = run_ip_netns(&name, &["addr", "add", &sandbox_cidr, "dev", &veth_sandbox]) { - let _ = run_ip(&["link", "delete", &veth_host]); - let _ = run_ip(&["netns", "delete", &name]); - return Err(e); - } - - if let Err(e) = run_ip_netns(&name, &["link", "set", &veth_sandbox, "up"]) { - let _ = run_ip(&["link", "delete", &veth_host]); - let _ = run_ip(&["netns", "delete", &name]); - return Err(e); - } - - // Bring up loopback in namespace - if let Err(e) = run_ip_netns(&name, &["link", "set", "lo", "up"]) { - let _ = run_ip(&["link", "delete", &veth_host]); - let _ = run_ip(&["netns", "delete", &name]); - return Err(e); - } - - // Add default route via host - let host_ip_str = host_ip.to_string(); - if let Err(e) = run_ip_netns(&name, &["route", "add", "default", "via", &host_ip_str]) { - let _ = run_ip(&["link", "delete", &veth_host]); - let _ = run_ip(&["netns", "delete", &name]); - return Err(e); - } - - // Open the namespace file descriptor for later use with setns - let ns_path = openshell_core::container_paths::netns_path(&name); - let ns_fd = match nix::fcntl::open( - ns_path.as_path(), - nix::fcntl::OFlag::O_RDONLY, - nix::sys::stat::Mode::empty(), - ) { - Ok(fd) => Some(fd), - Err(e) => { - warn!(error = %e, "Failed to open namespace fd, will use nsenter fallback"); - None - } - }; + let ns_fd = Some(ns_fd); openshell_ocsf::ocsf_emit!( openshell_ocsf::ConfigStateChangeBuilder::new(openshell_ocsf::ctx::ctx()) @@ -337,10 +274,11 @@ impl NetworkNamespace { // default route. Install only the active synthetic IPv6 epoch so the // kernel reaches the nft OUTPUT hook; REDIRECT then reroutes it to // the local transparent listener. - run_ip_netns( - &self.name, - &["-6", "route", "replace", synthetic_ipv6_cidr, "dev", "lo"], - )?; + let ns_fd = self + .ns_fd + .ok_or_else(|| miette::miette!("no namespace fd available for route setup"))?; + let v6_cidr: ipnet::IpNet = synthetic_ipv6_cidr.parse().into_diagnostic()?; + netlink::replace_route_dev_lo_in_netns(ns_fd, v6_cidr)?; let nft_path = find_nft().ok_or_else(|| { miette::miette!( "trusted nft helper not found; policy DNS and transparent TCP require nftables" @@ -385,9 +323,11 @@ impl NetworkNamespace { .parse::() .into_diagnostic()?, ]; - for family in ["-4", "-6"] { - let routes = - run_ip_netns_output(&self.name, &[family, "route", "show", "table", "all"])?; + let ns_fd = self + .ns_fd + .ok_or_else(|| miette::miette!("no namespace fd available for route validation"))?; + for v6 in [false, true] { + let routes = netlink::dump_route_prefixes_in_netns(ns_fd, v6)?; if let Some((route, pool)) = first_route_overlap(&routes, &reserved) { return Err(miette::miette!( "synthetic address pool {pool} overlaps workload route {route}; refusing to enable policy DNS" @@ -552,13 +492,9 @@ impl Drop for NetworkNamespace { fn drop(&mut self) { debug!(namespace = %self.name, "Cleaning up network namespace"); - // Close the fd if we have one - if let Some(fd) = self.ns_fd.take() { - let _ = nix::unistd::close(fd); - } - - // Delete the host-side veth (this also removes the peer) - if let Err(e) = run_ip(&["link", "delete", &self.veth_host]) { + // Delete the host-side veth (this also removes the sandbox peer). + // Do this before freeing the namespace so ordering stays explicit. + if let Err(e) = netlink::delete_link(&self.veth_host) { warn!( error = %e, veth = %self.veth_host, @@ -566,13 +502,16 @@ impl Drop for NetworkNamespace { ); } - // Delete the namespace - if let Err(e) = run_ip(&["netns", "delete", &self.name]) { - warn!( - error = %e, - namespace = %self.name, - "Failed to delete network namespace" - ); + // Free the namespace: close the fd, unmount the bind mount, remove the + // target file (no `ip netns delete`). + if let Some(fd) = self.ns_fd.take() { + if let Err(e) = netlink::destroy_netns(&self.name, fd) { + warn!( + error = %e, + namespace = %self.name, + "Failed to remove network namespace mount" + ); + } } openshell_ocsf::ocsf_emit!( @@ -596,9 +535,9 @@ impl Drop for NetworkNamespace { /// # Errors /// /// Returns an error if proxy mode is requested but the namespace cannot be -/// created (e.g., missing `CAP_NET_ADMIN` / `CAP_SYS_ADMIN` or `iproute2`). -/// Failure to install nftables bypass-detection rules is non-fatal and is -/// reported via OCSF instead. +/// created (e.g., missing `CAP_NET_ADMIN` / `CAP_SYS_ADMIN`). Failure to +/// install nftables bypass-detection rules is non-fatal and is reported via +/// OCSF instead. pub fn create_netns_for_proxy( policy: &openshell_core::policy::SandboxPolicy, ) -> Result> { @@ -632,7 +571,7 @@ pub fn create_netns_for_proxy( } Err(e) => Err(miette::miette!( "Network namespace creation failed and proxy mode requires isolation. \ - Ensure CAP_NET_ADMIN and CAP_SYS_ADMIN are available and iproute2 is installed. \ + Ensure CAP_NET_ADMIN and CAP_SYS_ADMIN are available. \ Error: {e}" )), } @@ -825,29 +764,6 @@ fn cleanup_sidecar_iptables_legacy_rule_families(ipv4_cmd: &str, ipv6_cmd: Optio } } -/// Run an `ip` command on the host. -fn run_ip(args: &[&str]) -> Result<()> { - let ip_path = find_trusted_binary("ip", IP_SEARCH_PATHS)?; - - debug!(command = %format!("{ip_path} {}", args.join(" ")), "Running ip command"); - - let output = Command::new(ip_path) - .args(args) - .output() - .into_diagnostic()?; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(miette::miette!( - "{ip_path} {} failed: {}", - args.join(" "), - stderr.trim() - )); - } - - Ok(()) -} - fn run_iptables_legacy_current_namespace(iptables_cmd: &str, args: &[&str]) -> Result<()> { debug!( command = %format!("{iptables_cmd} {}", args.join(" ")), @@ -910,73 +826,25 @@ fn run_nft_commands_current_namespace( Ok(()) } -/// Run an `ip` command inside a network namespace via `nsenter --net=`. -/// -/// We use `nsenter` instead of `ip netns exec` because `ip netns exec` -/// remounts `/sys` to reflect the target namespace's sysfs entries. That -/// sysfs remount requires real `CAP_SYS_ADMIN` in the host user namespace, -/// which is unavailable in rootless container runtimes (e.g. rootless -/// Podman). `nsenter --net=` enters only the network namespace without -/// changing the mount namespace, avoiding the sysfs remount entirely. -/// The supervisor's operations (addr add, link set, route add) are all -/// netlink-based and do not need sysfs access. -fn run_ip_netns(netns: &str, args: &[&str]) -> Result<()> { - run_ip_netns_output(netns, args).map(|_| ()) -} - -fn run_ip_netns_output(netns: &str, args: &[&str]) -> Result { - let ip_path = find_trusted_binary("ip", IP_SEARCH_PATHS)?; - let nsenter_path = find_trusted_binary("nsenter", NSENTER_SEARCH_PATHS)?; - let ns_path = openshell_core::container_paths::netns_path(netns); - let net_flag = format!("--net={}", ns_path.display()); - - let mut full_args = vec![net_flag.as_str(), "--", ip_path]; - full_args.extend(args); - - debug!( - command = %format!("{nsenter_path} {}", full_args.join(" ")), - "Running ip in namespace via nsenter" - ); - - let output = Command::new(nsenter_path) - .args(&full_args) - .output() - .into_diagnostic()?; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(miette::miette!( - "{nsenter_path} --net={} {ip_path} {} failed: {}", - ns_path.display(), - args.join(" "), - stderr.trim() - )); - } - - Ok(String::from_utf8_lossy(&output.stdout).into_owned()) -} - fn first_route_overlap( - routes: &str, + routes: &[ipnet::IpNet], reserved: &[ipnet::IpNet], ) -> Option<(ipnet::IpNet, ipnet::IpNet)> { - routes.lines().find_map(|line| { - line.split_whitespace().find_map(|token| { - let route = token - .parse::() - .ok() - .or_else(|| token.parse::().ok().map(ipnet::IpNet::from))?; - reserved - .iter() - .copied() - .find(|pool| { - let same_family = route.addr().is_ipv4() == pool.addr().is_ipv4(); - let overlaps = - route.contains(&pool.network()) || pool.contains(&route.network()); - same_family && overlaps - }) - .map(|pool| (route, pool)) - }) + routes.iter().find_map(|route| { + // A default route (0.0.0.0/0 or ::/0) covers everything but never + // shadows a specific synthetic pool, so it is not a real overlap. + if route.prefix_len() == 0 { + return None; + } + reserved + .iter() + .copied() + .find(|pool| { + let same_family = route.addr().is_ipv4() == pool.addr().is_ipv4(); + let overlaps = route.contains(&pool.network()) || pool.contains(&route.network()); + same_family && overlaps + }) + .map(|pool| (*route, pool)) }) } @@ -1111,6 +979,19 @@ mod tests { // These tests require root and network namespace support // Run with: sudo cargo test -- --ignored + /// Root-only: create() builds a working namespace with host veth, sandbox + /// address, and default route — with no external `ip`/`nsenter` process. + #[test] + #[ignore = "requires root / CAP_NET_ADMIN"] + fn create_builds_namespace_via_netlink() { + let ns = NetworkNamespace::create().expect("create netns"); + assert_eq!(ns.host_ip().to_string(), "10.200.0.1"); + assert_eq!(ns.sandbox_ip().to_string(), "10.200.0.2"); + assert!(ns.name().starts_with("sandbox-")); + assert!(ns.ns_fd().is_some(), "namespace must be FD-owned"); + // Dropping ns tears everything down via netlink + fd close. + } + #[test] fn find_trusted_binary_uses_absolute_existing_file() { let tempdir = tempfile::tempdir().unwrap(); @@ -1191,8 +1072,11 @@ fe800000000000000000000000000001 02 40 20 80 eth0 "198.18.1.0/25".parse().unwrap(), "fd23:6f70:656e:1::/120".parse().unwrap(), ]; - let routes = "default via 10.200.0.1 dev veth\n198.18.0.0/15 dev eth1\n"; - let (route, pool) = first_route_overlap(routes, &reserved).expect("collision"); + let routes: Vec = ["0.0.0.0/0", "198.18.0.0/15"] + .iter() + .map(|s| s.parse().unwrap()) + .collect(); + let (route, pool) = first_route_overlap(&routes, &reserved).expect("collision"); assert_eq!(route.to_string(), "198.18.0.0/15"); assert_eq!(pool.to_string(), "198.18.1.0/25"); } @@ -1203,8 +1087,11 @@ fe800000000000000000000000000001 02 40 20 80 eth0 "198.18.1.0/25".parse().unwrap(), "fd23:6f70:656e:1::/120".parse().unwrap(), ]; - let routes = "default via 10.200.0.1 dev veth\n10.200.0.0/24 dev veth\n"; - assert_eq!(first_route_overlap(routes, &reserved), None); + let routes: Vec = ["0.0.0.0/0", "10.200.0.0/24"] + .iter() + .map(|s| s.parse().unwrap()) + .collect(); + assert_eq!(first_route_overlap(&routes, &reserved), None); } #[test] From e75e50296d80e0ad69ed43f4c62d1a2735e40b5a Mon Sep 17 00:00:00 2001 From: Akram Date: Fri, 11 Sep 2026 18:09:43 +0400 Subject: [PATCH 9/9] fix(supervisor): run netlink on a dedicated thread to avoid nested runtime NetworkNamespace::create() is invoked from within a tokio runtime on the podman driver, so building a local current-thread runtime on the calling thread panicked with "Cannot start a runtime from within a runtime". Route all host-side netlink work through a fresh std::thread (on_thread), matching the setns path, so block_on never nests in the caller's runtime. Refs #3281 Signed-off-by: Akram --- .../src/netns/netlink.rs | 129 ++++++++++-------- 1 file changed, 75 insertions(+), 54 deletions(-) diff --git a/crates/openshell-supervisor-process/src/netns/netlink.rs b/crates/openshell-supervisor-process/src/netns/netlink.rs index e4e50fd464..a9019b419d 100644 --- a/crates/openshell-supervisor-process/src/netns/netlink.rs +++ b/crates/openshell-supervisor-process/src/netns/netlink.rs @@ -135,9 +135,32 @@ pub fn destroy_netns(name: &str, fd: RawFd) -> Result<()> { unmount_and_remove(&ns_path) } +/// Run `work` on a fresh, short-lived OS thread and wait for its result. +/// +/// `create()` is invoked from within a tokio runtime on some drivers, so the +/// local `current_thread` runtime in [`block_on_netlink`] must never be built +/// on the caller's thread (that panics with "Cannot start a runtime from +/// within a runtime"). Running on a dedicated thread also gives the setns path +/// a thread whose namespace state is discarded on exit. +fn on_thread(work: W) -> Result +where + T: Send + 'static, + W: FnOnce() -> Result + Send + 'static, +{ + let (tx, rx) = std::sync::mpsc::channel::>(); + std::thread::spawn(move || { + let _ = tx.send(work()); + }); + rx.recv() + .map_err(|_| miette!("netlink worker thread panicked"))? +} + /// Build a local current-thread runtime, open a route-netlink connection /// scoped to the current thread's network namespace, run `f`, then tear the /// connection task down. +/// +/// Must be called on a dedicated thread (see [`on_thread`]) — never directly +/// on a thread already driving a tokio runtime. fn block_on_netlink(f: impl FnOnce(rtnetlink::Handle) -> F) -> Result where F: Future>, @@ -177,42 +200,44 @@ pub fn setup_host_side( ) -> Result<()> { let veth_host = veth_host.to_string(); let veth_sandbox = veth_sandbox.to_string(); - block_on_netlink(move |handle| async move { - // Create veth pair. - handle - .link() - .add() - .veth(veth_host.clone(), veth_sandbox.clone()) - .execute() - .await - .into_diagnostic()?; - - // Move the sandbox peer into the target namespace by fd. - let sandbox_idx = link_index_by_name(&handle, &veth_sandbox).await?; - handle - .link() - .set(sandbox_idx) - .setns_by_fd(ns_fd) - .execute() - .await - .into_diagnostic()?; - - // Configure the host end: address + up. - let host_idx = link_index_by_name(&handle, &veth_host).await?; - handle - .address() - .add(host_idx, host_ip, prefix) - .execute() - .await - .into_diagnostic()?; - handle - .link() - .set(host_idx) - .up() - .execute() - .await - .into_diagnostic()?; - Ok(()) + on_thread(move || { + block_on_netlink(move |handle| async move { + // Create veth pair. + handle + .link() + .add() + .veth(veth_host.clone(), veth_sandbox.clone()) + .execute() + .await + .into_diagnostic()?; + + // Move the sandbox peer into the target namespace by fd. + let sandbox_idx = link_index_by_name(&handle, &veth_sandbox).await?; + handle + .link() + .set(sandbox_idx) + .setns_by_fd(ns_fd) + .execute() + .await + .into_diagnostic()?; + + // Configure the host end: address + up. + let host_idx = link_index_by_name(&handle, &veth_host).await?; + handle + .address() + .add(host_idx, host_ip, prefix) + .execute() + .await + .into_diagnostic()?; + handle + .link() + .set(host_idx) + .up() + .execute() + .await + .into_diagnostic()?; + Ok(()) + }) }) } @@ -226,20 +251,14 @@ where T: Send + 'static, W: FnOnce() -> Result + Send + 'static, { - let (tx, rx) = std::sync::mpsc::channel::>(); - std::thread::spawn(move || { - let result = (|| -> Result { - // SAFETY: setns on a dedicated, short-lived thread. - #[allow(unsafe_code)] - if unsafe { libc::setns(ns_fd, libc::CLONE_NEWNET) } != 0 { - return Err(miette!("setns failed: {}", std::io::Error::last_os_error())); - } - work() - })(); - let _ = tx.send(result); - }); - rx.recv() - .map_err(|_| miette!("netns worker thread panicked"))? + on_thread(move || { + // SAFETY: setns on a dedicated, short-lived thread. + #[allow(unsafe_code)] + if unsafe { libc::setns(ns_fd, libc::CLONE_NEWNET) } != 0 { + return Err(miette!("setns failed: {}", std::io::Error::last_os_error())); + } + work() + }) } /// Configure the sandbox end inside the namespace: address, link up, loopback @@ -293,10 +312,12 @@ pub fn setup_sandbox_side( /// veth end removes its peer too. pub fn delete_link(name: &str) -> Result<()> { let name = name.to_string(); - block_on_netlink(move |handle| async move { - let idx = link_index_by_name(&handle, &name).await?; - handle.link().del(idx).execute().await.into_diagnostic()?; - Ok(()) + on_thread(move || { + block_on_netlink(move |handle| async move { + let idx = link_index_by_name(&handle, &name).await?; + handle.link().del(idx).execute().await.into_diagnostic()?; + Ok(()) + }) }) }