diff --git a/src/RemSound.App/MainForm.cs b/src/RemSound.App/MainForm.cs index 5101c63..51c9653 100644 --- a/src/RemSound.App/MainForm.cs +++ b/src/RemSound.App/MainForm.cs @@ -8881,16 +8881,24 @@ public sealed partial class MainForm : Form private void SendRemoteControl(RemoteControlKind kind, sbyte delta) { if (!connected) return; + // No password → no key → no remote control, same mandatory-encryption rule as audio. The + // command is SEALED with the audio key (2026-07-26 security audit): only a password-holder + // can drive a peer's volume — a forged source IP is no longer enough. + if (currentAudioKey is not { } key) + { + logFile.Event($"remote-control NOT sent (no profile password set) kind={kind}"); + return; + } var endpoints = SelectedSendEndpoints(); if (endpoints.Length == 0) return; - Span packet = stackalloc byte[RemPacket.HeaderSize + RemPacket.ControlPayloadSize]; + var sealedPayload = ControlSealing.Seal(key, kind, delta, DateTimeOffset.UtcNow.ToUnixTimeSeconds()); + var bytes = new byte[RemPacket.HeaderSize + sealedPayload.Length]; // streamId 0xFFFE for control packets (heartbeat already uses 0xFFFF). Distinct value // makes diag logs easier to read; the receiver doesn't actually filter on it. var seq = unchecked((uint)Interlocked.Increment(ref remoteControlSequence)); - RemPacket.WriteHeader(packet, RemPacketType.Control, 0xFFFE, seq); - RemPacket.WriteControlPayload(packet[RemPacket.HeaderSize..], kind, delta); - var bytes = packet.ToArray(); + RemPacket.WriteHeader(bytes, RemPacketType.Control, 0xFFFE, seq); + sealedPayload.CopyTo(bytes, RemPacket.HeaderSize); var sentTo = 0; foreach (var ep in endpoints) @@ -8908,6 +8916,9 @@ public sealed partial class MainForm : Form } private int remoteControlSequence; + // Receiver-side gate for sealed control commands: authenticates against the profile key, bounds + // clock skew, and blocks replays of captured packets. UI-thread only (see the BeginInvoke above). + private readonly ControlReceiveGuard remoteControlGuard = new(); /// /// Handler for incoming Control packets. Runs on the network thread — marshal to UI before @@ -8917,16 +8928,18 @@ public sealed partial class MainForm : Form /// even when playback is currently off, because the next time the user enables receive /// they'll hear it at the right level. /// - private void HandleRemoteControlPacket(RemoteControlKind kind, sbyte delta, IPEndPoint remote) + private void HandleRemoteControlPacket(byte[] sealedPayload, IPEndPoint remote) { // Marshal to the UI thread FIRST. The allow-list scan reads selectedPeerEndpoints — a plain // Dictionary owned and mutated by the UI thread — so enumerating it here on the receiver's // network thread races a concurrent peer tick/untick (a caught "collection was modified" that - // silently drops the remote command). Running the whole check on the UI thread removes the race. + // silently drops the remote command). Running the whole check on the UI thread removes the race + // (and gives the replay guard single-threaded access). BeginInvoke(() => { // Allow-list match by IP only — the sender's source port is their ephemeral outbound, - // not their announced audio port. + // not their announced audio port. This is a coarse first gate; the REAL authentication + // is the seal check below (source IPs are forgeable, the audio key is not). var allowed = false; foreach (var ep in selectedPeerEndpoints.Values) { @@ -8934,12 +8947,26 @@ public sealed partial class MainForm : Form } if (!allowed) { - logFile.Event($"remote-control IGNORED (not in allow-list) kind={kind} delta={delta} from={remote}"); + logFile.Event($"remote-control IGNORED (not in allow-list) from={remote}"); return; } if (!settings.LoadAcceptRemoteVolumeCommands()) { - logFile.Event($"remote-control IGNORED (Accept remote volume commands is off) kind={kind} delta={delta} from={remote}"); + logFile.Event($"remote-control IGNORED (Accept remote volume commands is off) from={remote}"); + return; + } + // Authenticate: the command must decrypt with OUR profile key (proves the sender knows + // the password), be fresh, and not be a replayed capture. 2026-07-26 security audit — + // spoofing an allowed IP was previously enough to drive system volume/mute, and muting + // a blind user's machine mutes their screen reader. + if (currentAudioKey is not { } key) + { + logFile.Event($"remote-control IGNORED (no profile password set) from={remote}"); + return; + } + if (!remoteControlGuard.TryAccept(key, sealedPayload, DateTime.UtcNow, out var kind, out var delta, out var why)) + { + logFile.Event($"remote-control REJECTED ({why}) from={remote}"); return; } diff --git a/src/RemSound.App/SelfTest.cs b/src/RemSound.App/SelfTest.cs index fae70bf..044655f 100644 --- a/src/RemSound.App/SelfTest.cs +++ b/src/RemSound.App/SelfTest.cs @@ -112,6 +112,8 @@ internal static class SelfTest RunStep(results, "Main window builds where process-loopback is unsupported (Win7 launch, issue #22)", Win7SendModeConstruction); RunStep(results, "Menu shortcuts don't clash with controls", MenuShortcutsDontClashWithControls); RunStep(results, "Service log discovery (newest activity log)", ServiceLogDiscovery); + RunStep(results, "Sealed remote control (auth + replay + skew) + nonce discipline", SealedRemoteControl); + RunStep(results, "Service folder lockdown args (cross-user LPE hardening)", ServiceDirHardeningArgs); var failed = results.Count(r => r.Status == "FAIL"); var skipped = results.Count(r => r.Status == "SKIP"); @@ -2322,6 +2324,84 @@ internal static class SelfTest return "reachable armed; long-unreachable dropped; grace kept; carve-out honoured; signature order-free"; } + /// 2026-07-26 security audit: remote-control commands are SEALED with the audio key — + /// a forged source IP is no longer enough to drive a peer's system volume/mute (which, for our + /// users, mutes the screen reader). Pins: seal/accept round-trip, wrong-key reject, tampered + /// reject, legacy-plaintext reject, replay reject, stale reject — and the counter-based nonce + /// discipline that removes the random-nonce birthday bound on long-lived audio keys. + private static string? SealedRemoteControl() + { + var key = RemSoundCrypto.DeriveKey("test-password"); + var wrongKey = RemSoundCrypto.DeriveKey("other-password"); + var now = new DateTime(2026, 7, 26, 3, 0, 0, DateTimeKind.Utc); + var nowSecs = new DateTimeOffset(now).ToUnixTimeSeconds(); + + var guard = new ControlReceiveGuard(); + var sealedOk = ControlSealing.Seal(key, RemoteControlKind.SystemMuteToggle, 0, nowSecs); + Check(sealedOk.Length == ControlSealing.SealedPayloadBytes, "sealed payload must be the documented wire size"); + Check(guard.TryAccept(key, sealedOk, now, out var kind, out _, out _) && kind == RemoteControlKind.SystemMuteToggle, + "a genuine sealed command must authenticate and round-trip its kind"); + Check(!guard.TryAccept(key, sealedOk, now, out _, out _, out var whyReplay) && whyReplay.StartsWith("replay"), + "the SAME packet accepted twice is a replay and must be rejected"); + + Check(!new ControlReceiveGuard().TryAccept(wrongKey, ControlSealing.Seal(key, RemoteControlKind.VolumeUp, 5, nowSecs), now, out _, out _, out _), + "a command sealed with a different password must not authenticate"); + var tampered = ControlSealing.Seal(key, RemoteControlKind.VolumeUp, 5, nowSecs); + tampered[^1] ^= 0x01; + Check(!new ControlReceiveGuard().TryAccept(key, tampered, now, out _, out _, out _), "a tampered payload must fail the auth tag"); + + Span legacy = stackalloc byte[RemPacket.ControlPayloadSize]; + RemPacket.WriteControlPayload(legacy, RemoteControlKind.SystemVolumeUp, 5); + Check(!new ControlReceiveGuard().TryAccept(key, legacy, now, out _, out _, out _), + "a legacy plaintext control payload must be rejected, never acted on"); + + var stale = ControlSealing.Seal(key, RemoteControlKind.VolumeDown, 5, nowSecs - 3600); + Check(!new ControlReceiveGuard().TryAccept(key, stale, now, out _, out _, out var whyStale) && whyStale.StartsWith("stale"), + "a captured command replayed an hour later must be rejected as stale"); + var skewed = ControlSealing.Seal(key, RemoteControlKind.VolumeDown, 5, nowSecs - 300); + Check(new ControlReceiveGuard().TryAccept(key, skewed, now, out _, out _, out _), + "a command from a peer whose clock is 5 minutes off must still be accepted"); + + // Nonce discipline: prefix constant per sequence, counter strictly advancing, instances differ. + var seqA = new RemSoundCrypto.NonceSequence(); + var n1 = new byte[12]; var n2 = new byte[12]; + seqA.FillNext(n1); seqA.FillNext(n2); + Check(!n1.AsSpan().SequenceEqual(n2), "consecutive nonces from one sequence must differ"); + Check(n1.AsSpan(0, 4).SequenceEqual(n2.AsSpan(0, 4)), "the 4-byte prefix must stay constant within a sequence"); + Check(n2[4] == 1 && n1[4] == 0, "the counter half must advance arithmetically (uniqueness by construction)"); + + return "sealed + replay/stale/wrong-key/plaintext all rejected; skew tolerated; nonces counter-based"; + } + + /// The icacls contract for the service-folder lockdown (cross-user LPE fix): inheritance + /// stripped (kills the CREATOR OWNER hole), exactly SYSTEM + Administrators + the installing user + /// granted, recursive. And the SID recording round-trips with garbage rejected. + private static string? ServiceDirHardeningArgs() + { + var args = ServiceControl.BuildServiceDirAclArgs(@"C:\ProgramData\RemSound\service", "S-1-5-21-111-222-333-1001"); + Check(args.Contains("/inheritance:r"), "must strip inherited ACEs — the CREATOR OWNER grant is the hole"); + Check(args.Contains("*S-1-5-18:(OI)(CI)F"), "SYSTEM keeps Full (the service runs here)"); + Check(args.Contains("*S-1-5-32-544:(OI)(CI)F"), "Administrators keep Full"); + Check(args.Contains("*S-1-5-21-111-222-333-1001:(OI)(CI)(M)"), "the installing user keeps Modify (profile saves + test builds)"); + Check(args.Contains("/T"), "must re-stamp existing files (app-source.txt is the target of the attack)"); + Check(args.Split("/grant").Length == 4, "exactly three grants — nobody else survives the reset"); + + var savedOverride = ServiceStore.TestDirectoryOverride; + var tmp = Path.Combine(Path.GetTempPath(), "remsound-selftest-sid-" + Guid.NewGuid().ToString("N")); + try + { + ServiceStore.TestDirectoryOverride = tmp; + Check(ServiceStore.LoadInstallingUserSid() is null, "no recorded SID must read back null"); + ServiceStore.SaveInstallingUserSid("S-1-5-21-111-222-333-1001"); + Check(ServiceStore.LoadInstallingUserSid() == "S-1-5-21-111-222-333-1001", "the recorded SID must round-trip"); + ServiceStore.SaveInstallingUserSid("not-a-sid \"quotes\""); + Check(ServiceStore.LoadInstallingUserSid() is null, "garbage in the SID file must be ignored, never fed to icacls"); + } + finally { ServiceStore.TestDirectoryOverride = savedOverride; try { Directory.Delete(tmp, recursive: true); } catch { } } + + return "inheritance stripped; SYSTEM/Admins/installing-user only; SID recorded + garbage-proofed"; + } + /// Issue #23 boot self-heal decision core. Scenario: at the boot lock screen the machine's /// speakers audibly play (Windows tune, NVDA) but a capture attached in the first seconds of boot /// taps an engine mix the logon-session audio was never wired into — the endpoint's own METER shows diff --git a/src/RemSound.App/ServiceControl.cs b/src/RemSound.App/ServiceControl.cs index cfea050..c0e7212 100644 --- a/src/RemSound.App/ServiceControl.cs +++ b/src/RemSound.App/ServiceControl.cs @@ -109,7 +109,7 @@ public static class ServiceControl /// reports success. public static int DoInstall() { - if (IsInstalled()) return 0; + if (IsInstalled()) { HardenServiceDirectory(); return 0; } // re-harden pre-audit installs var exe = Environment.ProcessPath; if (string.IsNullOrEmpty(exe)) return 2; var sourceDir = Path.GetDirectoryName(exe); @@ -120,8 +120,10 @@ public static class ServiceControl try { CopyProgramTo(sourceDir, ServiceStore.BinDirectory); } catch (Exception ex) { ServiceStore.AppendServiceEvent($"install: copy program failed: {ex.GetType().Name}: {ex.Message}"); return 5; } // Remember where the app lives, so the SYSTEM service can watch it and auto-update itself when the - // app's auto-updater drops a newer build there (no UAC — see ServiceUpdate). + // app's auto-updater drops a newer build there (no UAC — see ServiceUpdate). Record the installing + // user's SID alongside it, so the SYSTEM-side re-hardening grants the right account. ServiceStore.SaveAppSourcePath(sourceDir); + ServiceStore.SaveInstallingUserSid(InstallingUserSid()); var rc = RunSc(BuildCreateArgs(ServiceStore.BinExePath)); if (rc != 0) return rc; @@ -138,9 +140,61 @@ public static class ServiceControl // with no UAC. (Trust note: a user-writable folder whose contents run as SYSTEM is the same posture // as the auto-update copy; fine for this app, a hardened build would code-sign instead.) GrantUsersWriteToBin(); + // Close the cross-user escalation the 2026-07-26 security audit found: the SYSTEM service + // trusts app-source.txt (which folder to self-update FROM), and if ANOTHER local user had + // pre-created ProgramData\RemSound\service (e.g. by saving the service config dialog before + // the install), Windows made them its owner with inheritable Full Control — letting them + // repoint the file at a folder of theirs and get their code run as SYSTEM. Reset ownership + // and the ACL here, while elevated, so only SYSTEM, Administrators and the installing user + // remain. Must run AFTER the grants above (inheritance reset re-derives file ACLs). + HardenServiceDirectory(); return 0; } + /// Take ownership of the service's ProgramData folder for Administrators and reset its + /// ACL to exactly SYSTEM + Administrators (Full) + the recorded installing user (Modify — they + /// save the service profile from the non-elevated dialog and drop test builds in bin). Removes + /// any ACE another account picked up by creating the folder first; ownership must move too, + /// because an owner can always rewrite the ACL back. Runs elevated (install) or as SYSTEM + /// (self-update) — both hold take-ownership rights. When called as SYSTEM with no recorded + /// installing-user SID (an install predating this), it SKIPS rather than lock the user out of + /// their own no-admin workflow; the next elevated service action records the SID and hardens. + /// Best-effort with loud logging. + internal static void HardenServiceDirectory() + { + var sid = ServiceStore.LoadInstallingUserSid(); + if (sid is null) + { + var current = InstallingUserSid(); + if (current == "S-1-5-18") + { + ServiceStore.AppendServiceEvent("harden: no installing-user SID recorded and running as SYSTEM — deferred to the next elevated install/update"); + return; + } + sid = current; + ServiceStore.SaveInstallingUserSid(sid); + } + var dir = ServiceStore.Directory; + try { Directory.CreateDirectory(dir); } catch { /* the icacls below will report */ } + // takeown /a → owner becomes the Administrators GROUP (not the current user); /r /d y recurses. + var own = RunProcessCaptured("takeown.exe", $"/f \"{dir}\" /a /r /d y", 30000); + if (!own.Started || !own.Exited || own.ExitCode != 0) + ServiceStore.AppendServiceEvent($"harden: takeown on service dir returned {(own.Exited ? own.ExitCode : -1)}: {own.StdErr}"); + var acl = RunProcessCaptured("icacls.exe", BuildServiceDirAclArgs(dir, sid), 30000); + if (!acl.Started || !acl.Exited || acl.ExitCode != 0) + ServiceStore.AppendServiceEvent($"harden: icacls reset on service dir returned {(acl.Exited ? acl.ExitCode : -1)}: {acl.StdErr}{acl.StdOut}"); + else + ServiceStore.AppendServiceEvent("harden: service folder ownership + ACL locked to SYSTEM/Administrators/installing user"); + } + + /// Pure, testable: the icacls arguments that lock the service folder down. + /// /inheritance:r strips inherited ACEs (ProgramData grants CREATOR OWNER full control — + /// the exact hole); explicit grants only: SYSTEM + Administrators Full, installing user + /// Modify. /T re-stamps existing files (app-source.txt included), /C continues past + /// per-file errors. + internal static string BuildServiceDirAclArgs(string dir, string installingUserSid) => + $"\"{dir}\" /inheritance:r /grant \"*S-1-5-18:(OI)(CI)F\" /grant \"*S-1-5-32-544:(OI)(CI)F\" /grant \"*{installingUserSid}:(OI)(CI)(M)\" /T /C"; + /// The SID to grant the no-admin service rights to: the account that installed it (the /// elevated install runs as the same interactive user with an elevated token, so its SID is that user). /// Scoping the grants to ONE account instead of all Users/Authenticated-Users keeps the effortless @@ -292,6 +346,10 @@ public static class ServiceControl // CopyProgramTo already excludes every user-state folder — same routine the installer uses. try { CopyProgramTo(appDir, ServiceStore.BinDirectory); Log("copied the new build into bin"); } catch (Exception ex) { Log($"COPY FAILED ({ex.GetType().Name}: {ex.Message}) — starting the existing build"); } + // Re-assert the service-folder lockdown on every self-update (we're SYSTEM here, which + // can take ownership too). This is how installs that predate the 2026-07-26 hardening + // pick it up without a reinstall. + try { HardenServiceDirectory(); } catch { /* logged inside; never block the update */ } } else { diff --git a/src/RemSound.Core/ControlSealing.cs b/src/RemSound.Core/ControlSealing.cs new file mode 100644 index 0000000..035422b --- /dev/null +++ b/src/RemSound.Core/ControlSealing.cs @@ -0,0 +1,107 @@ +using System.Buffers.Binary; + +namespace RemSound.Core; + +/// +/// Authenticated remote-control commands (2026-07-26 security audit, finding: Control packets were +/// the ONE thing on the wire that wasn't cryptographically protected — a 2-byte plaintext payload +/// gated only by a forgeable source-IP check. Anyone who learned an allowed peer's address could +/// spoof it and drive the receiving machine's SYSTEM volume/mute — and muting a blind user's +/// machine mutes their screen reader). +/// +/// Control payloads are now sealed with the SAME AES-256-GCM key the audio uses (derived from the +/// profile password), so only a password-holder can issue one. The sealed plaintext carries the +/// command plus the sender's UTC timestamp; the receiving side () +/// rejects anything outside a generous clock-skew window and remembers recent nonces inside it, so +/// a captured packet can't be replayed later to re-toggle mute. Legacy plaintext control packets +/// no longer verify and are dropped — peers on older builds must update before remote volume works +/// again (called out in the release notes). +/// +public static class ControlSealing +{ + /// kind(1) + delta(1) + unix-utc-seconds(8). + private const int PlainBytes = 10; + + /// Sealed wire size: nonce+tag overhead over the 10-byte plaintext (= 38). The receiver + /// uses this to cheaply distinguish a sealed payload from legacy 2-byte plaintext. + public const int SealedPayloadBytes = PlainBytes + RemSoundCrypto.EncryptionOverheadBytes; + + /// Seal a control command. is the profile's audio key — no key, + /// no remote control (same rule as audio: encryption is mandatory). + public static byte[] Seal(byte[] key, RemoteControlKind kind, sbyte delta, long unixUtcSeconds) + { + Span plain = stackalloc byte[PlainBytes]; + plain[0] = (byte)kind; + plain[1] = unchecked((byte)delta); + BinaryPrimitives.WriteInt64LittleEndian(plain[2..], unixUtcSeconds); + return RemSoundCrypto.Encrypt(key, plain); + } + + /// Open a sealed control payload. False when the size is wrong, the auth tag fails + /// (wrong password / tampered / legacy plaintext), or the command byte isn't a known kind. + public static bool TryUnseal(byte[] key, ReadOnlySpan payload, + out RemoteControlKind kind, out sbyte delta, out long unixUtcSeconds, out ulong nonceId) + { + kind = RemoteControlKind.VolumeUp; + delta = 0; + unixUtcSeconds = 0; + nonceId = 0; + if (payload.Length != SealedPayloadBytes) return false; + if (!RemSoundCrypto.TryDecrypt(key, payload, out var plain) || plain.Length != PlainBytes) return false; + if (!RemPacket.TryReadControl(plain, out kind, out delta)) return false; + unixUtcSeconds = BinaryPrimitives.ReadInt64LittleEndian(plain.AsSpan(2)); + // First 8 nonce bytes as the replay id — the nonce is random per seal, so this is unique + // per packet for the guard's purposes (a collision just double-rejects, never double-accepts). + nonceId = BinaryPrimitives.ReadUInt64LittleEndian(payload[..8]); + return true; + } +} + +/// Receiver-side gate for sealed control commands: authenticates (via the key), bounds +/// staleness against clock skew, and blocks replays of captured packets inside the window. One +/// instance per receiving app; not thread-safe (call from one thread — the UI marshal point). +public sealed class ControlReceiveGuard(TimeSpan? maxSkew = null) +{ + /// Generous on purpose: peers' clocks are not synchronised, and a rejected volume + /// command because someone's PC clock drifted eight minutes would be a miserable support case. + /// Within the window, the nonce memory below blocks replays; outside it, staleness rejects. + public static readonly TimeSpan DefaultMaxSkew = TimeSpan.FromMinutes(10); + + private readonly TimeSpan maxSkew = maxSkew ?? DefaultMaxSkew; + private readonly Dictionary seenNonces = []; + + /// Authenticate + accept a sealed control payload. On false, + /// says why in one word (for the log line): not-sealed / stale / replay. + public bool TryAccept(byte[] key, ReadOnlySpan payload, DateTime utcNow, + out RemoteControlKind kind, out sbyte delta, out string rejectReason) + { + delta = 0; + if (!ControlSealing.TryUnseal(key, payload, out kind, out delta, out var ts, out var nonceId)) + { + rejectReason = "not-sealed (wrong password, tampered, or a pre-5.6 peer)"; + return false; + } + var sentUtc = DateTimeOffset.FromUnixTimeSeconds(ts).UtcDateTime; + var age = utcNow - sentUtc; + if (age > maxSkew || age < -maxSkew) + { + rejectReason = $"stale (sent {Math.Abs(age.TotalMinutes):0.0} min from now, window {maxSkew.TotalMinutes:0} min)"; + return false; + } + if (seenNonces.ContainsKey(nonceId)) + { + rejectReason = "replay (nonce already accepted)"; + return false; + } + // Remember this nonce for the life of the skew window; prune opportunistically. Control + // commands are human-rate (hotkey presses), so this stays tiny. + if (seenNonces.Count >= 256) + { + foreach (var stale in seenNonces.Where(kv => utcNow - kv.Value > maxSkew + maxSkew).Select(kv => kv.Key).ToList()) + seenNonces.Remove(stale); + } + seenNonces[nonceId] = utcNow; + rejectReason = ""; + return true; + } +} diff --git a/src/RemSound.Core/RemSoundCrypto.cs b/src/RemSound.Core/RemSoundCrypto.cs index 1dd6cb8..6c71b18 100644 --- a/src/RemSound.Core/RemSoundCrypto.cs +++ b/src/RemSound.Core/RemSoundCrypto.cs @@ -141,17 +141,40 @@ public static class RemSoundCrypto /// Low-allocation encrypt straight into a destination span. Layout written: /// nonce(12) || tag(16) || ciphertext. Returns the number of bytes written /// (= plaintext.Length + ). must - /// be at least that big. Generates a fresh random nonce per call (safe at our packet rates). - public static int EncryptInto(AesGcm gcm, ReadOnlySpan plaintext, Span dst) + /// be at least that big. The nonce comes from — counter-based, so + /// it CANNOT repeat on a long-lived key (see ; 2026-07-26 audit: + /// per-packet random nonces carry a birthday-collision bound a multi-day stream can reach). + public static int EncryptInto(AesGcm gcm, NonceSequence nonces, ReadOnlySpan plaintext, Span dst) { var total = plaintext.Length + EncryptionOverheadBytes; if (dst.Length < total) throw new ArgumentException("Encrypt destination too small", nameof(dst)); - var nonce = dst[..NonceBytes]; - RandomNumberGenerator.Fill(nonce); - gcm.Encrypt(nonce, plaintext, dst.Slice(NonceBytes + TagBytes, plaintext.Length), dst.Slice(NonceBytes, TagBytes)); + nonces.FillNext(dst[..NonceBytes]); + gcm.Encrypt(dst[..NonceBytes], plaintext, dst.Slice(NonceBytes + TagBytes, plaintext.Length), dst.Slice(NonceBytes, TagBytes)); return total; } + /// The nonce generator for a hot-path encryptor: a random 4-byte prefix chosen at + /// construction plus a 64-bit counter — the textbook fix the old per-packet-random comment + /// pointed at. Uniqueness within one instance is arithmetic (the counter), not probabilistic; + /// across instances (each sender lane, each session) the random prefix keeps streams apart. + /// The receiver just reads the nonce from the packet, so this changes nothing on the wire. + /// NOT thread-safe — one per encryptor, same ownership rule as the AesGcm itself. + public sealed class NonceSequence + { + private readonly byte[] prefix = new byte[4]; + private ulong counter; + + public NonceSequence() => RandomNumberGenerator.Fill(prefix); + + /// Write the next 12-byte nonce: prefix(4) || counter(8), then advance. + public void FillNext(Span nonce12) + { + prefix.CopyTo(nonce12); + System.Buffers.Binary.BinaryPrimitives.WriteUInt64LittleEndian(nonce12[4..], counter); + counter++; + } + } + /// Low-allocation decrypt of an packet into a destination /// span. Returns true and the plaintext length on success; false if the packet is too short, /// the destination too small, or the auth tag fails (wrong key / tampered). diff --git a/src/RemSound.Core/ServiceStore.cs b/src/RemSound.Core/ServiceStore.cs index 587ce57..ad9b94f 100644 --- a/src/RemSound.Core/ServiceStore.cs +++ b/src/RemSound.Core/ServiceStore.cs @@ -143,6 +143,31 @@ public static class ServiceStore catch { return null; } } + // === Installing user's SID (for the service-folder ACL hardening) === + // Recorded at elevated install time — the account that owns the no-admin stop/update workflow. + // The SYSTEM-side self-update re-asserts the folder lockdown and must grant THIS user, not + // "whoever I currently am" (which under the service is SYSTEM itself). + private static string InstallingUserSidFile => Path.Combine(Directory, "installing-user.txt"); + + /// Record the installing user's SID. Elevated install path only. Never throws. + public static void SaveInstallingUserSid(string sid) + { + try { System.IO.Directory.CreateDirectory(Directory); File.WriteAllText(InstallingUserSidFile, sid); } + catch { /* best-effort */ } + } + + /// The recorded installing-user SID, or null if none / unreadable. + public static string? LoadInstallingUserSid() + { + try + { + var sid = File.Exists(InstallingUserSidFile) ? File.ReadAllText(InstallingUserSidFile).Trim() : null; + // A SID is S-1-... only; anything else (tampered/corrupt) is ignored rather than fed to icacls. + return sid is not null && sid.StartsWith("S-1-", StringComparison.Ordinal) && sid.Length < 200 ? sid : null; + } + catch { return null; } + } + // === Update log (ALWAYS written, not gated on the service-logging toggle) === // Updates are rare but important, so we always keep a small trail of them where the user can find it. public static string UpdateLogPath => Path.Combine(Directory, "update.log"); diff --git a/src/RemSound.Receiver/AudioReceiver.cs b/src/RemSound.Receiver/AudioReceiver.cs index 27c712d..a565a06 100644 --- a/src/RemSound.Receiver/AudioReceiver.cs +++ b/src/RemSound.Receiver/AudioReceiver.cs @@ -935,13 +935,13 @@ public sealed class AudioReceiver : IDisposable /// socket on either end any more. public Action? OnHeartbeatReceived { get; set; } - /// Hook for Control packets that arrive on the audio receiver's socket. The - /// App wires this to a handler that validates the source against the allow-list (the - /// peer must be in the user's selected-peers set), checks the user's "accept remote - /// volume commands" preference, and applies the requested change to the local volume - /// slider. Set this BEFORE starting the receiver; null = packet is silently dropped. + /// Hook for Control packets that arrive on the audio receiver's socket. Since 5.6 the + /// payload is SEALED with the profile's audio key (ControlSealing), so this hands the RAW payload + /// up — the App authenticates it (key + replay guard), validates the source against the + /// allow-list, checks the user's "accept remote volume commands" preference, and applies the + /// change. Set this BEFORE starting the receiver; null = packet is silently dropped. /// Travels on the same UDP socket as audio + heartbeat (single-port model 2026-05-07). - public Action? OnRemoteControlReceived { get; set; } + public Action? OnRemoteControlReceived { get; set; } private void HandleRawPacket(byte[] packet, int length, IPEndPoint remote) { @@ -975,12 +975,15 @@ public sealed class AudioReceiver : IDisposable OnHeartbeatReceived?.Invoke(packet, length, remote); break; case RemPacketType.Control: - // Remote-control message (volume up/down, mute toggle). Parse the payload - // here so the handler doesn't need to know about RemPacket layout. Caller - // is expected to gate on allow-list AND the user's opt-in preference. - if (RemPacket.TryReadControl(payload, out var ctrlKind, out var ctrlDelta)) + // Remote-control message (volume up/down, mute toggle). Since 5.6 the payload is + // SEALED with the profile's audio key (ControlSealing) — the receiver stays + // crypto-dumb and hands the raw payload up; the App authenticates it against its + // key + replay guard, then gates on allow-list AND the user's opt-in preference. + // A legacy 2-byte plaintext payload (pre-5.6 peer) fails the size check here and + // is dropped — an unauthenticated command must never reach the handler. + if (payload.Length == ControlSealing.SealedPayloadBytes) { - OnRemoteControlReceived?.Invoke(ctrlKind, ctrlDelta, remote); + OnRemoteControlReceived?.Invoke(payload.ToArray(), remote); } else { diff --git a/src/RemSound.Sender/SenderLane.cs b/src/RemSound.Sender/SenderLane.cs index afcec77..cb037f6 100644 --- a/src/RemSound.Sender/SenderLane.cs +++ b/src/RemSound.Sender/SenderLane.cs @@ -46,6 +46,7 @@ internal sealed class SenderLane // cipherScratch holds the per-frame ciphertext (plaintext + 28 bytes overhead); 4096 covers // the largest single frame (Opus 20 ms or PCM 5 ms) with room to spare. private AesGcm? cryptoGcm; + private RemSoundCrypto.NonceSequence? cryptoNonces; private byte[]? cryptoKeyCached; private readonly byte[] cipherScratch = new byte[4096]; @@ -317,7 +318,7 @@ internal sealed class SenderLane // Encrypt the whole PCM frame, then split the ciphertext across as many parts as the // Ethernet payload budget needs (the +28-byte crypto overhead can push a 5 ms frame over // a single datagram). The receiver reassembles the parts and then decrypts. - var ctLen = RemSoundCrypto.EncryptInto(cryptoGcm, int24, cipherScratch); + var ctLen = RemSoundCrypto.EncryptInto(cryptoGcm, cryptoNonces!, int24, cipherScratch); var maxPart = RemPacket.MaxAudioPayloadBytes; var totalParts = (byte)((ctLen + maxPart - 1) / maxPart); pcmFrameId++; @@ -353,7 +354,7 @@ internal sealed class SenderLane } EnsureCrypto(); if (cryptoGcm is null) return; // no password yet → never send audio in the clear - var ctLen = RemSoundCrypto.EncryptInto(cryptoGcm, opusPlainScratch.AsSpan(0, encLen), cipherScratch); + var ctLen = RemSoundCrypto.EncryptInto(cryptoGcm, cryptoNonces!, opusPlainScratch.AsSpan(0, encLen), cipherScratch); Interlocked.Increment(ref audioFramesSent); SendAudio(cipherScratch.AsSpan(0, ctLen)); } @@ -403,6 +404,9 @@ internal sealed class SenderLane if (ReferenceEquals(key, cryptoKeyCached)) return; cryptoGcm?.Dispose(); cryptoGcm = key is null ? null : RemSoundCrypto.CreateGcm(key); + // Fresh nonce sequence with the fresh cipher: new random prefix, counter from zero — + // a rebuilt key never continues an old counter, and an old key never sees a reused one. + cryptoNonces = key is null ? null : new RemSoundCrypto.NonceSequence(); cryptoKeyCached = key; }