diff --git a/dokimi/changelog.d/20261029_120000_vlad-perevezentsev_example-docker-hardening.md b/dokimi/changelog.d/20261029_120000_vlad-perevezentsev_example-docker-hardening.md new file mode 100644 index 000000000..bc31d982b --- /dev/null +++ b/dokimi/changelog.d/20261029_120000_vlad-perevezentsev_example-docker-hardening.md @@ -0,0 +1,14 @@ + + +### Security + +- **The `docker` backend of example execution drops every capability** — a + fenced example ran as the image's root with docker's default capabilities, + and a setuid binary in the image (`su` in `python:3.11-slim`) still worked. + The container now starts with `--cap-drop ALL` and `--security-opt + no-new-privileges`, as `repo run` containers do. The snippet's directory is + mounted world-readable (0755, the snippet 0644), because a root without + capabilities cannot enter the host user's private temp directory. diff --git a/dokimi/docs/example-execution.md b/dokimi/docs/example-execution.md index 74626b4bd..4922e8ce2 100644 --- a/dokimi/docs/example-execution.md +++ b/dokimi/docs/example-execution.md @@ -36,7 +36,7 @@ metrics: | --- | --- | --- | | `none` (default) | Nothing runs; examples reported as `skipped` | Untrusted corpora, or you only want extraction | | `subprocess` | Host subprocess with per-process CPU/memory caps, minimal env, temp cwd; its process group is killed when it exits or times out. **No network isolation and no process-count cap.** Requires `allow_host: true`. | Trusted, self-owned documentation | -| `docker` | Container with `--network none`, read-only mounts, CPU/memory/pid limits, per-example timeout | Untrusted documentation | +| `docker` | Container with `--network none`, no capabilities (`--cap-drop ALL`, `no-new-privileges`), read-only mounts, CPU/memory/pid limits, per-example timeout | Untrusted documentation | ### Why `none` is the default @@ -65,4 +65,7 @@ DOC_BENCH_EXAMPLE_BACKEND=none dokimi run --spec benchmarks/spec.v1.yaml counts all of the user's processes, not the snippet's). Only use it on documentation you trust. - For anything else, use `docker`, which adds `--network none` and a read-only - filesystem. See also [SECURITY.md](../SECURITY.md). + filesystem, and drops every capability with `no-new-privileges`, so the + image's root cannot gain any back through a setuid binary. The snippet's + temp directory is mounted world-readable for that capability-less root. + See also [SECURITY.md](../SECURITY.md). diff --git a/dokimi/dokimi/metrics/example_runner.py b/dokimi/dokimi/metrics/example_runner.py index 77cbbc647..22490e039 100644 --- a/dokimi/dokimi/metrics/example_runner.py +++ b/dokimi/dokimi/metrics/example_runner.py @@ -14,8 +14,9 @@ ``allow_host=True`` (an explicit opt-in). Use only for **trusted, self-owned** documentation. Network is *not* isolated in this mode, and the number of processes a snippet spawns is not capped. -* ``docker`` — run inside a locked-down container (``--network none``, - read-only mounts, CPU/memory/time limits). Preferred for untrusted docs. +* ``docker`` — run inside a locked-down container (``--network none``, no + capabilities, ``no-new-privileges``, read-only mounts, CPU/memory/process/time + limits). Preferred for untrusted docs. See ``docs/example-execution.md``. """ @@ -187,15 +188,20 @@ def _run_subprocess(lang: str, code: str, policy: ExecutionPolicy, def _run_docker(lang: str, code: str, policy: ExecutionPolicy, runner: process.Runner) -> tuple[bool, str | None]: - """Run code inside a locked-down container (no network, read-only, limited).""" + """Run code inside a locked-down container (no network, no capabilities, read-only, limited).""" suffix, argv_prefix = EXECUTORS[lang] with tempfile.TemporaryDirectory() as tmp: script = Path(tmp) / f"snippet{suffix}" script.write_text(code, encoding="utf-8") - flags = ContainerPolicy(network="none", cpus=policy.cpus, memory_mb=policy.mem_mb, pids_limit=64, - read_only=True) + # Root without CAP_DAC_OVERRIDE cannot enter a 0700 dir it does not own (#229), + # so the :ro mount is world-readable; it holds only the document's own snippet. + os.chmod(tmp, 0o755) # nosec B103 - read and entered as "other", not writable + script.chmod(0o644) + flags = ContainerPolicy(network="none", no_new_privileges=True, cap_drop_all=True, cpus=policy.cpus, + memory_mb=policy.mem_mb, pids_limit=64, read_only=True) cmd = [ "docker", "run", "--rm", + *flags.hardening_argv(), *flags.network_argv(), *flags.resource_argv(), *flags.pids_argv(), diff --git a/dokimi/tests/test_container_argv_characterization.py b/dokimi/tests/test_container_argv_characterization.py index 01c5069e9..505d64903 100644 --- a/dokimi/tests/test_container_argv_characterization.py +++ b/dokimi/tests/test_container_argv_characterization.py @@ -120,6 +120,7 @@ def test_example_docker_argv_default_policy_pins_every_flag_in_order(): argv, timeout = _docker_cmd(ExecutionPolicy(backend="docker", timeout=7)) assert argv == [ "docker", "run", "--rm", + "--security-opt", "no-new-privileges", "--cap-drop", "ALL", "--network", "none", "--cpus", "1.0", "--memory", "512m", @@ -138,6 +139,7 @@ def test_example_docker_argv_resource_values_come_from_the_policy(): argv, _ = _docker_cmd(policy, lang="bash") assert argv == [ "docker", "run", "--rm", + "--security-opt", "no-new-privileges", "--cap-drop", "ALL", "--network", "none", "--cpus", "2.5", "--memory", "1024m", @@ -156,9 +158,10 @@ def test_example_docker_argv_never_sets_network_from_the_policy(): assert argv[argv.index("--network") + 1] == "none" -def test_example_docker_argv_has_no_init_no_cap_drop_no_new_privileges(): - # Recorded gap: this site carries none of the repos/container.py hardening today. - # Adding it is an argv change and is a separate follow-up, so this test pins the absence. - argv, _ = _docker_cmd(ExecutionPolicy(backend="docker", timeout=5)) - for flag in ("--init", "--cap-drop", "no-new-privileges"): - assert flag not in argv +@pytest.mark.parametrize("lang", sorted(EXECUTORS)) +def test_example_docker_argv_drops_every_capability_and_new_privileges(lang): + # The snippet runs as the image's user, root in the default image: it keeps no capability + # and a setuid binary gives it none, as in repos/container.py. + argv, _ = _docker_cmd(ExecutionPolicy(backend="docker", timeout=5), lang=lang) + assert argv[argv.index("--cap-drop") + 1] == "ALL" + assert argv[argv.index("--security-opt") + 1] == "no-new-privileges" diff --git a/dokimi/tests/test_example_runner.py b/dokimi/tests/test_example_runner.py index 336577398..316b04abe 100644 --- a/dokimi/tests/test_example_runner.py +++ b/dokimi/tests/test_example_runner.py @@ -5,7 +5,9 @@ from __future__ import annotations +import os import tempfile +from pathlib import Path import pytest @@ -209,6 +211,27 @@ def test_docker_backend_locks_the_container_down(): assert argv[argv.index("timeout") + 1] == "7" and call.timeout == 37 +@pytest.mark.skipif(os.name == "nt", reason="POSIX permission bits: Windows chmod sets only read-only") +def test_docker_backend_mount_is_readable_by_a_root_without_capabilities(): + # The container's root has no CAP_DAC_OVERRIDE, and the mount belongs to the host user: + # it can enter the directory and read the snippet only through the "other" permission bits. + seen = {} + + def _modes(call): + work = Path(next(a for a in call.argv if a.endswith(":/work:ro")).removesuffix(":/work:ro")) + seen["dir"] = work.stat().st_mode & 0o777 + seen["script"] = (work / "snippet.py").stat().st_mode & 0o777 + return reply() + + old = os.umask(0o077) + try: + res = run_example("python", "print(1)", ExecutionPolicy(backend="docker"), FakeRunner(_modes)) + finally: + os.umask(old) + assert res.status == "passed" + assert seen == {"dir": 0o755, "script": 0o644} + + def test_docker_backend_reports_a_timeout(): """A docker run killed at the deadline is a timeout, not exit 124 from the snippet.""" runner = FakeRunner(lambda call: reply(timed_out=True))