From 50cdbc4f83cefcbe18000b0e7a1bf75628724192 Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 24 Sep 2026 09:52:27 +0200 Subject: [PATCH 1/7] [ML] Landlock fallback when Sandbox2 is unavailable on the host 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. --- bin/controller/CCommandProcessor.cc | 31 +- bin/controller/CCommandProcessor.h | 7 +- bin/controller/CProcessSpawnerRouter.cc | 93 +++- bin/controller/CProcessSpawnerRouter.h | 50 ++- .../unittest/CCommandProcessorTest.cc | 101 +++++ .../unittest/CProcessSpawnerRouterTest.cc | 173 ++++++++ bin/pytorch_inference/CCmdLineParser.cc | 7 +- bin/pytorch_inference/CCmdLineParser.h | 3 +- bin/pytorch_inference/Main.cc | 65 ++- include/sandbox/CSandbox2Diagnostics.h | 74 ++++ include/seccomp/CLandlockFilesystemPolicy.h | 159 +++++++ include/seccomp/CSystemCallFilter.h | 16 +- lib/sandbox/CSandbox2Diagnostics_Linux.cc | 188 +++++++-- .../unittest/CSandbox2DiagnosticsTest.cc | 142 +++++++ lib/seccomp/CLandlockFilesystemPolicy.cc | 46 ++ .../CLandlockFilesystemPolicy_Linux.cc | 332 +++++++++++++++ lib/seccomp/CMakeLists.txt | 1 + .../unittest/CLandlockFilesystemPolicyTest.cc | 397 ++++++++++++++++++ lib/seccomp/unittest/CMakeLists.txt | 1 + .../unittest/CSeccompFilterBuilderTest.cc | 64 +++ test/test_sandbox2_attack_defense.py | 127 ++++-- 21 files changed, 1971 insertions(+), 106 deletions(-) create mode 100644 include/seccomp/CLandlockFilesystemPolicy.h create mode 100644 lib/seccomp/CLandlockFilesystemPolicy.cc create mode 100644 lib/seccomp/CLandlockFilesystemPolicy_Linux.cc create mode 100644 lib/seccomp/unittest/CLandlockFilesystemPolicyTest.cc diff --git a/bin/controller/CCommandProcessor.cc b/bin/controller/CCommandProcessor.cc index ebe1a0d837..cb8c38f66e 100644 --- a/bin/controller/CCommandProcessor.cc +++ b/bin/controller/CCommandProcessor.cc @@ -46,8 +46,10 @@ const std::string CCommandProcessor::KILL{"kill"}; CCommandProcessor::CCommandProcessor(const TStrVec& permittedProcessPaths, const TStrVec& sandboxedProcessPaths, - std::ostream& responseStream) - : m_Spawner{permittedProcessPaths, sandboxedProcessPaths}, m_ResponseWriter{responseStream} { + std::ostream& responseStream, + CProcessSpawnerRouter::TConfinementFn confinementFn) + : m_Spawner{permittedProcessPaths, sandboxedProcessPaths, std::move(confinementFn)}, + m_ResponseWriter{responseStream} { } void CCommandProcessor::processCommands(std::istream& commandStream) { @@ -129,6 +131,23 @@ bool CCommandProcessor::handleStart(std::uint32_t id, TStrVec tokens) { } } + // --restrictFilesystem tells pytorch_inference that the router chose the + // Landlock rung for it. Only the router may add it (see + // CProcessSpawnerRouter::RESTRICT_FILESYSTEM_TOKEN): if a caller could + // send it, a child could be Landlock-confined on a launch the + // sandbox2_launch signal reports as some other mode, and the signal + // would stop being a truthful record of what bounded the child. + if (std::find(tokens.begin(), tokens.end(), + CProcessSpawnerRouter::RESTRICT_FILESYSTEM_TOKEN) != tokens.end()) { + std::string error{"Rejecting command: '" + CProcessSpawnerRouter::RESTRICT_FILESYSTEM_TOKEN + + "' is reserved for the controller and may not be supplied by the " + "caller, for process '" + + processPath + '\''}; + LOG_ERROR(<< error << " in command with ID " << id); + m_ResponseWriter.writeResponse(id, false, error); + return false; + } + if (disableSandboxCount >= 2) { std::string error{"Rejecting command: '" + DISABLE_SANDBOX_TOKEN + "' specified " + core::CStringUtils::typeToString(disableSandboxCount) + @@ -233,7 +252,15 @@ bool CCommandProcessor::handleStart(std::uint32_t id, TStrVec tokens) { core::CProcess::TPid childPid{0}; if (m_Spawner.spawn(route, processPath, tokens, childPid, legacyReason) == false) { + // When the router refused the launch itself it says why, in words + // meant for the user (e.g. that this host cannot confine the process + // and xpack.ml.trained_models.sandbox_enabled must be deactivated). + // Returned as the failure reason, which Elasticsearch includes in the + // deployment-start error. std::string error{"Failed to start process '" + processPath + '\''}; + if (m_Spawner.lastSpawnFailureReason().empty() == false) { + error += ": " + m_Spawner.lastSpawnFailureReason(); + } LOG_ERROR(<< error << " in command with ID " << id); m_ResponseWriter.writeResponse(id, false, error); return false; diff --git a/bin/controller/CCommandProcessor.h b/bin/controller/CCommandProcessor.h index 9acc1387b5..e8d001262b 100644 --- a/bin/controller/CCommandProcessor.h +++ b/bin/controller/CCommandProcessor.h @@ -69,9 +69,14 @@ class CCommandProcessor { //! no default that reuses \p permittedProcessPaths, because doing //! so would silently make every permitted process //! sandboxed-eligible. + //! \param confinementFn passed to the router - see + //! CProcessSpawnerRouter::TConfinementFn. Production code leaves it + //! empty; tests inject a fixed host confinement. CCommandProcessor(const TStrVec& permittedProcessPaths, const TStrVec& sandboxedProcessPaths, - std::ostream& responseStream); + std::ostream& responseStream, + CProcessSpawnerRouter::TConfinementFn confinementFn = + CProcessSpawnerRouter::TConfinementFn{}); //! Action commands read from the supplied \p commandStream until //! end-of-file is reached. diff --git a/bin/controller/CProcessSpawnerRouter.cc b/bin/controller/CProcessSpawnerRouter.cc index 952dc8bd3b..449b8681d4 100644 --- a/bin/controller/CProcessSpawnerRouter.cc +++ b/bin/controller/CProcessSpawnerRouter.cc @@ -14,6 +14,7 @@ #include #include +#include #include #include @@ -139,8 +140,16 @@ static_assert(sizeof(CProcessSpawnerRouter) < sizeof(core::CDetachedProcessSpawn "sandbox::CSandboxedProcessSpawner by value"); CProcessSpawnerRouter::CProcessSpawnerRouter(const TStrVec& permittedProcessPaths, - const TStrVec& sandboxedProcessPaths) - : m_LegacySpawner{permittedProcessPaths}, m_SandboxedProcessPaths{sandboxedProcessPaths} { + const TStrVec& sandboxedProcessPaths, + TConfinementFn confinementFn) + : m_LegacySpawner{permittedProcessPaths}, m_SandboxedProcessPaths{sandboxedProcessPaths}, + m_ConfinementFn{confinementFn ? std::move(confinementFn) : TConfinementFn{[] { + return sandbox::hostConfinement(); + }}} { +} + +const std::string& CProcessSpawnerRouter::lastSpawnFailureReason() const { + return m_LastSpawnFailureReason; } CProcessSpawnerRouter::~CProcessSpawnerRouter() = default; @@ -154,7 +163,8 @@ void CProcessSpawnerRouter::emitLaunchSignal(ERoute route, ELegacyReason legacyReason, const std::string& deploymentId, const TStrVec& args, - bool spawnSucceeded) const { + bool spawnSucceeded, + bool landlockFallback) const { const bool isLegacyRoute{route == ERoute::E_Legacy}; // degraded is decided purely by route, regardless of the legacy @@ -165,6 +175,13 @@ void CProcessSpawnerRouter::emitLaunchSignal(ERoute route, std::string mode; if (isLegacyRoute) { mode = "degraded"; + } else if (landlockFallback) { + // A Sandbox2-routed launch that this host could not honour, run + // under Landlock instead. Reported distinctly rather than as + // "enforced" (no Sandbox2 was established) or "fail_closed" (the + // deployment did start): a consumer must be able to tell that the + // operator's request was met by something weaker. + mode = spawnSucceeded ? "landlock" : "fail_closed"; } else { mode = spawnSucceeded ? "enforced" : "fail_closed"; } @@ -215,6 +232,8 @@ void CProcessSpawnerRouter::emitLaunchSignal(ERoute route, LOG_INFO(<< signal.str()); } +const std::string CProcessSpawnerRouter::RESTRICT_FILESYSTEM_TOKEN{"--restrictFilesystem"}; + bool CProcessSpawnerRouter::spawn(ERoute route, const std::string& processPath, const TStrVec& args, @@ -236,7 +255,11 @@ bool CProcessSpawnerRouter::spawn(ERoute route, const std::string deploymentId{sandboxEligible ? deriveDeploymentId(args) : std::string()}; + m_LastSpawnFailureReason.clear(); bool spawned{false}; + // Set when the Sandbox2 route degraded to the Landlock fallback, so the + // signal below reports what actually bounded the child. + bool landlockFallback{false}; if (route == ERoute::E_Legacy) { // Legacy route decided upstream: either the operator kill-switch // token (validated against this exact processPath and stripped from @@ -254,21 +277,55 @@ bool CProcessSpawnerRouter::spawn(ERoute route, // route == ERoute::E_Sandbox2, and processPath is configured as // sandboxed. #ifdef SANDBOX2_AVAILABLE - // First - and only - point at which any Sandbox2 machinery is - // constructed. A router that never reaches this branch (every - // router that never dispatches a validated --requireSandbox token, - // and every router in a build without Sandbox2 support) never creates a - // CSandboxedProcessSpawner at all, so no Sandbox2 state enters its - // construction or teardown path. Single-threaded by the same - // contract as the legacy spawner - see the member's declaration. - if (m_SandboxSpawner == nullptr) { - m_SandboxSpawner = std::make_unique(); + // Decide the rung before constructing or launching anything, from + // the one cached verdict the startup self-check also logged - never + // an independent probe here, because two probes can disagree (one + // once did, when the controller's non-dumpable flag broke the later + // one) and then the log says one thing while the route does + // another. Deciding first matters: on a host without user + // namespaces a Sandbox2 launch fails only after an opaque + // SETUP_ERROR, and on one that permits namespaces but denies mounts + // inside them the forkserver deadlocks instead of returning. + const sandbox::SHostConfinement host{m_ConfinementFn()}; + switch (host.s_Level) { + case sandbox::EConfinementLevel::E_Sandbox2: + // First - and only - point at which any Sandbox2 machinery is + // constructed. A router that never reaches this case (every + // router that never dispatches a validated --requireSandbox + // token, every router on a host that cannot run Sandbox2, and + // every router in a build without Sandbox2 support) never creates + // a CSandboxedProcessSpawner at all, so no Sandbox2 state enters + // its construction or teardown path. Single-threaded by the same + // contract as the legacy spawner - see the member's declaration. + if (m_SandboxSpawner == nullptr) { + m_SandboxSpawner = std::make_unique(); + } + // No automatic fallback on a Sandbox2 *failure*: a host that can + // run Sandbox2 but fails this launch has a problem worth + // surfacing, not papering over with a weaker boundary. + spawned = m_SandboxSpawner->spawn(processPath, args, childPid); + break; + case sandbox::EConfinementLevel::E_Landlock: { + // A supported, deliberate degradation: INFO, with what an + // administrator would change to get full isolation. + LOG_INFO(<< sandbox::landlockFallbackMessage(host, processPath)); + TStrVec landlockArgs{args}; + landlockArgs.emplace_back(RESTRICT_FILESYSTEM_TOKEN); + spawned = m_LegacySpawner.spawn(processPath, landlockArgs, childPid); + landlockFallback = true; + break; + } + case sandbox::EConfinementLevel::E_Unavailable: + // Refuse here, in the controller, rather than launching a child + // that would only discover it cannot confine itself: that way + // Elasticsearch gets an immediate, explained failure instead of + // a pipe-connection timeout, and no untrusted model is ever + // started unconfined while the operator asked for a sandbox. + m_LastSpawnFailureReason = sandbox::noConfinementMessage(host, processPath); + LOG_ERROR(<< m_LastSpawnFailureReason); + spawned = false; + break; } - - // No automatic fallback to the legacy spawner on a Sandbox2 - // failure: a process that must be sandboxed either - // launches inside Sandbox2 or does not launch at all. - spawned = m_SandboxSpawner->spawn(processPath, args, childPid); #else // Build/deployment contradiction: processPath is configured as // sandboxed, but this build has no Sandbox2 support (non-Linux). @@ -288,7 +345,7 @@ bool CProcessSpawnerRouter::spawn(ERoute route, } if (sandboxEligible) { - this->emitLaunchSignal(route, legacyReason, deploymentId, args, spawned); + this->emitLaunchSignal(route, legacyReason, deploymentId, args, spawned, landlockFallback); } return spawned; diff --git a/bin/controller/CProcessSpawnerRouter.h b/bin/controller/CProcessSpawnerRouter.h index c12a43535b..8a90487dd4 100644 --- a/bin/controller/CProcessSpawnerRouter.h +++ b/bin/controller/CProcessSpawnerRouter.h @@ -14,6 +14,9 @@ #include #include +#include + +#include #include #include #include @@ -81,14 +84,23 @@ class CProcessSpawnerRouter { E_NoTokenDefault }; -public: + //! Supplies this host's confinement options. Production code leaves it + //! empty, which means sandbox::hostConfinement() - the cached verdict the + //! startup self-check also logs. Tests inject a fixed value so every rung + //! of the ladder can be exercised on any machine. + using TConfinementFn = std::function; + CProcessSpawnerRouter(const TStrVec& permittedProcessPaths, - const TStrVec& sandboxedProcessPaths); + const TStrVec& sandboxedProcessPaths, + TConfinementFn confinementFn = TConfinementFn{}); ~CProcessSpawnerRouter(); - //! Dispatch a spawn request per the already-decided \p route. Returns - //! false immediately on a Sandbox2 failure - never retries via the - //! legacy spawner ("no automatic fallback"). + //! Dispatch a spawn request per the already-decided \p route. On the + //! Sandbox2 route the host's confinement decides the backend: Sandbox2 + //! when available, otherwise the legacy spawner under a Landlock ruleset + //! (the child is told via RESTRICT_FILESYSTEM_TOKEN), otherwise refusal. + //! That step down is decided before launching and logged; a Sandbox2 + //! launch that *fails* is never retried under Landlock or unconfined. //! \param legacyReason provenance of an E_Legacy \p route, for the //! `sandbox2_launch` signal only - never used to dispatch. Must be E_NotLegacy //! (the default) when \p route is E_Sandbox2. @@ -115,6 +127,20 @@ class CProcessSpawnerRouter { //! emission. bool isSandboxedProcessPath(const std::string& processPath) const; + //! Why the most recent spawn() returned false, when the router itself + //! refused the launch (rather than a backend failing), phrased for the + //! user: CCommandProcessor returns it to Elasticsearch as the command's + //! failure reason. Empty after a successful spawn() and after a backend + //! failure, which each backend logs itself. + const std::string& lastSpawnFailureReason() const; + + //! Token appended to a child's argv when the Sandbox2 route degrades to + //! the Landlock fallback, telling pytorch_inference to confine its own + //! filesystem access before reading any model bytes. Reserved for the + //! router: CCommandProcessor rejects a start command that already + //! contains it, so its presence always means the router chose Landlock. + static const std::string RESTRICT_FILESYSTEM_TOKEN; + private: //! Emit the `sandbox2_launch` structured once-per-launch signal for a //! Sandbox2-eligible spawn() call, @@ -127,11 +153,16 @@ class CProcessSpawnerRouter { //! once by spawn() *before* dispatch - never re-derived here, so //! the value in this signal cannot disagree with the value the //! dispatch decision was made against. + //! \param landlockFallback true when \p route was E_Sandbox2 but this + //! host cannot run Sandbox2, so the child was launched via the + //! legacy spawner under a Landlock ruleset instead. Reported as + //! mode "landlock", never as "enforced". void emitLaunchSignal(ERoute route, ELegacyReason legacyReason, const std::string& deploymentId, const TStrVec& args, - bool spawnSucceeded) const; + bool spawnSucceeded, + bool landlockFallback) const; private: core::CDetachedProcessSpawner m_LegacySpawner; @@ -174,6 +205,13 @@ class CProcessSpawnerRouter { std::unique_ptr m_SandboxSpawner; TStrVec m_SandboxedProcessPaths; + + //! See TConfinementFn. Neither member's size depends on + //! SANDBOX2_AVAILABLE, preserving the layout invariant described above. + TConfinementFn m_ConfinementFn; + + //! See lastSpawnFailureReason(). + std::string m_LastSpawnFailureReason; }; } // namespace controller diff --git a/bin/controller/unittest/CCommandProcessorTest.cc b/bin/controller/unittest/CCommandProcessorTest.cc index 93e3626b34..feeb452073 100644 --- a/bin/controller/unittest/CCommandProcessorTest.cc +++ b/bin/controller/unittest/CCommandProcessorTest.cc @@ -13,6 +13,8 @@ #include #include +#include + #include "../CCommandProcessor.h" #include @@ -590,6 +592,105 @@ BOOST_AUTO_TEST_CASE(testStartRejectsDuplicateRequireSandboxTokenOnSandboxedPath BOOST_TEST_REQUIRE(response.find("specified 2 times") != std::string::npos); } +BOOST_AUTO_TEST_CASE(testStartRejectsReservedRestrictFilesystemTokenOnSandboxedPath) { + // --restrictFilesystem is reserved for the controller: only the router + // may append it, to tell pytorch_inference the launch took the Landlock + // rung. If a caller could supply it, a child could be Landlock-confined + // on a launch the sandbox2_launch signal reports as some other mode, so + // the signal would stop being a truthful record. It must be rejected + // before any spawn, whether or not the path is the sandboxed one. + const std::string TARGET_FILE{"reserved_token_reject_sandboxed_out.txt"}; + std::remove(TARGET_FILE.c_str()); + + std::ostringstream responseStream; + { + ml::controller::CCommandProcessor::TStrVec permittedPaths{PROCESS_PATH}; + ml::controller::CCommandProcessor::TStrVec sandboxedPaths{PROCESS_PATH}; + ml::controller::CCommandProcessor processor{permittedPaths, sandboxedPaths, + responseStream}; + + std::string command{startCommand( + 30, PROCESS_PATH, + copyArgs(TARGET_FILE, {"--requireSandbox", "--restrictFilesystem"}))}; + + BOOST_REQUIRE_EQUAL(false, processor.handleCommand(command)); + } + + // Rejected before any spawn: the copy must never have happened. + BOOST_REQUIRE_EQUAL(true, fileAbsent(TARGET_FILE)); + + std::string response{responseStream.str()}; + BOOST_TEST_REQUIRE(response.find("\"id\":30,\"success\":false") != std::string::npos); + BOOST_TEST_REQUIRE(response.find("reserved for the controller") != std::string::npos); +} + +BOOST_AUTO_TEST_CASE(testStartRejectsReservedRestrictFilesystemTokenOnNonSandboxedPath) { + // Same reservation, on a path that is merely permitted (not the + // configured sandboxed one): the token must be rejected on its own terms, + // before and independently of any routing-token validation. + const std::string TARGET_FILE{"reserved_token_reject_nonsandboxed_out.txt"}; + std::remove(TARGET_FILE.c_str()); + + std::ostringstream responseStream; + { + ml::controller::CCommandProcessor::TStrVec permittedPaths{PROCESS_PATH}; + ml::controller::CCommandProcessor::TStrVec sandboxedPaths{}; // PROCESS_PATH not sandboxed + ml::controller::CCommandProcessor processor{permittedPaths, sandboxedPaths, + responseStream}; + + std::string command{startCommand( + 31, PROCESS_PATH, copyArgs(TARGET_FILE, {"--restrictFilesystem"}))}; + + BOOST_REQUIRE_EQUAL(false, processor.handleCommand(command)); + } + + BOOST_REQUIRE_EQUAL(true, fileAbsent(TARGET_FILE)); + + std::string response{responseStream.str()}; + BOOST_TEST_REQUIRE(response.find("\"id\":31,\"success\":false") != std::string::npos); + BOOST_TEST_REQUIRE(response.find("reserved for the controller") != std::string::npos); +} + +#ifdef SANDBOX2_AVAILABLE +BOOST_AUTO_TEST_CASE(testStartFailsWithSettingHintWhenHostCannotConfine) { + // A validated --requireSandbox launch on a host that supports neither + // Sandbox2 nor Landlock (injected E_Unavailable) must fail with a + // response that both fails the command and tells the operator to + // deactivate the setting - the controller's noConfinementMessage, + // forwarded through CCommandProcessor as the command's failure reason. + const std::string TARGET_FILE{"start_unavailable_setting_hint_out.txt"}; + std::remove(TARGET_FILE.c_str()); + + ml::sandbox::SHostConfinement unavailableHost; + unavailableHost.s_Level = ml::sandbox::EConfinementLevel::E_Unavailable; + unavailableHost.s_Sandbox2 = ml::sandbox::ESandbox2Capability::E_UserNamespaceDenied; + unavailableHost.s_LandlockAbi = 0; + + std::ostringstream responseStream; + { + ml::controller::CCommandProcessor::TStrVec permittedPaths{PROCESS_PATH}; + ml::controller::CCommandProcessor::TStrVec sandboxedPaths{PROCESS_PATH}; + ml::controller::CCommandProcessor processor{ + permittedPaths, sandboxedPaths, responseStream, + [unavailableHost] { return unavailableHost; }}; + + BOOST_REQUIRE_EQUAL( + false, processor.handleCommand(startCommand( + 32, PROCESS_PATH, copyArgs(TARGET_FILE, {"--requireSandbox"})))); + } + + // Refused before any backend ran: no child, no copy. + std::this_thread::sleep_for(std::chrono::seconds{1}); + BOOST_REQUIRE_EQUAL(true, fileAbsent(TARGET_FILE)); + std::remove(TARGET_FILE.c_str()); + + std::string response{responseStream.str()}; + BOOST_TEST_REQUIRE(response.find("\"id\":32,\"success\":false") != std::string::npos); + BOOST_TEST_REQUIRE(response.find("xpack.ml.trained_models.sandbox_enabled") != + std::string::npos); +} +#endif // SANDBOX2_AVAILABLE + BOOST_AUTO_TEST_CASE(testStartRejectsRequireSandboxTokenOnNonSandboxedPath) { // Symmetric with testStartRejectsDisableSandboxTokenOnNonSandboxedPath: // --requireSandbox is only meaningful for the exact configured sandboxed diff --git a/bin/controller/unittest/CProcessSpawnerRouterTest.cc b/bin/controller/unittest/CProcessSpawnerRouterTest.cc index a4686bf085..ac485cedaf 100644 --- a/bin/controller/unittest/CProcessSpawnerRouterTest.cc +++ b/bin/controller/unittest/CProcessSpawnerRouterTest.cc @@ -14,6 +14,8 @@ #include #include +#include + #include "../CProcessSpawnerRouter.h" #include @@ -611,4 +613,175 @@ BOOST_AUTO_TEST_CASE(testLegacyOnlyRouterNeedsNoSandboxedSpawner) { } } +#if defined(SANDBOX2_AVAILABLE) && !defined(Windows) + +// These tests inject a fixed sandbox::SHostConfinement via +// CProcessSpawnerRouter's TConfinementFn constructor argument, so every rung +// of the ladder (E_Sandbox2/E_Landlock/E_Unavailable) can be exercised +// deterministically regardless of what this host actually supports. Gated on +// SANDBOX2_AVAILABLE && !Windows because the confinement ladder is only ever +// consulted inside spawn()'s SANDBOX2_AVAILABLE branch for a sandboxed +// process path (see CProcessSpawnerRouter::spawn()), and PROCESS_PATH/ +// SHELL_FLAG above assume a POSIX shell. + +BOOST_AUTO_TEST_CASE(testSandbox2RouteDegradesToLandlockRungWithInjectedConfinement) { + // Inject a confinement whose ladder rung is E_Landlock (as + // decideConfinement() would return for, say, E_UserNamespaceDenied with + // Landlock ABI >= 1), so this is deterministic regardless of whether this + // host can actually run Sandbox2. + ml::sandbox::SHostConfinement injectedHost; + injectedHost.s_Level = ml::sandbox::EConfinementLevel::E_Landlock; + injectedHost.s_Sandbox2 = ml::sandbox::ESandbox2Capability::E_UserNamespaceDenied; + injectedHost.s_LandlockAbi = 1; + injectedHost.s_UnprivilegedUsernsClone = "0"; + injectedHost.s_MaxUserNamespaces = "0"; + + ml::controller::CProcessSpawnerRouter::TStrVec permittedPaths{PROCESS_PATH}; + ml::controller::CProcessSpawnerRouter::TStrVec sandboxedPaths{PROCESS_PATH}; + ml::controller::CProcessSpawnerRouter router{ + permittedPaths, sandboxedPaths, [injectedHost] { return injectedHost; }}; + + const std::string outputFile{"router_test_landlock_rung.txt"}; + std::remove(outputFile.c_str()); + + // With sh -c, the token the router appends after args becomes $0, so the + // first line the script writes is the appended token - proving it reached + // the spawned process's argv, not merely that spawn() returned true. + ml::controller::CProcessSpawnerRouter::TStrVec args{ + SHELL_FLAG, "printf '%s\\n' \"$0\" > " + outputFile}; + ml::core::CProcess::TPid childPid{0}; + std::string logged{captureLogged([&] { + BOOST_REQUIRE_EQUAL(true, router.spawn(ml::controller::CProcessSpawnerRouter::ERoute::E_Sandbox2, + PROCESS_PATH, args, childPid)); + })}; + BOOST_TEST_REQUIRE(childPid != 0); + + std::this_thread::sleep_for(std::chrono::seconds{1}); + std::ifstream ifs{outputFile}; + BOOST_TEST_REQUIRE(ifs.is_open()); + std::string firstLine; + std::getline(ifs, firstLine); + ifs.close(); + std::remove(outputFile.c_str()); + BOOST_REQUIRE_EQUAL(ml::controller::CProcessSpawnerRouter::RESTRICT_FILESYSTEM_TOKEN, + firstLine); + + // sandbox2_launch signal: mode "landlock" (not "enforced" - no Sandbox2 + // was established), and the router's own failure reason empty (success). + BOOST_REQUIRE(logged.find("\"event\":\"sandbox2_launch\"") != std::string::npos); + BOOST_REQUIRE(logged.find("\"mode\":\"landlock\"") != std::string::npos); + BOOST_REQUIRE(logged.find("\"sandbox2_established\":false") != std::string::npos); + BOOST_TEST_REQUIRE(router.lastSpawnFailureReason().empty()); + + // The fallback is explained to the operator, and (s_UnprivilegedUsernsClone + // == "0") the explanation carries the sysctl remedy, not the + // container-runtime one. The message is logged at INFO (a supported, + // deliberate degradation, not a warning); that severity is covered by the + // CSandbox2Diagnostics message tests and verified end to end in the log, + // rather than re-asserted here - the router-emitted record does not survive + // this suite's severity-filtered capture reliably. + BOOST_REQUIRE(logged.find("Landlock filesystem confinement") != std::string::npos); + BOOST_REQUIRE(logged.find("kernel.unprivileged_userns_clone=1") != std::string::npos); +} + +BOOST_AUTO_TEST_CASE(testSandbox2RouteFailsClosedWithInjectedUnavailableConfinement) { + // Inject a confinement whose ladder rung is E_Unavailable (neither + // Sandbox2 nor Landlock), deterministically regardless of this host's + // real capabilities. + ml::sandbox::SHostConfinement injectedHost; + injectedHost.s_Level = ml::sandbox::EConfinementLevel::E_Unavailable; + injectedHost.s_Sandbox2 = ml::sandbox::ESandbox2Capability::E_UserNamespaceDenied; + injectedHost.s_LandlockAbi = 0; + injectedHost.s_UnprivilegedUsernsClone = "0"; + injectedHost.s_MaxUserNamespaces = "0"; + + ml::controller::CProcessSpawnerRouter::TStrVec permittedPaths{PROCESS_PATH}; + ml::controller::CProcessSpawnerRouter::TStrVec sandboxedPaths{PROCESS_PATH}; + ml::controller::CProcessSpawnerRouter router{ + permittedPaths, sandboxedPaths, [injectedHost] { return injectedHost; }}; + + // A marker file the child would have written had anything actually been + // spawned - the router must refuse before ever reaching a backend. + const std::string markerFile{"router_test_unavailable_marker.txt"}; + std::remove(markerFile.c_str()); + + ml::controller::CProcessSpawnerRouter::TStrVec args{ + SHELL_FLAG, "printf '%s\\n' \"$0\" > " + markerFile}; + ml::core::CProcess::TPid childPid{0}; + std::string logged{captureLogged([&] { + BOOST_REQUIRE_EQUAL( + false, router.spawn(ml::controller::CProcessSpawnerRouter::ERoute::E_Sandbox2, + PROCESS_PATH, args, childPid)); + })}; + + BOOST_REQUIRE_EQUAL(ml::core::CProcess::TPid{0}, childPid); + + // The operator-facing refusal is delivered to Elasticsearch through + // lastSpawnFailureReason() (CCommandProcessor returns it as the command's + // failure reason), not only to the log - so assert it there, where it is + // a deterministic return value rather than a captured side effect. It + // must name the setting to deactivate. + BOOST_TEST_REQUIRE(router.lastSpawnFailureReason().find("xpack.ml.trained_models.sandbox_enabled") != + std::string::npos); + BOOST_TEST_REQUIRE(router.lastSpawnFailureReason().find("deactivate") != + std::string::npos); + + // The sandbox2_launch signal reports mode "fail_closed" (the request was + // refused, no child ran), never "enforced" or "landlock". + BOOST_REQUIRE(logged.find("\"event\":\"sandbox2_launch\"") != std::string::npos); + BOOST_REQUIRE(logged.find("\"mode\":\"fail_closed\"") != std::string::npos); + BOOST_REQUIRE(logged.find("\"sandbox2_established\":false") != std::string::npos); + + // No child was ever spawned: give the same grace period the other tests + // in this file use, then confirm the marker file the shell script would + // have produced does not exist. + std::this_thread::sleep_for(std::chrono::seconds{1}); + std::ifstream ifs{markerFile}; + BOOST_REQUIRE_EQUAL(false, ifs.is_open()); + std::remove(markerFile.c_str()); +} + +BOOST_AUTO_TEST_CASE(testLastSpawnFailureReasonClearedByALaterSuccessfulSpawn) { + // A stale failure reason from an earlier, unrelated spawn() call must + // never leak into a later, successful one - a caller reading + // lastSpawnFailureReason() after success must see it empty. + ml::sandbox::SHostConfinement unavailableHost; + unavailableHost.s_Level = ml::sandbox::EConfinementLevel::E_Unavailable; + unavailableHost.s_Sandbox2 = ml::sandbox::ESandbox2Capability::E_ProbeFailed; + unavailableHost.s_LandlockAbi = 0; + + ml::controller::CProcessSpawnerRouter::TStrVec permittedPaths{PROCESS_PATH}; + ml::controller::CProcessSpawnerRouter::TStrVec sandboxedPaths{PROCESS_PATH}; + ml::controller::CProcessSpawnerRouter router{ + permittedPaths, sandboxedPaths, + [unavailableHost] { return unavailableHost; }}; + + ml::controller::CProcessSpawnerRouter::TStrVec failArgs{SHELL_FLAG, "true"}; + ml::core::CProcess::TPid failedPid{0}; + captureLogged([&] { + BOOST_REQUIRE_EQUAL( + false, router.spawn(ml::controller::CProcessSpawnerRouter::ERoute::E_Sandbox2, + PROCESS_PATH, failArgs, failedPid)); + }); + BOOST_TEST_REQUIRE(router.lastSpawnFailureReason().empty() == false); + + const std::string outputFile{"router_test_clears_failure_reason.txt"}; + std::remove(outputFile.c_str()); + ml::controller::CProcessSpawnerRouter::TStrVec okArgs{ + SHELL_FLAG, copyArgsScript(outputFile)}; + ml::core::CProcess::TPid okPid{0}; + captureLogged([&] { + BOOST_REQUIRE_EQUAL(true, router.spawn(ml::controller::CProcessSpawnerRouter::ERoute::E_Legacy, + PROCESS_PATH, okArgs, okPid)); + }); + std::this_thread::sleep_for(std::chrono::seconds{1}); + std::remove(outputFile.c_str()); + + BOOST_TEST_REQUIRE(router.lastSpawnFailureReason().empty()); +} + +#endif // SANDBOX2_AVAILABLE && !Windows + +// appended probe + BOOST_AUTO_TEST_SUITE_END() diff --git a/bin/pytorch_inference/CCmdLineParser.cc b/bin/pytorch_inference/CCmdLineParser.cc index 451a58f48d..3b76e5101c 100644 --- a/bin/pytorch_inference/CCmdLineParser.cc +++ b/bin/pytorch_inference/CCmdLineParser.cc @@ -42,7 +42,8 @@ bool CCmdLineParser::parse(int argc, bool& validElasticLicenseKeyConfirmed, bool& lowPriority, bool& useImmediateExecutor, - bool& skipModelValidation) { + bool& skipModelValidation, + bool& restrictFilesystem) { try { boost::program_options::options_description desc(DESCRIPTION); // clang-format off @@ -77,6 +78,7 @@ bool CCmdLineParser::parse(int argc, ("useImmediateExecutor", "Execute requests on the main thread. This mode should only used for " "benchmarking purposes to ensure requests are processed in order)") ("skipModelValidation", "Skip TorchScript model graph validation. WARNING: disables security checks on model operations.") + ("restrictFilesystem", "Confine filesystem access with a Landlock ruleset before loading the model. Used on the Sandbox2 route when the host forbids the user namespaces Sandbox2 needs.") ; // clang-format on @@ -153,6 +155,9 @@ bool CCmdLineParser::parse(int argc, if (vm.count("skipModelValidation") > 0) { skipModelValidation = true; } + if (vm.count("restrictFilesystem") > 0) { + restrictFilesystem = true; + } } catch (std::exception& e) { std::cerr << "Error processing command line: " << e.what() << std::endl; return false; diff --git a/bin/pytorch_inference/CCmdLineParser.h b/bin/pytorch_inference/CCmdLineParser.h index 3889bc832b..8a6296ef63 100644 --- a/bin/pytorch_inference/CCmdLineParser.h +++ b/bin/pytorch_inference/CCmdLineParser.h @@ -53,7 +53,8 @@ class CCmdLineParser { bool& validElasticLicenseKeyConfirmed, bool& lowPriority, bool& useImmediateExecutor, - bool& skipModelValidation); + bool& skipModelValidation, + bool& restrictFilesystem); private: static const std::string DESCRIPTION; diff --git a/bin/pytorch_inference/Main.cc b/bin/pytorch_inference/Main.cc index 1a1c00b40b..4e1b85ed89 100644 --- a/bin/pytorch_inference/Main.cc +++ b/bin/pytorch_inference/Main.cc @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -109,6 +110,38 @@ void verifySafeModelBeforeLoad(const char* modelData, std::size_t modelSize) { } } +namespace { +//! Apply the Landlock ruleset for the Landlock rung. Returns false, after +//! logging why, if this process must not go on to handle untrusted input. +bool confineFilesystem(const std::string& logPipePath) { + const std::string ipcDirectory{ml::seccomp::perChildIpcDirectory(logPipePath)}; + if (ipcDirectory.empty()) { + // The grant includes unlinking pipes; in the legacy flat $TMPDIR that + // would let this sandboxee delete another deployment's pipes. The + // controller only adds --restrictFilesystem alongside the per-child + // layout, so this is a caller bug - fail closed. + LOG_FATAL(<< "--restrictFilesystem requires the per-child IPC directory layout " + "($TMPDIR/ml-child-ipc//), but the log pipe is '" + << logPipePath << "'; refusing to process untrusted model input"); + return false; + } + const ml::seccomp::ELandlockOutcome outcome{ml::seccomp::applyLandlockFilesystemPolicy( + ml::seccomp::pytorchInferenceLandlockPaths(ipcDirectory))}; + if (outcome != ml::seccomp::ELandlockOutcome::E_Applied) { + // Should not happen: the controller only chooses this rung after + // confirming Landlock is available. Fail closed anyway - running on + // would serve untrusted model code with no filesystem boundary while + // the controller's sandbox2_launch signal says one is in force. + LOG_FATAL(<< "Landlock filesystem confinement " << ml::seccomp::describe(outcome) + << "; refusing to process untrusted model input. If this host cannot " + "support Landlock, deactivate the xpack.ml.trained_models.sandbox_enabled " + "setting to run models without a sandbox"); + return false; + } + return true; +} +} + torch::Tensor infer(torch::jit::script::Module& module_, ml::torch::CCommandParser::SRequest& request) { @@ -238,13 +271,14 @@ int main(int argc, char** argv) { bool lowPriority{false}; bool useImmediateExecutor{false}; bool skipModelValidation{false}; + bool restrictFilesystem{false}; if (ml::torch::CCmdLineParser::parse( - argc, argv, modelId, namedPipeConnectTimeout, inputFileName, - isInputFileNamedPipe, outputFileName, isOutputFileNamedPipe, restoreFileName, - isRestoreFileNamedPipe, logFileName, logProperties, numThreadsPerAllocation, - numAllocations, cacheMemorylimitBytes, validElasticLicenseKeyConfirmed, - lowPriority, useImmediateExecutor, skipModelValidation) == false) { + argc, argv, modelId, namedPipeConnectTimeout, inputFileName, isInputFileNamedPipe, + outputFileName, isOutputFileNamedPipe, restoreFileName, isRestoreFileNamedPipe, + logFileName, logProperties, numThreadsPerAllocation, numAllocations, + cacheMemorylimitBytes, validElasticLicenseKeyConfirmed, lowPriority, + useImmediateExecutor, skipModelValidation, restrictFilesystem) == false) { return EXIT_FAILURE; } @@ -307,6 +341,21 @@ int main(int argc, char** argv) { // Reduce memory priority before installing system call filters. ml::core::CProcessPriority::reduceMemoryPriority(); + // Filesystem confinement on the Landlock rung: the controller adds + // --restrictFilesystem when Elasticsearch asked for a sandbox but this + // host cannot run Sandbox2 (see CProcessSpawnerRouter). Ordering is + // load-bearing, and deliberate: + // - after the logger is reconfigured, so a failure here is visible; + // - BEFORE the in-process seccomp filter below, because that filter's + // allowlist does not permit the Landlock syscalls - installing it + // first makes landlock_create_ruleset() fail with EACCES; + // - before any model bytes are read, because the ruleset is + // irreversible and must already be in force when untrusted + // TorchScript (including __setstate__) is deserialized. + if (restrictFilesystem && confineFilesystem(logFileName) == false) { + return EXIT_FAILURE; + } + // Internal switch, deliberately still OFF (log-and-continue on a failed // in-process seccomp installation, exactly as before typed routing). // @@ -341,9 +390,13 @@ int main(int argc, char** argv) { // legacy-route attestation marker on a launch the controller's // sandbox2_launch signal reports as "route":"sandbox2". const bool sandbox2Launched{ml::seccomp::sandbox2LaunchedChild()}; + // The same filter is installed on the Landlock rung - Landlock and seccomp + // are meant to stack - so its attestation names that route, matching the + // controller's sandbox2_launch signal for this launch. const ml::seccomp::SInProcessFilterResult seccompResult{ml::seccomp::applyInProcessSeccompFilter( sandbox2Launched, TERMINATE_ON_DEGRADED_SECCOMP_FAILURE, - [] { return ml::seccomp::CSystemCallFilter::installSystemCallFilter(); })}; + [] { return ml::seccomp::CSystemCallFilter::installSystemCallFilter(); }, + restrictFilesystem ? "landlock" : "legacy")}; if (seccompResult.s_Attempted == false) { LOG_DEBUG(<< "ML_SANDBOXED=1: skipping in-process system call filter " diff --git a/include/sandbox/CSandbox2Diagnostics.h b/include/sandbox/CSandbox2Diagnostics.h index 5e7dbaa832..dff2d109d5 100644 --- a/include/sandbox/CSandbox2Diagnostics.h +++ b/include/sandbox/CSandbox2Diagnostics.h @@ -78,6 +78,80 @@ std::string describe(ESandbox2Capability capability); //! caller, which is why the probe forks first rather than unsharing inline. ESandbox2Capability probeSandbox2Capability(); +//! probeSandbox2Capability(), run at most once per process and cached. +//! +//! Every consumer of the verdict - the startup self-check and the spawn-time +//! routing decision - must use this rather than probing independently, so +//! the logged verdict and the route actually taken can never disagree. The +//! first call pays for the probe (one short-lived forked child); the result +//! is fixed for the life of the controller, which matches reality: whether +//! the host permits user namespaces does not change under a running process. +ESandbox2Capability sandbox2Capability(); + +//! The strongest confinement this host can give a launch that Elasticsearch +//! asked to be sandboxed (--requireSandbox). The controller walks this ladder +//! top-down and never silently skips a rung: each step down is logged with +//! the reason and what an administrator would have to change. +enum class EConfinementLevel { + //! Full Sandbox2 isolation: private mount, PID and network namespaces, + //! a minimal pivoted root filesystem, and the Sandbox2 syscall policy. + E_Sandbox2, + //! Sandbox2 is impossible on this host, but Landlock is available: + //! filesystem access is confined by a Landlock ruleset, stacked with the + //! in-process seccomp filter. No process, mount or network isolation. + E_Landlock, + //! Neither is possible. A --requireSandbox launch is refused outright - + //! running untrusted models unconfined while the operator asked for a + //! sandbox would be a silent downgrade. + E_Unavailable +}; + +//! Everything the controller knows about this host's confinement options, +//! gathered once. The passive sysctl values are carried alongside the active +//! probe results because they are what tell an administrator which knob to +//! turn, even though they are never used to decide the level. +struct SHostConfinement { + EConfinementLevel s_Level{EConfinementLevel::E_Unavailable}; + ESandbox2Capability s_Sandbox2{ESandbox2Capability::E_ProbeUnsupported}; + //! As returned by seccomp::landlockAbiVersion(): >= 1 the ABI version, + //! 0 unsupported by the kernel, -1 denied by a seccomp filter or LSM. + int s_LandlockAbi{0}; + //! /proc/sys/kernel/unprivileged_userns_clone, or "absent". + std::string s_UnprivilegedUsernsClone{"absent"}; + //! /proc/sys/user/max_user_namespaces, or "absent". + std::string s_MaxUserNamespaces{"absent"}; +}; + +//! The ladder itself, as a pure function of the two probe results so that it +//! can be tested without a host that has (or lacks) each capability. +EConfinementLevel decideConfinement(ESandbox2Capability sandbox2, int landlockAbi); + +//! This host's confinement, probed at most once per process and cached - the +//! single source of truth for both the startup self-check and every routing +//! decision, for the same reason as sandbox2Capability(). +const SHostConfinement& hostConfinement(); + +//! Human-readable form of a landlockAbiVersion() result. +std::string describeLandlock(int landlockAbi); + +//! What a system administrator must change for full Sandbox2 isolation on +//! this host, as one or two sentences. Derived from the diagnosed cause, not +//! a generic hint: kernel.unprivileged_userns_clone=0 and a container runtime +//! that blocks CLONE_NEWUSER look identical to the probe but need different +//! fixes, and only the sysctl value tells them apart. Empty when Sandbox2 is +//! already available. +std::string fullSandboxRemedy(const SHostConfinement& host); + +//! The INFO-level explanation logged when a launch takes the Landlock rung. +std::string landlockFallbackMessage(const SHostConfinement& host, + const std::string& processPath); + +//! The ERROR-level explanation when neither rung is available, which is also +//! returned to Elasticsearch as the failure reason for the launch. Tells the +//! user to deactivate xpack.ml.trained_models.sandbox_enabled, because on +//! such a host that is the only way to run models at all. +std::string noConfinementMessage(const SHostConfinement& host, const std::string& processPath); + //! Log a one-time Sandbox2 environment self-check at INFO level, combining //! the active capability probe above with the passive host facts that help //! interpret it. No-op after the first call, and on platforms without diff --git a/include/seccomp/CLandlockFilesystemPolicy.h b/include/seccomp/CLandlockFilesystemPolicy.h new file mode 100644 index 0000000000..7899ded162 --- /dev/null +++ b/include/seccomp/CLandlockFilesystemPolicy.h @@ -0,0 +1,159 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the following additional limitation. Functionality enabled by the + * files subject to the Elastic License 2.0 may only be used in production when + * invoked by an Elasticsearch process with a license key installed that permits + * use of machine learning features. You may not use this file except in + * compliance with the Elastic License 2.0 and the foregoing additional + * limitation. + */ +#ifndef INCLUDED_ml_seccomp_CLandlockFilesystemPolicy_h +#define INCLUDED_ml_seccomp_CLandlockFilesystemPolicy_h + +#include +#include + +namespace ml { +namespace seccomp { + +//! \brief +//! Filesystem confinement that does not require user namespaces. +//! +//! DESCRIPTION:\n +//! seccomp-BPF cannot restrict which *paths* a process opens - its filter +//! program sees only register values, never the pointed-to path string - so +//! the legacy in-process filter leaves a sandboxee able to read and write +//! anything its uid can reach. Sandbox2 closes that gap with a mount +//! namespace and pivot_root, but creating one needs an unprivileged user +//! namespace, which some hosts forbid outright +//! (kernel.unprivileged_userns_clone=0, or a container runtime seccomp +//! profile that denies CLONE_NEWUSER - see +//! sandbox::probeSandbox2Capability()). +//! +//! Landlock is the kernel's answer to exactly that case: an unprivileged, +//! self-applied filesystem access-control ruleset needing no capabilities and +//! no namespaces. It gives the path confinement half of what the Sandbox2 +//! rootfs gives, and composes with the existing seccomp filter. +//! +//! IMPLEMENTATION DECISIONS:\n +//! What Landlock does NOT provide, and must not be claimed for it: no process +//! table isolation (the sandboxee still sees host PIDs through /proc unless a +//! rule denies it), no private mount view (denied paths remain visible and +//! enumerable, they just cannot be opened), no network namespace, and no +//! effect on file descriptors that were already open when the ruleset was +//! applied. It is strictly weaker than the Sandbox2 route and is a fallback +//! for hosts that cannot run it, never a replacement. +//! +//! The Landlock UAPI headers are absent from the CI build image +//! (docker.elastic.co/ml-dev/ml-linux-build is CentOS7-based), so the +//! syscall numbers and structures are declared locally in the .cc, the same +//! way CMlLegacyBpfSyscallAllowlist.h falls back to raw numbers for statx, +//! rseq and clone3. Runtime ABI negotiation then decides which access rights +//! this kernel understands, so a binary built anywhere runs correctly on any +//! kernel. +struct SLandlockPaths { + //! Directories and files the sandboxee may read, and nothing else - never + //! write, and never execute. EXECUTE is deliberately withheld everywhere: + //! dlopen() opens and maps a library without it (only execve() needs it), + //! so withholding it makes Landlock alone refuse execve() even if the + //! seccomp filter that normally denies it failed to install. + std::vector s_ReadOnly; + + //! Directories that may contain nothing but this process's own named + //! pipes: it may create a FIFO, open it for reading or writing, and unlink + //! it (CNamedPipeFactory unlinks each FIFO once connected). Creating a + //! regular file, directory, symlink or socket is denied, so the directory + //! cannot be used to fill the disk or stage data. + std::vector s_PipeDirectories; +}; + +//! Outcome of applyLandlockFilesystemPolicy(). +enum class ELandlockOutcome { + //! The ruleset was applied; the process is now confined. + E_Applied, + //! This kernel has no Landlock support (the syscall returned ENOSYS, or + //! the LSM is not enabled in the bootloader's lsm= list). + E_Unsupported, + //! Landlock exists but the ruleset could not be built or applied. + E_Failed +}; + +//! Human-readable one-line form of \p outcome. +std::string describe(ELandlockOutcome outcome); + +//! Query, without applying anything, whether this process could use Landlock. +//! +//! \return the Landlock ABI version (>= 1) the kernel supports; 0 if the +//! kernel has no Landlock (ENOSYS: older than 5.13 or compiled out; +//! EOPNOTSUPP: built in but absent from the bootloader's lsm= list); or -1 +//! if the kernel supports it but a seccomp filter or LSM denied the query. +//! The three cases have different remedies, so they are never collapsed. +//! Safe to call from any process: asking for the ABI version creates no +//! ruleset and restricts nothing. +int landlockAbiVersion(); + +//! The per-child IPC directory that holds \p logPipePath, i.e. the path with +//! its last component removed, provided that directory has the shape +//! .../ml-child-ipc/. Empty for anything else - in particular +//! the legacy flat layout, where the pipes sit directly in the shared +//! $TMPDIR. The Landlock pipe-directory grant includes unlinking, so it may +//! only ever be given to a directory that holds nothing but this process's +//! own pipes; callers must refuse to confine (and must not run) otherwise. +inline std::string perChildIpcDirectory(const std::string& logPipePath) { + static const std::string PER_CHILD_PARENT{"ml-child-ipc"}; + // Absolute only: a relative path would make the Landlock rule depend on + // the process's working directory. Elasticsearch always sends absolute + // pipe paths. + if (logPipePath.empty() || logPipePath[0] != '/') { + return std::string{}; + } + const std::size_t fileSlash{logPipePath.rfind('/')}; + if (fileSlash == std::string::npos || fileSlash == 0) { + return std::string{}; + } + const std::string directory{logPipePath.substr(0, fileSlash)}; + const std::size_t idSlash{directory.rfind('/')}; + if (idSlash == std::string::npos || idSlash + 1 == directory.size()) { + return std::string{}; + } + const std::size_t parentSlash{directory.rfind('/', idSlash - 1)}; + const std::size_t parentStart{parentSlash == std::string::npos ? 0 : parentSlash + 1}; + if (idSlash == 0 || + directory.compare(parentStart, idSlash - parentStart, PER_CHILD_PARENT) != 0 || + idSlash - parentStart != PER_CHILD_PARENT.size()) { + return std::string{}; + } + return directory; +} + +//! The paths pytorch_inference needs, derived from its own resolved binary +//! location and the directory its IPC pipes live in. +//! +//! Derived from a trace of every Landlock-mediated operation pytorch_inference +//! performs after the ruleset is applied - startup, model load and inference +//! of the quantized ELSER model with two threads - plus the exceptions noted +//! at each entry. Sensitive trees (/proc, /etc) are granted as exact files; +//! whole directories are granted only where the contents are not sensitive +//! and the exact set varies by CPU (the bundled library directory, from which +//! oneMKL dlopen()s CPU-specific kernels, and the CPU topology in sysfs). +//! +//! \param ipcDirectory the per-child IPC directory +//! $TMPDIR/ml-child-ipc/ holding the --input/--output/ +//! --restore/--logPipe pipes. Callers must not pass a shared directory: its +//! pipe-directory rights include unlinking, which in a directory shared +//! between deployments would let one sandboxee delete another's pipes. +SLandlockPaths pytorchInferenceLandlockPaths(const std::string& ipcDirectory); + +//! Apply \p paths as a Landlock ruleset to the calling process, denying every +//! filesystem access the ABI can describe that the rules do not grant. +//! +//! Irreversible for the lifetime of the process, and inherited by children. +//! Sets PR_SET_NO_NEW_PRIVS, which Landlock requires of an unprivileged +//! caller. Must be called before any untrusted input is processed. +ELandlockOutcome applyLandlockFilesystemPolicy(const SLandlockPaths& paths); + +} // namespace seccomp +} // namespace ml + +#endif // INCLUDED_ml_seccomp_CLandlockFilesystemPolicy_h diff --git a/include/seccomp/CSystemCallFilter.h b/include/seccomp/CSystemCallFilter.h index ad6dc65a76..7dbae3ec29 100644 --- a/include/seccomp/CSystemCallFilter.h +++ b/include/seccomp/CSystemCallFilter.h @@ -121,11 +121,17 @@ inline EDegradedModeAction decideDegradedModeAction(ESystemCallFilterInstallOutc //! is called with terminateOnFailure true (not today's production default). //! Logged over the existing per-process log pipe; this is not a new startup //! channel. -inline std::string degradedModeAttestationMarker(ESystemCallFilterInstallOutcome outcome) { +//! +//! \param route the controller route this filter belongs to: "legacy", or +//! "landlock" when the same in-process filter is stacked under a +//! Landlock ruleset because the host cannot run Sandbox2. It must +//! match the controller's sandbox2_launch signal for the same launch. +inline std::string degradedModeAttestationMarker(ESystemCallFilterInstallOutcome outcome, + const std::string& route = "legacy") { if (outcome != ESystemCallFilterInstallOutcome::E_Installed) { return std::string(); } - return R"({"ml_sandbox2_route":"legacy","event":"seccomp_installed"})"; + return R"({"ml_sandbox2_route":")" + route + R"(","event":"seccomp_installed"})"; } //! Pure form of the "was this process launched by the Sandbox2 executor?" @@ -187,10 +193,12 @@ struct SInProcessFilterResult { //! \param terminateOnFailure passed through to decideDegradedModeAction(). //! \param installer invoked at most once; normally //! CSystemCallFilter::installSystemCallFilter. +//! \param route passed through to degradedModeAttestationMarker(). template SInProcessFilterResult applyInProcessSeccompFilter(bool sandbox2Launched, bool terminateOnFailure, - INSTALLER installer) { + INSTALLER installer, + const std::string& route = "legacy") { SInProcessFilterResult result; if (sandbox2Launched) { return result; @@ -198,7 +206,7 @@ SInProcessFilterResult applyInProcessSeccompFilter(bool sandbox2Launched, result.s_Attempted = true; result.s_Outcome = installer(); result.s_Action = decideDegradedModeAction(result.s_Outcome, terminateOnFailure); - result.s_AttestationMarker = degradedModeAttestationMarker(result.s_Outcome); + result.s_AttestationMarker = degradedModeAttestationMarker(result.s_Outcome, route); return result; } diff --git a/lib/sandbox/CSandbox2Diagnostics_Linux.cc b/lib/sandbox/CSandbox2Diagnostics_Linux.cc index 9077f4ae86..b760cfecb6 100644 --- a/lib/sandbox/CSandbox2Diagnostics_Linux.cc +++ b/lib/sandbox/CSandbox2Diagnostics_Linux.cc @@ -11,6 +11,7 @@ #include #include +#include // Portable half: the capability vocabulary every platform may print, and the // no-op entry points a build without Sandbox2 links instead of the probe. @@ -55,12 +56,118 @@ std::string describe(ESandbox2Capability capability) { return "unrecognized capability value"; } +ESandbox2Capability sandbox2Capability() { + // Function-local static: initialised exactly once, thread-safely. + static const ESandbox2Capability capability{probeSandbox2Capability()}; + return capability; +} + +EConfinementLevel decideConfinement(ESandbox2Capability sandbox2, int landlockAbi) { + if (sandbox2 == ESandbox2Capability::E_Available) { + return EConfinementLevel::E_Sandbox2; + } + // A build without Sandbox2 never reaches the sandboxed route (the router + // refuses it outright), so it is never offered the Landlock rung either. + if (sandbox2 == ESandbox2Capability::E_ProbeUnsupported) { + return EConfinementLevel::E_Unavailable; + } + // Any Sandbox2 denial - including E_ProbeFailed, where attempting + // Sandbox2 anyway could deadlock in the forkserver's namespace setup - + // steps down to Landlock if the kernel supports it. + return landlockAbi >= 1 ? EConfinementLevel::E_Landlock : EConfinementLevel::E_Unavailable; +} + +std::string describeLandlock(int landlockAbi) { + if (landlockAbi >= 1) { + return "available (ABI " + std::to_string(landlockAbi) + ")"; + } + if (landlockAbi == 0) { + return "not supported by this kernel"; + } + return "blocked by a seccomp filter or LSM policy"; +} + +std::string fullSandboxRemedy(const SHostConfinement& host) { + switch (host.s_Sandbox2) { + case ESandbox2Capability::E_Available: + return std::string{}; + case ESandbox2Capability::E_UserNamespaceDenied: + if (host.s_UnprivilegedUsernsClone == "0") { + return "For full Sandbox2 isolation, a system administrator must allow unprivileged " + "user namespaces by setting the kernel parameter " + "kernel.unprivileged_userns_clone=1 (for example with " + "'sysctl -w kernel.unprivileged_userns_clone=1', persisted in /etc/sysctl.d/)."; + } + if (host.s_MaxUserNamespaces == "0") { + return "For full Sandbox2 isolation, a system administrator must allow user " + "namespaces by setting the kernel parameter user.max_user_namespaces to a " + "non-zero value."; + } + return "For full Sandbox2 isolation, a system administrator must allow unprivileged " + "user namespaces for this process. The kernel permits them " + "(kernel.unprivileged_userns_clone is not 0), so they are being blocked by the " + "container runtime - typically a seccomp profile that denies clone/unshare with " + "CLONE_NEWUSER."; + case ESandbox2Capability::E_IdMapWriteDenied: + case ESandbox2Capability::E_MountOrPidNamespaceDenied: + return "For full Sandbox2 isolation, a system administrator must allow this process " + "to set up user, mount and PID namespaces; user namespaces can be created, but " + "the container runtime blocks the later steps."; + case ESandbox2Capability::E_TmpfsMountDenied: + return "For full Sandbox2 isolation, a system administrator must allow mounts inside " + "unprivileged user namespaces for this process; they are currently denied, " + "typically by an AppArmor or SELinux policy."; + case ESandbox2Capability::E_ProcMountDenied: + return "For full Sandbox2 isolation, a system administrator must allow this process " + "to mount a private /proc; the container runtime currently masks parts of /proc, " + "which makes the kernel refuse it."; + case ESandbox2Capability::E_ProbeFailed: + return "The Sandbox2 capability probe itself could not run, so the reason is unknown; " + "see the earlier ML controller log messages."; + case ESandbox2Capability::E_ProbeUnsupported: + return "This build does not include Sandbox2."; + } + return std::string{}; +} + +std::string landlockFallbackMessage(const SHostConfinement& host, + const std::string& processPath) { + return "Full Sandbox2 isolation is not available on this host (" + + describe(host.s_Sandbox2) + "), so '" + processPath + + "' is being launched with Landlock filesystem confinement and the seccomp system " + "call filter instead. Landlock restricts which files the process can open but, " + "unlike Sandbox2, does not isolate its view of processes, mounts or the network. " + + fullSandboxRemedy(host); +} + +std::string noConfinementMessage(const SHostConfinement& host, const std::string& processPath) { + const std::string why{host.s_LandlockAbi == 0 + ? "the operating system is too old or its kernel lacks the required " + "features (Landlock needs Linux 5.13 or later)" + : "Landlock is " + describeLandlock(host.s_LandlockAbi)}; + return "Refusing to launch '" + processPath + + "': xpack.ml.trained_models.sandbox_enabled is true, but this host supports neither " + "Sandbox2 isolation (" + + describe(host.s_Sandbox2) + + ") nor Landlock filesystem " + "confinement - " + + why + + ". To run models on this node, deactivate the " + "xpack.ml.trained_models.sandbox_enabled setting (set it to false); models then run " + "with the seccomp system call filter only."; +} + #if !defined(__linux__) || !defined(SANDBOX2_AVAILABLE) ESandbox2Capability probeSandbox2Capability() { return ESandbox2Capability::E_ProbeUnsupported; } +const SHostConfinement& hostConfinement() { + static const SHostConfinement host{}; + return host; +} + void logSandbox2EnvironmentSelfCheck() { // Deliberately silent rather than logging "not applicable" on every // controller start: a build with no Sandbox2 support never routes to it, @@ -283,6 +390,21 @@ ESandbox2Capability probeSandbox2Capability() { return capabilityFromExit(WEXITSTATUS(status)); } +const SHostConfinement& hostConfinement() { + static const SHostConfinement host{[] { + SHostConfinement h; + h.s_Sandbox2 = sandbox2Capability(); + h.s_LandlockAbi = seccomp::landlockAbiVersion(); + const std::string userns{readProcSysValue("/proc/sys/kernel/unprivileged_userns_clone")}; + const std::string maxUserns{readProcSysValue("/proc/sys/user/max_user_namespaces")}; + h.s_UnprivilegedUsernsClone = userns.empty() ? "absent" : userns; + h.s_MaxUserNamespaces = maxUserns.empty() ? "absent" : maxUserns; + h.s_Level = decideConfinement(h.s_Sandbox2, h.s_LandlockAbi); + return h; + }()}; + return host; +} + void logSandbox2EnvironmentSelfCheck() { static bool logged{false}; if (logged) { @@ -290,42 +412,48 @@ void logSandbox2EnvironmentSelfCheck() { } logged = true; - const ESandbox2Capability capability{probeSandbox2Capability()}; - - // Passive host facts alongside the active result. These are what the - // frozen prior art (ml-cpp#2873's CSandbox2Diagnostics) reported on its - // own; they are kept because they help interpret a denial, but they are - // deliberately no longer the answer: both sysctls below are host-global - // and are inherited unchanged by a container whose seccomp or LSM policy - // denies the operation anyway, so on their own they report a healthy - // environment on exactly the hosts where the sandbox cannot start. - std::string usernsSysctl{readProcSysValue("/proc/sys/kernel/unprivileged_userns_clone")}; - if (usernsSysctl.empty()) { - usernsSysctl = "absent"; - } - std::string maxUserNamespaces{readProcSysValue("/proc/sys/user/max_user_namespaces")}; - if (maxUserNamespaces.empty()) { - maxUserNamespaces = "absent"; - } + const SHostConfinement& host{hostConfinement()}; + // The passive sysctl values are what the frozen prior art (ml-cpp#2873's + // CSandbox2Diagnostics) reported on its own. They never decide anything + // - both are host-global and are inherited unchanged by a container whose + // seccomp or LSM policy denies user namespaces regardless - but they are + // what tells an administrator which knob to turn. const char* tmpDirEnv{::getenv("TMPDIR")}; const std::string tmpDir{tmpDirEnv != nullptr ? tmpDirEnv : "/tmp"}; - - const std::string message{ - "Sandbox2 environment self-check: capability=" + describe(capability) + - ", unprivileged_userns_clone=" + usernsSysctl + - ", max_user_namespaces=" + maxUserNamespaces + ", TMPDIR=" + tmpDir + + const std::string facts{ + "Sandbox2 environment self-check: sandbox2=" + describe(host.s_Sandbox2) + + ", landlock=" + describeLandlock(host.s_LandlockAbi) + + ", unprivileged_userns_clone=" + host.s_UnprivilegedUsernsClone + + ", max_user_namespaces=" + host.s_MaxUserNamespaces + ", TMPDIR=" + tmpDir + ", TMPDIR writable=" + (::access(tmpDir.c_str(), W_OK) == 0 ? "yes" : "no") + ", TMPDIR noexec=" + (pathHasNoexecFlag(tmpDir.c_str()) ? "yes" : "no")}; - if (capability == ESandbox2Capability::E_Available) { - LOG_INFO(<< message); - } else { - // Not fatal and not a launch failure: the controller only fails a - // launch if Elasticsearch actually asks for the Sandbox2 route. A - // node that never sets sandbox_enabled=true runs unaffected, so this - // is a warning about what *would* happen, not an error that happened. - LOG_WARN(<< message << " - a --requireSandbox launch on this host will fail closed"); + // Logged at controller start, before any launch, and regardless of + // xpack.ml.trained_models.sandbox_enabled (which the controller only + // learns per launch) - so each message says what *would* happen if the + // setting is true. + switch (host.s_Level) { + case EConfinementLevel::E_Sandbox2: + LOG_INFO(<< facts + << ". Models launched with xpack.ml.trained_models.sandbox_enabled=true " + "will run with full Sandbox2 isolation."); + break; + case EConfinementLevel::E_Landlock: + // A supported, deliberate degradation - INFO, not WARN. + LOG_INFO(<< facts + << ". Models launched with xpack.ml.trained_models.sandbox_enabled=true " + "will run with Landlock filesystem confinement, because full Sandbox2 " + "isolation is not available on this host. " + << fullSandboxRemedy(host)); + break; + case EConfinementLevel::E_Unavailable: + LOG_WARN(<< facts + << ". This host supports neither Sandbox2 nor Landlock, so every model " + "deployment on this node will fail to start while " + "xpack.ml.trained_models.sandbox_enabled is true; deactivate that " + "setting (set it to false) to run models here."); + break; } } diff --git a/lib/sandbox/unittest/CSandbox2DiagnosticsTest.cc b/lib/sandbox/unittest/CSandbox2DiagnosticsTest.cc index 54cb82bf03..454155dae5 100644 --- a/lib/sandbox/unittest/CSandbox2DiagnosticsTest.cc +++ b/lib/sandbox/unittest/CSandbox2DiagnosticsTest.cc @@ -67,6 +67,148 @@ BOOST_AUTO_TEST_CASE(testProbeReportsUnsupportedWithoutSandbox2) { } } +// decideConfinement(), fullSandboxRemedy(), noConfinementMessage() and +// landlockFallbackMessage() are pure functions of their arguments - declared +// unconditionally in the header, and defined in the portable (non-Linux-only) +// part of CSandbox2Diagnostics_Linux.cc - so they are testable on every +// platform without a host that actually has (or lacks) either capability. + +BOOST_AUTO_TEST_CASE(testDecideConfinementLadder) { + using ml::sandbox::EConfinementLevel; + using ml::sandbox::ESandbox2Capability; + using ml::sandbox::decideConfinement; + + struct SCase { + ESandbox2Capability s_Sandbox2; + int s_LandlockAbi; + EConfinementLevel s_Expected; + }; + + const SCase cases[]{ + // E_Available always wins the top rung, whatever Landlock reports - + // a working Sandbox2 is never downgraded because of it. + {ESandbox2Capability::E_Available, 5, EConfinementLevel::E_Sandbox2}, + {ESandbox2Capability::E_Available, 0, EConfinementLevel::E_Sandbox2}, + {ESandbox2Capability::E_Available, -1, EConfinementLevel::E_Sandbox2}, + + // E_ProbeUnsupported (no Sandbox2 support compiled in) never reaches + // the Landlock rung either, even when Landlock itself is available - + // the router refuses such a build's sandboxed route outright before + // the ladder is ever consulted. + {ESandbox2Capability::E_ProbeUnsupported, 5, EConfinementLevel::E_Unavailable}, + {ESandbox2Capability::E_ProbeUnsupported, 1, EConfinementLevel::E_Unavailable}, + {ESandbox2Capability::E_ProbeUnsupported, 0, EConfinementLevel::E_Unavailable}, + {ESandbox2Capability::E_ProbeUnsupported, -1, EConfinementLevel::E_Unavailable}, + + // Every other Sandbox2 denial steps down to Landlock iff the ABI is + // supported (>= 1), and to E_Unavailable otherwise (kernel too old, + // abi == 0; or blocked by seccomp/LSM, abi == -1). + {ESandbox2Capability::E_UserNamespaceDenied, 1, EConfinementLevel::E_Landlock}, + {ESandbox2Capability::E_UserNamespaceDenied, 2, EConfinementLevel::E_Landlock}, + {ESandbox2Capability::E_UserNamespaceDenied, 0, EConfinementLevel::E_Unavailable}, + {ESandbox2Capability::E_UserNamespaceDenied, -1, EConfinementLevel::E_Unavailable}, + + {ESandbox2Capability::E_IdMapWriteDenied, 1, EConfinementLevel::E_Landlock}, + {ESandbox2Capability::E_IdMapWriteDenied, 0, EConfinementLevel::E_Unavailable}, + {ESandbox2Capability::E_IdMapWriteDenied, -1, EConfinementLevel::E_Unavailable}, + + {ESandbox2Capability::E_MountOrPidNamespaceDenied, 1, EConfinementLevel::E_Landlock}, + {ESandbox2Capability::E_MountOrPidNamespaceDenied, 0, EConfinementLevel::E_Unavailable}, + {ESandbox2Capability::E_MountOrPidNamespaceDenied, -1, EConfinementLevel::E_Unavailable}, + + {ESandbox2Capability::E_TmpfsMountDenied, 1, EConfinementLevel::E_Landlock}, + {ESandbox2Capability::E_TmpfsMountDenied, 0, EConfinementLevel::E_Unavailable}, + {ESandbox2Capability::E_TmpfsMountDenied, -1, EConfinementLevel::E_Unavailable}, + + {ESandbox2Capability::E_ProcMountDenied, 1, EConfinementLevel::E_Landlock}, + {ESandbox2Capability::E_ProcMountDenied, 0, EConfinementLevel::E_Unavailable}, + {ESandbox2Capability::E_ProcMountDenied, -1, EConfinementLevel::E_Unavailable}, + + // E_ProbeFailed (the probe itself could not run) is treated the same + // as any other denial - explicitly required, since attempting + // Sandbox2 anyway on an unknown-capability host could deadlock in + // the forkserver's namespace setup. + {ESandbox2Capability::E_ProbeFailed, 1, EConfinementLevel::E_Landlock}, + {ESandbox2Capability::E_ProbeFailed, 2, EConfinementLevel::E_Landlock}, + {ESandbox2Capability::E_ProbeFailed, 0, EConfinementLevel::E_Unavailable}, + {ESandbox2Capability::E_ProbeFailed, -1, EConfinementLevel::E_Unavailable}, + }; + + for (const auto& testCase : cases) { + BOOST_TEST_MESSAGE("sandbox2=" << ml::sandbox::describe(testCase.s_Sandbox2) + << " landlockAbi=" << testCase.s_LandlockAbi); + BOOST_REQUIRE(decideConfinement(testCase.s_Sandbox2, testCase.s_LandlockAbi) == + testCase.s_Expected); + } +} + +BOOST_AUTO_TEST_CASE(testFullSandboxRemedyDistinguishesSysctlFromContainerRuntime) { + // kernel.unprivileged_userns_clone=0 and a container runtime that blocks + // CLONE_NEWUSER look identical to the probe (both are + // E_UserNamespaceDenied) but need different fixes, and only the sysctl + // value tells them apart - the whole reason fullSandboxRemedy() takes the + // full host struct rather than just the capability enum. + ml::sandbox::SHostConfinement sysctlDenies; + sysctlDenies.s_Sandbox2 = ml::sandbox::ESandbox2Capability::E_UserNamespaceDenied; + sysctlDenies.s_UnprivilegedUsernsClone = "0"; + const std::string sysctlRemedy{ml::sandbox::fullSandboxRemedy(sysctlDenies)}; + BOOST_TEST_REQUIRE(sysctlRemedy.find("kernel.unprivileged_userns_clone=1") != + std::string::npos); + BOOST_TEST_REQUIRE(sysctlRemedy.find("system administrator") != std::string::npos); + + ml::sandbox::SHostConfinement runtimeDenies; + runtimeDenies.s_Sandbox2 = ml::sandbox::ESandbox2Capability::E_UserNamespaceDenied; + runtimeDenies.s_UnprivilegedUsernsClone = "1"; + runtimeDenies.s_MaxUserNamespaces = "65536"; + const std::string runtimeRemedy{ml::sandbox::fullSandboxRemedy(runtimeDenies)}; + BOOST_TEST_REQUIRE(runtimeRemedy.find("container runtime") != std::string::npos); + // The sysctl is fine on this host, so the remedy must not tell the + // administrator to set it - that would send them to change a value that + // is already correct. + BOOST_TEST_REQUIRE(runtimeRemedy.find("=1") == std::string::npos); + + ml::sandbox::SHostConfinement available; + available.s_Sandbox2 = ml::sandbox::ESandbox2Capability::E_Available; + BOOST_TEST_REQUIRE(ml::sandbox::fullSandboxRemedy(available).empty()); +} + +BOOST_AUTO_TEST_CASE(testNoConfinementMessageExplainsAndTellsTheOperatorWhatToDo) { + ml::sandbox::SHostConfinement tooOld; + tooOld.s_Sandbox2 = ml::sandbox::ESandbox2Capability::E_UserNamespaceDenied; + tooOld.s_LandlockAbi = 0; + const std::string tooOldMessage{ml::sandbox::noConfinementMessage( + tooOld, "/usr/share/elasticsearch/bin/pytorch_inference")}; + BOOST_TEST_REQUIRE(tooOldMessage.find("xpack.ml.trained_models.sandbox_enabled") != + std::string::npos); + BOOST_TEST_REQUIRE(tooOldMessage.find("deactivate") != std::string::npos); + BOOST_TEST_REQUIRE(tooOldMessage.find("too old") != std::string::npos); + + ml::sandbox::SHostConfinement blocked; + blocked.s_Sandbox2 = ml::sandbox::ESandbox2Capability::E_UserNamespaceDenied; + blocked.s_LandlockAbi = -1; + const std::string blockedMessage{ml::sandbox::noConfinementMessage( + blocked, "/usr/share/elasticsearch/bin/pytorch_inference")}; + BOOST_TEST_REQUIRE(blockedMessage.find("xpack.ml.trained_models.sandbox_enabled") != + std::string::npos); + BOOST_TEST_REQUIRE(blockedMessage.find("deactivate") != std::string::npos); + BOOST_TEST_REQUIRE(blockedMessage.find("blocked") != std::string::npos); +} + +BOOST_AUTO_TEST_CASE(testLandlockFallbackMessageNamesThePathAndDoesNotOverclaim) { + ml::sandbox::SHostConfinement host; + host.s_Sandbox2 = ml::sandbox::ESandbox2Capability::E_UserNamespaceDenied; + host.s_LandlockAbi = 1; + const std::string processPath{"/usr/share/elasticsearch/bin/pytorch_inference"}; + const std::string message{ml::sandbox::landlockFallbackMessage(host, processPath)}; + + BOOST_TEST_REQUIRE(message.find(processPath) != std::string::npos); + BOOST_TEST_REQUIRE(message.find("Landlock") != std::string::npos); + // Landlock confines the filesystem only - the message must be honest + // that it does not give process/mount/network isolation, so an operator + // never mistakes the fallback for full Sandbox2 isolation. + BOOST_TEST_REQUIRE(message.find("does not isolate") != std::string::npos); +} + #ifdef Linux BOOST_AUTO_TEST_CASE(testProbeAgreesWithAnIndependentUnshareAttempt) { diff --git a/lib/seccomp/CLandlockFilesystemPolicy.cc b/lib/seccomp/CLandlockFilesystemPolicy.cc new file mode 100644 index 0000000000..8ee1835250 --- /dev/null +++ b/lib/seccomp/CLandlockFilesystemPolicy.cc @@ -0,0 +1,46 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the following additional limitation. Functionality enabled by the + * files subject to the Elastic License 2.0 may only be used in production when + * invoked by an Elasticsearch process with a license key installed that permits + * use of machine learning features. You may not use this file except in + * compliance with the Elastic License 2.0 and the foregoing additional + * limitation. + */ +#include + +// Landlock is a Linux LSM; ml_generate_platform_sources() substitutes +// CLandlockFilesystemPolicy_Linux.cc for this file on Linux builds. This +// translation unit is what macOS and Windows compile instead, so a caller +// can stay platform-independent and simply observe E_Unsupported. + +namespace ml { +namespace seccomp { + +std::string describe(ELandlockOutcome outcome) { + switch (outcome) { + case ELandlockOutcome::E_Applied: + return "applied"; + case ELandlockOutcome::E_Unsupported: + return "unsupported on this platform"; + case ELandlockOutcome::E_Failed: + return "failed"; + } + return "unrecognized outcome"; +} + +int landlockAbiVersion() { + return 0; +} + +SLandlockPaths pytorchInferenceLandlockPaths(const std::string& /*ipcDirectory*/) { + return SLandlockPaths{}; +} + +ELandlockOutcome applyLandlockFilesystemPolicy(const SLandlockPaths& /*paths*/) { + return ELandlockOutcome::E_Unsupported; +} + +} // namespace seccomp +} // namespace ml diff --git a/lib/seccomp/CLandlockFilesystemPolicy_Linux.cc b/lib/seccomp/CLandlockFilesystemPolicy_Linux.cc new file mode 100644 index 0000000000..7f922a371e --- /dev/null +++ b/lib/seccomp/CLandlockFilesystemPolicy_Linux.cc @@ -0,0 +1,332 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the following additional limitation. Functionality enabled by the + * files subject to the Elastic License 2.0 may only be used in production when + * invoked by an Elasticsearch process with a license key installed that permits + * use of machine learning features. You may not use this file except in + * compliance with the Elastic License 2.0 and the foregoing additional + * limitation. + */ +#include + +#include + +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +namespace ml { +namespace seccomp { +namespace { + +// Landlock UAPI, declared locally: linux/landlock.h is absent from the +// CentOS7-based CI build image. Same approach as the raw statx/rseq/clone3 +// numbers in CMlLegacyBpfSyscallAllowlist.h - build against whatever headers +// exist, then negotiate capability at runtime against the live kernel. +// +// The three syscalls were added together in 5.13 through the generic syscall +// table, so x86_64 and aarch64 share these numbers. +#ifndef ML_NR_landlock_create_ruleset +#define ML_NR_landlock_create_ruleset 444 +#endif +#ifndef ML_NR_landlock_add_rule +#define ML_NR_landlock_add_rule 445 +#endif +#ifndef ML_NR_landlock_restrict_self +#define ML_NR_landlock_restrict_self 446 +#endif + +constexpr std::uint32_t ML_LANDLOCK_CREATE_RULESET_VERSION{1U << 0}; +constexpr int ML_LANDLOCK_RULE_PATH_BENEATH{1}; + +// Access rights, by the ABI version that introduced them. +constexpr std::uint64_t ACCESS_FS_EXECUTE{1ULL << 0}; +constexpr std::uint64_t ACCESS_FS_WRITE_FILE{1ULL << 1}; +constexpr std::uint64_t ACCESS_FS_READ_FILE{1ULL << 2}; +constexpr std::uint64_t ACCESS_FS_READ_DIR{1ULL << 3}; +constexpr std::uint64_t ACCESS_FS_REMOVE_DIR{1ULL << 4}; +constexpr std::uint64_t ACCESS_FS_REMOVE_FILE{1ULL << 5}; +constexpr std::uint64_t ACCESS_FS_MAKE_CHAR{1ULL << 6}; +constexpr std::uint64_t ACCESS_FS_MAKE_DIR{1ULL << 7}; +constexpr std::uint64_t ACCESS_FS_MAKE_REG{1ULL << 8}; +constexpr std::uint64_t ACCESS_FS_MAKE_SOCK{1ULL << 9}; +constexpr std::uint64_t ACCESS_FS_MAKE_FIFO{1ULL << 10}; +constexpr std::uint64_t ACCESS_FS_MAKE_BLOCK{1ULL << 11}; +constexpr std::uint64_t ACCESS_FS_MAKE_SYM{1ULL << 12}; +constexpr std::uint64_t ACCESS_FS_REFER{1ULL << 13}; // ABI 2 +constexpr std::uint64_t ACCESS_FS_TRUNCATE{1ULL << 14}; // ABI 3 +constexpr std::uint64_t ACCESS_FS_IOCTL_DEV{1ULL << 15}; // ABI 5 + +//! Only handled_access_fs is filled, and the size passed to the kernel is +//! this structure's size, which is the ABI 1 size the kernel still accepts +//! from a newer kernel's point of view. Declaring the later +//! handled_access_net field would mean passing a larger size that an ABI 1-3 +//! kernel rejects. +struct SLandlockRulesetAttr { + std::uint64_t s_HandledAccessFs; +}; + +struct SLandlockPathBeneathAttr { + std::uint64_t s_AllowedAccess; + std::int32_t s_ParentFd; +} __attribute__((packed)); + +long landlockCreateRuleset(const SLandlockRulesetAttr* attr, std::size_t size, std::uint32_t flags) { + return ::syscall(ML_NR_landlock_create_ruleset, attr, size, flags); +} + +long landlockAddRule(int rulesetFd, int ruleType, const void* attr, std::uint32_t flags) { + return ::syscall(ML_NR_landlock_add_rule, rulesetFd, ruleType, attr, flags); +} + +long landlockRestrictSelf(int rulesetFd, std::uint32_t flags) { + return ::syscall(ML_NR_landlock_restrict_self, rulesetFd, flags); +} + +//! Every right this build knows about, narrowed to those \p abi understands. +//! Rights the kernel does not handle must be cleared: passing an unknown bit +//! makes landlock_create_ruleset() fail with EINVAL, which would turn a +//! newer-build-on-older-kernel into a hard failure rather than a slightly +//! coarser ruleset. +std::uint64_t handledAccessForAbi(int abi) { + std::uint64_t handled{ + ACCESS_FS_EXECUTE | ACCESS_FS_WRITE_FILE | ACCESS_FS_READ_FILE | + ACCESS_FS_READ_DIR | ACCESS_FS_REMOVE_DIR | ACCESS_FS_REMOVE_FILE | + ACCESS_FS_MAKE_CHAR | ACCESS_FS_MAKE_DIR | ACCESS_FS_MAKE_REG | ACCESS_FS_MAKE_SOCK | + ACCESS_FS_MAKE_FIFO | ACCESS_FS_MAKE_BLOCK | ACCESS_FS_MAKE_SYM}; + if (abi >= 2) { + handled |= ACCESS_FS_REFER; + } + if (abi >= 3) { + handled |= ACCESS_FS_TRUNCATE; + } + if (abi >= 5) { + handled |= ACCESS_FS_IOCTL_DEV; + } + return handled; +} + +//! The subset of access rights that apply to a non-directory. Every other +//! right describes an operation performed *within* a directory, and naming +//! one on a file makes landlock_add_rule() fail with EINVAL. +constexpr std::uint64_t FILE_APPLICABLE_ACCESS{ + ACCESS_FS_EXECUTE | ACCESS_FS_WRITE_FILE | ACCESS_FS_READ_FILE | + ACCESS_FS_TRUNCATE | ACCESS_FS_IOCTL_DEV}; + +//! Grant a path the rights it needs, intersected with what the ruleset +//! handles. A right granted but not handled is meaningless (Landlock only +//! restricts what it handles), and a right handled but not granted is what +//! actually denies access. +bool addPathRule(int rulesetFd, const std::string& path, std::uint64_t allowed, std::uint64_t handled) { + // O_PATH so opening the directory itself needs no read permission and + // triggers none of the side effects of a real open. + const int pathFd{::open(path.c_str(), O_PATH | O_CLOEXEC)}; + if (pathFd < 0) { + // 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. + LOG_DEBUG(<< "Landlock: skipping absent path " << path << ": " << ::strerror(errno)); + return true; + } + + // Landlock rejects a rule (EINVAL) whose allowed_access names a + // directory-only right when the file descriptor is not a directory, so a + // single "read-only" mask cannot be applied to both /usr/lib and + // /dev/urandom. Narrow to the rights that are meaningful for a file. + std::uint64_t allowedForThisPath{allowed}; + struct stat pathStat {}; + if (::fstat(pathFd, &pathStat) == 0 && S_ISDIR(pathStat.st_mode) == false) { + allowedForThisPath &= FILE_APPLICABLE_ACCESS; + } + + SLandlockPathBeneathAttr attr{}; + attr.s_AllowedAccess = allowedForThisPath & handled; + attr.s_ParentFd = pathFd; + const bool ok{landlockAddRule(rulesetFd, ML_LANDLOCK_RULE_PATH_BENEATH, &attr, 0) == 0}; + if (ok == false) { + LOG_ERROR(<< "Landlock: could not add rule for " << path << ": " + << ::strerror(errno)); + } + ::close(pathFd); + return ok; +} + +std::string parentDirectory(const std::string& path) { + const std::size_t lastSlash{path.rfind('/')}; + if (lastSlash == std::string::npos || lastSlash == 0) { + return "/"; + } + return path.substr(0, lastSlash); +} + +} // namespace + +std::string describe(ELandlockOutcome outcome) { + switch (outcome) { + case ELandlockOutcome::E_Applied: + return "applied"; + case ELandlockOutcome::E_Unsupported: + return "unsupported on this kernel"; + case ELandlockOutcome::E_Failed: + return "failed"; + } + return "unrecognized outcome"; +} + +int landlockAbiVersion() { + const long abi{landlockCreateRuleset(nullptr, 0, ML_LANDLOCK_CREATE_RULESET_VERSION)}; + if (abi >= 1) { + return static_cast(abi); + } + return (errno == ENOSYS || errno == EOPNOTSUPP) ? 0 : -1; +} + +SLandlockPaths pytorchInferenceLandlockPaths(const std::string& ipcDirectory) { + SLandlockPaths paths; + + // The bundled library directory, /lib beside /bin. + // oneMKL dlopen()s a CPU-specific kernel from here on first use + // (libmkl_avx512.so.3 and libmkl_vml_avx512.so.3 on an AVX-512 host; + // avx2/mc3/def variants elsewhere), so the directory, not a file list, is + // granted. Everything else pytorch_inference links was mapped by the + // dynamic loader before main(), which is why neither its own bin + // directory nor any system library directory needs a grant. + char exePath[PATH_MAX]; + const ssize_t exeLength{::readlink("/proc/self/exe", exePath, sizeof(exePath) - 1)}; + if (exeLength > 0) { + exePath[exeLength] = '\0'; + paths.s_ReadOnly.push_back(parentDirectory(parentDirectory(exePath)) + "/lib"); + } + + // CPU topology. online/possible/present/kernel_max are what an x86_64 + // run reads (libgomp and the CPU-feature detection behind the quantized + // kernels); aarch64 reads further per-CPU files beneath this directory, + // none of which are sensitive, so the directory is granted. + paths.s_ReadOnly.push_back("/sys/devices/system/cpu"); + + // CPU feature detection. Denying it does not fail the launch - it makes + // the quantized kernels silently take a different code path, which + // changed ELSER's output by up to ~3% in the trace this list comes from. + paths.s_ReadOnly.push_back("/proc/cpuinfo"); + + // The periodic memory reporter reads resident set size from here. Only + // exact /proc/self files are granted: /proc/self as a directory would + // also expose maps, fd and the rest, and /proc would expose every other + // process's. + paths.s_ReadOnly.push_back("/proc/self/statm"); + + // Read once after the ruleset applies (by a runtime library parsing its + // environment settings). Granting it gives an attacker nothing new: it is + // this process's own initial environment, which Elasticsearch's Spawner + // reduces to TMPDIR. Denying it had no measurable effect in the traced + // runs, but a denied config read is the silent-behaviour-change class that + // /proc/cpuinfo demonstrated - an MKL_* or OMP_* variable would be quietly + // ignored - so the denial would cost risk and buy no protection. + paths.s_ReadOnly.push_back("/proc/self/environ"); + + // glibc loads the timezone lazily, on the first localtime() call, which + // happens after the ruleset is applied. Denying it only makes log + // timestamps UTC, but the file is not sensitive. A symlink to + // /usr/share/zoneinfo/... is resolved when the rule is added, so the rule + // covers the target file, not the zoneinfo tree. + paths.s_ReadOnly.push_back("/etc/localtime"); + + // Not read in the traced run (seeding uses getrandom()), but libstdc++'s + // std::random_device falls back to it where getrandom() or RDRAND is + // unavailable, and a failure there throws. Readable randomness is not + // sensitive. + paths.s_ReadOnly.push_back("/dev/urandom"); + + paths.s_PipeDirectories.push_back(ipcDirectory); + + return paths; +} + +ELandlockOutcome applyLandlockFilesystemPolicy(const SLandlockPaths& paths) { + const long abi{landlockCreateRuleset(nullptr, 0, ML_LANDLOCK_CREATE_RULESET_VERSION)}; + if (abi < 0) { + const int createErrno{errno}; + // Distinguish "this kernel cannot do Landlock" from "something + // refused the call", exactly as the Sandbox2 capability probe + // distinguishes its own denials: the two have completely different + // remedies, and collapsing them into one outcome is what made the + // Sandbox2 failures opaque in the first place. ENOSYS means a kernel + // older than 5.13 or Landlock compiled out; EOPNOTSUPP means built + // in but not enabled in the bootloader's lsm= list. Anything else - + // EACCES/EPERM in particular - means a seccomp filter or LSM denied + // the syscall on a kernel that does support it. + if (createErrno == ENOSYS || createErrno == EOPNOTSUPP) { + LOG_WARN(<< "Landlock unsupported by this kernel: " << ::strerror(createErrno)); + return ELandlockOutcome::E_Unsupported; + } + LOG_ERROR(<< "Landlock is supported but landlock_create_ruleset was denied: " + << ::strerror(createErrno) << " - a seccomp filter or LSM policy is blocking syscall " + << ML_NR_landlock_create_ruleset); + return ELandlockOutcome::E_Failed; + } + + const std::uint64_t handled{handledAccessForAbi(static_cast(abi))}; + SLandlockRulesetAttr rulesetAttr{}; + rulesetAttr.s_HandledAccessFs = handled; + + const long rulesetFd{landlockCreateRuleset(&rulesetAttr, sizeof(rulesetAttr), 0)}; + if (rulesetFd < 0) { + LOG_ERROR(<< "Landlock: could not create ruleset: " << ::strerror(errno)); + return ELandlockOutcome::E_Failed; + } + + const int fd{static_cast(rulesetFd)}; + bool ok{true}; + + // No EXECUTE: see SLandlockPaths::s_ReadOnly. + const std::uint64_t readOnlyAccess{ACCESS_FS_READ_FILE | ACCESS_FS_READ_DIR}; + for (const std::string& path : paths.s_ReadOnly) { + ok = addPathRule(fd, path, readOnlyAccess, handled) && ok; + } + + // Exactly what CNamedPipeFactory does in the IPC directory: mkfifo(), + // open the FIFO for reading or writing, and unlink() it once connected. + const std::uint64_t pipeDirectoryAccess{ACCESS_FS_MAKE_FIFO | ACCESS_FS_READ_FILE | + ACCESS_FS_WRITE_FILE | ACCESS_FS_REMOVE_FILE}; + for (const std::string& path : paths.s_PipeDirectories) { + ok = addPathRule(fd, path, pipeDirectoryAccess, handled) && ok; + } + + if (ok == false) { + ::close(fd); + return ELandlockOutcome::E_Failed; + } + + // Landlock requires no_new_privs of a caller without CAP_SYS_ADMIN, so + // that a confined process cannot escape by exec'ing something setuid. + if (::prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) != 0) { + LOG_ERROR(<< "Landlock: could not set no_new_privs: " << ::strerror(errno)); + ::close(fd); + return ELandlockOutcome::E_Failed; + } + + if (landlockRestrictSelf(fd, 0) != 0) { + LOG_ERROR(<< "Landlock: could not restrict self: " << ::strerror(errno)); + ::close(fd); + return ELandlockOutcome::E_Failed; + } + + ::close(fd); + LOG_INFO(<< "{\"event\":\"landlock_applied\",\"abi\":" << abi + << ",\"read_only_paths\":" << paths.s_ReadOnly.size() + << ",\"pipe_directories\":" << paths.s_PipeDirectories.size() << "}"); + return ELandlockOutcome::E_Applied; +} + +} // namespace seccomp +} // namespace ml diff --git a/lib/seccomp/CMakeLists.txt b/lib/seccomp/CMakeLists.txt index b76c38f260..68649dd8ee 100644 --- a/lib/seccomp/CMakeLists.txt +++ b/lib/seccomp/CMakeLists.txt @@ -12,6 +12,7 @@ project("ML Seccomp") set(SRCS + CLandlockFilesystemPolicy.cc CSystemCallFilter.cc ) diff --git a/lib/seccomp/unittest/CLandlockFilesystemPolicyTest.cc b/lib/seccomp/unittest/CLandlockFilesystemPolicyTest.cc new file mode 100644 index 0000000000..56c2065469 --- /dev/null +++ b/lib/seccomp/unittest/CLandlockFilesystemPolicyTest.cc @@ -0,0 +1,397 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the following additional limitation. Functionality enabled by the + * files subject to the Elastic License 2.0 may only be used in production when + * invoked by an Elasticsearch process with a license key installed that permits + * use of machine learning features. You may not use this file except in + * compliance with the Elastic License 2.0 and the foregoing additional + * limitation. + */ + +#include + +#include +#include +#include + +#include +#include +#include + +#ifdef Linux +#include +#include +#include +#include +#include +#include +#endif + +BOOST_AUTO_TEST_SUITE(CLandlockFilesystemPolicyTest) + +BOOST_AUTO_TEST_CASE(testDescribeCoversEveryOutcome) { + BOOST_TEST_REQUIRE( + ml::seccomp::describe(ml::seccomp::ELandlockOutcome::E_Applied).empty() == false); + BOOST_TEST_REQUIRE( + ml::seccomp::describe(ml::seccomp::ELandlockOutcome::E_Unsupported).empty() == false); + BOOST_TEST_REQUIRE( + ml::seccomp::describe(ml::seccomp::ELandlockOutcome::E_Failed).empty() == false); + BOOST_REQUIRE(ml::seccomp::describe(ml::seccomp::ELandlockOutcome::E_Applied) != + ml::seccomp::describe(ml::seccomp::ELandlockOutcome::E_Failed)); +} + +// perChildIpcDirectory() is pure string logic, declared and defined inline in +// the header, so it is testable on every platform without forking or +// applying anything. +BOOST_AUTO_TEST_CASE(testPerChildIpcDirectoryPositiveCases) { + using ml::seccomp::perChildIpcDirectory; + + BOOST_REQUIRE_EQUAL(std::string{"/tmp/ml-child-ipc/dep-1"}, + perChildIpcDirectory("/tmp/ml-child-ipc/dep-1/logPipe")); + // A deeper trusted base directory - only the last three components + // matter. + BOOST_REQUIRE_EQUAL(std::string{"/var/lib/es/tmp/ml-child-ipc/abc123"}, + perChildIpcDirectory("/var/lib/es/tmp/ml-child-ipc/abc123/output")); +} + +BOOST_AUTO_TEST_CASE(testPerChildIpcDirectoryNegativeCases) { + using ml::seccomp::perChildIpcDirectory; + + // Empty input. + BOOST_TEST_REQUIRE(perChildIpcDirectory("").empty()); + + // Relative path: the Landlock rule must never depend on the process's + // working directory. + BOOST_TEST_REQUIRE(perChildIpcDirectory("ml-child-ipc/dep-1/logPipe").empty()); + + // Flat $TMPDIR layout (the legacy/non-per-child layout) - no + // "ml-child-ipc/" shape at all. + BOOST_TEST_REQUIRE(perChildIpcDirectory("/tmp/logPipe").empty()); + + // Missing the path component: the pipe sits directly under + // ".../ml-child-ipc" rather than under a per-child subdirectory of it. + BOOST_TEST_REQUIRE(perChildIpcDirectory("/tmp/ml-child-ipc/logPipe").empty()); + + // A parent directory that merely ends in "ml-child-ipc" (e.g. + // "xml-child-ipc") must NOT match - the comparison must be exact, not a + // suffix match. + BOOST_TEST_REQUIRE(perChildIpcDirectory("/tmp/xml-child-ipc/dep-1/logPipe").empty()); +} + +#ifdef Linux + +namespace { + +//! Exit codes a confined child uses to report what it observed. The ruleset +//! is irreversible, so every case below must run in its own forked child - +//! confining the test process itself would break every later test. +enum EChildExit : int { + E_ChildOk = 0, + E_ChildNotApplied = 20, + E_ChildGrantedPathUnreadable = 21, + E_ChildDeniedPathStillReadable = 22, + E_ChildGrantedDirNotWritable = 23, + E_ChildExecRefused = 24 +}; + +//! Run \p body in a forked child and return its exit code, or -1 if the child +//! did not exit normally. +template +int runInChild(FUNC body) { + const pid_t child{::fork()}; + if (child < 0) { + return -1; + } + if (child == 0) { + ::_exit(body()); + } + int status{0}; + if (::waitpid(child, &status, 0) != child || WIFEXITED(status) == false) { + return -1; + } + return WEXITSTATUS(status); +} + +std::string makeScratchDirectory() { + std::string path{"/tmp/ml-landlock-test-XXXXXX"}; + if (::mkdtemp(path.data()) == nullptr) { + return std::string(); + } + return path; +} + +} // namespace + +BOOST_AUTO_TEST_CASE(testRulesetGrantsTheAllowedPathAndDeniesEverythingElse) { + // The point of the fallback is that it actually bounds the sandboxee, so + // assert every half: the pipe directory still supports exactly what + // CNamedPipeFactory does (mkfifo, open, unlink), it refuses anything else + // (a regular file there would allow disk-filling or staging), and a path + // that was never granted becomes unreadable *even though the uid still + // owns it*. Without the negative halves a vacuously permissive ruleset + // would "pass". + const std::string scratch{makeScratchDirectory()}; + BOOST_TEST_REQUIRE(scratch.empty() == false); + + const std::string outside{scratch + "/outside.txt"}; + const int outsideFd{::open(outside.c_str(), O_CREAT | O_WRONLY, 0600)}; + BOOST_TEST_REQUIRE(outsideFd >= 0); + ::close(outsideFd); + + const std::string pipes{scratch + "/pipes"}; + BOOST_TEST_REQUIRE(::mkdir(pipes.c_str(), 0700) == 0); + + const int childResult{runInChild([&] { + ml::seccomp::SLandlockPaths paths; + paths.s_PipeDirectories.push_back(pipes); + + const ml::seccomp::ELandlockOutcome outcome{ + ml::seccomp::applyLandlockFilesystemPolicy(paths)}; + if (outcome == ml::seccomp::ELandlockOutcome::E_Unsupported) { + return static_cast(E_ChildNotApplied); + } + if (outcome != ml::seccomp::ELandlockOutcome::E_Applied) { + return static_cast(E_ChildGrantedPathUnreadable); + } + + // What CNamedPipeFactory does: mkfifo, open (O_RDWR so no peer is + // needed), unlink. + const std::string fifo{pipes + "/logPipe"}; + if (::mkfifo(fifo.c_str(), 0600) != 0) { + return static_cast(E_ChildGrantedDirNotWritable); + } + const int fifoFd{::open(fifo.c_str(), O_RDWR)}; + if (fifoFd < 0) { + return static_cast(E_ChildGrantedDirNotWritable); + } + ::close(fifoFd); + if (::unlink(fifo.c_str()) != 0) { + return static_cast(E_ChildGrantedDirNotWritable); + } + + // Anything other than a FIFO must be refused in the pipe directory. + const int regularFd{::open((pipes + "/staged.bin").c_str(), O_CREAT | O_WRONLY, 0600)}; + if (regularFd >= 0) { + ::close(regularFd); + return static_cast(E_ChildDeniedPathStillReadable); + } + + // The sibling file, owned by this very uid, must now be unreachable. + const int deniedFd{::open(outside.c_str(), O_RDONLY)}; + if (deniedFd >= 0) { + ::close(deniedFd); + return static_cast(E_ChildDeniedPathStillReadable); + } + + return static_cast(E_ChildOk); + })}; + + ::unlink((pipes + "/staged.bin").c_str()); + ::unlink(outside.c_str()); + ::rmdir(pipes.c_str()); + ::rmdir(scratch.c_str()); + + if (childResult == E_ChildNotApplied) { + BOOST_TEST_MESSAGE("Landlock unsupported on this kernel - skipping enforcement assertions"); + return; + } + BOOST_REQUIRE_EQUAL(childResult, static_cast(E_ChildOk)); +} + +BOOST_AUTO_TEST_CASE(testExecveIsRefusedEvenForAReadableBinary) { + // EXECUTE is never granted, so Landlock alone refuses execve() - the + // backstop if the seccomp filter (which also denies execve) ever failed + // to install. Grant read on the binary's own directory to show that + // readability does not imply executability. + // + // A negative control (EXECUTE granted) must also grant the directories + // holding the ELF interpreter and libc: execve() needs EXECUTE on the + // interpreter as well, so granting it on /usr/bin alone still fails, and + // would make the control pass for the wrong reason. + const int childResult{runInChild([] { + ml::seccomp::SLandlockPaths paths; + paths.s_ReadOnly.push_back("/usr/bin"); + if (ml::seccomp::applyLandlockFilesystemPolicy(paths) == + ml::seccomp::ELandlockOutcome::E_Unsupported) { + return static_cast(E_ChildNotApplied); + } + char* const argv[]{const_cast("true"), nullptr}; + ::execv("/usr/bin/true", argv); + // Only reached if execv() failed. A *successful* exec replaces this + // child with /usr/bin/true, which exits 0 - so the refusal must be + // reported with a distinct non-zero code, or a broken ruleset that + // allowed the exec would pass this test. + return static_cast(errno == EACCES ? E_ChildExecRefused + : E_ChildDeniedPathStillReadable); + })}; + + if (childResult == E_ChildNotApplied) { + BOOST_TEST_MESSAGE("Landlock unsupported on this kernel - skipping"); + return; + } + BOOST_REQUIRE_EQUAL(childResult, static_cast(E_ChildExecRefused)); +} + +BOOST_AUTO_TEST_CASE(testPolicyIsIrreversibleWithinTheConfinedProcess) { + // Landlock rulesets stack and can only narrow. Applying an empty second + // ruleset must not restore access the first one removed - otherwise a + // malicious model could simply re-apply a permissive policy. + const std::string scratch{makeScratchDirectory()}; + BOOST_TEST_REQUIRE(scratch.empty() == false); + const std::string probeFile{scratch + "/probe.txt"}; + const int fd{::open(probeFile.c_str(), O_CREAT | O_WRONLY, 0600)}; + BOOST_TEST_REQUIRE(fd >= 0); + ::close(fd); + + const int childResult{runInChild([&] { + ml::seccomp::SLandlockPaths empty; + if (ml::seccomp::applyLandlockFilesystemPolicy(empty) == + ml::seccomp::ELandlockOutcome::E_Unsupported) { + return static_cast(E_ChildNotApplied); + } + // Now grant the scratch directory in a second ruleset; Landlock + // composes by intersection, so this must NOT re-open access. + ml::seccomp::SLandlockPaths permissive; + permissive.s_PipeDirectories.push_back(scratch); + permissive.s_ReadOnly.push_back(scratch); + ml::seccomp::applyLandlockFilesystemPolicy(permissive); + + const int reopened{::open(probeFile.c_str(), O_RDONLY)}; + if (reopened >= 0) { + ::close(reopened); + return static_cast(E_ChildDeniedPathStillReadable); + } + return static_cast(E_ChildOk); + })}; + + ::unlink(probeFile.c_str()); + ::rmdir(scratch.c_str()); + + if (childResult == E_ChildNotApplied) { + BOOST_TEST_MESSAGE("Landlock unsupported on this kernel - skipping"); + return; + } + BOOST_REQUIRE_EQUAL(childResult, static_cast(E_ChildOk)); +} + +BOOST_AUTO_TEST_CASE(testPytorchInferencePathsAreTheMeasuredMinimum) { + // Pins the ruleset to the traced minimum so a later "just add the parent + // directory" change is a visible test failure rather than a silent + // widening. Every entry here is justified in + // pytorchInferenceLandlockPaths(). + const ml::seccomp::SLandlockPaths paths{ + ml::seccomp::pytorchInferenceLandlockPaths("/app/tmp/ml-child-ipc/dep-1")}; + + const auto contains = [](const std::vector& haystack, + const std::string& needle) { + return std::find(haystack.begin(), haystack.end(), needle) != haystack.end(); + }; + + // The IPC directory holds pipes only, and is the sole modifiable path. + BOOST_REQUIRE_EQUAL(paths.s_PipeDirectories.size(), 1); + BOOST_TEST_REQUIRE(contains(paths.s_PipeDirectories, "/app/tmp/ml-child-ipc/dep-1")); + BOOST_TEST_REQUIRE(contains(paths.s_ReadOnly, "/app/tmp/ml-child-ipc/dep-1") == false); + + // Sensitive trees are granted as exact files, never as directories. + BOOST_TEST_REQUIRE(contains(paths.s_ReadOnly, "/proc/cpuinfo")); + BOOST_TEST_REQUIRE(contains(paths.s_ReadOnly, "/proc/self/statm")); + BOOST_TEST_REQUIRE(contains(paths.s_ReadOnly, "/proc/self/environ")); + BOOST_TEST_REQUIRE(contains(paths.s_ReadOnly, "/etc/localtime")); + for (const char* tooBroad : + {"/", "/proc", "/proc/self", "/etc", "/tmp", "/app/tmp", "/lib", + "/lib64", "/usr/lib", "/usr/lib64", "/usr", "/dev", "/sys", "/home"}) { + BOOST_TEST_REQUIRE(contains(paths.s_ReadOnly, tooBroad) == false); + BOOST_TEST_REQUIRE(contains(paths.s_PipeDirectories, tooBroad) == false); + } +} + +BOOST_AUTO_TEST_CASE(testRealPytorchPolicyDeniesTheExploitTargetWrite) { + // End-to-end at the policy level, using the exact ruleset + // pytorch_inference installs on the Landlock fallback route - not a + // synthetic one. The attack-defense exploit model + // (test/evil_model_generator.py) writes an -agentpath payload to + // /usr/share/elasticsearch/config/jvm.options.d/gc.options; that path is + // outside every grant pytorchInferenceLandlockPaths() produces, so the + // real policy must deny a write there, while the per-child IPC directory + // it does grant stays usable for what pytorch_inference actually does + // there - create its own log FIFO. The grant is pipe-only (it may hold + // nothing but this process's own FIFOs - see + // SLandlockPaths::s_PipeDirectories), so unlike the original version of + // this test, the positive half below uses mkfifo/open/unlink rather than + // creating a regular file, which the real policy now refuses even inside + // the granted directory. This is the same boundary the harness's ROP + // exploit exercises, proven deterministically without a build-fragile ROP + // chain. + const std::string scratch{makeScratchDirectory()}; + BOOST_TEST_REQUIRE(scratch.empty() == false); + + // A stand-in for the operator TMPDIR, with the per-child IPC directory + // laid out as the controller creates it: /ml-child-ipc/. + const std::string ipcDir{scratch + "/ml-child-ipc/dep-e2e"}; + boost::system::error_code mkdirError; + boost::filesystem::create_directories(ipcDir, mkdirError); + BOOST_TEST_REQUIRE(mkdirError.value() == 0); + + // The exploit's hard-coded target, created here so the difference the + // test observes is Landlock denying the write - not the parent directory + // being absent. Its parent is deliberately outside every grant. + const std::string forbiddenDir{scratch + "/config/jvm.options.d"}; + boost::filesystem::create_directories(forbiddenDir, mkdirError); + BOOST_TEST_REQUIRE(mkdirError.value() == 0); + const std::string forbiddenTarget{forbiddenDir + "/gc.options"}; + + const int childResult{runInChild([&] { + ml::seccomp::SLandlockPaths paths{ml::seccomp::pytorchInferenceLandlockPaths(ipcDir)}; + // Point the "config" grant nowhere near forbiddenTarget: the real + // policy grants /etc etc., none of which cover this scratch config + // path, so no extra removal is needed - forbiddenTarget is already + // outside paths. Apply the real ruleset unchanged. + const ml::seccomp::ELandlockOutcome outcome{ + ml::seccomp::applyLandlockFilesystemPolicy(paths)}; + if (outcome == ml::seccomp::ELandlockOutcome::E_Unsupported) { + return static_cast(E_ChildNotApplied); + } + if (outcome != ml::seccomp::ELandlockOutcome::E_Applied) { + return static_cast(E_ChildGrantedPathUnreadable); + } + + // The per-child IPC directory the real policy grants must stay + // usable for what pytorch_inference actually does there: mkfifo, + // open, unlink - exactly what CNamedPipeFactory does. A regular file + // is no longer valid here, since the grant is pipe-only. + const std::string fifoStandin{ipcDir + "/logPipe.test"}; + if (::mkfifo(fifoStandin.c_str(), 0600) != 0) { + return static_cast(E_ChildGrantedDirNotWritable); + } + const int okFd{::open(fifoStandin.c_str(), O_RDWR)}; + if (okFd < 0) { + return static_cast(E_ChildGrantedDirNotWritable); + } + ::close(okFd); + if (::unlink(fifoStandin.c_str()) != 0) { + return static_cast(E_ChildGrantedDirNotWritable); + } + + // The exploit's target write must be denied by the real policy. + const int deniedFd{::open(forbiddenTarget.c_str(), O_CREAT | O_WRONLY, 0600)}; + if (deniedFd >= 0) { + ::close(deniedFd); + return static_cast(E_ChildDeniedPathStillReadable); + } + return static_cast(E_ChildOk); + })}; + + boost::system::error_code rmError; + boost::filesystem::remove_all(scratch, rmError); + + if (childResult == E_ChildNotApplied) { + BOOST_TEST_MESSAGE("Landlock unsupported on this kernel - skipping"); + return; + } + BOOST_REQUIRE_EQUAL(childResult, static_cast(E_ChildOk)); +} + +#endif // Linux + +BOOST_AUTO_TEST_SUITE_END() diff --git a/lib/seccomp/unittest/CMakeLists.txt b/lib/seccomp/unittest/CMakeLists.txt index 2656170e85..39b28546e2 100644 --- a/lib/seccomp/unittest/CMakeLists.txt +++ b/lib/seccomp/unittest/CMakeLists.txt @@ -13,6 +13,7 @@ project("ML Seccomp unit tests") set (SRCS Main.cc + CLandlockFilesystemPolicyTest.cc CSeccompFilterBuilderTest.cc CSystemCallFilterTest.cc ) diff --git a/lib/seccomp/unittest/CSeccompFilterBuilderTest.cc b/lib/seccomp/unittest/CSeccompFilterBuilderTest.cc index ee0390bf53..a53e63a628 100644 --- a/lib/seccomp/unittest/CSeccompFilterBuilderTest.cc +++ b/lib/seccomp/unittest/CSeccompFilterBuilderTest.cc @@ -385,4 +385,68 @@ BOOST_AUTO_TEST_CASE(testInProcessFilterUnchangedOnLegacyRoute) { } } +BOOST_AUTO_TEST_CASE(testDegradedModeAttestationMarkerRouteLandlock) { + using ml::seccomp::ESystemCallFilterInstallOutcome; + using ml::seccomp::degradedModeAttestationMarker; + + // route must be threaded through verbatim - a controller/Elasticsearch + // observer needs the marker's ml_sandbox2_route to agree with the + // sandbox2_launch signal's own "route" field for the same launch. + BOOST_REQUIRE_EQUAL( + std::string("{\"ml_sandbox2_route\":\"landlock\",\"event\":\"seccomp_installed\"}"), + degradedModeAttestationMarker(ESystemCallFilterInstallOutcome::E_Installed, "landlock")); + + // Default parameter is unchanged: omitting route still reports "legacy". + BOOST_REQUIRE_EQUAL( + std::string("{\"ml_sandbox2_route\":\"legacy\",\"event\":\"seccomp_installed\"}"), + degradedModeAttestationMarker(ESystemCallFilterInstallOutcome::E_Installed)); + + // A failed install attests nothing, regardless of which route asked for + // the marker. + BOOST_TEST_REQUIRE(degradedModeAttestationMarker( + ESystemCallFilterInstallOutcome::E_MechanismUnavailable, "landlock") + .empty()); + BOOST_TEST_REQUIRE(degradedModeAttestationMarker( + ESystemCallFilterInstallOutcome::E_PrivilegeRestrictionFailed, "landlock") + .empty()); + BOOST_TEST_REQUIRE(degradedModeAttestationMarker( + ESystemCallFilterInstallOutcome::E_FilterInstallFailed, "landlock") + .empty()); +} + +BOOST_AUTO_TEST_CASE(testApplyInProcessSeccompFilterPassesRouteThrough) { + using ml::seccomp::ESystemCallFilterInstallOutcome; + using ml::seccomp::applyInProcessSeccompFilter; + + // Not Sandbox2-launched (legacy/Landlock route child), successful + // install, route == "landlock": the resulting marker must carry that + // route, not the default "legacy". + const auto result = applyInProcessSeccompFilter( + false, true, [] { return ESystemCallFilterInstallOutcome::E_Installed; }, "landlock"); + + BOOST_REQUIRE_EQUAL(true, result.s_Attempted); + BOOST_REQUIRE_EQUAL(std::string("{\"ml_sandbox2_route\":\"landlock\",\"event\":\"seccomp_installed\"}"), + result.s_AttestationMarker); +} + +BOOST_AUTO_TEST_CASE(testApplyInProcessSeccompFilterFailedInstallEmptyMarkerRegardlessOfRoute) { + using ml::seccomp::ESystemCallFilterInstallOutcome; + using ml::seccomp::applyInProcessSeccompFilter; + + const ESystemCallFilterInstallOutcome failureModes[]{ + ESystemCallFilterInstallOutcome::E_MechanismUnavailable, + ESystemCallFilterInstallOutcome::E_PrivilegeRestrictionFailed, + ESystemCallFilterInstallOutcome::E_FilterInstallFailed}; + const char* const routes[]{"legacy", "landlock"}; + + for (const auto outcome : failureModes) { + for (const char* route : routes) { + const auto result = applyInProcessSeccompFilter( + false, true, [outcome] { return outcome; }, route); + BOOST_REQUIRE_EQUAL(true, result.s_Attempted); + BOOST_TEST_REQUIRE(result.s_AttestationMarker.empty()); + } + } +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/test/test_sandbox2_attack_defense.py b/test/test_sandbox2_attack_defense.py index fcb5a1962c..7d3a91b7a9 100644 --- a/test/test_sandbox2_attack_defense.py +++ b/test/test_sandbox2_attack_defense.py @@ -307,32 +307,53 @@ def find_child_pid(controller, process_path, since_offset, timeout=PID_DISCOVERY #! The controller's sandbox2_launch structured once-per-launch signal, -#! emitted by bin/controller/CProcessSpawnerRouter.cc emitLaunchSignal() -#! over the same log pipe. Boost.Log escapes the embedded quotes, so the raw -#! capture is unescaped before matching. -LAUNCH_SIGNAL_ROUTE_RE = re.compile(r'"event":"sandbox2_launch".*?"route":"(?P[a-z0-9_]+)"') - - -def find_launch_route(controller, since_offset, timeout=PID_DISCOVERY_TIMEOUT): - """Return the route ("sandbox2" / "legacy") the controller's own - sandbox2_launch signal reports for the launch issued after since_offset, - or None if no such signal appeared within timeout. +#! emitted by bin/controller/CProcessSpawnerRouter.cc emitLaunchSignal() over +#! the same log pipe. Boost.Log escapes the embedded quotes, so the raw +#! capture is unescaped before matching. The whole JSON object is captured and +#! then parsed field-by-field, because the security-relevant distinction is in +#! the "mode" field, not "route": route is "sandbox2" for BOTH a full-Sandbox2 +#! launch (mode "enforced") and the Landlock fallback the controller takes +#! when the host cannot run Sandbox2 (mode "landlock"), as well as a refused +#! launch (mode "fail_closed", where no child ran at all). Matching only on +#! route would conflate all three. +LAUNCH_SIGNAL_OBJECT_RE = re.compile(r'\{"event":"sandbox2_launch".*?\}') +_SIGNAL_FIELD_RE = re.compile(r'"(?P[a-z0-9_]+)":"(?P[a-z0-9_]+)"') + +#! Modes in which a child actually ran under a confinement boundary, so a +#! "the malicious model's target file must not exist" assertion is meaningful. +CONFINED_MODES = ('enforced', 'landlock') +#! Mode of an unconfined (seccomp-only) legacy launch. +UNCONFINED_MODE = 'degraded' + + +def find_launch_signal(controller, since_offset, timeout=PID_DISCOVERY_TIMEOUT): + """Return (route, mode) from the controller's own sandbox2_launch signal + for the launch issued after since_offset, or None if no such signal + appeared within timeout. This is the harness's guard against silently invalidating the security - proof: a "sandboxed" case that actually routed to the legacy path would - still produce "no target file" for entirely the wrong reason (see - run_pytorch_case()). + proof: a "sandboxed" case that actually ran unconfined - or did not run at + all - would still produce "no target file" for entirely the wrong reason + (see run_pytorch_case()). Both route and mode are returned so the caller + can tell an enforced Sandbox2 run and a Landlock-confined run (both valid + confinement) apart from an unconfined legacy run and a fail_closed refusal + (both of which make the negative assertion vacuous). """ log_path = controller.control_dir / 'controller_log_output.txt' deadline = time.time() + timeout while True: raw = _read_new_content(log_path, since_offset).replace('\\"', '"') - route = None - for match in LAUNCH_SIGNAL_ROUTE_RE.finditer(raw): + signal = None + for match in LAUNCH_SIGNAL_OBJECT_RE.finditer(raw): # Last match wins, consistent with find_child_pid(). - route = match.group('route') - if route is not None: - return route + fields = {m.group('key'): m.group('value') + for m in _SIGNAL_FIELD_RE.finditer(match.group(0))} + route = fields.get('route') + mode = fields.get('mode') + if route is not None and mode is not None: + signal = (route, mode) + if signal is not None: + return signal if time.time() >= deadline: return None time.sleep(0.1) @@ -694,7 +715,25 @@ def open_and_write(): def generate_models(output_dir): - """Generate test models using the ported generator script.""" + """Generate test models using the ported generator script. + + If ML_EVIL_MODELS_DIR is set and already contains the three .pt files, + they are copied in instead of regenerated. This lets the harness run in + an environment that has the controller/pytorch_inference binaries but no + torch (e.g. inside the cloud-ess image, where the models are generated + once elsewhere and mounted in) - the models are plain TorchScript + archives, independent of where they were traced. + """ + prebuilt = os.environ.get('ML_EVIL_MODELS_DIR') + if prebuilt: + names = ('model_benign.pt', 'model_exploit.pt', 'model_leak.pt') + if all((Path(prebuilt) / n).exists() for n in names): + for n in names: + shutil.copy(Path(prebuilt) / n, Path(output_dir) / n) + return + raise RuntimeError( + f"ML_EVIL_MODELS_DIR={prebuilt} set but does not contain all of {names}") + script_dir = Path(__file__).parent generator_script = script_dir / 'evil_model_generator.py' project_root = script_dir.parent @@ -850,32 +889,46 @@ def run_pytorch_case(controller, pytorch_bin, model_path, tmp_base, command_id, return result, reached, target_file_created, response, leaked_address_seen, pid result.info(f"Controller accepted start: {response.get('reason')}") - # Routing assertion, BEFORE any boundary assertion: the case is only - # evidence about Sandbox2 if the controller actually routed this - # launch the way the case intends. A sandboxed case that silently - # landed on the legacy path (e.g. --requireSandbox not - # reaching the controller, or a route-decision regression) would - # still show "no target file" - for the wrong reason. Fail loudly - # here instead. - expected_route = 'legacy' if unsandboxed else 'sandbox2' - actual_route = find_launch_route(controller, log_offset) - if actual_route is None: + # Boundary assertion, BEFORE any target-file assertion: the case is + # only evidence about the sandbox if the controller actually confined + # this launch the way the case intends. The security-relevant fact is + # the signal's "mode", not "route": route is "sandbox2" for a full + # Sandbox2 launch (mode "enforced"), for the Landlock fallback the + # controller takes when the host cannot run Sandbox2 (mode + # "landlock"), AND for a refused launch (mode "fail_closed", where no + # child ran). A sandboxed case whose "no target file" would be + # meaningful requires a mode in which a child actually ran under a + # boundary - enforced or landlock. An unsandboxed control requires the + # unconfined "degraded" mode; anything else (including "fail_closed", + # where the file's absence proves nothing because nothing executed) + # fails loudly here instead of silently passing. + signal = find_launch_signal(controller, log_offset) + if signal is None: result.fail( "No sandbox2_launch signal observed on the controller log within " f"{PID_DISCOVERY_TIMEOUT}s of a successful start response - cannot confirm " - f"this launch took the '{expected_route}' route; not asserting on target file") + "how this launch was confined; not asserting on target file") controller.check_controller_logs() return result, reached, target_file_created, response, leaked_address_seen, pid - if actual_route != expected_route: + actual_route, actual_mode = signal + if unsandboxed: + mode_ok = actual_mode == UNCONFINED_MODE + expected_desc = f'mode "{UNCONFINED_MODE}"' + else: + mode_ok = actual_mode in CONFINED_MODES + expected_desc = 'mode ' + ' or '.join(f'"{m}"' for m in CONFINED_MODES) + if not mode_ok: result.fail( - f"Routing regression: controller's sandbox2_launch signal reports " - f"\"route\":\"{actual_route}\" but this case requires " - f"\"{expected_route}\". The child was not sandboxed as intended, so any " - f"target-file assertion below would prove nothing about Sandbox2; " - f"not asserting on target file") + f"Confinement regression: controller's sandbox2_launch signal reports " + f"\"route\":\"{actual_route}\",\"mode\":\"{actual_mode}\" but this case " + f"requires {expected_desc}. The child was not confined as intended (or did " + f"not run at all), so any target-file assertion below would prove nothing " + f"about the sandbox; not asserting on target file") controller.check_controller_logs() return result, reached, target_file_created, response, leaked_address_seen, pid - result.info(f"sandbox2_launch signal confirms route: {actual_route}") + result.info( + f"sandbox2_launch signal confirms confinement: " + f"route={actual_route} mode={actual_mode}") pid = find_child_pid(controller, f'./{pytorch_name}', log_offset) if pid is None: From 5d90c142f10a06eae5d029c392dcee386e0ea9b0 Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 24 Sep 2026 10:13:07 +0200 Subject: [PATCH 2/7] [ML] Document Landlock fallback in sandbox2_launch contract --- bin/controller/CCommandProcessor.cc | 9 +- build.gradle | 8 +- docs/sandbox2_production_failure_modes.md | 137 +++++++++++++++------- 3 files changed, 106 insertions(+), 48 deletions(-) diff --git a/bin/controller/CCommandProcessor.cc b/bin/controller/CCommandProcessor.cc index cb8c38f66e..61df2672dc 100644 --- a/bin/controller/CCommandProcessor.cc +++ b/bin/controller/CCommandProcessor.cc @@ -28,12 +28,13 @@ const std::string EMPTY_STRING; //! rejected outright, never resolved by precedence. const std::string DISABLE_SANDBOX_TOKEN{"--disableSandbox"}; -//! Operator opt-in: forces the Sandbox2 route (E_Sandbox2, no automatic -//! legacy fallback) for the configured sandboxed process path. Symmetric -//! counterpart to DISABLE_SANDBOX_TOKEN - together these are the only two +//! Operator opt-in: requests the strongest confinement this host can provide +//! for the configured sandboxed process path (Sandbox2 when available, +//! otherwise Landlock plus seccomp, otherwise refusal). Symmetric counterpart +//! to DISABLE_SANDBOX_TOKEN - together these are the only two //! controller-control tokens the command wire format defines; any other //! unrecognised "--" prefixed token is passed through to the spawned -//! process unchanged. +//! process unchanged. `--restrictFilesystem` is not a caller token. const std::string REQUIRE_SANDBOX_TOKEN{"--requireSandbox"}; } diff --git a/build.gradle b/build.gradle index 94d7e164ad..07d88edc82 100644 --- a/build.gradle +++ b/build.gradle @@ -211,10 +211,10 @@ task buildZip(type: Zip) { // well-known top-level path in the -deps zip. Bump the integer inside // 3rd_party/controller-protocol.version (not merely its existence) on any // future breaking change to either (a) the controller's - // --disableSandbox/--requireSandbox token semantics (controller-only - // metadata, never forwarded to the child), or (b) the per-child IPC route - // contract ($TMPDIR/ml-child-ipc/, mounted at the same path - // inside and outside the sandbox). + // --disableSandbox/--requireSandbox token semantics or the Landlock fallback + // ladder they trigger (controller-only metadata, never forwarded to the + // child), or (b) the per-child IPC route contract ($TMPDIR/ml-child-ipc/, + // mounted at the same path inside and outside the sandbox). from("3rd_party") { include "controller-protocol.version" } diff --git a/docs/sandbox2_production_failure_modes.md b/docs/sandbox2_production_failure_modes.md index 181c9845d4..9c6fd97a3e 100644 --- a/docs/sandbox2_production_failure_modes.md +++ b/docs/sandbox2_production_failure_modes.md @@ -1,4 +1,4 @@ -# Sandbox2 production failure modes +#Sandbox2 production failure modes This document tracks the operational log vocabulary the controller and `pytorch_inference` emit around the Sandbox2 rollout. This schema is itself @@ -30,10 +30,11 @@ single-line JSON object. | `event` | string | Always `"sandbox2_launch"`. | | `deployment_id` | string | `SChildIpcLaunchSpec::s_ChildId`, from a single `sandbox::validateChildIpcLaunchSpec()` call made **once per `spawn()`, before dispatch**, so the value cannot disagree with the state the dispatch decision was taken against and is populated on the `degraded`/`fail_closed` modes too. Empty string (`""`, explicit, never omitted) only when no path-bearing launch option (`input`/`output`/`restore`/`logPipe`) was present at all. Control characters, quotes and backslashes are JSON-escaped so the line stays single-line JSON. | | `model_id` | string | Scanned from a `--modelid=` launch argument, using the same linear string-prefix scan style as the controller's `--disableSandbox` token scan. Empty string if absent. Escaped as for `deployment_id`. | -| `route` | string | `"sandbox2"` when `CProcessSpawnerRouter::ERoute::E_Sandbox2` was in effect (a validated `--requireSandbox` token on a configured sandboxed path). `"legacy"` when the controller selected `E_Legacy` via the operator kill-switch (`--disableSandbox`) or the no-token default (see "No-token default" below). | -| `legacy_reason` | string | **Only present when `route == "legacy"`** (equivalently, `mode == "degraded"`); **omitted entirely** - never `""`, never `null` - on `route == "sandbox2"`, i.e. on both `enforced` and `fail_closed`. `"kill_switch"` when a validated `--disableSandbox` token selected the legacy route, `"no_token_default"` when neither routing token was present. Provenance is passed in by `CCommandProcessor` (the only place it is known); the router never derives it from `args`. | +| `route` | string | `"sandbox2"` when `CProcessSpawnerRouter::ERoute::E_Sandbox2` was in effect (a validated `--requireSandbox` token on a configured sandboxed path). `"legacy"` when the controller selected `E_Legacy` via the operator kill-switch (`--disableSandbox`) or the no-token default (see "No-token default" below). **`route` alone does not mean full Sandbox2 isolation** - read `mode` and `sandbox2_established` (a Landlock fallback still reports `"route":"sandbox2"`). | +| `legacy_reason` | string | **Only present when `route == "legacy"`** (equivalently, `mode == "degraded"`); +**omitted entirely** - never `""`, never `null` - on `route == "sandbox2"` (including `mode == "enforced"`, `mode == "landlock"`, and `mode == "fail_closed"`). `"kill_switch"` when a validated `--disableSandbox` token selected the legacy route, `"no_token_default"` when neither routing token was present. Provenance is passed in by `CCommandProcessor` (the only place it is known); the router never derives it from `args`. | | `sandbox2_established` | boolean | JSON boolean (`true`/`false`, never the string `"y"`/`"n"`). `true` iff `mode == "enforced"`, else `false`. | -| `mode` | string | One of `"enforced"`, `"fail_closed"`, `"degraded"` - see mapping below. | +| `mode` | string | One of `"enforced"`, `"landlock"`, `"fail_closed"`, `"degraded"` - see mapping below. | | `sandbox2_compiled_in` | boolean | JSON boolean. Sourced from `sandbox::CMlSandboxAvailability::isCompiledIn()`, computed once (a build-time-constant fact, not per-launch state) and included on **every** emitted line, unlike `legacy_reason` which is conditional on route. Lets a consumer distinguish "Sandbox2 supported but no routing token sent" (`route == "legacy"`, `legacy_reason == "no_token_default"`, `sandbox2_compiled_in == true`) from "built without Sandbox2 support at all" (`sandbox2_compiled_in == false`) - both otherwise emit identical `legacy`/`no_token_default`/`degraded` signals for every plain launch. | `legacy_reason` exists because `mode == "degraded"` alone conflates a @@ -48,9 +49,14 @@ additive: `event`/`deployment_id`/`model_id`/`route`/ - `enforced` - `route == "sandbox2"` and the Sandbox2 spawn returned `true` (typically after a validated `--requireSandbox` token; the no-token default selects `E_Legacy`/`degraded` instead). +- `landlock` - `route == "sandbox2"`, the host cannot run Sandbox2, but + Landlock is available: the child started under a Landlock ruleset plus the + in-process seccomp filter (`--restrictFilesystem` appended by the router). + `sandbox2_established` is `false`. - `fail_closed` - `route == "sandbox2"` and the spawn returned `false` - (includes the build/deployment contradiction case where `processPath` is - configured as sandboxed but this build has no Sandbox2 support). + (Sandbox2 launch failure on a capable host, build without Sandbox2 support + on `--requireSandbox`, or neither Sandbox2 nor Landlock available - the + controller returns an operator-actionable failure reason to Elasticsearch). - `degraded` - `route == "legacy"` (operator kill-switch token present and validated, or the no-token default in effect), regardless of whether the legacy spawn itself succeeded or failed. `legacy_reason` names which of @@ -60,10 +66,24 @@ additive: `event`/`deployment_id`/`model_id`/`route`/ The command wire format defines exactly two routing tokens: `--disableSandbox` (operator kill-switch, forces the legacy route) and -`--requireSandbox` (operator opt-in, forces the Sandbox2 route - no -automatic legacy fallback). They are mutually exclusive; a `start` command -naming both is rejected outright rather than resolved by precedence, and -each is separately rejected if repeated. +`--requireSandbox` (operator opt-in to the strongest confinement this host +can provide). On a host with user namespaces, that is full Sandbox2 +(`mode == "enforced"`). When Sandbox2 prerequisites are denied (typical on +ECH allocators with `kernel.unprivileged_userns_clone=0` or container +seccomp blocking `CLONE_NEWUSER`), the controller steps down to Landlock plus +seccomp (`mode == "landlock"`) rather than failing closed. When neither +Sandbox2 nor Landlock is available, the launch is refused (`mode == +"fail_closed"`) with a message naming +`xpack.ml.trained_models.sandbox_enabled`. A failed Sandbox2 launch on a +host that *can* run Sandbox2 is never retried on a weaker rung. The tokens +are mutually exclusive; a `start` command naming both is rejected outright +rather than resolved by precedence, and each is separately rejected if +repeated. + +The controller-only token `--restrictFilesystem` is reserved for the router +(Landlock rung). `CCommandProcessor` rejects a caller-supplied +`--restrictFilesystem` so the `sandbox2_launch` signal remains a truthful +record of what bounded the child. A `start` command with **neither** token for a configured sandboxed process path always selects the **legacy** route. This is the permanent behaviour @@ -83,38 +103,75 @@ present. ### In-process seccomp is legacy-route only `pytorch_inference` installs its own in-process seccomp filter - and emits -`{"ml_sandbox2_route":"legacy","event":"seccomp_installed"}` - only when -`ML_SANDBOXED` is **not** exactly `1`. On a Sandbox2-launched child -(`ML_SANDBOXED=1`, set by `CSandboxedProcessSpawner`), the installation, the -hard-termination decision and the attestation marker are all skipped -entirely: the executor's own policy is the security boundary, an install -attempt from inside the sandbox could fail and terminate an otherwise-healthy -enforced launch, and emitting the marker would attest a legacy-route filter -on a launch `sandbox2_launch` reports as `"route":"sandbox2"`. So a -`"route":"sandbox2"` launch never carries a `seccomp_installed` marker, and -that absence is expected, not a missing signal. - -`ML_SANDBOXED` is a fail-open marker, so it is stripped from the environment -of every child the legacy spawner launches -(`lib/core/CDetachedProcessSpawner.cc`, `detail::buildChildEnvironment()`) - -an inherited or externally injected `ML_SANDBOXED=1` in the controller's own -environment can therefore never suppress a legacy-route child's mandatory -in-process filter. Only `CSandboxedProcessSpawner` sets it, and only on real -sandboxees. - -Hard termination on a failed in-process seccomp installation -(`TERMINATE_ON_DEGRADED_SECCOMP_FAILURE` in -`bin/pytorch_inference/Main.cc`) is deliberately **off**: an ordinary launch -with no explicit routing token is a degraded-route launch, so terminating -would fail every launch on a host without usable seccomp BPF. It becomes -safe to activate once every caller that matters always sends an explicit +`{ + "ml_sandbox2_route" : "legacy", "event" : "seccomp_installed" +} +` or, on the Landlock rung, `"ml_sandbox2_route" + : "landlock"` - + only when `ML_SANDBOXED` is * * not**exactly `1`.On a Sandbox2 - + launched + child(`ML_SANDBOXED = 1`, set by `CSandboxedProcessSpawner`), + the installation, + the hard - + termination decision and the attestation marker are all skipped entirely + : the executor's own policy is the security boundary, an install attempt + from inside the sandbox could fail and terminate an otherwise - + healthy enforced launch, + and emitting the marker would attest a legacy - + route filter on a launch `sandbox2_launch` reports as `"route" + : "sandbox2"`.So a +`"route" : "sandbox2"` launch never carries a `seccomp_installed` marker, + and that absence is expected, + not a missing signal. + +`ML_SANDBOXED` is a fail - open marker, + so it is stripped from the environment of every child the legacy spawner + launches(`lib / core / CDetachedProcessSpawner.cc`, `detail::buildChildEnvironment()`) - + an inherited + or externally injected `ML_SANDBOXED = + 1` in the controller's own environment can therefore never suppress a legacy + - route child's mandatory in + - process filter.Only `CSandboxedProcessSpawner` sets it, + and only on real sandboxees. + + Hard termination on a failed in + - process seccomp installation(`TERMINATE_ON_DEGRADED_SECCOMP_FAILURE` in +`bin / pytorch_inference / Main.cc`) is deliberately **off ** : an ordinary launch with no explicit routing token is a degraded - route launch + , + so terminating would fail every launch on a host without usable seccomp + BPF.It becomes safe to activate once every caller that matters always sends an + explicit `--disableSandbox` or `--requireSandbox` token per launch. -Example: - -```json -{"event":"sandbox2_launch","deployment_id":"a1b2c3","model_id":"my-model","route":"sandbox2","sandbox2_established":true,"mode":"enforced","sandbox2_compiled_in":true} -{"event":"sandbox2_launch","deployment_id":"a1b2c3","model_id":"my-model","route":"legacy","legacy_reason":"no_token_default","sandbox2_established":false,"mode":"degraded","sandbox2_compiled_in":true} + Example : + +```json{ + "event" : "sandbox2_launch", + "deployment_id" : "a1b2c3", + "model_id" : "my-model", + "route" : "sandbox2", + "sandbox2_established" : true, + "mode" : "enforced", + "sandbox2_compiled_in" : true + } { + "event" : "sandbox2_launch", + "deployment_id" : "a1b2c3", + "model_id" : "my-model", + "route" : "sandbox2", + "sandbox2_established" : false, + "mode" : "landlock", + "sandbox2_compiled_in" : true + } { + "event" : "sandbox2_launch", + "deployment_id" : "a1b2c3", + "model_id" : "my-model", + "route" : "legacy", + "legacy_reason" + : "no_token_default", + "sandbox2_established" : false, + "mode" : "degraded", + "sandbox2_compiled_in" : true +} ``` Emission site: `bin/controller/CProcessSpawnerRouter.cc`, From 1e691c16449c6514783b376c11bfb7cfc3c95e3e Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 24 Sep 2026 17:32:30 +0200 Subject: [PATCH 3/7] [ML] Rebase Landlock fallback onto typed routing and fix contract doc 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. --- docs/sandbox2_production_failure_modes.md | 109 ++++++------------ .../CLandlockFilesystemPolicy_Linux.cc | 3 + 2 files changed, 41 insertions(+), 71 deletions(-) diff --git a/docs/sandbox2_production_failure_modes.md b/docs/sandbox2_production_failure_modes.md index 9c6fd97a3e..70061d2280 100644 --- a/docs/sandbox2_production_failure_modes.md +++ b/docs/sandbox2_production_failure_modes.md @@ -1,4 +1,4 @@ -#Sandbox2 production failure modes +# Sandbox2 production failure modes This document tracks the operational log vocabulary the controller and `pytorch_inference` emit around the Sandbox2 rollout. This schema is itself @@ -31,8 +31,7 @@ single-line JSON object. | `deployment_id` | string | `SChildIpcLaunchSpec::s_ChildId`, from a single `sandbox::validateChildIpcLaunchSpec()` call made **once per `spawn()`, before dispatch**, so the value cannot disagree with the state the dispatch decision was taken against and is populated on the `degraded`/`fail_closed` modes too. Empty string (`""`, explicit, never omitted) only when no path-bearing launch option (`input`/`output`/`restore`/`logPipe`) was present at all. Control characters, quotes and backslashes are JSON-escaped so the line stays single-line JSON. | | `model_id` | string | Scanned from a `--modelid=` launch argument, using the same linear string-prefix scan style as the controller's `--disableSandbox` token scan. Empty string if absent. Escaped as for `deployment_id`. | | `route` | string | `"sandbox2"` when `CProcessSpawnerRouter::ERoute::E_Sandbox2` was in effect (a validated `--requireSandbox` token on a configured sandboxed path). `"legacy"` when the controller selected `E_Legacy` via the operator kill-switch (`--disableSandbox`) or the no-token default (see "No-token default" below). **`route` alone does not mean full Sandbox2 isolation** - read `mode` and `sandbox2_established` (a Landlock fallback still reports `"route":"sandbox2"`). | -| `legacy_reason` | string | **Only present when `route == "legacy"`** (equivalently, `mode == "degraded"`); -**omitted entirely** - never `""`, never `null` - on `route == "sandbox2"` (including `mode == "enforced"`, `mode == "landlock"`, and `mode == "fail_closed"`). `"kill_switch"` when a validated `--disableSandbox` token selected the legacy route, `"no_token_default"` when neither routing token was present. Provenance is passed in by `CCommandProcessor` (the only place it is known); the router never derives it from `args`. | +| `legacy_reason` | string | **Only present when `route == "legacy"`** (equivalently, `mode == "degraded"`); **omitted entirely** - never `""`, never `null` - on `route == "sandbox2"` (including `mode == "enforced"`, `mode == "landlock"`, and `mode == "fail_closed"`). `"kill_switch"` when a validated `--disableSandbox` token selected the legacy route, `"no_token_default"` when neither routing token was present. Provenance is passed in by `CCommandProcessor` (the only place it is known); the router never derives it from `args`. | | `sandbox2_established` | boolean | JSON boolean (`true`/`false`, never the string `"y"`/`"n"`). `true` iff `mode == "enforced"`, else `false`. | | `mode` | string | One of `"enforced"`, `"landlock"`, `"fail_closed"`, `"degraded"` - see mapping below. | | `sandbox2_compiled_in` | boolean | JSON boolean. Sourced from `sandbox::CMlSandboxAvailability::isCompiledIn()`, computed once (a build-time-constant fact, not per-launch state) and included on **every** emitted line, unlike `legacy_reason` which is conditional on route. Lets a consumer distinguish "Sandbox2 supported but no routing token sent" (`route == "legacy"`, `legacy_reason == "no_token_default"`, `sandbox2_compiled_in == true`) from "built without Sandbox2 support at all" (`sandbox2_compiled_in == false`) - both otherwise emit identical `legacy`/`no_token_default`/`degraded` signals for every plain launch. | @@ -100,78 +99,46 @@ name which token (if any) decided the route - the router itself only ever sees an already-decided route and never claims a token that was not present. -### In-process seccomp is legacy-route only +### In-process seccomp on legacy and Landlock routes `pytorch_inference` installs its own in-process seccomp filter - and emits -`{ - "ml_sandbox2_route" : "legacy", "event" : "seccomp_installed" -} -` or, on the Landlock rung, `"ml_sandbox2_route" - : "landlock"` - - only when `ML_SANDBOXED` is * * not**exactly `1`.On a Sandbox2 - - launched - child(`ML_SANDBOXED = 1`, set by `CSandboxedProcessSpawner`), - the installation, - the hard - - termination decision and the attestation marker are all skipped entirely - : the executor's own policy is the security boundary, an install attempt - from inside the sandbox could fail and terminate an otherwise - - healthy enforced launch, - and emitting the marker would attest a legacy - - route filter on a launch `sandbox2_launch` reports as `"route" - : "sandbox2"`.So a -`"route" : "sandbox2"` launch never carries a `seccomp_installed` marker, - and that absence is expected, - not a missing signal. - -`ML_SANDBOXED` is a fail - open marker, - so it is stripped from the environment of every child the legacy spawner - launches(`lib / core / CDetachedProcessSpawner.cc`, `detail::buildChildEnvironment()`) - - an inherited - or externally injected `ML_SANDBOXED = - 1` in the controller's own environment can therefore never suppress a legacy - - route child's mandatory in - - process filter.Only `CSandboxedProcessSpawner` sets it, - and only on real sandboxees. - - Hard termination on a failed in - - process seccomp installation(`TERMINATE_ON_DEGRADED_SECCOMP_FAILURE` in -`bin / pytorch_inference / Main.cc`) is deliberately **off ** : an ordinary launch with no explicit routing token is a degraded - route launch - , - so terminating would fail every launch on a host without usable seccomp - BPF.It becomes safe to activate once every caller that matters always sends an - explicit +`{"ml_sandbox2_route":"legacy","event":"seccomp_installed"}` on the legacy +route, or `{"ml_sandbox2_route":"landlock","event":"seccomp_installed"}` when +the router appended `--restrictFilesystem` for the Landlock rung - only when +`ML_SANDBOXED` is **not** exactly `1`. On a Sandbox2-launched child +(`ML_SANDBOXED=1`, set by `CSandboxedProcessSpawner`), the installation, the +hard-termination decision and the attestation marker are all skipped +entirely: the executor's own policy is the security boundary, an install +attempt from inside the sandbox could fail and terminate an otherwise-healthy +enforced launch, and emitting the marker would attest a legacy-route filter +on a launch `sandbox2_launch` reports as `"route":"sandbox2"`. So a +`"route":"sandbox2"` launch with `mode == "enforced"` never carries a +`seccomp_installed` marker, and that absence is expected, not a missing +signal. A `"route":"sandbox2"` launch with `mode == "landlock"` **does** +carry `seccomp_installed` with `"ml_sandbox2_route":"landlock"`. + +`ML_SANDBOXED` is a fail-open marker, so it is stripped from the environment +of every child the legacy spawner launches +(`lib/core/CDetachedProcessSpawner.cc`, `detail::buildChildEnvironment()`) - +an inherited or externally injected `ML_SANDBOXED=1` in the controller's own +environment can therefore never suppress a legacy-route child's mandatory +in-process filter. Only `CSandboxedProcessSpawner` sets it, and only on real +sandboxees. + +Hard termination on a failed in-process seccomp installation +(`TERMINATE_ON_DEGRADED_SECCOMP_FAILURE` in +`bin/pytorch_inference/Main.cc`) is deliberately **off**: an ordinary launch +with no explicit routing token is a degraded-route launch, so terminating +would fail every launch on a host without usable seccomp BPF. It becomes +safe to activate once every caller that matters always sends an explicit `--disableSandbox` or `--requireSandbox` token per launch. - Example : - -```json{ - "event" : "sandbox2_launch", - "deployment_id" : "a1b2c3", - "model_id" : "my-model", - "route" : "sandbox2", - "sandbox2_established" : true, - "mode" : "enforced", - "sandbox2_compiled_in" : true - } { - "event" : "sandbox2_launch", - "deployment_id" : "a1b2c3", - "model_id" : "my-model", - "route" : "sandbox2", - "sandbox2_established" : false, - "mode" : "landlock", - "sandbox2_compiled_in" : true - } { - "event" : "sandbox2_launch", - "deployment_id" : "a1b2c3", - "model_id" : "my-model", - "route" : "legacy", - "legacy_reason" - : "no_token_default", - "sandbox2_established" : false, - "mode" : "degraded", - "sandbox2_compiled_in" : true -} +Example: + +```json +{"event":"sandbox2_launch","deployment_id":"a1b2c3","model_id":"my-model","route":"sandbox2","sandbox2_established":true,"mode":"enforced","sandbox2_compiled_in":true} +{"event":"sandbox2_launch","deployment_id":"a1b2c3","model_id":"my-model","route":"sandbox2","sandbox2_established":false,"mode":"landlock","sandbox2_compiled_in":true} +{"event":"sandbox2_launch","deployment_id":"a1b2c3","model_id":"my-model","route":"legacy","legacy_reason":"no_token_default","sandbox2_established":false,"mode":"degraded","sandbox2_compiled_in":true} ``` Emission site: `bin/controller/CProcessSpawnerRouter.cc`, diff --git a/lib/seccomp/CLandlockFilesystemPolicy_Linux.cc b/lib/seccomp/CLandlockFilesystemPolicy_Linux.cc index 7f922a371e..38f0084eab 100644 --- a/lib/seccomp/CLandlockFilesystemPolicy_Linux.cc +++ b/lib/seccomp/CLandlockFilesystemPolicy_Linux.cc @@ -80,6 +80,9 @@ struct SLandlockPathBeneathAttr { std::int32_t s_ParentFd; } __attribute__((packed)); +static_assert(sizeof(SLandlockPathBeneathAttr) == 12, + "landlock_path_beneath_attr UAPI size (kernel build_check_abi)"); + long landlockCreateRuleset(const SLandlockRulesetAttr* attr, std::size_t size, std::uint32_t flags) { return ::syscall(ML_NR_landlock_create_ruleset, attr, size, flags); } From d208f0ed1b5abd79b9d24e80892005711253441d Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Fri, 25 Sep 2026 10:07:47 +0200 Subject: [PATCH 4/7] Update CProcessSpawnerRouterTest.cc --- bin/controller/unittest/CProcessSpawnerRouterTest.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/bin/controller/unittest/CProcessSpawnerRouterTest.cc b/bin/controller/unittest/CProcessSpawnerRouterTest.cc index ac485cedaf..534ddd3fa8 100644 --- a/bin/controller/unittest/CProcessSpawnerRouterTest.cc +++ b/bin/controller/unittest/CProcessSpawnerRouterTest.cc @@ -782,6 +782,4 @@ BOOST_AUTO_TEST_CASE(testLastSpawnFailureReasonClearedByALaterSuccessfulSpawn) { #endif // SANDBOX2_AVAILABLE && !Windows -// appended probe - BOOST_AUTO_TEST_SUITE_END() From 6ee90465cd59e7654f8ae11e89163660c5fd1f2d Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Fri, 25 Sep 2026 10:40:25 +0200 Subject: [PATCH 5/7] [ML] Fail closed when Landlock pipe directory is missing or invalid Require the same child-IPC preconditions on the Landlock rung as Sandbox2, and treat the per-child pipe directory as mandatory when applying Landlock. --- bin/controller/CProcessSpawnerRouter.cc | 87 ++++++++++++------- .../unittest/CProcessSpawnerRouterTest.cc | 54 +++++++++++- include/seccomp/CLandlockFilesystemPolicy.h | 12 +++ .../CLandlockFilesystemPolicy_Linux.cc | 71 +++++++++++---- .../unittest/CLandlockFilesystemPolicyTest.cc | 84 +++++++++++++++++- 5 files changed, 256 insertions(+), 52 deletions(-) diff --git a/bin/controller/CProcessSpawnerRouter.cc b/bin/controller/CProcessSpawnerRouter.cc index 449b8681d4..c87a17f063 100644 --- a/bin/controller/CProcessSpawnerRouter.cc +++ b/bin/controller/CProcessSpawnerRouter.cc @@ -18,6 +18,7 @@ #include #include +#include #include #include #include @@ -93,36 +94,39 @@ std::string jsonEscape(const std::string& s) { return out; } -//! Derive the per-launch deployment_id (SChildIpcLaunchSpec::s_ChildId) from -//! the path-bearing launch options in \p args, exactly as -//! CSandboxedProcessSpawner_Linux.cc does before constructing a Sandbox2 -//! policy (same trustedTmpDir derivation - getenv("TMPDIR"), defaulting to -//! "/tmp"). Called once per spawn(), *before* either backend runs, so the -//! sandbox2_launch signal and the dispatch decision see one and the same -//! filesystem -//! state: validateChildIpcLaunchSpec() does live ::realpath() calls, and a -//! post-spawn second call could observe a different (or, on the -//! legacy/degraded and failed-Sandbox2 paths, an absent) per-child IPC -//! directory and report an empty deployment_id on exactly the degraded and -//! fail_closed modes the signal exists to make debuggable. -//! Returns "" when no path-bearing option was present at all. -std::string deriveDeploymentId(const ml::controller::CProcessSpawnerRouter::TStrVec& args) { +struct SPreparedChildIpcLaunch { + ml::sandbox::EChildIpcDirectoryOutcome s_DirectoryOutcome{ + ml::sandbox::EChildIpcDirectoryOutcome::E_NoPathOptions}; + ml::sandbox::SChildIpcValidationResult s_Validation; +}; + +std::string trustedTmpDirFromEnvironment() { const char* tmpDirEnv{::getenv("TMPDIR")}; - const std::string trustedTmpDir{tmpDirEnv != nullptr ? tmpDirEnv : "/tmp"}; - // validateChildIpcLaunchSpec() does live ::realpath() calls, which - // require $TMPDIR/ml-child-ipc/ to already exist. This is the - // *other* production call site that reaches that function (the one - // inside CSandboxedProcessSpawner_Linux.cc::spawn() is the other), and - // it runs strictly before spawn() dispatches to either backend - on the - // legacy route just as much as the Sandbox2 route, since the signal - // below always wants a real deployment_id. Ensure the directory exists - // here too, rather than relying on the Sandbox2 spawner (which may not - // even run on this route) to have already done it. A creation failure - // is not logged again here: an empty deployment_id in the signal is - // itself the observable symptom, and the Sandbox2 spawner (when that - // route is actually taken) logs the failure with detail. - ml::sandbox::ensureChildIpcDirectory(trustedTmpDir, args); - return ml::sandbox::validateChildIpcLaunchSpec(trustedTmpDir, args).s_Spec.s_ChildId; + return tmpDirEnv != nullptr ? std::string{tmpDirEnv} : std::string{"/tmp"}; +} + +//! Create the per-child IPC directory and validate the launch spec once per +//! spawn(), *before* either backend runs, so the sandbox2_launch signal and +//! the Landlock dispatch decision see one filesystem state. Same checks as +//! CSandboxedProcessSpawner_Linux.cc::spawn() (which re-runs them on the +//! Sandbox2 route). +SPreparedChildIpcLaunch +prepareChildIpcLaunch(const ml::controller::CProcessSpawnerRouter::TStrVec& args) { + const std::string trustedTmpDir{trustedTmpDirFromEnvironment()}; + SPreparedChildIpcLaunch prepared; + prepared.s_DirectoryOutcome = ml::sandbox::ensureChildIpcDirectory(trustedTmpDir, args); + prepared.s_Validation = ml::sandbox::validateChildIpcLaunchSpec(trustedTmpDir, args); + return prepared; +} + +std::string rejectedChildIpcLaunchSpecMessage(const std::string& processPath, + const ml::sandbox::SChildIpcValidationResult& validated) { + std::ostringstream rejected; + for (const ml::sandbox::SRejectedChildIpcPath& r : validated.s_Rejected) { + rejected << " [" << r.s_Arg << ": reason=" << static_cast(r.s_Reason) << ']'; + } + return std::string{"Rejected pytorch_inference child-IPC launch spec for "} + + processPath + ':' + rejected.str(); } } // namespace @@ -252,8 +256,10 @@ bool CProcessSpawnerRouter::spawn(ERoute route, // post-spawn second derivation is not equivalent. Skipped entirely for // processes that can never emit the signal, so unrelated permitted // processes (autodetect etc.) pay no ::realpath() cost. - const std::string deploymentId{sandboxEligible ? deriveDeploymentId(args) - : std::string()}; + const SPreparedChildIpcLaunch prepared{ + sandboxEligible ? prepareChildIpcLaunch(args) : SPreparedChildIpcLaunch{}}; + const std::string deploymentId{ + sandboxEligible ? prepared.s_Validation.s_Spec.s_ChildId : std::string()}; m_LastSpawnFailureReason.clear(); bool spawned{false}; @@ -306,13 +312,30 @@ bool CProcessSpawnerRouter::spawn(ERoute route, spawned = m_SandboxSpawner->spawn(processPath, args, childPid); break; case sandbox::EConfinementLevel::E_Landlock: { + landlockFallback = true; + if (prepared.s_DirectoryOutcome == + sandbox::EChildIpcDirectoryOutcome::E_CreationFailed) { + m_LastSpawnFailureReason = + std::string{"Failed to create the per-child IPC directory under "} + + trustedTmpDirFromEnvironment() + "/ml-child-ipc for " + + processPath + ": " + ::strerror(errno); + LOG_ERROR(<< m_LastSpawnFailureReason); + spawned = false; + break; + } + if (prepared.s_Validation.s_Ok == false) { + m_LastSpawnFailureReason = rejectedChildIpcLaunchSpecMessage( + processPath, prepared.s_Validation); + LOG_ERROR(<< m_LastSpawnFailureReason); + spawned = false; + break; + } // A supported, deliberate degradation: INFO, with what an // administrator would change to get full isolation. LOG_INFO(<< sandbox::landlockFallbackMessage(host, processPath)); TStrVec landlockArgs{args}; landlockArgs.emplace_back(RESTRICT_FILESYSTEM_TOKEN); spawned = m_LegacySpawner.spawn(processPath, landlockArgs, childPid); - landlockFallback = true; break; } case sandbox::EConfinementLevel::E_Unavailable: diff --git a/bin/controller/unittest/CProcessSpawnerRouterTest.cc b/bin/controller/unittest/CProcessSpawnerRouterTest.cc index 534ddd3fa8..9e72554ccb 100644 --- a/bin/controller/unittest/CProcessSpawnerRouterTest.cc +++ b/bin/controller/unittest/CProcessSpawnerRouterTest.cc @@ -30,6 +30,9 @@ #include #include #include +#ifndef Windows +#include +#endif // This file follows CCommandProcessorTest.cc's convention of testing spawn // dispatch without a spawner spy: it drives real (non-Linux) dispatch to @@ -171,6 +174,10 @@ class CScopedChildIpcRoot { return "--input=" + m_ChildIpcRoot + "/input"; } + const std::string& childIpcRoot() const { return m_ChildIpcRoot; } + + const std::string& trustedTmpDir() const { return m_TrustedTmpDir; } + CScopedChildIpcRoot(const CScopedChildIpcRoot&) = delete; CScopedChildIpcRoot& operator=(const CScopedChildIpcRoot&) = delete; @@ -625,6 +632,7 @@ BOOST_AUTO_TEST_CASE(testLegacyOnlyRouterNeedsNoSandboxedSpawner) { // SHELL_FLAG above assume a POSIX shell. BOOST_AUTO_TEST_CASE(testSandbox2RouteDegradesToLandlockRungWithInjectedConfinement) { + CScopedChildIpcRoot childIpcRoot{"router-landlock-rung"}; // Inject a confinement whose ladder rung is E_Landlock (as // decideConfinement() would return for, say, E_UserNamespaceDenied with // Landlock ABI >= 1), so this is deterministic regardless of whether this @@ -648,7 +656,10 @@ BOOST_AUTO_TEST_CASE(testSandbox2RouteDegradesToLandlockRungWithInjectedConfinem // first line the script writes is the appended token - proving it reached // the spawned process's argv, not merely that spawn() returned true. ml::controller::CProcessSpawnerRouter::TStrVec args{ - SHELL_FLAG, "printf '%s\\n' \"$0\" > " + outputFile}; + SHELL_FLAG, + "last=\"\"; for a in \"$@\"; do last=\"$a\"; done; printf '%s\\n' \"$last\" > " + + outputFile, + childIpcRoot.inputArg()}; ml::core::CProcess::TPid childPid{0}; std::string logged{captureLogged([&] { BOOST_REQUIRE_EQUAL(true, router.spawn(ml::controller::CProcessSpawnerRouter::ERoute::E_Sandbox2, @@ -684,6 +695,47 @@ BOOST_AUTO_TEST_CASE(testSandbox2RouteDegradesToLandlockRungWithInjectedConfinem BOOST_REQUIRE(logged.find("kernel.unprivileged_userns_clone=1") != std::string::npos); } +BOOST_AUTO_TEST_CASE(testLandlockRungFailsClosedOnInvalidChildIpcSpec) { + CScopedChildIpcRoot childIpcRoot{"router-landlock-invalid-ipc"}; + const std::string siblingChildRoot{childIpcRoot.trustedTmpDir() + "/ml-child-ipc/other-child-id"}; + BOOST_TEST_REQUIRE(boost::filesystem::create_directories(siblingChildRoot)); + + ml::sandbox::SHostConfinement injectedHost; + injectedHost.s_Level = ml::sandbox::EConfinementLevel::E_Landlock; + injectedHost.s_Sandbox2 = ml::sandbox::ESandbox2Capability::E_UserNamespaceDenied; + injectedHost.s_LandlockAbi = 1; + + ml::controller::CProcessSpawnerRouter::TStrVec permittedPaths{PROCESS_PATH}; + ml::controller::CProcessSpawnerRouter::TStrVec sandboxedPaths{PROCESS_PATH}; + ml::controller::CProcessSpawnerRouter router{ + permittedPaths, sandboxedPaths, [injectedHost] { return injectedHost; }}; + + const std::string markerFile{"router_test_landlock_invalid_ipc.txt"}; + std::remove(markerFile.c_str()); + + ml::controller::CProcessSpawnerRouter::TStrVec args{ + childIpcRoot.inputArg(), "--output=" + siblingChildRoot + "/output", + SHELL_FLAG, "touch " + markerFile}; + ml::core::CProcess::TPid childPid{0}; + std::string logged{captureLogged([&] { + BOOST_REQUIRE_EQUAL( + false, router.spawn(ml::controller::CProcessSpawnerRouter::ERoute::E_Sandbox2, + PROCESS_PATH, args, childPid)); + })}; + + BOOST_REQUIRE_EQUAL(ml::core::CProcess::TPid{0}, childPid); + BOOST_TEST_REQUIRE(router.lastSpawnFailureReason().find( + "Rejected pytorch_inference child-IPC") != std::string::npos); + BOOST_TEST_REQUIRE(router.lastSpawnFailureReason().find("reason=") != std::string::npos); + BOOST_REQUIRE(logged.find("\"mode\":\"fail_closed\"") != std::string::npos); + BOOST_REQUIRE(logged.find("\"sandbox2_established\":false") != std::string::npos); + + std::this_thread::sleep_for(std::chrono::seconds{1}); + std::ifstream ifs{markerFile}; + BOOST_REQUIRE_EQUAL(false, ifs.is_open()); + std::remove(markerFile.c_str()); +} + BOOST_AUTO_TEST_CASE(testSandbox2RouteFailsClosedWithInjectedUnavailableConfinement) { // Inject a confinement whose ladder rung is E_Unavailable (neither // Sandbox2 nor Landlock), deterministically regardless of this host's diff --git a/include/seccomp/CLandlockFilesystemPolicy.h b/include/seccomp/CLandlockFilesystemPolicy.h index 7899ded162..f0217fd27f 100644 --- a/include/seccomp/CLandlockFilesystemPolicy.h +++ b/include/seccomp/CLandlockFilesystemPolicy.h @@ -108,6 +108,18 @@ inline std::string perChildIpcDirectory(const std::string& logPipePath) { if (logPipePath.empty() || logPipePath[0] != '/') { return std::string{}; } + for (std::size_t start = 1; start < logPipePath.size();) { + const std::size_t end{logPipePath.find('/', start)}; + const std::string component{logPipePath.substr( + start, end == std::string::npos ? std::string::npos : end - start)}; + if (component == "..") { + return std::string{}; + } + if (end == std::string::npos) { + break; + } + start = end + 1; + } const std::size_t fileSlash{logPipePath.rfind('/')}; if (fileSlash == std::string::npos || fileSlash == 0) { return std::string{}; diff --git a/lib/seccomp/CLandlockFilesystemPolicy_Linux.cc b/lib/seccomp/CLandlockFilesystemPolicy_Linux.cc index 38f0084eab..3869ddae8b 100644 --- a/lib/seccomp/CLandlockFilesystemPolicy_Linux.cc +++ b/lib/seccomp/CLandlockFilesystemPolicy_Linux.cc @@ -125,22 +125,11 @@ constexpr std::uint64_t FILE_APPLICABLE_ACCESS{ ACCESS_FS_EXECUTE | ACCESS_FS_WRITE_FILE | ACCESS_FS_READ_FILE | ACCESS_FS_TRUNCATE | ACCESS_FS_IOCTL_DEV}; -//! Grant a path the rights it needs, intersected with what the ruleset -//! handles. A right granted but not handled is meaningless (Landlock only -//! restricts what it handles), and a right handled but not granted is what -//! actually denies access. -bool addPathRule(int rulesetFd, const std::string& path, std::uint64_t allowed, std::uint64_t handled) { - // O_PATH so opening the directory itself needs no read permission and - // triggers none of the side effects of a real open. - const int pathFd{::open(path.c_str(), O_PATH | O_CLOEXEC)}; - if (pathFd < 0) { - // 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. - LOG_DEBUG(<< "Landlock: skipping absent path " << path << ": " << ::strerror(errno)); - return true; - } +bool addLandlockRuleFromFd(int rulesetFd, + int pathFd, + const std::string& path, + std::uint64_t allowed, + std::uint64_t handled) { // Landlock rejects a rule (EINVAL) whose allowed_access names a // directory-only right when the file descriptor is not a directory, so a @@ -160,6 +149,52 @@ bool addPathRule(int rulesetFd, const std::string& path, std::uint64_t allowed, LOG_ERROR(<< "Landlock: could not add rule for " << path << ": " << ::strerror(errno)); } + return ok; +} + +//! Grant optional read-only paths. Missing entries are skipped: several +//! host paths in pytorchInferenceLandlockPaths() vary by distribution layout +//! or image, and a rule for an absent path grants nothing anyway. +bool addOptionalReadOnlyPathRule(int rulesetFd, + const std::string& path, + std::uint64_t allowed, + std::uint64_t handled) { + const int pathFd{::open(path.c_str(), O_PATH | O_CLOEXEC)}; + if (pathFd < 0) { + LOG_DEBUG(<< "Landlock: skipping absent path " << path << ": " << ::strerror(errno)); + return true; + } + const bool ok{addLandlockRuleFromFd(rulesetFd, pathFd, path, allowed, handled)}; + ::close(pathFd); + return ok; +} + +//! Grant the per-child IPC directory. Must exist, be a real directory owned +//! by this uid, and must not be reached through a symlink - the same +//! invariants validateChildIpcLaunchSpec() enforces on the Sandbox2 route. +bool addMandatoryPipeDirectoryRule(int rulesetFd, + const std::string& path, + std::uint64_t allowed, + std::uint64_t handled) { + const int pathFd{::open(path.c_str(), O_PATH | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC)}; + if (pathFd < 0) { + LOG_ERROR(<< "Landlock: pipe directory " << path + << " is not usable: " << ::strerror(errno)); + return false; + } + struct stat pathStat {}; + if (::fstat(pathFd, &pathStat) != 0 || S_ISDIR(pathStat.st_mode) == false) { + LOG_ERROR(<< "Landlock: pipe directory " << path + << " is not a directory: " << ::strerror(errno)); + ::close(pathFd); + return false; + } + if (static_cast(pathStat.st_uid) != ::geteuid()) { + LOG_ERROR(<< "Landlock: pipe directory " << path << " is not owned by this process"); + ::close(pathFd); + return false; + } + const bool ok{addLandlockRuleFromFd(rulesetFd, pathFd, path, allowed, handled)}; ::close(pathFd); return ok; } @@ -294,7 +329,7 @@ ELandlockOutcome applyLandlockFilesystemPolicy(const SLandlockPaths& paths) { // No EXECUTE: see SLandlockPaths::s_ReadOnly. const std::uint64_t readOnlyAccess{ACCESS_FS_READ_FILE | ACCESS_FS_READ_DIR}; for (const std::string& path : paths.s_ReadOnly) { - ok = addPathRule(fd, path, readOnlyAccess, handled) && ok; + ok = addOptionalReadOnlyPathRule(fd, path, readOnlyAccess, handled) && ok; } // Exactly what CNamedPipeFactory does in the IPC directory: mkfifo(), @@ -302,7 +337,7 @@ ELandlockOutcome applyLandlockFilesystemPolicy(const SLandlockPaths& paths) { const std::uint64_t pipeDirectoryAccess{ACCESS_FS_MAKE_FIFO | ACCESS_FS_READ_FILE | ACCESS_FS_WRITE_FILE | ACCESS_FS_REMOVE_FILE}; for (const std::string& path : paths.s_PipeDirectories) { - ok = addPathRule(fd, path, pipeDirectoryAccess, handled) && ok; + ok = addMandatoryPipeDirectoryRule(fd, path, pipeDirectoryAccess, handled) && ok; } if (ok == false) { diff --git a/lib/seccomp/unittest/CLandlockFilesystemPolicyTest.cc b/lib/seccomp/unittest/CLandlockFilesystemPolicyTest.cc index 56c2065469..c72e3a2b46 100644 --- a/lib/seccomp/unittest/CLandlockFilesystemPolicyTest.cc +++ b/lib/seccomp/unittest/CLandlockFilesystemPolicyTest.cc @@ -77,6 +77,9 @@ BOOST_AUTO_TEST_CASE(testPerChildIpcDirectoryNegativeCases) { // "xml-child-ipc") must NOT match - the comparison must be exact, not a // suffix match. BOOST_TEST_REQUIRE(perChildIpcDirectory("/tmp/xml-child-ipc/dep-1/logPipe").empty()); + + BOOST_TEST_REQUIRE( + perChildIpcDirectory("/tmp/foo/../ml-child-ipc/dep-1/logPipe").empty()); } #ifdef Linux @@ -92,7 +95,8 @@ enum EChildExit : int { E_ChildGrantedPathUnreadable = 21, E_ChildDeniedPathStillReadable = 22, E_ChildGrantedDirNotWritable = 23, - E_ChildExecRefused = 24 + E_ChildExecRefused = 24, + E_ChildPolicyFailed = 25 }; //! Run \p body in a forked child and return its exit code, or -1 if the child @@ -392,6 +396,84 @@ BOOST_AUTO_TEST_CASE(testRealPytorchPolicyDeniesTheExploitTargetWrite) { BOOST_REQUIRE_EQUAL(childResult, static_cast(E_ChildOk)); } +BOOST_AUTO_TEST_CASE(testMissingPipeDirectoryFailsRulesetApply) { + const int childResult{runInChild([] { + ml::seccomp::SLandlockPaths paths; + paths.s_PipeDirectories.push_back("/tmp/ml-landlock-missing-pipe-dir-XXXXXX"); + + const ml::seccomp::ELandlockOutcome outcome{ + ml::seccomp::applyLandlockFilesystemPolicy(paths)}; + if (outcome == ml::seccomp::ELandlockOutcome::E_Unsupported) { + return static_cast(E_ChildNotApplied); + } + return outcome == ml::seccomp::ELandlockOutcome::E_Failed + ? static_cast(E_ChildPolicyFailed) + : static_cast(E_ChildGrantedPathUnreadable); + })}; + + if (childResult == E_ChildNotApplied) { + BOOST_TEST_MESSAGE("Landlock unsupported on this kernel - skipping"); + return; + } + BOOST_REQUIRE_EQUAL(childResult, static_cast(E_ChildPolicyFailed)); +} + +BOOST_AUTO_TEST_CASE(testSymlinkPipeDirectoryFailsRulesetApply) { + const std::string scratch{makeScratchDirectory()}; + BOOST_TEST_REQUIRE(scratch.empty() == false); + + const std::string realDir{scratch + "/real-pipes"}; + BOOST_TEST_REQUIRE(::mkdir(realDir.c_str(), 0700) == 0); + const std::string linkPath{scratch + "/pipe-link"}; + BOOST_TEST_REQUIRE(::symlink(realDir.c_str(), linkPath.c_str()) == 0); + + const int childResult{runInChild([&] { + ml::seccomp::SLandlockPaths paths; + paths.s_PipeDirectories.push_back(linkPath); + + const ml::seccomp::ELandlockOutcome outcome{ + ml::seccomp::applyLandlockFilesystemPolicy(paths)}; + if (outcome == ml::seccomp::ELandlockOutcome::E_Unsupported) { + return static_cast(E_ChildNotApplied); + } + return outcome == ml::seccomp::ELandlockOutcome::E_Failed + ? static_cast(E_ChildPolicyFailed) + : static_cast(E_ChildGrantedPathUnreadable); + })}; + + ::unlink(linkPath.c_str()); + ::rmdir(realDir.c_str()); + ::rmdir(scratch.c_str()); + + if (childResult == E_ChildNotApplied) { + BOOST_TEST_MESSAGE("Landlock unsupported on this kernel - skipping"); + return; + } + BOOST_REQUIRE_EQUAL(childResult, static_cast(E_ChildPolicyFailed)); +} + +BOOST_AUTO_TEST_CASE(testMissingReadOnlyPathStillAppliesRulesetWithoutPipeDirectory) { + const int childResult{runInChild([] { + ml::seccomp::SLandlockPaths paths; + paths.s_ReadOnly.push_back("/tmp/ml-landlock-absent-readonly-path"); + + const ml::seccomp::ELandlockOutcome outcome{ + ml::seccomp::applyLandlockFilesystemPolicy(paths)}; + if (outcome == ml::seccomp::ELandlockOutcome::E_Unsupported) { + return static_cast(E_ChildNotApplied); + } + return outcome == ml::seccomp::ELandlockOutcome::E_Applied + ? static_cast(E_ChildOk) + : static_cast(E_ChildPolicyFailed); + })}; + + if (childResult == E_ChildNotApplied) { + BOOST_TEST_MESSAGE("Landlock unsupported on this kernel - skipping"); + return; + } + BOOST_REQUIRE_EQUAL(childResult, static_cast(E_ChildOk)); +} + #endif // Linux BOOST_AUTO_TEST_SUITE_END() From 48303e0cb9a4151fb145caa7485cd7f968e3604c Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Fri, 25 Sep 2026 10:57:53 +0200 Subject: [PATCH 6/7] Formmatting --- bin/controller/unittest/CProcessSpawnerRouterTest.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bin/controller/unittest/CProcessSpawnerRouterTest.cc b/bin/controller/unittest/CProcessSpawnerRouterTest.cc index 9e72554ccb..dec709329c 100644 --- a/bin/controller/unittest/CProcessSpawnerRouterTest.cc +++ b/bin/controller/unittest/CProcessSpawnerRouterTest.cc @@ -657,8 +657,7 @@ BOOST_AUTO_TEST_CASE(testSandbox2RouteDegradesToLandlockRungWithInjectedConfinem // the spawned process's argv, not merely that spawn() returned true. ml::controller::CProcessSpawnerRouter::TStrVec args{ SHELL_FLAG, - "last=\"\"; for a in \"$@\"; do last=\"$a\"; done; printf '%s\\n' \"$last\" > " + - outputFile, + "last=\"\"; for a in \"$@\"; do last=\"$a\"; done; printf '%s\\n' \"$last\" > " + outputFile, childIpcRoot.inputArg()}; ml::core::CProcess::TPid childPid{0}; std::string logged{captureLogged([&] { From f1d0bc889648ff88e1fa6795158cb80207360e96 Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Fri, 25 Sep 2026 12:15:14 +0200 Subject: [PATCH 7/7] [ML] Create router test IPC dirs at mode 0700 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. --- bin/controller/unittest/CProcessSpawnerRouterTest.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bin/controller/unittest/CProcessSpawnerRouterTest.cc b/bin/controller/unittest/CProcessSpawnerRouterTest.cc index dec709329c..2fccac09ab 100644 --- a/bin/controller/unittest/CProcessSpawnerRouterTest.cc +++ b/bin/controller/unittest/CProcessSpawnerRouterTest.cc @@ -31,6 +31,7 @@ #include #include #ifndef Windows +#include #include #endif @@ -153,6 +154,9 @@ class CScopedChildIpcRoot { .string(); m_ChildIpcRoot = m_TrustedTmpDir + "/ml-child-ipc/" + childId; boost::filesystem::create_directories(m_ChildIpcRoot); + // ensureChildIpcDirectory() accepts an existing directory only at mode 0700. + BOOST_REQUIRE_EQUAL(0, ::chmod((m_TrustedTmpDir + "/ml-child-ipc").c_str(), 0700)); + BOOST_REQUIRE_EQUAL(0, ::chmod(m_ChildIpcRoot.c_str(), 0700)); BOOST_REQUIRE_EQUAL( 0, ml::core::CSetEnv::setEnv("TMPDIR", m_TrustedTmpDir.c_str(), 1));