unom/punktfunk
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases 26 Wiki Activity

refactor(host/W6.1): extract secret/config-dir helpers into the pf-paths leaf crate

Browse Source 
Second de-coupling for the host crate carve (plan §W6.1 leaf). config_dir /
create_private_dir / write_secret_file (+ the Windows DACL helpers) were pub(crate) in the
gamestream junk drawer, yet consumed by vdisplay, stats, gpu, library, mgmt_token,
native_pairing and the Windows service — many of which become pf-media / pf-vdisplay, for
which crate::gamestream would be an illegal upward edge. New leaf crate pf-paths (pure std
+ tracing) owns them; ~40 call sites across 14 files repoint to pf_paths::. gamestream
keeps only its own concerns.

Verified: Linux (home-worker-5) clippy -p pf-paths -p punktfunk-host --all-targets
-D warnings + tests (347 pass, incl. secrets_are_written_owner_only); Windows
(192.168.1.158) clippy --features nvenc,amf-qsv --all-targets green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Enrico Bühler
2026-07-17 01:07:26 +02:00
parent 2e3208f75e
commit c42ce88921
19 changed files with 248 additions and 218 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/punktfunk-host/src/gamestream/apps.rs
+1 -1
View File
@@ -28,7 +28,7 @@ pub struct AppEntry {
fn config_path() -> Option<std::path::PathBuf> {
// `config_dir()` resolves XDG/HOME on Linux and %APPDATA% on Windows (no HOME needed).
Some(super::config_dir().join("apps.json"))
Some(pf_paths::config_dir().join("apps.json"))
}
fn parse_compositor(s: &str) -> Option<crate::vdisplay::Compositor> {
crates/punktfunk-host/src/gamestream/cert.rs
+4 -4
View File
@@ -2,8 +2,8 @@
//! during pairing AND presented as the TLS server cert on 47984 (Moonlight pins it). The
//! cert's own X.509 signature bytes are an input to the pairing hashes, so we extract them.
use super::config_dir;
use anyhow::{anyhow, Context, Result};
use pf_paths::config_dir;
use rsa::pkcs1v15::SigningKey;
use rsa::pkcs8::DecodePrivateKey;
use rsa::RsaPrivateKey;
@@ -36,11 +36,11 @@ impl ServerIdentity {
// The private key is the trust root for EVERY surface (TLS server cert, pairing
// signing, the QUIC identity clients pin) — write it owner-only (0600 / SYSTEM-only
// DACL) so a local user can't read it and impersonate the host. The dir is 0700.
super::create_private_dir(&dir).ok();
super::write_secret_file(&key_path, k.as_bytes())
pf_paths::create_private_dir(&dir).ok();
pf_paths::write_secret_file(&key_path, k.as_bytes())
.with_context(|| format!("write {}", key_path.display()))?;
// The cert is public (handed to clients), but write it owner-only too for consistency.
super::write_secret_file(&cert_path, c.as_bytes())
pf_paths::write_secret_file(&cert_path, c.as_bytes())
.with_context(|| format!("write {}", cert_path.display()))?;
tracing::info!(path = %cert_path.display(), "generated punktfunk host certificate (RSA-2048, key 0600)");
(c, k)
crates/punktfunk-host/src/gamestream/mod.rs
+9 -172
View File
@@ -26,7 +26,6 @@ mod video;
use anyhow::{Context, Result};
use std::net::{IpAddr, Ipv4Addr, UdpSocket};
use std::path::PathBuf;
use std::sync::Arc;
/// nvhttp ports (Moonlight derives all stream ports by offset from the HTTP base 47989).
@@ -307,168 +306,6 @@ pub fn serve(
})
}
/// The host config dir (host identity, pairing state, mgmt token, library) — created on demand.
/// Linux: `$XDG_CONFIG_HOME/punktfunk` or `~/.config/punktfunk`. Windows: `%ProgramData%\punktfunk`
/// (machine-wide — the SYSTEM service and the interactive user share ONE dir that survives logout).
/// `PUNKTFUNK_CONFIG_DIR` overrides on both platforms (used by the Windows service config / tests).
pub(crate) fn config_dir() -> PathBuf {
if let Some(dir) = std::env::var_os("PUNKTFUNK_CONFIG_DIR").filter(|s| !s.is_empty()) {
return PathBuf::from(dir);
}
// Windows: %ProgramData% (e.g. C:\ProgramData\punktfunk) — machine-wide, SYSTEM-readable,
// persists across user logout, correct for a SYSTEM service. Falls back to %APPDATA% then CWD.
#[cfg(target_os = "windows")]
let base = std::env::var_os("ProgramData")
.or_else(|| std::env::var_os("APPDATA"))
.map(PathBuf::from)
.unwrap_or_else(|| PathBuf::from("."));
#[cfg(not(target_os = "windows"))]
let base = std::env::var_os("XDG_CONFIG_HOME")
.map(PathBuf::from)
.or_else(|| std::env::var_os("HOME").map(|h| PathBuf::from(h).join(".config")))
.unwrap_or_else(|| PathBuf::from("."));
base.join("punktfunk")
}
/// Create `dir` (and parents) owner-private — **0700** on Unix (so the host's secrets aren't readable
/// by other local users via a traversable config path). On Windows, applies a restrictive DACL
/// ([`restrict_dir_to_system_admins`]) so a local unprivileged user can't pre-create / plant files in
/// the config tree (the default `%ProgramData%` ACL grants Users *create*; security-review
/// 2026-06-28 #3/#11). Tightens (and re-owns) an already-existing dir too.
pub(crate) fn create_private_dir(dir: &std::path::Path) -> std::io::Result<()> {
#[cfg(unix)]
{
use std::os::unix::fs::{DirBuilderExt, PermissionsExt};
let r = std::fs::DirBuilder::new()
.recursive(true)
.mode(0o700)
.create(dir);
// `recursive` doesn't re-chmod an existing dir — tighten it so an old 0755 dir gets locked.
if dir.exists() {
let _ = std::fs::set_permissions(dir, std::fs::Permissions::from_mode(0o700));
}
r
}
#[cfg(not(unix))]
{
let r = std::fs::create_dir_all(dir);
#[cfg(windows)]
restrict_dir_to_system_admins(dir);
r
}
}
/// Best-effort Windows DACL lockdown of the config *directory* (the companion to
/// [`restrict_to_system_admins`] for files). The default `%ProgramData%` ACL lets `BUILTIN\Users`
/// create subfolders/files (and become `CREATOR OWNER`), so a non-admin could pre-create the
/// `punktfunk` dir or plant a `host.env`/`apps.json` that the privileged SYSTEM service then trusts
/// (LPE; security-review 2026-06-28 #3). This re-owns the dir to Administrators (defeating a
/// pre-creation), strips inheritance, and sets an explicit DACL: SYSTEM/Administrators/OWNER full
/// (object+container inherit so child files/dirs inherit it), and Users **read-only** (so existing
/// reads of non-secret config keep working but a local user can no longer write/plant). Secret files
/// are additionally locked to SYSTEM/Admins by [`write_secret_file`]. Hard-coded SIDs
/// (locale-independent) via the absolute `%SystemRoot%` path; never fatal.
#[cfg(windows)]
fn restrict_dir_to_system_admins(dir: &std::path::Path) {
let icacls = std::env::var("SystemRoot")
.map(|r| format!("{r}\\System32\\icacls.exe"))
.unwrap_or_else(|_| "icacls".to_string());
// Reset ownership of the directory object to Administrators first, so a dir a non-admin may have
// pre-created can't keep OWNER control (an owner can always rewrite the DACL). No `/T` — re-owning
// the dir itself is what defeats the pre-creation; recursing a large captures tree each call is
// needless churn (secret files are individually owner-locked by `write_secret_file`).
let _ = std::process::Command::new(&icacls)
.arg(dir.as_os_str())
.args(["/setowner", "*S-1-5-32-544"]) // BUILTIN\Administrators
.stdout(std::process::Stdio::null())
.stderr(std::process::Stdio::null())
.status();
let status = std::process::Command::new(&icacls)
.arg(dir.as_os_str())
.args([
"/inheritance:r",
"/grant:r",
"*S-1-5-18:(OI)(CI)(F)", // NT AUTHORITY\SYSTEM
"/grant:r",
"*S-1-5-32-544:(OI)(CI)(F)", // BUILTIN\Administrators
"/grant:r",
"*S-1-3-4:(OI)(CI)(F)", // OWNER RIGHTS
"/grant:r",
"*S-1-5-32-545:(OI)(CI)(RX)", // BUILTIN\Users — read-only (no create/write → no plant)
])
.stdout(std::process::Stdio::null())
.stderr(std::process::Stdio::null())
.status();
match status {
Ok(s) if s.success() => {}
_ => tracing::warn!(
dir = %dir.display(),
"config-dir DACL hardening did not fully succeed — a local user may be able to plant config files"
),
}
}
/// Write `contents` to `path` as an **owner-only secret**: created and re-chmod'd **0600** on Unix
/// (never even briefly group/world-readable), and DACL-restricted to SYSTEM/Administrators/owner on
/// Windows (the default `%ProgramData%` ACL is Users-readable). Mirrors the mgmt-token hardening; used
/// for the host private key and the persisted trust stores so a local unprivileged user can neither
/// read the key (impersonation) nor tamper with the paired allow-list (unauthorized pairing).
pub(crate) fn write_secret_file(path: &std::path::Path, contents: &[u8]) -> std::io::Result<()> {
use std::io::Write;
let mut opts = std::fs::OpenOptions::new();
opts.write(true).create(true).truncate(true);
#[cfg(unix)]
{
use std::os::unix::fs::OpenOptionsExt;
opts.mode(0o600);
}
let mut f = opts.open(path)?;
f.write_all(contents)?;
f.flush()?;
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
let _ = std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600));
}
#[cfg(windows)]
restrict_to_system_admins(path);
Ok(())
}
/// Best-effort Windows DACL lockdown of a secret file: strip inherited ACEs and grant Full only to
/// SYSTEM, Administrators, and OWNER RIGHTS (the creating account — the SYSTEM service or a manually
/// running user keeps access). Without this the host key under the default Users-readable
/// `%ProgramData%` ACL is readable by ANY local user. Uses `icacls` with hard-coded SIDs
/// (locale-independent) via the absolute `%SystemRoot%` path (a privileged service must not trust
/// `PATH`). Never fatal — on failure the file is simply left at the inherited ACL (today's behaviour).
#[cfg(windows)]
fn restrict_to_system_admins(path: &std::path::Path) {
let icacls = std::env::var("SystemRoot")
.map(|r| format!("{r}\\System32\\icacls.exe"))
.unwrap_or_else(|_| "icacls".to_string());
let status = std::process::Command::new(icacls)
.arg(path.as_os_str())
.args([
"/inheritance:r",
"/grant:r",
"*S-1-5-18:(F)", // NT AUTHORITY\SYSTEM
"/grant:r",
"*S-1-5-32-544:(F)", // BUILTIN\Administrators
"/grant:r",
"*S-1-3-4:(F)", // OWNER RIGHTS
])
.stdout(std::process::Stdio::null())
.stderr(std::process::Stdio::null())
.status();
match status {
Ok(s) if s.success() => {}
_ => tracing::warn!(
path = %path.display(),
"icacls hardening did not succeed — this secret may be readable by other local users"
),
}
}
fn hostname_string() -> String {
#[cfg(target_os = "windows")]
if let Some(n) = std::env::var_os("COMPUTERNAME") {
@@ -486,7 +323,7 @@ fn hostname_string() -> String {
/// Load the persisted host uniqueid, or mint one (from the kernel UUID source) and store it.
fn load_or_create_uniqueid() -> Result<String> {
let path = config_dir().join("uniqueid");
let path = pf_paths::config_dir().join("uniqueid");
if let Ok(s) = std::fs::read_to_string(&path) {
let t = s.trim();
if !t.is_empty() {
@@ -496,7 +333,7 @@ fn load_or_create_uniqueid() -> Result<String> {
let id = std::fs::read_to_string("/proc/sys/kernel/random/uuid")
.map(|u| u.trim().replace('-', ""))
.unwrap_or_else(|_| format!("{:016x}{:016x}", std::process::id(), HTTP_PORT));
std::fs::create_dir_all(config_dir()).ok();
std::fs::create_dir_all(pf_paths::config_dir()).ok();
std::fs::write(&path, &id).with_context(|| format!("write {}", path.display()))?;
Ok(id)
}
@@ -512,7 +349,7 @@ fn primary_local_ip() -> Option<IpAddr> {
/// Where the paired-client allow-list persists (survives host restarts, like Sunshine).
fn paired_path() -> Option<std::path::PathBuf> {
// Same dir as the host identity (HOME/.config/punktfunk on Linux, %APPDATA%\punktfunk on Windows).
Some(config_dir().join("paired.json"))
Some(pf_paths::config_dir().join("paired.json"))
}
/// Load the persisted paired-client certificate DERs (empty on first run / parse failure).
@@ -541,7 +378,7 @@ fn load_paired() -> Vec<Vec<u8>> {
pub(crate) fn save_paired(paired: &[Vec<u8>]) {
let Some(path) = paired_path() else { return };
if let Some(dir) = path.parent() {
let _ = create_private_dir(dir);
let _ = pf_paths::create_private_dir(dir);
}
let bytes = match serde_json::to_vec(paired) {
Ok(b) => b,
@@ -553,7 +390,7 @@ pub(crate) fn save_paired(paired: &[Vec<u8>]) {
// Write to a sibling temp file (owner-only, so a local user can't tamper the allow-list), then
// rename over the target (atomic replace on Unix and Windows). Never write `path` in place.
let tmp = path.with_extension("json.tmp");
if let Err(e) = write_secret_file(&tmp, &bytes) {
if let Err(e) = pf_paths::write_secret_file(&tmp, &bytes) {
tracing::warn!(error = %e, "persisting pairings failed (temp write)");
return;
}
@@ -565,24 +402,24 @@ pub(crate) fn save_paired(paired: &[Vec<u8>]) {
#[cfg(all(test, unix))]
mod tests {
use super::{create_private_dir, write_secret_file};
use std::os::unix::fs::PermissionsExt;
#[test]
fn secrets_are_written_owner_only() {
let dir = std::env::temp_dir().join(format!("pf-secret-test-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
create_private_dir(&dir).expect("create private dir");
pf_paths::create_private_dir(&dir).expect("create private dir");
let dmode = std::fs::metadata(&dir).unwrap().permissions().mode() & 0o777;
assert_eq!(dmode, 0o700, "config dir must be owner-only (0700)");
let key = dir.join("key.pem");
write_secret_file(&key, b"-----BEGIN PRIVATE KEY-----\n...").expect("write secret");
pf_paths::write_secret_file(&key, b"-----BEGIN PRIVATE KEY-----\n...")
.expect("write secret");
let fmode = std::fs::metadata(&key).unwrap().permissions().mode() & 0o777;
assert_eq!(fmode, 0o600, "private key must be owner-only (0600)");
// Overwriting an existing secret keeps it 0600 (the truncate+reopen path).
write_secret_file(&key, b"new contents").expect("rewrite secret");
pf_paths::write_secret_file(&key, b"new contents").expect("rewrite secret");
let fmode = std::fs::metadata(&key).unwrap().permissions().mode() & 0o777;
assert_eq!(fmode, 0o600);
let _ = std::fs::remove_dir_all(&dir);