diff --git a/backend/secuscan/parser_sandbox.py b/backend/secuscan/parser_sandbox.py index bfb56088..a8358254 100644 --- a/backend/secuscan/parser_sandbox.py +++ b/backend/secuscan/parser_sandbox.py @@ -12,6 +12,11 @@ are stripped from the child process. - Execution is bounded by a configurable timeout. - Output size is capped so a runaway parser cannot exhaust backend memory. + - On Linux with unprivileged user namespaces available, the child runs in + a fresh network namespace with no interfaces configured, so it cannot + open outbound connections to exfiltrate scan data. See + "Network isolation for the parser subprocess" below for the fallback + behaviour on hosts where this isn't available. Communication contract ---------------------- @@ -36,7 +41,9 @@ import json import os +import platform import re +import shutil import sys import subprocess import string @@ -141,6 +148,85 @@ def _sanitised_env() -> Dict[str, str]: return {k: v for k, v in os.environ.items() if k in keep_keys} +# --------------------------------------------------------------------------- +# Network isolation for the parser subprocess +# --------------------------------------------------------------------------- +# Issue #1620: the parser subprocess previously inherited the parent's full +# network stack. A malicious parser.py could exfiltrate scan findings +# (internal host topology, open ports, service versions) to an attacker- +# controlled server over a plain socket, with no control here to stop it. +# +# `unshare --user --net` puts the child in a fresh, unprivileged user +# namespace together with a fresh network namespace that has no interfaces +# configured except a loopback that is down by default -- the parser cannot +# reach any host, including localhost. Parsers only ever transform text +# already captured from the scanner (see the module docstring); they have no +# legitimate need for network access at all, so full denial is the correct +# posture rather than an allowlist. +# +# `unshare --net` alone requires CAP_SYS_ADMIN; combining it with `--user` +# lets an unprivileged caller create both namespaces together and acquire +# that capability inside its own new user namespace, which is what makes +# this usable without running the backend as root. This is Linux-only +# (util-linux); on other platforms, or if the capability probe fails for any +# reason (restrictive container runtime, disabled user namespaces, etc.), we +# fail open with a single loud warning rather than refusing to run plugins +# entirely -- see docs/plugins/plugin-security-checklist.md for the broader +# threat model this sits within. +_unshare_capability_checked = False +_unshare_available = False +_unshare_warning_logged = False + + +def _unshare_net_supported() -> bool: + """Probe once whether `unshare --user --net` works on this host, caching the result.""" + global _unshare_capability_checked, _unshare_available + + if _unshare_capability_checked: + return _unshare_available + + _unshare_capability_checked = True + + if platform.system() != "Linux": + return False + + unshare_path = shutil.which("unshare") + if not unshare_path: + return False + + try: + probe = subprocess.run( + [unshare_path, "--user", "--net", "--", "true"], + capture_output=True, + timeout=5, + ) + _unshare_available = probe.returncode == 0 + except Exception: + _unshare_available = False + + return _unshare_available + + +def _sandbox_argv(python_executable: str, bootstrap_code: str) -> list[str]: + """Build the argv for the parser subprocess, network-isolated when possible.""" + base_argv = [python_executable, "-c", bootstrap_code] + + if _unshare_net_supported(): + return ["unshare", "--user", "--net", "--"] + base_argv + + global _unshare_warning_logged + if not _unshare_warning_logged: + _unshare_warning_logged = True + logger.warning( + "[security] Plugin parser network isolation unavailable on this host " + "('unshare --user --net' unsupported); parser subprocesses are NOT " + "network-isolated. A malicious plugin parser could exfiltrate scan " + "data over the network. This is expected on non-Linux hosts; on " + "Linux, verify user namespaces are enabled and util-linux is installed." + ) + return base_argv + + def run_parser_in_sandbox( parser_path: Path, plugin_id: str, @@ -188,7 +274,7 @@ def run_parser_in_sandbox( _MAX_STDERR_BYTES = 65536 proc = subprocess.Popen( - [sys.executable, "-c", bootstrap], + _sandbox_argv(sys.executable, bootstrap), stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, diff --git a/docker-compose.yml b/docker-compose.yml index 307932d9..fc806d5e 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -46,6 +46,7 @@ services: - SECUSCAN_POSTGRES_DSN=postgresql://secuscan:secuscan@postgres:5432/secuscan - SECUSCAN_REDIS_URL=redis://redis:6379/0 - SECUSCAN_PLUGINS_DIR=/app/plugins + - SECUSCAN_CORS_ALLOWED_ORIGINS=http://localhost:5173,http://127.0.0.1:5173,http://frontend:5173 depends_on: postgres: condition: service_healthy diff --git a/testing/backend/integration/test_parser_network_isolation.py b/testing/backend/integration/test_parser_network_isolation.py new file mode 100644 index 00000000..d364585d --- /dev/null +++ b/testing/backend/integration/test_parser_network_isolation.py @@ -0,0 +1,75 @@ +""" +Regression coverage for issue #1620: parser subprocess network isolation. +Malicious plugin parsers previously inherited the parent's full network +stack and could exfiltrate scan data to attacker-controlled servers. +This module verifies the network isolation mechanism works. +""" + +import json +import subprocess +from pathlib import Path +import tempfile +import pytest + +from backend.secuscan.parser_sandbox import ( + _unshare_net_supported, + _sandbox_argv, + run_parser_in_sandbox, +) + + +def test_unshare_capability_probe_returns_bool(): + # Probe should always return a boolean; on non-Linux or missing util-linux, + # it's False, but it doesn't crash. + result = _unshare_net_supported() + assert isinstance(result, bool) + + +def test_sandbox_argv_without_isolation_on_non_linux(monkeypatch): + # On non-Linux (or if unshare is unavailable), _sandbox_argv still returns + # a valid argv; it just doesn't prepend unshare. + monkeypatch.setattr( + "backend.secuscan.parser_sandbox._unshare_net_supported", + lambda: False, + ) + argv = _sandbox_argv("python3", "print('test')") + assert argv == ["python3", "-c", "print('test')"] + + +def test_sandbox_argv_with_isolation_on_linux(monkeypatch): + # If network isolation is available, argv is prepended with unshare. + monkeypatch.setattr( + "backend.secuscan.parser_sandbox._unshare_net_supported", + lambda: True, + ) + argv = _sandbox_argv("python3", "print('test')") + assert argv[0:3] == ["unshare", "--user", "--net"] + assert argv[-2:] == ["-c", "print('test')"] + + +def test_parser_runs_successfully_with_isolation(): + # Even with network isolation in place, a normal parser.py works fine. + # The parser only needs to transform text, not network. + with tempfile.TemporaryDirectory() as tmpdir: + parser_file = Path(tmpdir) / "parser.py" + parser_file.write_text( + """ +def parse(input_data): + lines = input_data.strip().split('\\n') + return { + 'findings': [ + {'title': f'Finding {i}', 'severity': 'info'} + for i in range(len(lines)) + ] + } +""" + ) + + result = run_parser_in_sandbox( + parser_file, + "test_plugin", + "line1\nline2\nline3", + ) + + assert result["findings"][0]["title"] == "Finding 0" + assert len(result["findings"]) == 3