Files
punktfunk/design/security-posture-audit-2026-07-05.md
T
enricobuehler d76a42e0e9
android / android (push) Has been cancelled
apple / screenshots (push) Has been cancelled
apple / swift (push) Has been cancelled
arch / build-publish (push) Has been cancelled
audit / bun-audit (push) Has been cancelled
audit / cargo-audit (push) Has been cancelled
ci / rust (push) Has been cancelled
ci / web (push) Has been cancelled
ci / docs-site (push) Has been cancelled
ci / bench (push) Has been cancelled
deb / build-publish (push) Has been cancelled
decky / build-publish (push) Has been cancelled
docker / build-push (., web/Dockerfile, punktfunk-web) (push) Has been cancelled
docker / build-push (ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora-rpm) (push) Has been cancelled
docker / build-push (ci, ci/rust-ci.Dockerfile, punktfunk-rust-ci) (push) Has been cancelled
docker / build-push (docs-site, docs-site/Dockerfile, punktfunk-docs) (push) Has been cancelled
docker / deploy-docs (push) Has been cancelled
docker / build-push (--build-arg FEDORA_VERSION=44, ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora44-rpm) (push) Has been cancelled
rpm / build-publish (44, fedora-44, punktfunk-fedora44-rpm) (push) Has been cancelled
rpm / build-publish (43, bazzite, punktfunk-fedora-rpm) (push) Has been cancelled
windows-host / package (push) Has been cancelled
docs(security): add 2026-07-05 whole-project posture audit
Assessment of the overall security architecture, the state of the prior
reviews' remediations, and the process/supply-chain controls. Flags the
two web-console residuals (no brute-force throttle; cookie seal key from
the login password) that the accompanying web-hardening change fixes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-06 07:28:51 +02:00

18 KiB
Raw Blame History

punktfunk — security posture audit (2026-07-05)

Status: AUDIT COMPLETE (2026-07-05). Whole-project posture audit — not a finding hunt like security-review.md (2026-06-21) and security-review-2026-06-28.md (host follow-up), but an assessment of the overall security architecture, the state of the prior reviews' remediations, the delta landed since 2026-06-28 (display management Stages 08, WoL wake-until-up, --data-port, web display-config surface), and the process/supply-chain controls around the code. Method: single-reviewer read of the trust-model core (quic.rs, crypto.rs, session.rs, native_pairing.rs, punktfunk1.rs, mgmt.rs, mgmt_token.rs, the web BFF auth stack, gamestream/{pairing,crypto,control}.rs, discovery.rs, wol.rs), plus sweeps for TLS-bypass patterns, committed secrets, unsafe density, CI workflow hygiene, and the dependency/advisory pipeline.

Executive summary

The posture is strong — unusually so for a project of this size. The core trust architecture is sound and shows defense-in-depth discipline: a modern, mutually-authenticated native plane (QUIC + rustls, fingerprint pinning with real CertificateVerify verification, SPAKE2 pairing with cert-fingerprint identity binding); a management plane with a clean authn/authz split (loopback-confined bearer for admin, deny-by-default read-only allowlist for paired mTLS certs); a web console that never leaks the admin token to the browser and fails closed when misconfigured; legacy GameStream compatibility correctly quarantined behind an explicit opt-in; and a supply chain guarded by scheduled cargo audit, a tightly-justified ignore list, a license allowlist, and an exact-pinned toolchain. The two prior reviews' 18 findings are remediated or accepted-with-rationale, and the fixes are visible (and in several cases regression-tested) in the code today.

No new high-severity issue was found. The residual risk concentrates in the web console's password gate (no brute-force throttling; cookie-sealing key derived from the login password) and in second-tier supply-chain gaps (no advisory scanning for the Bun/Nitro stack; tag-pinned third-party CI actions holding deploy secrets). All are bounded by the documented threat model (trusted LAN / VPN, no WAN exposure) but are cheap to close and worth closing.

Remediation status (2026-07-05)

All findings from this audit were fixed the same day, plus one Windows LPE gap (F-8) surfaced by a reviewer during remediation. Verification notes are per-item.

