Create a Tor watchguard and make iptables non mandatory (#1152)

Fix https://github.com/calibrain/shelfmark/issues/1150
2026-09-24 13:50:22 +01:00 · 2026-07-30 23:41:55 -04:00
parent ce026e2eb8
commit 10554444a9
2 changed files with 393 additions and 41 deletions
@@ -1,9 +1,20 @@
 from __future__ import annotations

+import os
+import shutil
+import signal
+import socket
+import subprocess
+from contextlib import suppress
 from pathlib import Path

+import pytest
+
 TOR_SCRIPT_PATH = Path(__file__).resolve().parents[2] / "tor.sh"

+IPTABLES_BLOCK_START = 'echo "[*] Setting up iptables rules..."'
+IPTABLES_BLOCK_END = 'echo "[✓] Transparent Tor routing enabled."'
+

 def _generated_tor_healthcheck_script() -> str:
    script = TOR_SCRIPT_PATH.read_text()
@@ -14,17 +25,208 @@ def _generated_tor_healthcheck_script() -> str:


 def _tor_script_rule_lines() -> list[str]:
-    return [
-        line.strip()
-        for line in TOR_SCRIPT_PATH.read_text().splitlines()
-        if line.strip().startswith("iptables ")
-    ]
+    """Return the iptables invocations in tor.sh, in file order.
+
+    Rules that are applied on a best-effort basis are written as
+    ``if iptables ...; then``, so the shell wrapper is stripped to keep the
+    ordering assertions below comparing rules rather than syntax.
+    """
+    lines = []
+    for raw_line in TOR_SCRIPT_PATH.read_text().splitlines():
+        line = raw_line.strip().removeprefix("if ")
+        if line.startswith("iptables "):
+            lines.append(line)
+    return lines


 def _line_index(lines: list[str], needle: str) -> int:
    return next(index for index, line in enumerate(lines) if needle in line)


+def _tor_iptables_block() -> str:
+    """Extract the firewall setup section of tor.sh so it can be executed."""
+    script = TOR_SCRIPT_PATH.read_text()
+    start = script.index(IPTABLES_BLOCK_START)
+    end = script.index(IPTABLES_BLOCK_END)
+    return script[start:end]
+
+
+# Verbatim stderr of an iptables build whose kernel lacks the xt_owner module,
+# as reported from Synology DSM in issue #1150.
+MISSING_XT_OWNER_STDERR = (
+    "Warning: Extension owner revision 0 not supported, missing kernel module?\n"
+    "iptables: No chain/target/match by that name.\n"
+)
+
+IPTABLES_STUB = f"""#!/bin/bash
+if [ "$FAKE_OWNER_MATCH_SUPPORTED" != "1" ]; then
+    case "$*" in
+        *--uid-owner*)
+            printf '%s' {MISSING_XT_OWNER_STDERR!r} >&2
+            exit 1
+            ;;
+    esac
+fi
+echo "$*" >> "$FAKE_IPTABLES_LOG"
+exit 0
+"""
+
+# tor.sh resolves the Tor uid with `id -u debian-tor`; that account only exists
+# inside the image, so stub it out with the uid the Debian tor package uses.
+ID_STUB = """#!/bin/bash
+echo 107
+"""
+
+
+def _run_tor_iptables_block(tmp_path: Path, *, owner_match_supported: bool):
+    """Run tor.sh's firewall block against a fake iptables.
+
+    Returns the completed process plus the rules the fake iptables accepted.
+    """
+    bin_dir = tmp_path / "bin"
+    bin_dir.mkdir()
+    for name, source in (("iptables", IPTABLES_STUB), ("id", ID_STUB)):
+        stub = bin_dir / name
+        stub.write_text(source)
+        stub.chmod(0o755)
+
+    applied_rules_log = tmp_path / "applied-rules.log"
+    # tor.sh runs under `set -e`, which is what turns a rejected rule into a
+    # container restart loop, so reproduce that here.
+    result = subprocess.run(
+        ["bash", "-c", "set -e\n" + _tor_iptables_block()],
+        capture_output=True,
+        text=True,
+        env={
+            "PATH": f"{bin_dir}:/usr/bin:/bin",
+            "FAKE_IPTABLES_LOG": str(applied_rules_log),
+            "FAKE_OWNER_MATCH_SUPPORTED": "1" if owner_match_supported else "0",
+        },
+    )
+    applied_rules = applied_rules_log.read_text().splitlines() if applied_rules_log.exists() else []
+    return result, applied_rules
+
+
+requires_bash = pytest.mark.skipif(
+    shutil.which("bash") is None, reason="bash is required to execute tor.sh"
+)
+
+# The watchdog shells out to `timeout`, which is not present on stock macOS.
+requires_timeout = pytest.mark.skipif(
+    shutil.which("timeout") is None, reason="coreutils timeout is required"
+)
+
+SUPERVISORCTL_STUB = """#!/bin/bash
+# Emulates `supervisorctl status tor`. Normally reports FAKE_TOR_STATE, but when
+# FAKE_TOR_FAIL_FIRST is set the first call reports a transient failure so the
+# watchdog's retry behaviour can be exercised.
+if [ -n "$FAKE_TOR_FAIL_FIRST" ] && [ ! -f "$FAKE_TOR_CALL_MARKER" ]; then
+    touch "$FAKE_TOR_CALL_MARKER"
+    echo "tor    FATAL     Exited too quickly"
+    exit 3
+fi
+echo "tor    $FAKE_TOR_STATE   pid 42, uptime 0:01:00"
+"""
+
+
+def _watchdog_path_env(bin_dir: Path) -> str:
+    """PATH exposing the stubs plus the real bash/timeout the watchdog calls."""
+    parts = [str(bin_dir)]
+    for tool in ("bash", "timeout"):
+        resolved = shutil.which(tool)
+        if resolved:
+            parts.append(str(Path(resolved).parent))
+    parts += ["/usr/bin", "/bin"]
+    return ":".join(parts)
+
+
+def _run_tor_watchdog(
+    tmp_path: Path,
+    *,
+    tor_state: str = "RUNNING",
+    bootstrapped: bool = True,
+    trans_port_open: bool = True,
+    fail_first: bool = False,
+    run_for: float = 2.0,
+):
+    """Run tor.sh's watchdog against a fake Tor and see if it resets the container.
+
+    Returns ``(reset_signal, stdout)`` where ``reset_signal`` is the signal the
+    stand-in for PID 1 received, or ``None`` if it was left alone.
+    """
+    bin_dir = tmp_path / "bin"
+    bin_dir.mkdir()
+    supervisorctl = bin_dir / "supervisorctl"
+    supervisorctl.write_text(SUPERVISORCTL_STUB)
+    supervisorctl.chmod(0o755)
+
+    notices_log = tmp_path / "notices.log"
+    notices_log.write_text(
+        "Bootstrapped 100%: Done\n" if bootstrapped else "Bootstrapped 45%: Loading\n"
+    )
+
+    script = tmp_path / "tor_healthcheck.sh"
+    script.write_text(_generated_tor_healthcheck_script())
+    script.chmod(0o755)
+
+    listener = socket.socket()
+    listener.bind(("127.0.0.1", 0))
+    trans_port = listener.getsockname()[1]
+    if trans_port_open:
+        listener.listen(5)
+    else:
+        # Closing it leaves a port nothing is listening on, which is what a dead
+        # or wedged Tor looks like from the outside.
+        listener.close()
+
+    # Stands in for PID 1: the watchdog signals it instead of dumb-init.
+    container_process = subprocess.Popen(
+        ["sleep", "120"], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL
+    )
+
+    env = {
+        "PATH": _watchdog_path_env(bin_dir),
+        "TOR_CHECK_INTERVAL": "0.2",
+        "TOR_NOTICES_LOG": str(notices_log),
+        "TOR_TRANS_PORT": str(trans_port),
+        "TOR_CONTAINER_PID": str(container_process.pid),
+        "FAKE_TOR_STATE": tor_state,
+        "FAKE_TOR_CALL_MARKER": str(tmp_path / "supervisorctl.called"),
+    }
+    if fail_first:
+        env["FAKE_TOR_FAIL_FIRST"] = "1"
+
+    watchdog = subprocess.Popen(
+        ["bash", str(script)],
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+        text=True,
+        env=env,
+        start_new_session=True,
+    )
+    try:
+        try:
+            container_process.wait(timeout=run_for)
+        except subprocess.TimeoutExpired:
+            pass
+        reset_signal = (
+            -container_process.returncode if container_process.poll() is not None else None
+        )
+    finally:
+        # Only the watchdog gets its own session; signalling the group is how we
+        # reap the `sleep` and `timeout` children it spawns.
+        with suppress(ProcessLookupError, PermissionError):
+            os.killpg(os.getpgid(watchdog.pid), signal.SIGKILL)
+        with suppress(ProcessLookupError):
+            container_process.kill()
+        with suppress(OSError):
+            listener.close()
+        container_process.wait()
+        stdout = watchdog.communicate()[0] or ""
+
+    return reset_signal, stdout
+
+
 def test_tor_nat_rules_bypass_private_networks_before_tcp_redirect():
    lines = _tor_script_rule_lines()
    tcp_redirect_index = _line_index(lines, "--syn -j REDIRECT --to-ports 9040")
@@ -63,3 +265,119 @@ def test_tor_healthcheck_uses_local_tor_state_without_clear_net_probe():
    assert "curl " not in healthcheck_script
    assert "supervisorctl status tor" in healthcheck_script
    assert "Bootstrapped 100%" in healthcheck_script
+
+
+@requires_bash
+def test_tor_iptables_setup_survives_kernel_without_owner_match(tmp_path):
+    """Regression test for issue #1150.
+
+    Synology (and other NAS/embedded) kernels ship without xt_owner. The owner
+    exemption is a best-effort optimisation, so a kernel that rejects it must
+    not abort tor.sh and put the container into a restart loop.
+    """
+    result, _ = _run_tor_iptables_block(tmp_path, owner_match_supported=False)
+
+    assert result.returncode == 0, (
+        "tor.sh aborted on a kernel without xt_owner:\n"
+        f"stdout:\n{result.stdout}\nstderr:\n{result.stderr}"
+    )
+
+
+@requires_bash
+def test_tor_routing_still_applied_when_owner_match_unsupported(tmp_path):
+    """Degrading past the owner rule must not skip the rules that torify traffic."""
+    _, applied_rules = _run_tor_iptables_block(tmp_path, owner_match_supported=False)
+
+    assert any("--syn -j REDIRECT --to-ports 9040" in rule for rule in applied_rules)
+    for cidr in ("127.0.0.0/8", "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16"):
+        assert any(f"-d {cidr} -j RETURN" in rule for rule in applied_rules)
+    for protocol in ("udp", "tcp"):
+        assert any(f"-p {protocol} --dport 53" in rule for rule in applied_rules)
+
+
+@requires_bash
+def test_tor_warns_when_owner_match_unsupported(tmp_path):
+    """The degraded path has to be visible in the logs, not silent."""
+    result, _ = _run_tor_iptables_block(tmp_path, owner_match_supported=False)
+
+    assert "owner" in result.stdout.lower()
+    assert "[!]" in result.stdout
+
+
+@requires_bash
+def test_tor_exempts_own_traffic_when_owner_match_supported(tmp_path):
+    """On a normal kernel the exemption must still be installed."""
+    result, applied_rules = _run_tor_iptables_block(tmp_path, owner_match_supported=True)
+
+    assert result.returncode == 0, result.stderr
+    owner_rules = [rule for rule in applied_rules if "--uid-owner" in rule]
+    assert len(owner_rules) == 1
+    assert "-j RETURN" in owner_rules[0]
+
+    owner_index = applied_rules.index(owner_rules[0])
+    redirect_index = next(
+        index for index, rule in enumerate(applied_rules) if "--syn -j REDIRECT" in rule
+    )
+    assert owner_index < redirect_index
+
+
+@requires_bash
+@requires_timeout
+def test_tor_watchdog_resets_container_when_tor_dies(tmp_path):
+    """A Tor that is no longer running must take the container down with it."""
+    reset_signal, stdout = _run_tor_watchdog(tmp_path, tor_state="STOPPED")
+
+    assert reset_signal == signal.SIGTERM, f"container was not reset:\n{stdout}"
+
+
+@requires_bash
+@requires_timeout
+def test_tor_watchdog_resets_container_when_tor_stops_accepting_connections(tmp_path):
+    """Tor can be 'running' yet unusable; the container must still be reset."""
+    reset_signal, stdout = _run_tor_watchdog(tmp_path, tor_state="RUNNING", trans_port_open=False)
+
+    assert reset_signal == signal.SIGTERM, f"container was not reset:\n{stdout}"
+
+
+@requires_bash
+@requires_timeout
+def test_tor_watchdog_leaves_healthy_tor_alone(tmp_path):
+    """A healthy Tor must never be reset, no matter how many cycles run."""
+    reset_signal, stdout = _run_tor_watchdog(tmp_path)
+
+    assert reset_signal is None, f"healthy Tor was reset:\n{stdout}"
+
+
+@requires_bash
+@requires_timeout
+def test_tor_watchdog_retries_once_before_resetting(tmp_path):
+    """A single transient failure is retried, not escalated to a reset."""
+    reset_signal, stdout = _run_tor_watchdog(tmp_path, fail_first=True)
+
+    assert reset_signal is None, f"transient failure caused a reset:\n{stdout}"
+    assert "Healthcheck failed (1/2)" in stdout
+    assert "Tor recovered." in stdout
+
+
+@requires_bash
+@requires_timeout
+def test_tor_watchdog_waits_for_first_bootstrap_before_policing(tmp_path):
+    """A slow first bootstrap must not be mistaken for a failure.
+
+    Otherwise every user on a slow link would be reset into a restart loop
+    before Tor ever had a chance to come up.
+    """
+    reset_signal, stdout = _run_tor_watchdog(
+        tmp_path, tor_state="STOPPED", bootstrapped=False, trans_port_open=False
+    )
+
+    assert reset_signal is None, f"reset during initial bootstrap:\n{stdout}"
+    assert "Waiting for initial Tor bootstrap" in stdout
+
+
+def test_tor_watchdog_escalates_to_container_reset_not_in_place_restart():
+    """The watchdog must signal PID 1 rather than bounce Tor under supervisor."""
+    healthcheck_script = _generated_tor_healthcheck_script()
+
+    assert "kill -TERM" in healthcheck_script
+    assert "supervisorctl restart" not in healthcheck_script
@@ -129,57 +129,76 @@ EOF
 cat <<'HC' > /app/tor_healthcheck.sh
 #!/bin/bash

-# Function to dynamically wait for Tor bootstrap
-wait_for_tor() {
-    echo "$(date): Waiting for Tor to finish bootstrapping..."
+# Tor watchdog.
+#
+# Once the NAT rules are live every process in the container is pinned to Tor,
+# so a Tor that has died or wedged takes the whole app down with it and cannot
+# reliably be repaired in place. When Tor stops being usable, tear the container
+# down instead: the configured Docker restart policy brings it back and tor.sh
+# rebuilds torrc, supervisor and the iptables rules from scratch.

-    > /var/log/tor/notices.log 2>/dev/null || true
+CHECK_INTERVAL=${TOR_CHECK_INTERVAL:-30}
+# Strikes before the container is reset. 2 means "one retry": a single failed
+# probe is tolerated, a second consecutive failure resets.
+MAX_FAILURES=${TOR_MAX_FAILURES:-2}
+NOTICES_LOG=${TOR_NOTICES_LOG:-/var/log/tor/notices.log}
+TRANS_PORT=${TOR_TRANS_PORT:-9040}
+# PID 1 is dumb-init, which tears the container down when it is signalled.
+CONTAINER_PID=${TOR_CONTAINER_PID:-1}

-    sleep 10
+tor_process_running() {
+    supervisorctl status tor 2>/dev/null | grep -q "RUNNING"
+}

-    TIMEOUT=300
-    ELAPSED=0
-    while [ $ELAPSED -lt $TIMEOUT ]; do
-        if grep -q "Bootstrapped 100%" /var/log/tor/notices.log 2>/dev/null; then
-            echo "$(date): Tor bootstrap complete."
-            return 0
-        fi
-        sleep 5
-        ELAPSED=$((ELAPSED + 5))
-        # Show progress
-        CURRENT=$(tail -n 1 /var/log/tor/notices.log 2>/dev/null | grep -oP 'Bootstrapped \d+%' || echo "waiting...")
-        echo "$(date): Bootstrap progress: $CURRENT ($ELAPSED/${TIMEOUT}s)"
-    done
+tor_has_bootstrapped() {
+    grep -q "Bootstrapped 100%" "$NOTICES_LOG" 2>/dev/null
+}

-    echo "$(date): WARNING - Tor bootstrap timed out after ${TIMEOUT}s"
-    return 1
+# Liveness probe for "Tor is still running but no longer usable". Deliberately a
+# loopback check against Tor's own TransPort: probing the clear net from here
+# would leak and defeat the point of the proxy.
+tor_accepts_connections() {
+    timeout 5 bash -c "exec 3<>/dev/tcp/127.0.0.1/${TRANS_PORT}" 2>/dev/null
 }

 tor_is_healthy() {
-    supervisorctl status tor | grep -q "RUNNING" &&
-        grep -q "Bootstrapped 100%" /var/log/tor/notices.log 2>/dev/null
+    tor_process_running && tor_has_bootstrapped && tor_accepts_connections
 }

+reset_container() {
+    echo "$(date): Tor still unhealthy after ${MAX_FAILURES} checks - resetting container."
+    kill -TERM "$CONTAINER_PID" 2>/dev/null
+    exit 1
+}
+
+# Never police Tor before it has bootstrapped once. A first bootstrap can
+# legitimately take minutes on a slow link, and mistaking that for a failure
+# would reset the container into an endless restart loop.
+echo "$(date): Waiting for initial Tor bootstrap before monitoring..."
+while ! tor_has_bootstrapped; do
+    sleep "$CHECK_INTERVAL"
+done
+echo "$(date): Tor bootstrapped - watchdog active (every ${CHECK_INTERVAL}s, ${MAX_FAILURES} strikes)."
+
 FAIL_COUNT=0
 while true; do
    if tor_is_healthy; then
+        if [ "$FAIL_COUNT" -ne 0 ]; then
+            echo "$(date): Tor recovered."
+        fi
        FAIL_COUNT=0
    else
        FAIL_COUNT=$((FAIL_COUNT+1))
-        echo "$(date): Healthcheck failed (Count: $FAIL_COUNT)"
+        echo "$(date): Healthcheck failed (${FAIL_COUNT}/${MAX_FAILURES})"
+
+        if [ "$FAIL_COUNT" -ge "$MAX_FAILURES" ]; then
+            reset_container
+        fi
+
+        echo "$(date): Retrying in ${CHECK_INTERVAL}s before resetting..."
    fi

-    # If failed 3 times in a row, restart Tor
-    if [ "$FAIL_COUNT" -ge 3 ]; then
-        echo "$(date): restart trigger - Restarting Tor..."
-        supervisorctl restart tor
-        FAIL_COUNT=0
-
-        # Wait for it to come back using the dynamic check
-        wait_for_tor
-    fi
-
-    sleep 30
+    sleep "$CHECK_INTERVAL"
 done
 HC
 chmod +x /app/tor_healthcheck.sh
@@ -221,8 +240,23 @@ TOR_UID=$(id -u debian-tor)
 # Allow loopback
 iptables -t nat -A OUTPUT -o lo -j RETURN

-# Allow Tor itself to reach the network
-iptables -t nat -A OUTPUT -m owner --uid-owner "$TOR_UID" -j RETURN
+# Allow Tor itself to reach the network.
+#
+# The owner match needs the xt_owner kernel module, which some NAS and embedded
+# kernels (Synology DSM in particular) do not ship. There iptables rejects the
+# rule with "Extension owner revision 0 not supported, missing kernel module?",
+# and under `set -e` that aborted the whole script before any routing rule was
+# installed, leaving the container in a restart loop. Treat the exemption as
+# best-effort so those hosts keep working as they did before it was introduced.
+if iptables -t nat -A OUTPUT -m owner --uid-owner "$TOR_UID" -j RETURN 2>/dev/null; then
+    echo "[✓] Tor process (uid $TOR_UID) exempted from transparent redirect."
+else
+    echo "[!] Warning: this kernel has no iptables owner match (xt_owner module)."
+    echo "[!] Continuing without the Tor process exemption. Tor keeps using the"
+    echo "[!] connections it opened while bootstrapping, so traffic is still"
+    echo "[!] routed through Tor, but if Tor loses its guard relays it may need"
+    echo "[!] a container restart to recover."
+fi

 # For UDP DNS queries
 iptables -t nat -A OUTPUT -p udp --dport 53 ! -d 127.0.0.1 -j DNAT --to-destination 127.0.0.1:53