harden(left4me-overlay): fix idmap collision risk, gate test stubs on PRINT_ONLY, wrap os.stat

Issue #1: idmap target now uses parent+name (overlays_workshop instead of
workshop) to prevent basename collisions across allowlist roots; explicit
die() on collision detected in the loop.

Issue #2: env-var uid stubs (renamed to LEFT4ME_TEST_SANDBOX_UID etc.) are
only honoured when LEFT4ME_OVERLAY_PRINT_ONLY=1, so a misconfigured systemd
unit override cannot influence real uid mapping.

Issue #3: os.stat(lowerdir) is wrapped in try/except OSError with a die()
that shell-quotes the path and includes the exception, matching the helper's
existing error style.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
mwiegand 2026-05-14 23:53:32 +02:00
parent 2f6a9cfba0
commit 90531864b3
No known key found for this signature in database
2 changed files with 45 additions and 24 deletions

View file

@ -70,15 +70,19 @@ def _lookup_uid(username: str) -> tuple[int, int]:
def _get_user_ids() -> tuple[int, int, int, int]: def _get_user_ids() -> tuple[int, int, int, int]:
"""Return (sandbox_uid, sandbox_gid, left4me_uid, left4me_gid). """Return (sandbox_uid, sandbox_gid, left4me_uid, left4me_gid).
In normal operation, looks up the real system users. When the test-only In normal operation, looks up the real system users. In PRINT_ONLY
env vars _L4D2_SANDBOX_UID/_L4D2_SANDBOX_GID/_LEFT4ME_UID/_LEFT4ME_GID (test) mode the env vars LEFT4ME_TEST_SANDBOX_UID/LEFT4ME_TEST_SANDBOX_GID/
are set, those values are used directly so tests can run without root LEFT4ME_TEST_LEFT4ME_UID/LEFT4ME_TEST_LEFT4ME_GID may be used to inject
and without real system users being present. synthetic uids so tests can run without root and without real system
users present. The stubs are intentionally ignored outside PRINT_ONLY
mode so that a misconfigured systemd unit override cannot influence the
real uid mapping.
""" """
sandbox_uid_env = os.environ.get("_L4D2_SANDBOX_UID") if os.environ.get("LEFT4ME_OVERLAY_PRINT_ONLY") == "1":
sandbox_gid_env = os.environ.get("_L4D2_SANDBOX_GID") sandbox_uid_env = os.environ.get("LEFT4ME_TEST_SANDBOX_UID")
left4me_uid_env = os.environ.get("_LEFT4ME_UID") sandbox_gid_env = os.environ.get("LEFT4ME_TEST_SANDBOX_GID")
left4me_gid_env = os.environ.get("_LEFT4ME_GID") left4me_uid_env = os.environ.get("LEFT4ME_TEST_LEFT4ME_UID")
left4me_gid_env = os.environ.get("LEFT4ME_TEST_LEFT4ME_GID")
if all(v is not None for v in (sandbox_uid_env, sandbox_gid_env, if all(v is not None for v in (sandbox_uid_env, sandbox_gid_env,
left4me_uid_env, left4me_gid_env)): left4me_uid_env, left4me_gid_env)):
@ -223,13 +227,30 @@ def cmd_mount(name: str) -> None:
idmap_dir = runtime_name_dir / "idmap" idmap_dir = runtime_name_dir / "idmap"
final_lowerdirs: list[str] = [] final_lowerdirs: list[str] = []
bind_argvs: list[list[str]] = [] bind_argvs: list[list[str]] = []
seen_idmap_targets: dict[Path, str] = {}
for lowerdir in canonical_lowerdirs: for lowerdir in canonical_lowerdirs:
try:
st = os.stat(lowerdir) st = os.stat(lowerdir)
except OSError as exc:
die(f"failed to stat lowerdir {shlex.quote(lowerdir)}: {exc}")
if st.st_uid == sandbox_uid: if st.st_uid == sandbox_uid:
# This lowerdir needs idmap remapping. # This lowerdir needs idmap remapping.
overlay_id = Path(lowerdir).name # Include the parent dirname to avoid basename collisions between
idmap_target = idmap_dir / overlay_id # lowerdirs from different allowlist roots (e.g. overlays/foo and
# workshop_cache/foo would otherwise map to the same idmap target).
p = Path(lowerdir)
lowerdir_basename = f"{p.parent.name}_{p.name}"
idmap_target = idmap_dir / lowerdir_basename
# Belt-and-braces: detect if two different lowerdirs would collide
# on the same idmap target after the parent+name derivation.
if idmap_target in seen_idmap_targets:
die(
f"idmap target collision: lowerdirs {shlex.quote(seen_idmap_targets[idmap_target])}"
f" and {shlex.quote(lowerdir)} both map to {idmap_target}"
)
seen_idmap_targets[idmap_target] = lowerdir
if os.environ.get("LEFT4ME_OVERLAY_PRINT_ONLY") != "1": if os.environ.get("LEFT4ME_OVERLAY_PRINT_ONLY") != "1":
idmap_dir.mkdir(mode=0o700, exist_ok=True) idmap_dir.mkdir(mode=0o700, exist_ok=True)

View file

@ -42,10 +42,10 @@ def _run(args: list[str], root: Path, extra_env: dict[str, str] | None = None) -
"LEFT4ME_ROOT": str(root), "LEFT4ME_ROOT": str(root),
"LEFT4ME_OVERLAY_PRINT_ONLY": "1", "LEFT4ME_OVERLAY_PRINT_ONLY": "1",
# Inject synthetic user ids so tests work without real system users. # Inject synthetic user ids so tests work without real system users.
"_L4D2_SANDBOX_UID": str(FAKE_SANDBOX_UID), "LEFT4ME_TEST_SANDBOX_UID": str(FAKE_SANDBOX_UID),
"_L4D2_SANDBOX_GID": str(FAKE_SANDBOX_GID), "LEFT4ME_TEST_SANDBOX_GID": str(FAKE_SANDBOX_GID),
"_LEFT4ME_UID": str(FAKE_LEFT4ME_UID), "LEFT4ME_TEST_LEFT4ME_UID": str(FAKE_LEFT4ME_UID),
"_LEFT4ME_GID": str(FAKE_LEFT4ME_GID), "LEFT4ME_TEST_LEFT4ME_GID": str(FAKE_LEFT4ME_GID),
} }
if extra_env: if extra_env:
env.update(extra_env) env.update(extra_env)
@ -222,7 +222,7 @@ def test_mount_idmaps_sandbox_owned_lowerdir(tmp_path: Path) -> None:
assert f"--map-users={FAKE_SANDBOX_UID}:{FAKE_LEFT4ME_UID}:1" in bind_parts assert f"--map-users={FAKE_SANDBOX_UID}:{FAKE_LEFT4ME_UID}:1" in bind_parts
assert f"--map-groups={FAKE_SANDBOX_GID}:{FAKE_LEFT4ME_GID}:1" in bind_parts assert f"--map-groups={FAKE_SANDBOX_GID}:{FAKE_LEFT4ME_GID}:1" in bind_parts
assert bind_parts[-2] == str(overlay_dir) assert bind_parts[-2] == str(overlay_dir)
idmap_target = str(tmp_path / "runtime" / "alpha" / "idmap" / "workshop") idmap_target = str(tmp_path / "runtime" / "alpha" / "idmap" / "overlays_workshop")
assert bind_parts[-1] == idmap_target assert bind_parts[-1] == idmap_target
overlay_parts = shlex.split(lines[1]) overlay_parts = shlex.split(lines[1])