From 60de506f66b3cb87de9d88dff9bd0f89633f6ff2 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Mon, 29 Jun 2026 09:02:12 +0000 Subject: [PATCH] fix(host/gamestream): correct the rsa-Marvin (S7) rationale + cap pairing signatures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Red-team found the .cargo/audit.toml justification for RUSTSEC-2023-0071 was materially wrong: it claimed "Marvin targets decryption, so the vulnerable path isn't exercised" — but the advisory is a variable-time modexp of the secret exponent, which RSA *signing* (signing_key.sign) also runs. The accept is still correct, for the RIGHT reasons (no decryption/padding oracle; the signed serversecret is host-random not attacker-chosen; signing is operator-PIN-gated; GameStream is off by default and the native QUIC plane uses rustls, not rsa; Moonlight mandates RSA-2048 so the GameStream key can't move off it). Rewrite the rationale accordingly. Also shut the timing-sample amplifier the review surfaced: the pairing session was never marked after phase 3, so a peer past phase 1 could loop phase2/phase3 to harvest many RSA signing-time samples. Sign exactly once per ceremony (reject a repeated serverchallengeresp). Co-Authored-By: Claude Opus 4.8 (1M context) --- .cargo/audit.toml | 26 ++++++++++++++----- .../punktfunk-host/src/gamestream/pairing.rs | 13 ++++++++++ 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/.cargo/audit.toml b/.cargo/audit.toml index a2c475f..517486f 100644 --- a/.cargo/audit.toml +++ b/.cargo/audit.toml @@ -13,11 +13,25 @@ [advisories] ignore = [ - # rsa "Marvin Attack" — a timing sidechannel in RSA *decryption* (PKCS#1 v1.5 padding oracle). - # There is NO fixed rsa release (the constant-time rewrite is still unreleased upstream), and rsa - # is required for GameStream/Moonlight pairing. Crucially, the host uses rsa ONLY for PKCS#1 v1.5 - # SIGNING / VERIFYING (gamestream/cert.rs + gamestream/pairing.rs: SigningKey / VerifyingKey / - # Signer / Verifier) — it never performs RSA decryption, which is the operation Marvin targets. - # So the vulnerable code path is not exercised. Revisit if a fixed rsa ships or we add RSA decrypt. + # rsa "Marvin Attack" (RUSTSEC-2023-0071): a timing side-channel in the rsa crate's variable-time + # modular exponentiation of the SECRET exponent. IMPORTANT — this affects the RSA private-key op in + # general, INCLUDING signing (m^d mod n), which the host DOES perform (gamestream/pairing.rs + # `signing_key.sign(&serversecret)`). It is NOT, as an earlier version of this note wrongly claimed, + # limited to decryption — so "the vulnerable path isn't exercised" is false; signing exercises it. + # We accept it because the attack is not practically reachable here, NOT because the path is unused: + # * No RSA decryption / PKCS#1v1.5 padding oracle exists anywhere (every `decrypt` in the tree is + # AES/AES-GCM), so the classic Bleichenbacher/Marvin chosen-ciphertext oracle is absent. + # * The only signed message (`serversecret`) is HOST-generated random, never attacker-chosen — so + # there's no adaptive chosen-input probing (the lever remote RSA-timing key recovery needs); and + # signing is gated behind the operator-entered pairing PIN, ONE signature per ceremony (a + # repeated phase-3 is rejected — gamestream/pairing.rs — to deny a passive timing-sample harvester). + # * GameStream is OFF by default (bare `serve` is native-only); the secure native QUIC plane uses + # rustls' constant-time backend, NOT the rsa crate. RSA is touched only on the opt-in, + # trusted-LAN GameStream/Moonlight pairing handshake. Moonlight mandates RSA-2048, so the + # GameStream identity cannot move to Ed25519/ECDSA (only the native identity could, and it + # already avoids the rsa crate). + # There is NO fixed rsa release (the constant-time rewrite is still unreleased upstream). Revisit if: + # a constant-time rsa ships (then drop this), the host ever signs an attacker-chosen message with + # this key, or any RSA decryption / key-transport using the private key is added. "RUSTSEC-2023-0071", ] diff --git a/crates/punktfunk-host/src/gamestream/pairing.rs b/crates/punktfunk-host/src/gamestream/pairing.rs index 0f85f37..841b14e 100644 --- a/crates/punktfunk-host/src/gamestream/pairing.rs +++ b/crates/punktfunk-host/src/gamestream/pairing.rs @@ -101,6 +101,10 @@ struct Session { server_challenge: [u8; 16], /// The client's phase-3 hash, recomputed + checked in phase 4. client_hash: Vec, + /// Set once phase 3 has produced the RSA-signed serversecret. A repeated phase 3 is refused so a + /// peer past phase 1 can't loop phase2/phase3 to harvest many signing-time samples (a passive + /// timing-oracle amplifier vs. the rsa-crate Marvin side-channel; see `.cargo/audit.toml`). + responded: bool, } pub struct Pairing { @@ -155,6 +159,7 @@ impl Pairing { serversecret: [0; 16], server_challenge: [0; 16], client_hash: Vec::new(), + responded: false, }, ); tracing::info!( @@ -216,6 +221,14 @@ impl Pairing { bail!("short challenge response"); } s.client_hash = client_hash[..32].to_vec(); + // Sign the serversecret exactly ONCE per ceremony: refuse a repeated phase 3 so a peer that + // cleared phase 1 (operator PIN) can't replay it to collect many RSA signing-time samples + // (timing-oracle amplifier vs. RUSTSEC-2023-0071; see `.cargo/audit.toml`). A legit client + // signs once. The session stays for phase 4 (the cert-pin step) but won't re-sign. + if s.responded { + bail!("serverchallengeresp already answered for this pairing session"); + } + s.responded = true; let sig: Signature = id.signing_key.sign(&s.serversecret); let mut secret = Vec::with_capacity(16 + 256); secret.extend_from_slice(&s.serversecret);