diff --git a/src/RemSound.App/CheckedListAccessibility.cs b/src/RemSound.App/CheckedListAccessibility.cs index 2c39053..13e32c4 100644 --- a/src/RemSound.App/CheckedListAccessibility.cs +++ b/src/RemSound.App/CheckedListAccessibility.cs @@ -17,7 +17,7 @@ internal static class CheckedListAccessibility var lastIndex = 0; void Update(int? overrideIndex = null, bool? overrideChecked = null) - => SetStatus(list, statusLabel, itemKind, overrideIndex, overrideChecked); + => ApplyStatus(list, statusLabel, itemKind, overrideIndex, overrideChecked); void RestoreFocus() { @@ -67,12 +67,26 @@ internal static class CheckedListAccessibility Update(); } - // Exact copy of MainForm.UpdateCheckedListStatus so the spoken text is word-for-word identical. - private static void SetStatus(CheckedListBox list, Label statusLabel, string itemKind, int? overrideIndex, bool? overrideChecked) + /// The empty-list spoken text for a given item kind. The remembered-applications list + /// teaches its own lifecycle here (entries arrive when you TICK an app), since after clearing it + /// there is otherwise no cue how it refills; every other list is the plain "none available". + /// One home so the main window and the dialogs can never drift (they did once — a MainForm-only + /// copy of this branch, 2026-07-27). + public static string EmptyTextFor(string itemKind) => + itemKind == "remembered application" + ? "No remembered application. Tick an application in the list above and it will be remembered here." + : $"No {itemKind}s available."; + + /// THE single builder+applier for a CheckedListBox's spoken status. MainForm delegates + /// here too, so the wording is word-for-word identical in the main window and every dialog by + /// construction — not by a comment promising it. Writes the text into the status label and the + /// list's AccessibleDescription (and the label's, for a focused non-empty item) so NVDA reads + /// the item AND its checked state. + public static void ApplyStatus(CheckedListBox list, Label statusLabel, string itemKind, int? overrideIndex = null, bool? overrideChecked = null) { if (list.Items.Count == 0) { - var emptyText = $"No {itemKind}s available."; + var emptyText = EmptyTextFor(itemKind); statusLabel.Text = emptyText; list.AccessibleDescription = emptyText; return; diff --git a/src/RemSound.App/MainForm.Peers.cs b/src/RemSound.App/MainForm.Peers.cs index 235f624..5f54959 100644 --- a/src/RemSound.App/MainForm.Peers.cs +++ b/src/RemSound.App/MainForm.Peers.cs @@ -12,8 +12,10 @@ namespace RemSound.App; /// The PEER half of the main window — state, discovery/selection reconciliation, arming, naming and /// the remembered-peers plumbing — split out of MainForm.cs verbatim in the 2026-07-26 review's /// god-object shrink (same partial-class pattern Andre's SensorReadout form uses). Pure code motion: -/// same class, same members, no behaviour change — the compiler proves it. The shared LOGIC these -/// methods lean on (PeerArming, CaptureSpecBuilder, PeerAddress) already lives in Core. +/// same class, same members, no behaviour change — the compiler proves it. The shared arming/address +/// LOGIC these methods lean on (PeerArming, PeerAddress) lives in Core; the send-spec builder +/// (CaptureSpecBuilder) stays in App by necessity — it depends on App's AudioDefaultFollower and the +/// Sender's app enumerator — but is likewise shared with the service. /// public sealed partial class MainForm { diff --git a/src/RemSound.App/MainForm.cs b/src/RemSound.App/MainForm.cs index 7e39413..cfb4718 100644 --- a/src/RemSound.App/MainForm.cs +++ b/src/RemSound.App/MainForm.cs @@ -9648,31 +9648,12 @@ public sealed partial class MainForm : Form else Restore(); } + // Delegates to the ONE shared builder (CheckedListAccessibility.ApplyStatus) so the main + // window and the dialogs speak word-for-word identical status text — including the + // remembered-apps empty-state lifecycle line. (These used to be two hand-kept copies that + // drifted, 2026-07-27.) private static void UpdateCheckedListStatus(CheckedListBox list, Label statusLabel, string itemKind, int? overrideIndex = null, bool? overrideChecked = null) - { - if (list.Items.Count == 0) - { - // The remembered-apps list teaches its own lifecycle when empty (Ed, 2026-07-27: after - // clearing it he had no way to know how entries come back — they arrive when you TICK - // an app, so the empty state says exactly that, right where the question arises). - var emptyText = itemKind == "remembered application" - ? "No remembered application. Tick an application in the list above and it will be remembered here." - : $"No {itemKind}s available."; - statusLabel.Text = emptyText; - list.AccessibleDescription = emptyText; - return; - } - - var index = overrideIndex ?? (list.SelectedIndex >= 0 ? list.SelectedIndex : 0); - index = Math.Clamp(index, 0, list.Items.Count - 1); - var isChecked = overrideChecked ?? list.GetItemChecked(index); - var checkedText = isChecked ? "checked" : "not checked"; - var itemText = list.Items[index]?.ToString() ?? itemKind; - var text = $"{checkedText}, {itemText}. Item {index + 1} of {list.Items.Count}. Press Space to toggle."; - statusLabel.Text = text; - list.AccessibleDescription = text; - statusLabel.AccessibleDescription = text; - } + => CheckedListAccessibility.ApplyStatus(list, statusLabel, itemKind, overrideIndex, overrideChecked); /// /// Makes a NumericUpDown's text content fully selected whenever the control receives focus, diff --git a/src/RemSound.App/SelfTest.cs b/src/RemSound.App/SelfTest.cs index dce4cb7..85c23b9 100644 --- a/src/RemSound.App/SelfTest.cs +++ b/src/RemSound.App/SelfTest.cs @@ -43,6 +43,17 @@ internal static class SelfTest private static string Skip(string why) => throw new StepSkipped(why); + /// Non-overlapping count of in — + /// used by the log-rotation integrity checks to prove each line survives exactly once. + private static int CountOccurrences(string haystack, string needle) + { + if (string.IsNullOrEmpty(needle)) return 0; + var count = 0; + for (var i = haystack.IndexOf(needle, StringComparison.Ordinal); i >= 0; i = haystack.IndexOf(needle, i + needle.Length, StringComparison.Ordinal)) + count++; + return count; + } + public static int Run(string[] args) { var seconds = int.TryParse(ValueAfter(args, "--seconds"), out var s) && s is > 0 and <= 30 ? s : 3; @@ -2367,16 +2378,40 @@ internal static class SelfTest 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"); + // A FUTURE-dated command (sender clock ahead, or a crafted timestamp) beyond the skew window + // must also be rejected — the guard bounds skew in BOTH directions, not just the past. + var future = ControlSealing.Seal(key, RemoteControlKind.VolumeUp, 5, nowSecs + 3600); + Check(!new ControlReceiveGuard().TryAccept(key, future, now, out _, out _, out var whyFuture) && whyFuture.StartsWith("stale"), + "a command dated an hour in the FUTURE must be rejected (skew is bounded both ways)"); + // Replay cache under a sustained burst: 300 distinct genuine commands (> the 256 prune + // threshold) must all be accepted once and the dict must not reject a fresh nonce as a + // false replay — i.e. pruning must never evict an entry still inside the skew window. + var burstGuard = new ControlReceiveGuard(); + var accepted = 0; + for (var i = 0; i < 300; i++) + { + // Vary the timestamp by a second each so every packet has a distinct nonce+content. + var cmd = ControlSealing.Seal(key, RemoteControlKind.VolumeUp, 1, nowSecs - (i % 60)); + if (burstGuard.TryAccept(key, cmd, now, out _, out _, out _)) accepted++; + } + Check(accepted == 300, $"every one of 300 distinct genuine commands must be accepted across the prune threshold (got {accepted})"); - // Nonce discipline: prefix constant per sequence, counter strictly advancing, instances differ. + // Nonce discipline: 48-bit random prefix (bytes 0-5) constant within a sequence, 48-bit + // little-endian counter (bytes 6-11) advancing, and — the cross-session safety that matters + // — two independent instances draw DIFFERENT prefixes so their counter-0 nonces can't collide + // under the same long-lived key. 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)"); + Check(n1.AsSpan(0, 6).SequenceEqual(n2.AsSpan(0, 6)), "the 6-byte prefix must stay constant within a sequence"); + Check(n1[6] == 0 && n2[6] == 1, "the counter (byte 6, little-endian) must advance arithmetically"); + var seqB = new RemSoundCrypto.NonceSequence(); + var m1 = new byte[12]; seqB.FillNext(m1); + Check(!n1.AsSpan(0, 6).SequenceEqual(m1.AsSpan(0, 6)), + "two independent NonceSequence instances must draw different prefixes (else same-key counter-0 nonces collide)"); - return "sealed + replay/stale/wrong-key/plaintext all rejected; skew tolerated; nonces counter-based"; + return "sealed + replay/stale/wrong-key/plaintext all rejected; skew tolerated; nonce prefix 48-bit + per-instance-distinct"; } /// Release signing (2026-07-27): the updater refuses any release zip whose detached @@ -2408,7 +2443,12 @@ internal static class SelfTest var realKeyPath = Environment.GetEnvironmentVariable("REMSOUND_SIGNING_KEY") ?? @"D:\Dropbox\proj\rsound key\remsound-signing-key.pem"; if (!File.Exists(realKeyPath)) - return "mechanics proven with ephemeral key; embedded key parses (publisher key not on this machine — embed-match check skipped)"; + // Honest SKIP, not a caveat-PASS: the crypto mechanics above passed, but this step's + // headline job — proving the on-disk private key matches the EMBEDDED public key — did + // not run without the publisher key, and must show in the SKIPPED line rather than read + // as "verified". (The real safety net is build-release.ps1's --sign-update self-check, + // which aborts the publish on a mismatch; this step is the early-warning on the dev box.) + return Skip("publisher signing key not on this machine — embed-match check did not run (mechanics passed; publish pipeline self-checks regardless)"); var realSig = UpdateSignature.SignWithKey(data, File.ReadAllText(realKeyPath)); Check(UpdateSignature.Verify(data, realSig), "the on-disk private key MUST match the embedded public key — a mismatch ships a release every updater refuses"); @@ -2547,19 +2587,28 @@ internal static class SelfTest /// priority-mode scope decision — levers only while streaming, released after the hold-down. private static string? LongRunHygiene() { - // 1. Log rotation at the cap. Tiny cap via the internal seam; verify multiple real files. + // 1. Log rotation at the cap. Tiny cap via the internal seam; verify multiple real files, + // that NO line is lost or duplicated across the rolls (integrity — a dropped last-buffered + // line would slip past a count-only check), and that concurrent writers can't interleave + // (the entire documented reason for writeGate — a single-threaded test never exercises it). var createdLogs = new List(); var log = new RemSoundLog { Enabled = true, RollAfterBytes = 400 }; try { - for (var i = 0; i < 30; i++) + const int lineCount = 120; + for (var i = 0; i < lineCount; i++) { - log.Event($"rotation self-test line {i} — padding padding padding padding"); + log.Event($"ROTLINE#{i}# padding padding padding padding padding"); if (log.Path is { } p && !createdLogs.Contains(p)) createdLogs.Add(p); } - Check(log.RollCount >= 2, $"a 400-byte cap over 30 writes must roll at least twice (rolled {log.RollCount})"); + Check(log.RollCount >= 2, $"a 400-byte cap over {lineCount} writes must roll several times (rolled {log.RollCount})"); Check(createdLogs.Count >= 3, "each roll must continue into a NEW timestamped file"); Check(createdLogs.All(File.Exists), "every rolled file must exist on disk (the chain, not one giant)"); + // Integrity: read the whole chain back and confirm every ROTLINE#i# appears exactly once. + log.Dispose(); // flush + close so the last file is fully readable + var body = string.Concat(createdLogs.Where(File.Exists).Select(File.ReadAllText)); + var missing = Enumerable.Range(0, lineCount).Where(i => CountOccurrences(body, $"ROTLINE#{i}#") != 1).ToList(); + Check(missing.Count == 0, $"every logged line must survive rotation exactly once — {missing.Count} lost/duplicated across the roll"); } finally { @@ -2567,6 +2616,37 @@ internal static class SelfTest foreach (var f in createdLogs) { try { File.Delete(f); } catch { } } } + // 1b. Concurrency: many threads writing through ONE log must never interleave a line (the + // writeGate guarantee). Delete the lock and this is what fails — a count-only test wouldn't. + var concLogs = new List(); + // Cap high enough that 800 short lines never roll — this case is about the writeGate + // interleave guarantee, not rotation, so keep it to one file and capture every path anyway. + var clog = new RemSoundLog { Enabled = true, RollAfterBytes = 4 * 1024 * 1024 }; + try + { + const int threads = 4, perThread = 200; + Parallel.For(0, threads, t => + { + for (var i = 0; i < perThread; i++) + { + clog.Event($"T{t}L{i}-nointerleave-nointerleave-nointerleave"); + if (clog.Path is { } p && !concLogs.Contains(p)) concLogs.Add(p); + } + }); + clog.Dispose(); + var lines = concLogs.Where(File.Exists).SelectMany(File.ReadAllLines).ToList(); + var payload = lines.Where(l => l.Contains("-nointerleave")).ToList(); + Check(payload.Count == threads * perThread, + $"every concurrent write must be its own intact line ({threads * perThread} expected, got {payload.Count} — interleave = torn lines)"); + Check(payload.All(l => CountOccurrences(l, "-nointerleave") == 3), + "no line may contain a fragment of another (torn/interleaved write detector)"); + } + finally + { + clog.Dispose(); + foreach (var f in concLogs) { try { File.Delete(f); } catch { } } + } + // 2. Crash-report cap: 14 fake reports → newest 10 survive. var tmp = Path.Combine(Path.GetTempPath(), "remsound-selftest-crash-" + Guid.NewGuid().ToString("N")); try diff --git a/src/RemSound.Core/RemPacket.cs b/src/RemSound.Core/RemPacket.cs index c958ffe..eba3987 100644 --- a/src/RemSound.Core/RemPacket.cs +++ b/src/RemSound.Core/RemPacket.cs @@ -12,9 +12,11 @@ public enum RemPacketType : byte /// Remote-control message from one connected peer to another. Currently used to let a /// peer adjust the receiver-side volume slider on a peer it's connected to (so a user /// who's NVDA-Remote'd into another machine can still nudge the listening volume on - /// the machine they're physically at). Wire format: 1 byte - /// + 1 byte signed delta (interpreted as signed sbyte; range -128..127, percent points). - /// Old peers see "unknown packet type" and silently drop, so adding this is wire-safe. + /// the machine they're physically at). Since 5.6 the payload is SEALED ( + /// .Core ControlSealing): on the wire it is the 12-byte header + a GCM blob of + /// ControlSealing.SealedPayloadBytes. The 1-byte kind + 1-byte signed delta + /// () is now only the INNER plaintext inside that blob, never + /// travelling in the clear. Old peers see "unknown packet type" and silently drop. /// Control = 5, // 6-9 are the relay's lobby types (hello / roster / full / bye) — relay-side, not modelled here. @@ -112,9 +114,10 @@ public static class RemPacket /// public const int HeartbeatPayloadSize = 9; /// - /// Control payload: 1 byte + 1 signed byte delta. Total - /// 2 bytes, plus the 12-byte header = 14 bytes on the wire. See - /// for the rationale. + /// Control INNER plaintext: 1 byte + 1 signed byte delta = 2 bytes. + /// Since 5.6 this does NOT travel on the wire on its own — it's sealed by ControlSealing, and the + /// on-wire Control payload is ControlSealing.SealedPayloadBytes (nonce+tag over this plaintext plus + /// a timestamp). See . /// public const int ControlPayloadSize = 2; /// diff --git a/src/RemSound.Core/RemSoundCrypto.cs b/src/RemSound.Core/RemSoundCrypto.cs index 9fa7995..b2a53b2 100644 --- a/src/RemSound.Core/RemSoundCrypto.cs +++ b/src/RemSound.Core/RemSoundCrypto.cs @@ -164,25 +164,39 @@ public static class RemSoundCrypto 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. + /// The nonce generator for a hot-path encryptor: a random 48-bit prefix chosen at + /// construction plus a 48-bit counter, filling the 96-bit nonce. Two independent guarantees: + /// WITHIN one instance uniqueness is arithmetic (the counter, 2^48 packets ≈ tens of thousands + /// of years at our rates — effectively unlimited per session); ACROSS instances (every launch, + /// profile reload, and the two BothIndependent lanes all rebuild the sequence under the SAME + /// long-lived audio key) the random prefix keeps their counter ranges apart, with a birthday + /// bound at ~2^24 instances — negligible for any realistic number of app launches on one + /// password. + /// + /// Why 48/48 and not the interim 32/64: a fresh instance restarts the counter at 0, so two + /// instances that drew the SAME prefix would reuse nonce=prefix‖0,‖1,… under the same key — + /// catastrophic for AES-GCM. A 32-bit prefix collides at only ~2^16 instances (a real risk + /// over a multi-year, one-password lifetime); widening the prefix to 48 bits pushes that to + /// ~2^24 while 48 counter bits stay far beyond any session's packet count. This is strictly + /// safer than BOTH the interim scheme and the original per-packet 96-bit-random nonce (whose + /// birthday bound a heavy multi-day streamer could actually approach). The receiver just reads + /// the nonce off the packet, so the wire is unchanged. 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; + private const int PrefixBytes = 6; // 48-bit per-instance random prefix + private const int CounterBytes = NonceBytes - PrefixBytes; // 6 → 48-bit counter + private readonly byte[] prefix = new byte[PrefixBytes]; + private ulong counter; // only the low 48 bits are used (see CounterBytes) public NonceSequence() => RandomNumberGenerator.Fill(prefix); - /// Write the next 12-byte nonce: prefix(4) || counter(8), then advance. + /// Write the next 12-byte nonce: prefix(6) ‖ counter(6, little-endian), then advance. public void FillNext(Span nonce12) { prefix.CopyTo(nonce12); - System.Buffers.Binary.BinaryPrimitives.WriteUInt64LittleEndian(nonce12[4..], counter); - counter++; + var c = counter++; + for (var i = 0; i < CounterBytes; i++) nonce12[PrefixBytes + i] = (byte)(c >> (8 * i)); } }