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>
18 KiB
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) andsecurity-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 0–8, 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 | FIXED — bun 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-sideAcceptAnyClientCertlikewise 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 2sPAIRING_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 innative_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_PASSWORDis unset; constant-time compare; sealed (AES-GCM)httpOnlySameSite=laxsession 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.jsonexposure via a*.jsonallowlist was anticipated and avoided). - Gaps: no login throttling (F-1), cookie-sealing key derived from the password when
PUNKTFUNK_UI_SECRETis unset (F-2), process-wideNODE_TLS_REJECT_UNAUTHORIZED=0for 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 auditweekly + on everyCargo.lockchange + on demand (.gitea/workflows/audit.yml); the.cargo/audit.tomlignore 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 viacargo-about(no copyleft in the linked set). Exact toolchain pin (1.96.0). - Secrets hygiene: no committed key material or real
.envfiles (templates only); secret files routed throughwrite_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::sessionConfig — sha256("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 —
PinGatedoc + 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.tomlmatches the corrected version; one-signature-per-ceremony hardening referenced in the ignore justification.
Priorities
- 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.
- 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.
- 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.