Talon/RemSound
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Security phase 1: sealed remote control + service-folder lockdown + counter nonces

Browse Source 
From the 2026-07-26 security audit, the two must-fix findings plus the crypto tidy:

1. Remote-control commands are now SEALED with the profile's audio key (AES-256-GCM,
   ControlSealing in Core). Previously a 2-byte plaintext payload gated only by a
   forgeable source-IP check - anyone who learned an allowed peer's address could
   drive the receiving machine's SYSTEM volume/mute, and muting a blind user's
   machine mutes their screen reader. Now only a password-holder can issue a command;
   a ControlReceiveGuard also bounds clock skew (10 min) and remembers nonces so a
   captured packet can't be replayed to re-toggle mute. Legacy plaintext control from
   pre-5.6 peers is dropped at the receiver (never acted on) - release notes must say
   both ends need 5.6 for remote volume.

2. Cross-user LPE closed: the SYSTEM service trusts app-source.txt to decide what to
   copy+run on self-update, and ProgramData lets any user who pre-created the service
   folder own it (CREATOR OWNER inheritance) and repoint that file. Elevated install
   now records the installing user's SID, takes ownership for Administrators and
   resets the ACL to exactly SYSTEM + Administrators + installing user (takeown +
   icacls /inheritance:r). Re-asserted on every SYSTEM self-update so existing
   installs pick it up; as SYSTEM with no recorded SID it defers rather than lock the
   user out of their no-admin workflow.

3. Audio-path GCM nonces are now counter-based per lane (random 4-byte prefix +
   64-bit counter, fresh sequence with every key rebuild) - unique by arithmetic,
   removing the random-nonce birthday bound on a long-lived key. Wire format
   unchanged; the receiver reads the nonce from the packet as before.

New gate steps: sealed-control auth/replay/stale/wrong-key/plaintext matrix + nonce
discipline; service-folder lockdown args + SID recording garbage-proofing.
Gate 63/63.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Ednunp
2026-07-26 23:34:49 +01:00
co-authored by Claude Fable 5
parent 6f91e0ed5e
commit 9a18d18656
8 changed files with 356 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/RemSound.App/MainForm.cs
+36 -9
View File
@@ -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<byte> 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();
/// <summary>
/// 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.
/// </summary>
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;
}
src/RemSound.App/SelfTest.cs
+80
View File
@@ -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";
}
/// <summary>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.</summary>
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<byte> 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";
}
/// <summary>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.</summary>
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";
}
/// <summary>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
src/RemSound.App/ServiceControl.cs
+60 -2
View File
@@ -109,7 +109,7 @@ public static class ServiceControl
/// reports success.</summary>
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;
}
/// <summary>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.</summary>
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");
}
/// <summary>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.</summary>
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";
/// <summary>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
{