Repository navigation
[ML] Landlock fallback when Sandbox2 is unavailable on the host - #3215
Merged
Merged
Conversation
|
Pinging @elastic/ml-core (Team:ML) |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved security, confinement, build, and documentation issues remain.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (4)
What changed in this PR
Adds Landlock as a fallback when Sandbox2 is unavailable, with routing, confinement signaling, tests, and documentation.
Changes:
- Implements Landlock filesystem policies and host capability detection.
- Adds Sandbox2 → Landlock → refusal routing.
- Updates inference parsing, attestations, tests, and operational documentation.
| File | Reviewed change |
|---|---|
test/test_sandbox2_attack_defense.py |
Confinement-mode attack-defense checks |
lib/seccomp/unittest/CSeccompFilterBuilderTest.cc |
Route attestation tests |
lib/seccomp/unittest/CMakeLists.txt |
Test registration |
lib/seccomp/unittest/CLandlockFilesystemPolicyTest.cc |
Landlock policy tests |
lib/seccomp/CMakeLists.txt |
Landlock source registration |
lib/seccomp/CLandlockFilesystemPolicy.cc |
Portable Landlock stubs |
lib/seccomp/CLandlockFilesystemPolicy_Linux.cc |
Linux Landlock implementation |
lib/sandbox/unittest/CSandbox2DiagnosticsTest.cc |
Confinement diagnostics tests |
lib/sandbox/CSandboxedProcessSpawner_Linux.cc |
Spawner formatting changes |
lib/sandbox/CSandbox2Diagnostics_Linux.cc |
Cached capability diagnostics |
include/seccomp/CSystemCallFilter.h |
Route-aware seccomp attestation |
include/seccomp/CLandlockFilesystemPolicy.h |
Landlock policy API |
include/sandbox/CSandbox2Diagnostics.h |
Confinement ladder API |
docs/sandbox2_production_failure_modes.md |
Fallback-mode documentation |
build.gradle |
Protocol compatibility metadata |
bin/pytorch_inference/Main.cc |
Applies Landlock before processing |
bin/pytorch_inference/CCmdLineParser.h |
Parser interface updates |
bin/pytorch_inference/CCmdLineParser.cc |
Filesystem restriction parsing |
bin/controller/unittest/CProcessSpawnerRouterTest.cc |
Router fallback tests |
bin/controller/unittest/CCommandProcessorTest.cc |
Command validation tests |
bin/controller/CProcessSpawnerRouter.h |
Router interfaces |
bin/controller/CProcessSpawnerRouter.cc |
Confinement routing |
bin/controller/CCommandProcessor.h |
Command processor interfaces |
bin/controller/CCommandProcessor.cc |
Reserved-token validation |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This was referenced Sep 24, 2026
valeriy42
force-pushed
the
feature/sandbox2-pr-f-landlock-fallback
branch
from
September 24, 2026 15:34
b6557b1 to
516fae0
Compare
Base automatically changed from
feature/sandbox2-pr-e-typed-routing
to
main
September 24, 2026 18:20
valeriy42
added a commit
that referenced
this pull request
Sep 24, 2026
## Summary Stacks on #3187. Adds typed controller-side routing behind two symmetric controller tokens. Landlock fallback when Sandbox2 is unavailable is split to stacked #3215. - `CCommandProcessor` parses at most one of `--disableSandbox` (operator kill switch, forces legacy) or `--requireSandbox` (operator opt-in, forces Sandbox2 on capable hosts) for the exact configured `pytorch_inference` path. Duplicate occurrences of either token, a mismatched process path, or both tokens present together are all rejected before spawn. The selected token is stripped before the child ever sees it. - New `CProcessSpawnerRouter` dispatches the decided route to `CSandboxedProcessSpawner` or `CDetachedProcessSpawner` with no automatic fallback: a required Sandbox2 launch that fails returns a failed start response, it never retries through the legacy spawner. - A `start` command with neither token always takes the legacy route - permanent behaviour for any caller that sends no routing token (support/debug scripts, direct controller invocation, the test harness), not a rollout seam. Elasticsearch is expected to always send exactly one of the two tokens per launch, chosen from its own operator setting's live value. - Structured once-per-launch observability signal (`sandbox2_launch`: `deployment_id`, `model_id`, `route`, `sandbox2_established`, `mode`, `legacy_reason`, `sandbox2_compiled_in`) so a consumer can distinguish an operator kill switch from the no-token default, and "Sandbox2 supported but no routing token sent" from "built without Sandbox2 support." - `ML_SANDBOXED` is stripped from every legacy-route child's environment (POSIX and Windows) so an inherited or injected value can never fail-open the mandatory in-process seccomp filter. - Active Sandbox2 capability probe logged once at controller start (`CSandbox2Diagnostics`) so operators see whether `--requireSandbox` can succeed before a deployment fails closed. - Staged user-namespace capability probe plus CI wiring: aarch64 runs enforced-mode coverage; x86_64 runs fail-closed-only coverage, since x86_64 Buildkite k8s pods get `EPERM` on `mount("proc", ...)` and there is currently no userns-capable x86_64 CI runner. - Repaired `test_sandbox2_attack_defense.py`: reached markers, unsandboxed positive controls, per-case cleanup/reap assertions, the real `ml-child-ipc/<child-id>` IPC layout, an assertion that the controller's own `sandbox2_launch` signal reports the expected route before any security-boundary check runs, and explicit `--requireSandbox`/`--disableSandbox` tokens per case instead of a global env-var default. - Producer-side controller-protocol capability token (`3rd_party/controller-protocol.version`, now version 2) for a future cross-repo compatibility check. - Build packaging: per-library `install_libs()` guard and unconditional `$ORIGIN` RPATH on bundled ELF libraries so sibling `.so` dependencies resolve when the controller environment is cleared. ## Filesystem-policy fixes found during enforced-mode qualification End-to-end qualification on the qaf harness with `xpack.ml.trained_models.sandbox_enabled=true` (real `mode:enforced` route, not the legacy fallback) surfaced several launch/filesystem-policy gaps that previously only manifested once a real `pytorch_inference` ran under the enforced sandbox. Fixed here: - **Per-child IPC directory is created before validation.** The native controller now creates `$TMPDIR/ml-child-ipc/<child-id>` (mode 0700) before `validateChildIpcLaunchSpec()`'s live `realpath()` calls, on both spawn paths. Elasticsearch only ever constructs the IPC path strings; nothing created the directory they name. - **Per-child IPC root is mounted at the same path inside and outside the sandbox** (no `/run/elastic/ml-ipc` remap), because `pytorch_inference` receives its `--input=`/`--output=`/`--restore=`/`--logPipe=` argv as host paths under that root - a remap left them unresolvable in the sandbox mount namespace. - **Sandbox2-specific syscall allowlist** ported from the frozen reference (#2873): Sandbox2's namespace/threading setup exercises syscalls the legacy in-process BPF filter never needed, so `legacyBpfAllowedSyscalls()` alone is insufficient. - **`/proc` is mounted (PID-namespaced) inside the sandbox rootfs.** Sandbox2 mounts a fresh PID-namespaced procfs on the outer root, then `pivot_root`s into the chroot and detaches the old root, so the rootfs had no `/proc`. Without it `readlink(/proc/self/exe)` fails with `ENOENT`, breaking Intel oneMKL's runtime dispatcher (it reads `/proc/self/exe` to self-locate and `dlopen` its CPU-specific `libmkl_*.so.3` kernels) - every enforced `pytorch_inference` aborted with `Intel oneMKL FATAL ERROR: Cannot load <mkl-loader>`. The mount binds the already-namespaced procfs (never the host's): inside the sandbox `/proc` shows only the sandboxee's own PIDs. `/sys` is left unmounted. - **Regression test:** `ml_sandbox_probe` now checks `/proc/self/exe` is readable inside the real sandbox, asserted by `CPytorchInferenceSandboxPolicyMechanismTest_Linux` - turning the MKL crash into a build-time failure. Verified: ml-cpp sandbox unit tests 33/33; qaf boot check and `test_scenario_buildly` (8/8) pass under `mode:enforced` with zero MKL crashes and no legacy fallbacks.
When Elasticsearch sends --requireSandbox but the host cannot run Sandbox2 (user namespaces denied on ECH allocators and similar), step down to a Landlock filesystem ruleset plus the existing in-process seccomp filter instead of failing closed. The controller probes once at startup, caches hostConfinement(), and reports mode "landlock" on the sandbox2_launch signal. --restrictFilesystem is reserved for the router; refusal paths return an operator-actionable reason to Elasticsearch.
Rebase onto feature/sandbox2-pr-e-typed-routing with router and diagnostics conflicts resolved. Assert landlock_path_beneath_attr stays 12-byte packed UAPI. Restore sandbox2_production_failure_modes heading and seccomp/Landlock prose.
valeriy42
force-pushed
the
feature/sandbox2-pr-f-landlock-fallback
branch
from
September 24, 2026 18:20
516fae0 to
1e691c1
Compare
edsavage
approved these changes
Sep 24, 2026
|
|
||
| #endif // SANDBOX2_AVAILABLE && !Windows | ||
|
|
||
| // appended probe |
Contributor
There was a problem hiding this comment.
nit: remove the dangling comment
| // A path that simply does not exist on this host is not an error: | ||
| // the fixed list covers several distribution layouts (/lib64 exists | ||
| // on RHEL-family, is a symlink or absent elsewhere), and a rule for | ||
| // a missing path grants nothing anyway. |
Contributor
There was a problem hiding this comment.
I'm curious as to how this interacts with the pipe directory. If ipcDirectory doesn't exist at the moment the ruleset is applied, the grant is silently skipped and then the mkfifo gets denied. It fails closed, so its all safe, but the only hint that it happened is a DEBUG line. I'm thinking it might be worth double checking that the per-child IPC dir is guaranteed to exist before applyLandlockFilesystemPolicy(), and considering treating a missing pipe directory as an error rather than a skip.
Contributor
Author
There was a problem hiding this comment.
The controller already calls ensureChildIpcDirectory() before dispatch, but the Landlock rung ignored both that outcome and validateChildIpcLaunchSpec() (only s_ChildId fed the signal).
6ee9046 fixes that in three places:
-
Router (Landlock rung only): one prepareChildIpcLaunch() call now supplies both the signal and the routing decision. Refusal when mkdir fails or validation fails — mode fail_closed, LOG_ERROR, rejection text in lastSpawnFailureReason() for ES.
-
Child: pipe directories are mandatory in applyLandlockFilesystemPolicy() — O_PATH|O_DIRECTORY|O_NOFOLLOW, owner must be this uid; missing/symlink/wrong owner returns E_Failed (not a DEBUG skip). Optional read-only host paths still skip when absent.
-
perChildIpcDirectory() rejects .. components.
Unit tests cover missing/symlink pipe dirs, optional read-only skip, and router refusal on invalid IPC spec. #3217 does not address this (IPC reap + degraded seccomp only).
Require the same child-IPC preconditions on the Landlock rung as Sandbox2, and treat the per-child pipe directory as mandatory when applying Landlock.
CScopedChildIpcRoot used create_directories (umask 0755), so ensureChildIpcDirectory() rejected the pre-existing tree on the Landlock rung and the two injected-confinement router tests failed.
This was referenced Oct 9, 2026
valeriy42
added a commit
that referenced
this pull request
Oct 16, 2026
## Summary - Controller-owned `CChildIpcDirectoryReaper` removes `$TMPDIR/ml-child-ipc/<deployment-id>` after the bound child exits (legacy tracker or Sandbox2 monitor) or after a failed spawn with no live pid; - `TERMINATE_ON_DEGRADED_SECCOMP_FAILURE` is true in `include/seccomp/CSystemCallFilter.h` so legacy and Landlock `pytorch_inference` exits before model IO when in-process seccomp cannot install; - Stacks on #3215. Companion elasticsearch#159052 (`mode:"landlock"`) unchanged. closes #3214. closes #3213.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stacks on #3188. When Elasticsearch sends
--requireSandboxbut the host cannot run Sandbox2 (ECH allocators with user namespaces disabled, container seccomp blockingCLONE_NEWUSER, and similar), the controller steps down to a Landlock filesystem ruleset plus the existing in-process seccomp filter instead of failing every deployment closed.hostConfinement()probes once at controller start (same cached verdict as the startup self-check) and walks a fixed ladder: Sandbox2, then Landlock, then refusal with an operator-actionable message returned to Elasticsearch.sandbox2_launchmode"landlock"(route:"sandbox2",sandbox2_established:false). Consumers must not treatroutealone as full isolation.--restrictFilesystemtoken;CCommandProcessorrejects caller-supplied copies.docs/sandbox2_production_failure_modes.md. Companion: ML: Implement optional sandboxing for PyTorch models elasticsearch#159052 must acceptmode:"landlock".A failed Sandbox2 launch on a host that can run Sandbox2 is never retried under Landlock.