fix(security): anti-replay, 0600 client key, open redirect, supply-chain
Address findings from a repo security review:
- core: add a sliding-window anti-replay filter over the AEAD-authenticated
sequence in Session (poll_input/poll_frame), closing the input-replay gap the
data plane previously left to the LAN/VPN trust assumption. 4096-deep window,
unit-tested; the encrypted loopback suite confirms no false drops.
- clients: write the mTLS client private key 0600 and lock the config dir 0700
on Unix (it was world-readable at the umask default), re-locking existing
stores on load. pf-client-core::trust plus the probe's own identity writer.
Windows keeps the %APPDATA% ACL; Android/Apple already wrap the key.
- web: fix a post-login open redirect — resolve `next` via URL and require it to
stay same-origin, rejecting `/\evil.com` and tab/encoding variants the old
`!startsWith("//")` guard missed. Also fixes the dead safeNextPath helper.
- ci: SHA-256-pin the BtbN FFmpeg DLLs bundled into the signed Windows installer
(were fetched from the rolling `latest` tag unverified); fails closed on a
re-roll, matching the VB-CABLE gate.
- ci: fail-open fork-guard on the Windows/Apple host-mode PR build jobs that
share runner labels with the signing jobs. Definitive fix stays server-side
(Gitea outside-collaborator approval / isolated PR runners) — see the notes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -34,16 +34,61 @@ pub fn load_or_create_identity() -> Result<(String, String)> {
|
||||
let dir = config_dir()?;
|
||||
let (cp, kp) = (dir.join("client-cert.pem"), dir.join("client-key.pem"));
|
||||
if let (Ok(c), Ok(k)) = (std::fs::read_to_string(&cp), std::fs::read_to_string(&kp)) {
|
||||
// An older build wrote the key with a plain `fs::write`, which honors the umask and
|
||||
// typically lands 0644 — world-readable. Re-lock an existing store on load so upgrades
|
||||
// get fixed, not just fresh installs. Best-effort (a read-only store keeps what it has).
|
||||
#[cfg(unix)]
|
||||
lock_identity_perms(&dir, &kp);
|
||||
return Ok((c, k));
|
||||
}
|
||||
let (c, k) = endpoint::generate_identity().map_err(|e| anyhow!("generate identity: {e}"))?;
|
||||
std::fs::create_dir_all(&dir)?;
|
||||
// The private key authorizes this client for full remote control of a paired host, so it must
|
||||
// never be world-readable: lock the dir to the owner (0700) and create the key 0600 from the
|
||||
// start (`fs::write` alone honors the umask → typically 0644). The certificate is public. On
|
||||
// non-Unix the %APPDATA% profile ACL already scopes the dir to the user, so std perms suffice.
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700))?;
|
||||
}
|
||||
std::fs::write(&cp, &c)?;
|
||||
std::fs::write(&kp, &k)?;
|
||||
write_private_key(&kp, k.as_bytes())?;
|
||||
tracing::info!(cert = %cp.display(), "generated client identity");
|
||||
Ok((c, k))
|
||||
}
|
||||
|
||||
/// Write the client's mTLS private key owner-only. On Unix the file is created with mode 0600 from
|
||||
/// the outset — an `fs::write` + later `chmod` would briefly expose it at the umask default. On
|
||||
/// other platforms std's default perms plus the %APPDATA% profile ACL scope it to the user.
|
||||
fn write_private_key(path: &std::path::Path, bytes: &[u8]) -> Result<()> {
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::io::Write;
|
||||
use std::os::unix::fs::OpenOptionsExt;
|
||||
let mut f = std::fs::OpenOptions::new()
|
||||
.write(true)
|
||||
.create(true)
|
||||
.truncate(true)
|
||||
.mode(0o600)
|
||||
.open(path)?;
|
||||
f.write_all(bytes)?;
|
||||
}
|
||||
#[cfg(not(unix))]
|
||||
std::fs::write(path, bytes)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Best-effort re-lock of an already-present identity (dir 0700, key 0600) — for stores written by
|
||||
/// an older build that left the key world-readable. Errors are ignored: the worst case is the
|
||||
/// pre-existing perms, which this never loosens.
|
||||
#[cfg(unix)]
|
||||
fn lock_identity_perms(dir: &std::path::Path, key: &std::path::Path) {
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
let _ = std::fs::set_permissions(dir, std::fs::Permissions::from_mode(0o700));
|
||||
let _ = std::fs::set_permissions(key, std::fs::Permissions::from_mode(0o600));
|
||||
}
|
||||
|
||||
pub fn hex(fp: &[u8; 32]) -> String {
|
||||
fp.iter().map(|b| format!("{b:02x}")).collect()
|
||||
}
|
||||
|
||||
@@ -28,15 +28,18 @@ pub struct Frame {
|
||||
/// One end of a stream. Constructed for a single [`Role`]; calling the other role's
|
||||
/// methods returns [`PunktfunkError::InvalidArg`].
|
||||
///
|
||||
/// Note: the AEAD layer authenticates each datagram but does **not** provide anti-replay.
|
||||
/// Video replays are largely absorbed by the reassembler's per-frame dedup, but replayed
|
||||
/// input events are not yet filtered. A sliding-window replay filter keyed on the
|
||||
/// authenticated sequence belongs with the pairing/handshake layer (the GameStream host); until then,
|
||||
/// rely on the LAN/VPN transport assumption (plan §1).
|
||||
/// Anti-replay: the receive path runs each opened datagram's AEAD-authenticated sequence through a
|
||||
/// sliding-window filter ([`ReplayWindow`]), so a captured, validly-sealed datagram can't be replayed
|
||||
/// by an on-path attacker — closing the input-replay gap that previously rested solely on the
|
||||
/// LAN/VPN transport assumption (plan §1). Genuine reordering within the window is still accepted;
|
||||
/// video additionally benefits from the reassembler's per-frame dedup.
|
||||
pub struct Session {
|
||||
config: Config,
|
||||
coder: Box<dyn ErasureCoder>,
|
||||
crypto: Option<SessionCrypto>,
|
||||
/// Anti-replay window over the peer's authenticated sequence (receive side). `Some` exactly when
|
||||
/// `crypto` is — the plaintext probe path carries no sequence to filter on.
|
||||
replay: Option<ReplayWindow>,
|
||||
transport: Box<dyn Transport>,
|
||||
packetizer: Packetizer,
|
||||
reassembler: Reassembler,
|
||||
@@ -68,11 +71,15 @@ impl Session {
|
||||
let crypto = config
|
||||
.encrypt
|
||||
.then(|| SessionCrypto::new(&config.key, config.salt, config.role));
|
||||
// A receive-side replay window exists exactly when the datagrams are sealed (they carry the
|
||||
// authenticated sequence the window keys on). Both roles receive from their peer.
|
||||
let replay = config.encrypt.then(ReplayWindow::new);
|
||||
let packetizer = Packetizer::new(&config);
|
||||
let reassembler = Reassembler::new(ReassemblerLimits::from_config(&config));
|
||||
Ok(Session {
|
||||
coder,
|
||||
crypto,
|
||||
replay,
|
||||
transport,
|
||||
packetizer,
|
||||
reassembler,
|
||||
@@ -130,6 +137,16 @@ impl Session {
|
||||
}
|
||||
}
|
||||
|
||||
/// Feed an opened datagram's authenticated sequence to the anti-replay window: `true` = fresh
|
||||
/// (accept), `false` = a replay or older than the window (drop). Returns `true` when the session
|
||||
/// isn't encrypting (no window, and no sequence on the wire to key on).
|
||||
fn accept_seq(&mut self, seq: u64) -> bool {
|
||||
match self.replay.as_mut() {
|
||||
Some(w) => w.accept(seq),
|
||||
None => true,
|
||||
}
|
||||
}
|
||||
|
||||
// -- Host path --------------------------------------------------------
|
||||
|
||||
/// Host: FEC-protect, packetize, and seal one encoded access unit into wire packets WITHOUT
|
||||
@@ -225,6 +242,13 @@ impl Session {
|
||||
Ok(p) => p,
|
||||
Err(_) => continue, // drop undecryptable noise
|
||||
};
|
||||
// Anti-replay: a captured input datagram replayed by an on-path attacker opens cleanly
|
||||
// (its sequence + tag are still valid) — the window is what rejects the second copy.
|
||||
// `len >= 8` is guaranteed because the sealed-path open above succeeded.
|
||||
if self.replay.is_some() && !self.accept_seq(seq_of(&wire)) {
|
||||
StatsCounters::add(&self.stats.packets_dropped, 1);
|
||||
continue;
|
||||
}
|
||||
StatsCounters::add(&self.stats.packets_received, 1);
|
||||
if let Some(ev) = InputEvent::decode(&pkt) {
|
||||
return Ok(Some(ev));
|
||||
@@ -276,6 +300,13 @@ impl Session {
|
||||
Ok(p) => p,
|
||||
Err(_) => continue,
|
||||
};
|
||||
// Anti-replay (same rationale as poll_input): reject a datagram whose authenticated
|
||||
// sequence was already seen. Video also dedups per-frame downstream, but filtering here
|
||||
// is uniform and cheap. `len >= 8` because the sealed-path open above succeeded.
|
||||
if self.replay.is_some() && !self.accept_seq(seq_of(&self.recv_scratch[i][..len])) {
|
||||
StatsCounters::add(&self.stats.packets_dropped, 1);
|
||||
continue;
|
||||
}
|
||||
StatsCounters::add(&self.stats.packets_received, 1);
|
||||
StatsCounters::add(&self.stats.bytes_received, pkt.len() as u64);
|
||||
// The reassembler validates the packet via its parsed header (`magic`),
|
||||
@@ -347,3 +378,162 @@ impl Session {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
/// Extract the AEAD-authenticated 8-byte big-endian sequence prefix from a sealed wire datagram.
|
||||
/// Only called on the encrypted receive path, where a preceding successful open has already
|
||||
/// established `wire.len() >= 8`.
|
||||
fn seq_of(wire: &[u8]) -> u64 {
|
||||
u64::from_be_bytes(wire[..8].try_into().unwrap())
|
||||
}
|
||||
|
||||
/// Depth of the anti-replay window, in sequences. The sender advances its sequence once per
|
||||
/// datagram, so at the data plane's packet rate 4096 is roughly 33 ms of reorder tolerance for the
|
||||
/// video stream (well beyond any reordering still useful for a live frame) and effectively unbounded
|
||||
/// for the sparse input stream — while bounding how far back a replay could hide.
|
||||
const REPLAY_WINDOW: u64 = 4096;
|
||||
const REPLAY_WORDS: usize = (REPLAY_WINDOW / 64) as usize;
|
||||
|
||||
/// Sliding-window anti-replay filter over the AEAD-authenticated wire sequence. The sender counts
|
||||
/// its datagrams from 0, and the protocol never legitimately re-sends a sequence (FEC recovery
|
||||
/// shards get fresh ones), so a sequence seen twice is a replay. The AEAD tag already authenticates
|
||||
/// the sequence — a forged one can't open — so this only has to reject *duplicates* of validly
|
||||
/// sealed datagrams (and anything older than the window, which we can no longer prove is fresh).
|
||||
/// Genuine reordering within the window is accepted. Bitmap-per-sequence, indexed `seq % WINDOW`.
|
||||
struct ReplayWindow {
|
||||
/// Highest sequence accepted so far; `seen` stays false until the first datagram.
|
||||
highest: u64,
|
||||
seen: bool,
|
||||
/// One bit per in-window sequence in `(highest - WINDOW, highest]`.
|
||||
bits: [u64; REPLAY_WORDS],
|
||||
}
|
||||
|
||||
impl ReplayWindow {
|
||||
fn new() -> ReplayWindow {
|
||||
ReplayWindow {
|
||||
highest: 0,
|
||||
seen: false,
|
||||
bits: [0; REPLAY_WORDS],
|
||||
}
|
||||
}
|
||||
|
||||
#[inline]
|
||||
fn word_bit(seq: u64) -> (usize, u64) {
|
||||
let idx = (seq % REPLAY_WINDOW) as usize;
|
||||
(idx / 64, 1u64 << (idx % 64))
|
||||
}
|
||||
fn is_set(&self, seq: u64) -> bool {
|
||||
let (w, b) = Self::word_bit(seq);
|
||||
self.bits[w] & b != 0
|
||||
}
|
||||
fn set(&mut self, seq: u64) {
|
||||
let (w, b) = Self::word_bit(seq);
|
||||
self.bits[w] |= b;
|
||||
}
|
||||
fn unset(&mut self, seq: u64) {
|
||||
let (w, b) = Self::word_bit(seq);
|
||||
self.bits[w] &= !b;
|
||||
}
|
||||
|
||||
/// Record `seq`, returning `true` if it's fresh (accept) or `false` if it's a replay / too old.
|
||||
fn accept(&mut self, seq: u64) -> bool {
|
||||
if !self.seen {
|
||||
self.seen = true;
|
||||
self.highest = seq;
|
||||
self.set(seq);
|
||||
return true;
|
||||
}
|
||||
if seq > self.highest {
|
||||
// Advance the window. Sequences between the old and new high slide in unseen, so clear
|
||||
// their (possibly stale, from a full window ago) slots — unless we jumped an entire
|
||||
// window, in which case wipe the bitmap wholesale.
|
||||
if seq - self.highest >= REPLAY_WINDOW {
|
||||
self.bits = [0; REPLAY_WORDS];
|
||||
} else {
|
||||
let mut s = self.highest + 1;
|
||||
while s < seq {
|
||||
self.unset(s);
|
||||
s += 1;
|
||||
}
|
||||
}
|
||||
self.highest = seq;
|
||||
self.set(seq);
|
||||
true
|
||||
} else if self.highest - seq >= REPLAY_WINDOW || self.is_set(seq) {
|
||||
// Older than the window (can't prove it isn't a replay) or already seen (a duplicate) —
|
||||
// either way, drop it.
|
||||
false
|
||||
} else {
|
||||
self.set(seq); // in-window and not yet seen — a genuine reorder
|
||||
true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod replay_tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn accepts_in_order_and_rejects_duplicates() {
|
||||
let mut w = ReplayWindow::new();
|
||||
for seq in 0..1000 {
|
||||
assert!(w.accept(seq), "fresh in-order seq {seq} must be accepted");
|
||||
}
|
||||
// Every one of those is now a replay.
|
||||
for seq in 0..1000 {
|
||||
assert!(!w.accept(seq), "replayed seq {seq} must be rejected");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn accepts_reorder_within_window_once() {
|
||||
let mut w = ReplayWindow::new();
|
||||
assert!(w.accept(100));
|
||||
// Earlier-but-in-window sequences (a genuine reorder) are accepted exactly once.
|
||||
assert!(w.accept(80));
|
||||
assert!(!w.accept(80), "second copy of a reordered seq is a replay");
|
||||
assert!(w.accept(99));
|
||||
assert!(!w.accept(100), "the high-water seq itself can't be replayed");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_older_than_window() {
|
||||
let mut w = ReplayWindow::new();
|
||||
assert!(w.accept(REPLAY_WINDOW * 2));
|
||||
// Anything a full window or more behind the high-water mark is dropped (can't prove fresh).
|
||||
assert!(!w.accept(REPLAY_WINDOW * 2 - REPLAY_WINDOW));
|
||||
assert!(!w.accept(0));
|
||||
// But just inside the window is still accepted.
|
||||
assert!(w.accept(REPLAY_WINDOW * 2 - (REPLAY_WINDOW - 1)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn large_forward_jump_wipes_stale_bits() {
|
||||
let mut w = ReplayWindow::new();
|
||||
assert!(w.accept(5));
|
||||
// Jump far forward (more than a window). The slot for an old seq that aliases 5 mod WINDOW
|
||||
// must read as unseen afterward, i.e. the jump cleared it — so a NEW seq there is accepted.
|
||||
let far = 10 * REPLAY_WINDOW + 5;
|
||||
assert!(w.accept(far));
|
||||
assert!(!w.accept(5), "the pre-jump seq is now far older than the window");
|
||||
// A fresh seq aliasing 5 (mod WINDOW) but inside the new window is accepted, proving the
|
||||
// stale bit was cleared rather than mistaken for a replay.
|
||||
assert!(w.accept(far - REPLAY_WINDOW + 1));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn first_seq_need_not_be_zero() {
|
||||
// Startup loss can mean the first datagram we ever open isn't seq 0.
|
||||
let mut w = ReplayWindow::new();
|
||||
assert!(w.accept(42));
|
||||
assert!(!w.accept(42));
|
||||
assert!(w.accept(43));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn seq_of_reads_the_big_endian_prefix() {
|
||||
let mut wire = 0x0102_0304_0506_0708u64.to_be_bytes().to_vec();
|
||||
wire.extend_from_slice(b"ciphertext-and-tag");
|
||||
assert_eq!(seq_of(&wire), 0x0102_0304_0506_0708);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user