sec(updater): harden tarball extraction and verify sha256 before extract
Reject symlinks, hardlinks, device nodes and FIFOs in update tarballs; validate each member's resolved path stays under dest after symlink resolution; cap uncompressed size at 256 MiB to bound gzip-bomb damage; strip setuid/setgid bits from extracted modes. Add an optional sha256 form field to /update and /update-self; the master client computes and sends it on every push, the executor refuses to extract on mismatch. mTLS already authenticates the master, so this is defence-in-depth against in-transit corruption and gives operators a way to pin "exactly these bytes" for vetted releases.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user