diff --git a/decnet/swarm/updater_client.py b/decnet/swarm/updater_client.py index 753c5582..01223d47 100644 --- a/decnet/swarm/updater_client.py +++ b/decnet/swarm/updater_client.py @@ -12,6 +12,7 @@ the connection on purpose (the updater re-execs itself mid-response). """ from __future__ import annotations +import hashlib import ssl from typing import Any, Optional @@ -36,7 +37,12 @@ class UpdaterClient: address: Optional[str] = None, updater_port: int = 8766, identity: Optional[MasterIdentity] = None, + verify_hostname: Optional[bool] = None, ): + if verify_hostname is None: + from decnet.env import DECNET_VERIFY_HOSTNAME + verify_hostname = DECNET_VERIFY_HOSTNAME + self._verify_hostname = verify_hostname if host is not None: self._address = host["address"] self._host_name = host.get("name") @@ -56,7 +62,7 @@ class UpdaterClient: ) ctx.load_verify_locations(cafile=str(self._identity.ca_cert_path)) ctx.verify_mode = ssl.CERT_REQUIRED - ctx.check_hostname = False + ctx.check_hostname = self._verify_hostname return httpx.AsyncClient( base_url=f"https://{self._address}:{self._port}", verify=ctx, @@ -93,12 +99,13 @@ class UpdaterClient: """POST /update. Returns the Response so the caller can distinguish 200 / 409 / 500 — each means something different. """ + sha256 = hashlib.sha256(tarball).hexdigest() self._require().timeout = _TIMEOUT_UPDATE try: r = await self._require().post( "/update", files={"tarball": ("tree.tgz", tarball, "application/gzip")}, - data={"sha": sha}, + data={"sha": sha, "sha256": sha256}, ) finally: self._require().timeout = _TIMEOUT_CONTROL @@ -109,12 +116,13 @@ class UpdaterClient: usually drops mid-response; that's not an error. Callers should then poll /health until the new SHA appears. """ + sha256 = hashlib.sha256(tarball).hexdigest() self._require().timeout = _TIMEOUT_UPDATE try: r = await self._require().post( "/update-self", files={"tarball": ("tree.tgz", tarball, "application/gzip")}, - data={"sha": sha, "confirm_self": "true"}, + data={"sha": sha, "sha256": sha256, "confirm_self": "true"}, ) finally: self._require().timeout = _TIMEOUT_CONTROL diff --git a/decnet/updater/app.py b/decnet/updater/app.py index 91cbd0e2..3f3b437c 100644 --- a/decnet/updater/app.py +++ b/decnet/updater/app.py @@ -129,12 +129,14 @@ async def releases() -> dict: async def update( tarball: UploadFile = File(..., description="tar.gz of the working tree"), sha: str = Form("", description="git SHA of the tree for provenance"), + sha256: str = Form("", description="hex SHA-256 of the tarball bytes; verified before extract"), ) -> dict: body = await tarball.read() try: return _exec.run_update( body, sha=sha or None, install_dir=_Config.install_dir, agent_dir=_Config.agent_dir, + expected_sha256=sha256 or None, ) except _exec.UpdateError as exc: status = 409 if exc.rolled_back else 500 @@ -148,6 +150,7 @@ async def update( async def update_self( tarball: UploadFile = File(...), sha: str = Form(""), + sha256: str = Form("", description="hex SHA-256 of the tarball bytes; verified before extract"), confirm_self: str = Form("", description="Must be 'true' to proceed"), ) -> dict: if confirm_self.lower() != "true": @@ -160,6 +163,7 @@ async def update_self( return _exec.run_update_self( body, sha=sha or None, updater_install_dir=_Config.updater_install_dir, + expected_sha256=sha256 or None, ) except _exec.UpdateError as exc: raise HTTPException( diff --git a/decnet/updater/executor.py b/decnet/updater/executor.py index a618f4a8..1e62bd88 100644 --- a/decnet/updater/executor.py +++ b/decnet/updater/executor.py @@ -21,6 +21,7 @@ without actually touching the filesystem's Python toolchain. from __future__ import annotations import dataclasses +import hashlib import os import pathlib import shutil @@ -46,6 +47,15 @@ AGENT_PROBE_ATTEMPTS = 10 AGENT_PROBE_BACKOFF_S = 1.0 AGENT_RESTART_GRACE_S = 10.0 +# Hard cap on the post-decompression size of an update tarball. The DECNET +# source tree is on the order of single-digit MiB; 256 MiB is far above +# that but small enough to bound damage from a gzip bomb. mTLS already +# authenticates the master, but the worker still treats the bytes as +# untrusted because (a) a compromised master shouldn't get arbitrary RAM +# exhaustion on every agent, (b) bugs in the master tarball builder +# shouldn't brick agents. +MAX_TARBALL_UNCOMPRESSED_BYTES = 256 * 1024 * 1024 + # ------------------------------------------------------------------- errors @@ -188,17 +198,53 @@ def clean_stale_staging(install_dir: pathlib.Path) -> None: def extract_tarball(tarball_bytes: bytes, dest: pathlib.Path) -> None: """Extract a gzipped tarball into ``dest`` (must not pre-exist). - Rejects absolute paths and ``..`` traversal in the archive. + Hardening on top of stdlib ``tarfile``: + + * Rejects absolute paths and ``..`` traversal in member names. + * Rejects anything that isn't a regular file or directory — no symlinks, + hardlinks, devices, or FIFOs. A symlink pointing outside ``dest`` + would let later writes (pip install, etc.) escape the staging tree. + * Validates that each member's *resolved* destination path stays under + ``dest`` after symlink resolution, in case a parent directory in the + tarball is itself a symlink we missed. + * Caps total uncompressed size to bound gzip-bomb damage. + * Strips suid/sgid bits from extracted file modes. """ import io dest.mkdir(parents=True, exist_ok=False) + dest_resolved = dest.resolve() + total_size = 0 with tarfile.open(fileobj=io.BytesIO(tarball_bytes), mode="r:gz") as tar: - for member in tar.getmembers(): + members = tar.getmembers() + for member in members: name = member.name if name.startswith("/") or ".." in pathlib.PurePosixPath(name).parts: raise UpdateError(f"unsafe path in tarball: {name!r}") - tar.extractall(dest) # nosec B202 — validated above + if not (member.isfile() or member.isdir()): + raise UpdateError( + f"unsupported tar entry {name!r}: type={member.type!r}; " + "only regular files and directories are allowed" + ) + total_size += max(member.size, 0) + if total_size > MAX_TARBALL_UNCOMPRESSED_BYTES: + raise UpdateError( + f"tarball exceeds size cap " + f"({total_size} > {MAX_TARBALL_UNCOMPRESSED_BYTES} bytes)" + ) + # Strip setuid/setgid/sticky bits — extracted files should never + # acquire elevated mode, even if the master built the tarball + # against a tree that has them by accident. + member.mode = member.mode & 0o777 & ~0o7000 + for member in members: + target = (dest / member.name).resolve() + try: + target.relative_to(dest_resolved) + except ValueError: + raise UpdateError( + f"resolved path escapes dest: {member.name!r} -> {target}" + ) from None + tar.extractall(dest) # nosec B202 — every member validated above # ---------------------------------------------------------------- seams @@ -529,14 +575,39 @@ def _point_current_at(install_dir: pathlib.Path, target: pathlib.Path) -> None: os.replace(tmp, link) +def _verify_tarball_sha256(tarball_bytes: bytes, expected_sha256: Optional[str]) -> None: + """Refuse to extract a tarball whose SHA-256 disagrees with the operator-supplied digest. + + mTLS already authenticates the master, so a network MITM can't forge + bytes. This check exists for two narrower cases: catching corruption + in transit (proxies, broken disks) before we explode a half-decoded + archive into the staging tree, and giving the operator a way to pin + "exactly these bytes" when distributing a vetted release. The form + field is optional — if the caller doesn't send one, we skip the + check rather than reject (no breaking change for older masters). + """ + if not expected_sha256: + return + expected = expected_sha256.strip().lower() + if len(expected) != 64 or any(c not in "0123456789abcdef" for c in expected): + raise UpdateError(f"sha256 digest is not a 64-char hex string: {expected_sha256!r}") + actual = hashlib.sha256(tarball_bytes).hexdigest() + if actual != expected: + raise UpdateError( + f"tarball sha256 mismatch (expected={expected[:16]}…, got={actual[:16]}…)" + ) + + def run_update( tarball_bytes: bytes, sha: Optional[str], install_dir: pathlib.Path = DEFAULT_INSTALL_DIR, agent_dir: pathlib.Path = pki.DEFAULT_AGENT_DIR, + expected_sha256: Optional[str] = None, ) -> dict[str, Any]: """Apply an update atomically. Rolls back on probe failure.""" log.info("update received sha=%s bytes=%d install_dir=%s", sha, len(tarball_bytes), install_dir) + _verify_tarball_sha256(tarball_bytes, expected_sha256) clean_stale_staging(install_dir) staging = _staging_dir(install_dir) @@ -628,6 +699,7 @@ def run_update_self( sha: Optional[str], updater_install_dir: pathlib.Path, exec_cb: Optional[Callable[[list[str]], None]] = None, + expected_sha256: Optional[str] = None, ) -> dict[str, Any]: """Replace the updater's own source tree, then re-exec this process. @@ -635,6 +707,7 @@ def run_update_self( returns new SHA within 30s" as success. """ log.info("self-update received sha=%s bytes=%d install_dir=%s", sha, len(tarball_bytes), updater_install_dir) + _verify_tarball_sha256(tarball_bytes, expected_sha256) clean_stale_staging(updater_install_dir) staging = _staging_dir(updater_install_dir) log.info("extracting tarball -> %s", staging) diff --git a/tests/updater/test_updater_app.py b/tests/updater/test_updater_app.py index 12b0c83e..15382dda 100644 --- a/tests/updater/test_updater_app.py +++ b/tests/updater/test_updater_app.py @@ -51,7 +51,7 @@ def test_health_returns_role_and_releases(client: TestClient, monkeypatch: pytes def test_update_happy_path(client: TestClient, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( ex, "run_update", - lambda data, sha, install_dir, agent_dir: {"status": "updated", "release": {"slot": "active", "sha": sha}, "probe": "ok"}, + lambda data, sha, install_dir, agent_dir, expected_sha256=None: {"status": "updated", "release": {"slot": "active", "sha": sha}, "probe": "ok"}, ) r = client.post( "/update", @@ -97,7 +97,7 @@ def test_update_self_requires_confirm(client: TestClient) -> None: def test_update_self_happy_path(client: TestClient, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( ex, "run_update_self", - lambda data, sha, updater_install_dir: {"status": "self_update_queued", "argv": ["python", "-m", "decnet", "updater"]}, + lambda data, sha, updater_install_dir, expected_sha256=None: {"status": "self_update_queued", "argv": ["python", "-m", "decnet", "updater"]}, ) r = client.post( "/update-self", diff --git a/tests/updater/test_updater_executor.py b/tests/updater/test_updater_executor.py index 7be0f85a..1766b562 100644 --- a/tests/updater/test_updater_executor.py +++ b/tests/updater/test_updater_executor.py @@ -96,6 +96,116 @@ def test_extract_happy_path(tmp_path: pathlib.Path) -> None: assert (out / "a" / "b.txt").read_text() == "hello" +def _tarball_with_link(linkname: str, target: str, *, hard: bool = False) -> bytes: + buf = io.BytesIO() + with tarfile.open(fileobj=buf, mode="w:gz") as tar: + info = tarfile.TarInfo(name=linkname) + info.type = tarfile.LNKTYPE if hard else tarfile.SYMTYPE + info.linkname = target + tar.addfile(info) + return buf.getvalue() + + +def test_extract_rejects_symlinks(tmp_path: pathlib.Path) -> None: + evil = _tarball_with_link("link.txt", "/etc/passwd") + with pytest.raises(ex.UpdateError, match="only regular files"): + ex.extract_tarball(evil, tmp_path / "out") + + +def test_extract_rejects_hardlinks(tmp_path: pathlib.Path) -> None: + evil = _tarball_with_link("link.txt", "real.txt", hard=True) + with pytest.raises(ex.UpdateError, match="only regular files"): + ex.extract_tarball(evil, tmp_path / "out") + + +def test_extract_rejects_device_nodes(tmp_path: pathlib.Path) -> None: + buf = io.BytesIO() + with tarfile.open(fileobj=buf, mode="w:gz") as tar: + info = tarfile.TarInfo(name="dev_null") + info.type = tarfile.CHRTYPE + info.devmajor = 1 + info.devminor = 3 + tar.addfile(info) + with pytest.raises(ex.UpdateError, match="only regular files"): + ex.extract_tarball(buf.getvalue(), tmp_path / "out") + + +def test_extract_rejects_oversized_tarball( + tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch, +) -> None: + # Lower the cap rather than building a 256 MiB tarball in memory. + monkeypatch.setattr(ex, "MAX_TARBALL_UNCOMPRESSED_BYTES", 32) + big = _make_tarball({"big.txt": "x" * 64}) + with pytest.raises(ex.UpdateError, match="exceeds size cap"): + ex.extract_tarball(big, tmp_path / "out") + + +def test_extract_strips_setuid_bit(tmp_path: pathlib.Path) -> None: + buf = io.BytesIO() + payload = b"hello" + with tarfile.open(fileobj=buf, mode="w:gz") as tar: + info = tarfile.TarInfo(name="suid.bin") + info.size = len(payload) + info.mode = 0o4755 # setuid + rwxr-xr-x + tar.addfile(info, io.BytesIO(payload)) + out = tmp_path / "out" + ex.extract_tarball(buf.getvalue(), out) + mode = (out / "suid.bin").stat().st_mode & 0o7777 + assert mode & 0o4000 == 0, f"setuid bit should be stripped, got {oct(mode)}" + + +# ----------------------------------------------------------- sha256 verify + +def test_run_update_rejects_sha256_mismatch( + monkeypatch: pytest.MonkeyPatch, + install_dir: pathlib.Path, + agent_dir: pathlib.Path, +) -> None: + monkeypatch.setattr(ex, "_run_pip", lambda release: _PipOK()) + monkeypatch.setattr(ex, "_stop_agent", lambda *a, **k: None) + monkeypatch.setattr(ex, "_spawn_agent", lambda *a, **k: 1) + monkeypatch.setattr(ex, "_probe_agent", lambda **_: (True, "ok")) + tb = _make_tarball({"marker.txt": "new"}) + bad = "0" * 64 + with pytest.raises(ex.UpdateError, match="sha256 mismatch"): + ex.run_update( + tb, sha="S", install_dir=install_dir, agent_dir=agent_dir, + expected_sha256=bad, + ) + # Mismatch must abort before staging is left around. + assert not (install_dir / "releases" / "active.new").exists() + + +def test_run_update_accepts_correct_sha256( + monkeypatch: pytest.MonkeyPatch, + install_dir: pathlib.Path, + agent_dir: pathlib.Path, +) -> None: + import hashlib as _hl + monkeypatch.setattr(ex, "_run_pip", lambda release: _PipOK()) + monkeypatch.setattr(ex, "_stop_agent", lambda *a, **k: None) + monkeypatch.setattr(ex, "_spawn_agent", lambda *a, **k: 1) + monkeypatch.setattr(ex, "_probe_agent", lambda **_: (True, "ok")) + tb = _make_tarball({"marker.txt": "new"}) + digest = _hl.sha256(tb).hexdigest() + result = ex.run_update( + tb, sha="S", install_dir=install_dir, agent_dir=agent_dir, + expected_sha256=digest, + ) + assert result["status"] == "updated" + + +def test_run_update_rejects_malformed_sha256( + install_dir: pathlib.Path, agent_dir: pathlib.Path, +) -> None: + tb = _make_tarball({"x.txt": "y"}) + with pytest.raises(ex.UpdateError, match="not a 64-char hex"): + ex.run_update( + tb, sha="S", install_dir=install_dir, agent_dir=agent_dir, + expected_sha256="not-a-hex-digest", + ) + + def test_clean_stale_staging(install_dir: pathlib.Path) -> None: staging = install_dir / "releases" / "active.new" staging.mkdir()