# Sev Status
F-1 Medium FIXED — per-IP exponential-backoff login throttle (web/server/util/loginThrottle.ts + login.post.ts); 5 free attempts then 1s→5min backoff, global floor, size-capped map. Behaviorally tested (backoff, unlock, IP-independence, success-clear).
F-2 Medium FIXED — cookie-seal key now derived from the CSPRNG mgmt token (sessionConfig in auth.ts), not the login password; password-derivation kept only as a no-token dev fallback. A captured cookie is no longer an offline password oracle.
F-3 Low FIXED — accept-any-cert scoped to the loopback proxy hop via Bun per-request tls (routes/api/[...].ts, gated on isLoopbackUrl); process-wide NODE_TLS_REJECT_UNAUTHORIZED=0 removed from all 4 deploy files + 3 docs. A non-loopback mgmt URL now verifies normally.
F-4 Low FIXEDbun audit job added to audit.yml (weekly + on web/bun.lock change), same fail-on-vulnerability stance as cargo audit.
F-5 Low FIXED — the three secret-touching third-party actions SHA-pinned with version comments: appleboy/scp-action, appleboy/ssh-action (deploy SSH key + registry token, docker.yml), android-actions/setup-android (signing keystore + Play SA, android.yml/android-screenshots.yml). GitHub-owned actions/* left on major tags (org controls the tag).
F-6 Info No change (accepted) — tray loopback accept-any-cert; spoofed-status-only, local-process out of scope.
F-7 Info No change (documented) — session-layer input replay; native plane rides replay-protected QUIC, GameStream covered by the trusted-LAN caveat.
F-8 Medium FIXED — see below. driver install executed/trusted files from --dir with no check the dir was admin-only → local EoP if the payload is staged somewhere user-writable. Added a DACL/owner check (ensure_admin_only_source in windows/install.rs) that fails closed. Verified: Win32 FFI typechecks against windows 0.62.2 for x86_64-pc-windows-msvc.

F-8 (Medium) — driver install trusts a non-admin-writable source directory

Reported by the user during remediation; confirmed. punktfunk-host driver install --dir <stage> runs elevated (the Inno [Run] section, or a manual admin invocation) and, from --dir, executes nefconc.exe, trusts a .cer into the machine Root/TrustedPublisher stores, and pnputil /add-drivers the package — with no check that --dir is writable only by administrators. If the staging directory is writable by a non-admin, a local unprivileged user can plant a malicious nefconc.exe (arbitrary code as the elevated installer → SYSTEM) or swap the .cer (poisoning the machine trust store) before the elevated step consumes it — a local elevation of privilege.

Not exploitable in the default install (correctly noted by the reporter): the shipped installer stages into {tmp} under a DefaultDirName={autopf} (Program Files), PrivilegesRequired=admin Inno setup, which restricts the staging dir to Administrators/SYSTEM. The gap bites only when the payload is staged somewhere user-writable — a custom install directory, or a hand-run driver install --dir <user-writable>. Fix: ensure_admin_only_source reads the directory's owner + DACL (GetNamedSecurityInfoW) and refuses to proceed unless every principal with a create/write/delete/DACL-rewrite right is SYSTEM/Administrators/TrustedInstaller (or CREATOR-OWNER under a privileged owner). Fails closed — an unreadable ACL is treated as unsafe — and, like every other step in the best-effort installer, a refusal only degrades the host to a physical display; it never aborts the install.

Trust architecture assessment

Native plane (punktfunk/1) — sound

  • Transport: QUIC via quinn 0.11.11 / rustls 0.23.41 (ring). Data-plane sessions are created with encrypt: true (punktfunk1.rs:959) and per-session random key+salt.
  • Identity & pinning: hosts serve a persisted self-signed cert; clients pin its SHA-256 (TOFU only as the documented bootstrap special case, endpoint::client_insecure = client_pinned(None)). Crucially, the pin verifier still verifies TLS 1.2/1.3 handshake signatures for real (quic.rs:2064-2096) — possession of the pinned cert's key is proven, the classic pin-without-CertificateVerify hole is explicitly avoided. The host-side AcceptAnyClientCert likewise verifies the handshake signature, so a client fingerprint is proof of key possession, not just a presented blob.
  • Pairing (SPAKE2): the ceremony binds both certificate fingerprints as SPAKE2 identities, so a MITM presenting different certs to each leg cannot reach a shared key. Key-confirmation MACs are compared constant-time. The PIN is CSPRNG-minted, single-use and consumed before the client's proof is read (punktfunk1.rs, pair_ceremony) — an attacker gets exactly one online guess per operator arming, rate-limited further by the 2s PAIRING_COOLDOWN. Arming windows can be fingerprint-bound (prior review #9), and the delegated-approval pending queue is flood-resistant (per-source-IP cap, parked-knock eviction protection, TTL — prior review #13, all regression-tested in native_pairing.rs).
  • Session AEAD: AES-128-GCM with a documented nonce-uniqueness contract (per-direction salt bit, per-session key+salt, zero-key rejected by config validation), sequence bound as AAD. Wire decoders bound every attacker-controllable header field (packet.rs:323, ReassemblerLimits); pairing/control messages are length-checked with exact-trailing-bytes rejection.
  • Untrusted-input hygiene: client-supplied device names are sanitized against control chars, ANSI escapes, and Unicode bidi overrides before storage/log/UI (native_pairing.rs::sanitize_device_name) — approval-UI spoofing is handled.
  • Trust store: atomic temp+rename writes, owner-only permissions (0600/DACL via write_secret_file), in-memory rollback on persist failure.

Management plane — sound

  • HTTPS always; bearer token always required (env > owner-only persisted file > generated; mgmt_token.rs). Token comparison hashes both sides before comparing (mgmt.rs::token_eq), neutralizing timing.
  • The bearer (full-admin) path is honored only from loopback peers, even though the listener binds all interfaces by default; LAN callers must present a paired mTLS cert and are confined to a deny-by-default, GET-only allowlist (cert_may_access) — a paired streaming client cannot administer the host (unpair others, read/arm PINs, stop sessions, or mutate the library/display config). The new display-management routes stayed off the cert allowlist, with a regression test (display_settings_surface stays read-only).
  • /api/v1/local/summary (tray status) is unauthenticated but loopback-only and deliberately non-sensitive.

Web console (Nitro/Bun BFF) — good design, two hardening gaps (F-1, F-2)

  • Single shared password gate; fails closed (503) when PUNKTFUNK_UI_PASSWORD is unset; constant-time compare; sealed (AES-GCM) httpOnly SameSite=lax session cookie with a 7-day TTL; open-redirect guard on the post-login path; /api/** always gated; the mgmt bearer token is injected server-side only and browser cookies are stripped from the upstream request. Public-path allowlisting is by path, not extension (with the reasoning documented — openapi.json exposure via a *.json allowlist was anticipated and avoided).
  • Gaps: no login throttling (F-1), cookie-sealing key derived from the password when PUNKTFUNK_UI_SECRET is unset (F-2), process-wide NODE_TLS_REJECT_UNAUTHORIZED=0 for the loopback proxy hop (F-3).

GameStream/Moonlight compatibility — correctly quarantined

Off by default; enabled only by --gamestream/--moonlight, and documented (SECURITY.md, --help) as legacy-crypto, trusted-LAN-only. Within that constraint the implementation is defensively built: PIN delivery only via the authenticated management API (never nvhttp — prior #1), parked-handshake caps (#12), one RSA signature per ceremony (Marvin-amplifier hardening, S7), phase-repeat rejection, RTSP gated on a paired /launch bound to the launching peer's IP (#4). The AES-128-ECB pairing crypto and mostly-plaintext media streams are wire-compatibility constraints of the Moonlight protocol, not implementation choices, and the input-replay gap at the raw-UDP session layer is documented in code (session.rs:31-35) and covered by the trusted-LAN assumption.

Discovery & Wake-on-LAN — appropriately advisory

mDNS TXT records (fp, mac, pair, mgmt) are treated as unauthenticated hints everywhere they're consumed: pinning still gates the actual connection, and a spoofed MAC only makes a wake fail (magic packets are inert). wol.rs detect-and-warn never mutates NIC state.

Supply chain & process — mature

  • Rust: cargo audit weekly + on every Cargo.lock change + on demand (.gitea/workflows/audit.yml); the .cargo/audit.toml ignore list is tight, justified, and self-correcting (the RUSTSEC-2023-0071 entry documents that its own earlier rationale was wrong and re-justifies the accept on corrected grounds — exemplary). Key crypto deps are current (rustls 0.23.41, quinn 0.11.11, ring 0.17.14). License allowlist enforced via cargo-about (no copyleft in the linked set). Exact toolchain pin (1.96.0).
  • Secrets hygiene: no committed key material or real .env files (templates only); secret files routed through write_secret_file (0600 + Windows SYSTEM/Admins DACL).
  • Process: SECURITY.md with private reporting, coordinated disclosure, and safe harbor; two prior deep reviews with per-finding remediation tracking; findings referenced by number in code comments and regression tests.
  • Gaps: no JS-side advisory scanning (F-4); third-party actions tag-pinned, not SHA-pinned (F-5).

Findings (this audit)

No High findings. Severities assume the documented LAN/VPN threat model.

# Sev Component Finding
F-1 Medium web console No throttling on POST /_auth/login (web/server/routes/_auth/login.post.ts). The constant-time compare stops timing leaks but not volume: a LAN peer can brute-force PUNKTFUNK_UI_PASSWORD at network speed against a console that is, by design, LAN-exposed. Every other password/PIN gate in the project is rate-limited (pairing cooldown, single-use PINs) — this is the one unthrottled secret. Fix: small in-memory failure counter with exponential backoff (per source IP + global), mirroring PAIRING_COOLDOWN's philosophy.
F-2 Medium web console Cookie-sealing key is derived from the login password when PUNKTFUNK_UI_SECRET is unset (web/server/util/auth.ts::sessionConfigsha256("punktfunk-session-v1:" + password)). A captured session cookie (e.g. sniffed over the plain-HTTP dev/LAN mode, where Secure is off by default) becomes an offline dictionary oracle for the password: an attacker tries candidate passwords by deriving the key and attempting to unseal — no server round-trips, so F-1's fix doesn't help. Fix: generate and persist a random 32-byte secret on first start when PUNKTFUNK_UI_SECRET is unset (same pattern as mgmt_token.rs), instead of deriving from the password.
F-3 Low web console NODE_TLS_REJECT_UNAUTHORIZED=0 is process-wide, not scoped to the loopback mgmt hop it's documented for (web/.env.example:25, README). Today the BFF makes no other outbound TLS call, so the practical scope holds — but it's a global switch that silently unverifies any future fetch (an update check, an art CDN, a webhook). Fix: drop the env var and pass the host's own cert.pem as a per-fetch CA (Bun fetch tls.ca / undici Agent), which also removes a scary line from the deployment docs.
F-4 Low supply chain No advisory scanning for the web stack. audit.yml covers the Rust tree only; the Bun/Nitro BFF — the component that holds the login gate, session sealing, and the mgmt token — has a bun.lock but no scheduled vulnerability scan. Fix: add a job running bun audit (or osv-scanner --lockfile web/bun.lock) on the same weekly + lockfile-change triggers.
F-5 Low CI Third-party actions are tag-pinned, not SHA-pinned. appleboy/ssh-action@v1.2.5 / appleboy/scp-action@v0.1.7 receive deploy SSH credentials, and actions/checkout@v4 / cache@v4 run on every job on self-hosted runners; a moved tag = code execution on the runners plus secret exfiltration. Given how rigorous the Rust-side supply chain is, this is the soft spot. Fix: pin at least the secret-touching actions to full commit SHAs.
F-6 Info tray / local The tray's status fetch (punktfunk-tray/src/status.rs) uses the accept-any-cert verifier against 127.0.0.1:47990. If the host isn't running, any local unprivileged process can squat the port and feed the tray fake status. Impact is spoofed tray UI only (the tray sends no secrets and the summary is non-sensitive) — consistent with the documented "local processes are out of scope" stance. No action needed; recorded for completeness.
F-7 Info core session Input-event replay at the raw-UDP session layer remains unfiltered — already documented in session.rs:31-35 with the correct future home (a sliding window keyed on the authenticated sequence, in the GameStream host). The native plane is unaffected (client input rides QUIC datagrams, which are inherently replay-protected). Keep the doc note honest until the GameStream path grows the window; it stays covered by the opt-in/trusted-LAN caveat.

Prior-review remediations — spot-verified present

  • #1 (PIN only via bearer mgmt API): confirmed — PinGate doc + no nvhttp PIN route.
  • #2 (mgmt token via write_secret_file): confirmed — mgmt_token.rs:62.
  • #9 (fingerprint-bound arming windows): confirmed — arm_for/pin_for_attempt + tests.
  • #12 (parked-waiter cap): confirmed — MAX_PARKED_WAITERS + atomic slot reservation.
  • #13 (per-IP pending cap, parked-knock protection): confirmed — constants + tests.
  • S7 (rsa Marvin accept): rationale in .cargo/audit.toml matches the corrected version; one-signature-per-ceremony hardening referenced in the ignore justification.

Priorities

  1. F-1 + F-2 together (web console password gate): a login backoff plus a generated-not-derived cookie secret close the only realistic LAN-attacker path to admin found in this audit. Both are small, isolated changes.
  2. F-4 + F-5 (supply chain parity): one CI job and a handful of SHA pins bring the JS stack and the workflows up to the standard the Rust tree already meets.
  3. F-3 (scoped CA instead of the global TLS switch): low urgency, high docs/appearance value.

Nothing found here changes the documented threat model or contradicts SECURITY.md's stated limits. The next full finding-hunt review should focus on the Windows vdisplay/driver admission surface (largest post-2026-06-28 delta) and the web console once F-1/F-2 land.