fix(bot-screen): hygiene — private alloc lock, private lease files, cookie off argv, real Fedora packages
Four small exposures, none a same-UID boundary (which the design does not claim), all about other local users and wrong install hints: - The host-wide display-allocation lock sat at a predictable name in world-writable /tmp, where any local user could pre-create or squat it. It now lives under XDG_RUNTIME_DIR (else ~/.cache), created 0700; still host-wide, outside every profile home, because all profiles allocate from one band. - lease.py hand-rolled mkdir + plain writes, so a takeover recorded before start() ever ran left bot-desktop/ 0755 and lease.json / lease.lock 0644 under the umask — who holds the screen and the lock the RFB bridge serialises on, readable by everyone. It now uses secure_parent_dir and creates every file 0600 via an opener; the fcntl-less (Windows) fallback is untouched. - launcher.sh passed the X cookie on xauth's argv, visible in ps to other local UIDs. The cookie is now fed on stdin through `xauth source -`. - The Fedora map named tigervnc-server-minimal (only a Provides of tigervnc-x11-server, which actually ships Xvnc) and dbus-x11 for dbus-run-session (dbus-daemon owns it; dbus-x11 ships dbus-launch). Both BINARY_PACKAGES and PACKAGES are corrected together.
This commit is contained in:
@@ -178,3 +178,26 @@ def test_lease_works_without_fcntl(tmp_path):
|
||||
out = subprocess.run([sys.executable, "-c", probe], capture_output=True, text=True, encoding="utf-8", timeout=60,
|
||||
stdin=subprocess.DEVNULL, env={**os.environ, "HERMES_HOME": str(tmp_path)})
|
||||
assert out.stdout.strip() == "OK", out.stderr
|
||||
|
||||
|
||||
@pytest.mark.linux_only
|
||||
def test_lease_files_are_private_even_when_the_lease_is_written_before_the_screen_exists(tmp_path, monkeypatch):
|
||||
"""A takeover can be recorded before start() ever created bot-desktop/ 0700. The lease path then created
|
||||
the directory and files with the umask (0755 / 0644): who holds the screen, and the lock the RFB bridge
|
||||
serialises on, readable and clobberable by every other local user. Every piece must be owner-only."""
|
||||
import os
|
||||
import stat
|
||||
|
||||
home = tmp_path / "deep" / "home"
|
||||
home.mkdir(parents=True)
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
monkeypatch.setattr(lease, "get_hermes_home", lambda: home)
|
||||
old = os.umask(0o022)
|
||||
try:
|
||||
lease.acquire("v1")
|
||||
finally:
|
||||
os.umask(old)
|
||||
sd = home / "bot-desktop"
|
||||
for path in (sd, sd / "lease.json", sd / "lease.lock"):
|
||||
assert path.exists(), path
|
||||
assert stat.S_IMODE(path.stat().st_mode) & 0o077 == 0, f"{path.name} is {oct(path.stat().st_mode)}"
|
||||
|
||||
@@ -49,7 +49,10 @@ if [[ -e "$xlock" ]] && ! kill -0 "$(tr -d ' ' < "$xlock" 2>/dev/null)" 2>/dev/n
|
||||
rm -f "$xlock" "/tmp/.X11-unix/X${HERMES_BD_DISPLAY_NUM}"
|
||||
fi
|
||||
: > "$XAUTHORITY"; chmod 600 "$XAUTHORITY"
|
||||
xauth -q -f "$XAUTHORITY" add "$DISPLAY" MIT-MAGIC-COOKIE-1 "$(od -An -N16 -tx1 /dev/urandom | tr -d ' \n')"
|
||||
# The cookie goes in on stdin, not argv: a command line is readable by every local user via ps.
|
||||
xauth -q -f "$XAUTHORITY" source - <<COOKIE
|
||||
add $DISPLAY MIT-MAGIC-COOKIE-1 $(od -An -N16 -tx1 /dev/urandom | tr -d ' \n')
|
||||
COOKIE
|
||||
|
||||
# ---- look: dark theme from whatever the host ships (first match wins), Hermes wallpaper ----
|
||||
pick_theme() { local d t; for t in "$@"; do for d in /usr/share/themes "$HOME/.themes"; do [[ -d "$d/$t" ]] && { echo "$t"; return; }; done; done; echo "$1"; }
|
||||
|
||||
@@ -23,7 +23,7 @@ from dataclasses import asdict, dataclass, field
|
||||
from pathlib import Path
|
||||
from typing import Callable, Dict, List, Optional
|
||||
|
||||
from hermes_constants import get_hermes_home, hermes_home_key
|
||||
from hermes_constants import get_hermes_home, hermes_home_key, secure_parent_dir
|
||||
|
||||
try:
|
||||
import fcntl
|
||||
@@ -87,10 +87,23 @@ def _read(path: Path) -> Lease:
|
||||
return Lease(holder=HUMAN, viewer_id="unreadable-lease", reason="lease file corrupt")
|
||||
|
||||
|
||||
def _write(path: Path, lease: Lease) -> None:
|
||||
def _private_dir(path: Path) -> None:
|
||||
"""``bot-desktop/`` owner-only even when the lease is the first thing written there (a takeover can be
|
||||
recorded before start() ever ran, and the umask would otherwise leave it 0755)."""
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
secure_parent_dir(path)
|
||||
|
||||
|
||||
def _open_private(path: str | bytes | os.PathLike, flags: int) -> int:
|
||||
"""``open(..., opener=_open_private)``: the file is created 0600 regardless of the umask."""
|
||||
return os.open(path, flags, 0o600)
|
||||
|
||||
|
||||
def _write(path: Path, lease: Lease) -> None:
|
||||
_private_dir(path)
|
||||
tmp = path.with_suffix(".json.tmp")
|
||||
tmp.write_text(json.dumps(lease.as_dict()), encoding="utf-8")
|
||||
with open(tmp, "w", encoding="utf-8", opener=_open_private) as fh:
|
||||
fh.write(json.dumps(lease.as_dict()))
|
||||
os.replace(tmp, path)
|
||||
|
||||
|
||||
@@ -104,8 +117,8 @@ class _locked:
|
||||
def __enter__(self):
|
||||
if fcntl is None:
|
||||
return self
|
||||
self._lockfile.parent.mkdir(parents=True, exist_ok=True)
|
||||
self._fh = open(self._lockfile, "a+", encoding="utf-8") # noqa: SIM115 — closed in __exit__
|
||||
_private_dir(self._lockfile)
|
||||
self._fh = open(self._lockfile, "a+", encoding="utf-8", opener=_open_private) # noqa: SIM115 — closed in __exit__
|
||||
fcntl.flock(self._fh.fileno(), fcntl.LOCK_EX)
|
||||
return self
|
||||
|
||||
|
||||
@@ -48,8 +48,10 @@ BINARY_PACKAGES = {
|
||||
"apt": {"Xvnc": "tigervnc-standalone-server", "xfwm4": "xfwm4", "xfce4-panel": "xfce4-panel",
|
||||
"xfdesktop": "xfdesktop4", "xfsettingsd": "xfce4-settings", "dbus-run-session": "dbus-x11",
|
||||
"xauth": "xauth", "xdpyinfo": "x11-utils", "setxkbmap": "x11-xkb-utils", "xprop": "x11-utils"},
|
||||
"dnf": {"Xvnc": "tigervnc-server-minimal", "xfwm4": "xfwm4", "xfce4-panel": "xfce4-panel",
|
||||
"xfdesktop": "xfdesktop", "xfsettingsd": "xfce4-settings", "dbus-run-session": "dbus-x11",
|
||||
# tigervnc-x11-server is the real package (tigervnc-server-minimal is only a Provides on it); dbus-run-session
|
||||
# is in dbus-daemon (dbus-x11 ships dbus-launch only).
|
||||
"dnf": {"Xvnc": "tigervnc-x11-server", "xfwm4": "xfwm4", "xfce4-panel": "xfce4-panel",
|
||||
"xfdesktop": "xfdesktop", "xfsettingsd": "xfce4-settings", "dbus-run-session": "dbus-daemon",
|
||||
"xauth": "xorg-x11-xauth", "xdpyinfo": "xdpyinfo", "setxkbmap": "setxkbmap", "xprop": "xprop"},
|
||||
"pacman": {"Xvnc": "tigervnc", "xfwm4": "xfwm4", "xfce4-panel": "xfce4-panel", "xfdesktop": "xfdesktop",
|
||||
"xfsettingsd": "xfce4-settings", "dbus-run-session": "dbus",
|
||||
@@ -60,8 +62,8 @@ PACKAGES = {
|
||||
"apt": ["tigervnc-standalone-server", "xfce4-panel", "xfwm4", "xfdesktop4", "xfce4-settings",
|
||||
"xfce4-terminal", "dbus-x11", "x11-xserver-utils", "x11-utils", "x11-xkb-utils", "xauth",
|
||||
"fonts-dejavu-core"],
|
||||
"dnf": ["tigervnc-server-minimal", "xfce4-panel", "xfwm4", "xfdesktop", "xfce4-settings",
|
||||
"xfce4-terminal", "dbus-x11", "xsetroot", "xset", "xdpyinfo", "xprop", "xorg-x11-xauth", "setxkbmap",
|
||||
"dnf": ["tigervnc-x11-server", "xfce4-panel", "xfwm4", "xfdesktop", "xfce4-settings",
|
||||
"xfce4-terminal", "dbus-daemon", "xsetroot", "xset", "xdpyinfo", "xprop", "xorg-x11-xauth", "setxkbmap",
|
||||
"dejavu-sans-fonts"],
|
||||
"pacman": ["tigervnc", "xfce4-panel", "xfwm4", "xfdesktop", "xfce4-settings", "xfce4-terminal", "dbus",
|
||||
"xorg-xsetroot", "xorg-xset", "xorg-xdpyinfo", "xorg-xprop", "xorg-xauth", "xorg-setxkbmap",
|
||||
@@ -229,7 +231,9 @@ def _kill_group_then_wait(pgid: Optional[int], pid: int, grace: float = 2.0) ->
|
||||
_signal(signal.SIGKILL) # windows-footgun: ok — Linux-only runtime (is_supported_host gates start/stop)
|
||||
|
||||
|
||||
_ALLOC_LOCK = Path("/tmp/.hermes-bot-desktop-alloc.lock") # host-wide: profiles allocate from one band
|
||||
# Host-wide (every profile allocates from one band), so it lives outside any profile home — but not in
|
||||
# world-writable /tmp, where a predictable name lets another local user pre-create or squat the file.
|
||||
_ALLOC_LOCK = Path(os.environ.get("XDG_RUNTIME_DIR") or Path.home() / ".cache") / "hermes-bot-desktop-alloc.lock"
|
||||
|
||||
|
||||
@contextlib.contextmanager
|
||||
@@ -256,6 +260,7 @@ def _pick_display() -> int:
|
||||
|
||||
|
||||
def _allocate_display() -> int:
|
||||
_ALLOC_LOCK.parent.mkdir(parents=True, exist_ok=True, mode=0o700)
|
||||
with _flocked(_ALLOC_LOCK):
|
||||
return _pick_display()
|
||||
|
||||
@@ -371,6 +376,7 @@ def start(*, wait_seconds: float = 15.0) -> DesktopStatus:
|
||||
return status()
|
||||
if _launcher_pid() is None:
|
||||
_reap_orphaned_server(sd)
|
||||
_ALLOC_LOCK.parent.mkdir(parents=True, exist_ok=True, mode=0o700)
|
||||
with _flocked(_ALLOC_LOCK):
|
||||
return _spawn_and_wait(sd, _pick_display(), wait_seconds)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user