Test gaps 4-6: relay unit tests, updater-refusal, password walk-through
Closing the coverage gaps the review flagged as blind spots we'd be relying on at release: 4. RELAY LOGIC TESTS. server/test_relay.py (stdlib unittest + a FakeSocket, no network) covers the address-proof end to end: cookie issued on join, wrong cookie rejected, right cookie verifies once; enforce mode WITHHOLDS forwarding from an unverified address then delivers after it proves itself; watch-only forwards but records would-block; the per-IP cap counts across BOTH v1 and v2; a NAT-rebind clears verification (spoof-takeover guard); a forged BYE from another address can't evict the victim; and bad/short/unknown-version headers are refused. Wired into run-tests.ps1 (Start-Process from server\, SKIPs loudly if no Python) so a relay change can no longer ship past the gate untested. The relay had ZERO automated coverage before and auto-updates every user. 5. UPDATER SIGNATURE ENFORCEMENT. Extracted the two refusal branches into a pure VerifyStagedRelease gate and added UpdaterRefusesUnsignedRelease: no-sig refused, wrong-key refused, garbage refused, tamper (good sig over changed bytes) refused, genuine release accepted. ReleaseSigning only proved the crypto; this proves the updater actually REFUSES - the hijacked-release-stream threat. 6. STREAMING PASSWORD STRENGTHENING. The accept decision is now a pure ProfilePasswordDialog.RejectionAdviceFor shared by BOTH password dialogs (also fixes the App-review trim inconsistency - manager dialog compared untrimmed). Test pins the load-bearing rule: requireStrong DISABLES the unchanged-exemption so an existing weak "Games" can't keep streaming, while casual mode still grandfathers an unchanged password and blocks a new weak one, trim-safe. Plus the NVDA-hang cache assertions in PasswordRules (miss->hit, same-instance repeat, Prewarm, empty/weak = no work). Gate 71/71 + 7 relay tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -13,6 +13,21 @@ namespace RemSound.App;
|
||||
/// </summary>
|
||||
internal static class ProfilePasswordDialog
|
||||
{
|
||||
/// <summary>The pure "should this password entry be rejected, and why" decision, shared by both
|
||||
/// password dialogs and unit-testable without any UI (2026-07-27). Returns the plain-English
|
||||
/// advice to show, or null to accept. Rules: an empty entry is not judged here (the
|
||||
/// requireNonEmpty gate owns that); a CHANGED entry is always judged; an UNCHANGED entry is
|
||||
/// exempt UNLESS <paramref name="requireStrong"/> — the streaming gate's mode, where the whole
|
||||
/// point is that the current password already failed the rule, so re-entering it must be
|
||||
/// refused. Both sides are compared trimmed (fixes the App-review trim inconsistency).</summary>
|
||||
internal static string? RejectionAdviceFor(string entered, string current, bool requireStrong)
|
||||
{
|
||||
entered = entered.Trim();
|
||||
if (entered.Length == 0) return null;
|
||||
if (!requireStrong && string.Equals(entered, current.Trim(), StringComparison.Ordinal)) return null;
|
||||
return RemSound.Core.PasswordStrength.Critique(entered);
|
||||
}
|
||||
|
||||
public static string? Show(string profileTitle, string currentPassword, bool requireNonEmpty = false, bool requireStrong = false)
|
||||
{
|
||||
var (dialog, textBox) = Build(profileTitle, currentPassword, requireNonEmpty, requireStrong);
|
||||
@@ -92,15 +107,9 @@ internal static class ProfilePasswordDialog
|
||||
textBox.SelectAll();
|
||||
return;
|
||||
}
|
||||
// Strength gate (2026-07-27, with the derivation-cost raise). Normally NEW or CHANGED
|
||||
// passwords only — re-accepting the existing password unchanged passes, so an old weak
|
||||
// password never traps the user inside a casual visit to this dialog. requireStrong is
|
||||
// the STREAMING gate's mode: there the whole point is that the current password failed
|
||||
// the rule, so the unchanged exemption is off and a stronger one must be entered before
|
||||
// audio can flow (Ed, 2026-07-27). The critique text says exactly what to do instead.
|
||||
if (entered.Length > 0
|
||||
&& (requireStrong || !string.Equals(entered, currentPassword.Trim(), StringComparison.Ordinal))
|
||||
&& RemSound.Core.PasswordStrength.Critique(entered) is { } advice)
|
||||
// Strength gate (2026-07-27) — the decision lives in the pure RejectionAdviceFor so a
|
||||
// test can pin it without a modal dialog (and both password dialogs share one rule).
|
||||
if (RejectionAdviceFor(entered, currentPassword, requireStrong) is { } advice)
|
||||
{
|
||||
var page = new TaskDialogPage
|
||||
{
|
||||
|
||||
@@ -86,10 +86,9 @@ internal static class ProfilePasswordManagerDialog
|
||||
{
|
||||
foreach (var (title, original, box) in rows)
|
||||
{
|
||||
var entered = box.Text.Trim();
|
||||
if (entered.Length > 0
|
||||
&& !string.Equals(entered, original, StringComparison.Ordinal)
|
||||
&& PasswordStrength.Critique(entered) is { } advice)
|
||||
// Same shared decision as the single-password dialog (casual mode: unchanged is
|
||||
// exempt, changed-and-weak is refused) — one rule at every door, compared trimmed.
|
||||
if (ProfilePasswordDialog.RejectionAdviceFor(box.Text, original, requireStrong: false) is { } advice)
|
||||
{
|
||||
var page = new TaskDialogPage
|
||||
{
|
||||
|
||||
@@ -217,27 +217,19 @@ internal sealed class RemSoundUpdater
|
||||
await src.CopyToAsync(dst, token).ConfigureAwait(false);
|
||||
}
|
||||
|
||||
// Signature enforcement (2026-07-27): the zip must carry a valid signature by the
|
||||
// embedded release key, or it is NOT installed — this is what stops a compromised
|
||||
// release stream (e.g. a hijacked GitHub account) from silently shipping code to
|
||||
// every user. Missing signature = refused too: every genuine release from 5.6 on is
|
||||
// signed by build-release.ps1, so "no .sig asset" is itself a red flag, not a legacy
|
||||
// case (older releases are BELOW this version and the updater never downgrades).
|
||||
if (string.IsNullOrEmpty(info.SignatureUrl))
|
||||
{
|
||||
Log?.Invoke("updater: REFUSED — release has no signature file; a genuine RemSound release always ships one. Install left untouched.");
|
||||
TryDeleteDirectory(stageRoot);
|
||||
return false;
|
||||
}
|
||||
var signatureBase64 = await http.GetStringAsync(info.SignatureUrl, token).ConfigureAwait(false);
|
||||
// Signature enforcement (2026-07-27) — the release must carry a valid signature by the
|
||||
// embedded key or it is NOT installed. The decision is a pure gate (VerifyStagedRelease)
|
||||
// so a test can pin the control flow — the thing ReleaseSigning's crypto test can't see —
|
||||
// independently of the HTTP/Process machinery around it.
|
||||
var signatureBase64 = string.IsNullOrEmpty(info.SignatureUrl)
|
||||
? null
|
||||
: await http.GetStringAsync(info.SignatureUrl, token).ConfigureAwait(false);
|
||||
var zipBytes = await File.ReadAllBytesAsync(zipPath, token).ConfigureAwait(false);
|
||||
if (!UpdateSignature.Verify(zipBytes, signatureBase64))
|
||||
if (!VerifyStagedRelease(zipBytes, info.SignatureUrl, signatureBase64, Log))
|
||||
{
|
||||
Log?.Invoke("updater: REFUSED — the release signature does not verify (tampered download or not signed by the RemSound release key). Install left untouched.");
|
||||
TryDeleteDirectory(stageRoot);
|
||||
TryDeleteDirectory(stageRoot); // refused → leave the install untouched
|
||||
return false;
|
||||
}
|
||||
Log?.Invoke("updater: release signature verified");
|
||||
|
||||
Log?.Invoke($"updater: extracting to {appDir}");
|
||||
System.IO.Compression.ZipFile.ExtractToDirectory(zipPath, appDir, overwriteFiles: true);
|
||||
@@ -291,6 +283,30 @@ internal sealed class RemSoundUpdater
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>The pure signature gate: may this downloaded release be installed? False (refuse,
|
||||
/// install left untouched) when there is no signature asset, or the signature doesn't verify
|
||||
/// against the embedded release key — the two branches that stop a hijacked release stream from
|
||||
/// shipping code to every user. Extracted from <see cref="DownloadAndStageInstallAsync"/> so the
|
||||
/// control flow is unit-testable apart from the HTTP/extract/Process machinery (the crypto alone
|
||||
/// is covered elsewhere; this pins that the updater actually REFUSES). A missing signature is
|
||||
/// refused, not tolerated: every genuine release from 5.6 on is signed, and the updater never
|
||||
/// downgrades, so "no .sig" is a red flag, not a legacy case.</summary>
|
||||
internal static bool VerifyStagedRelease(byte[] zipBytes, string? signatureUrl, string? signatureBase64, Action<string>? log)
|
||||
{
|
||||
if (string.IsNullOrEmpty(signatureUrl) || string.IsNullOrEmpty(signatureBase64))
|
||||
{
|
||||
log?.Invoke("updater: REFUSED — release has no signature file; a genuine RemSound release always ships one. Install left untouched.");
|
||||
return false;
|
||||
}
|
||||
if (!UpdateSignature.Verify(zipBytes, signatureBase64))
|
||||
{
|
||||
log?.Invoke("updater: REFUSED — the release signature does not verify (tampered download or not signed by the RemSound release key). Install left untouched.");
|
||||
return false;
|
||||
}
|
||||
log?.Invoke("updater: release signature verified");
|
||||
return true;
|
||||
}
|
||||
|
||||
/// <summary>If the zip extracted to a single subfolder (typical when GitHub zips a tag),
|
||||
/// return that subfolder so the copy works from the inner level. Otherwise return the
|
||||
/// staging dir itself.</summary>
|
||||
|
||||
@@ -129,6 +129,8 @@ internal static class SelfTest
|
||||
RunStep(results, "Service startup volume (boot-once decision + settings round-trip)", ServiceStartupVolume);
|
||||
RunStep(results, "Update install window (same-day, wraparound, retry timing)", UpdateInstallWindow);
|
||||
RunStep(results, "Release signing (verify, tamper, key-embed match)", ReleaseSigning);
|
||||
RunStep(results, "Updater refuses an unsigned or badly-signed release (enforcement flow)", UpdaterRefusesUnsignedRelease);
|
||||
RunStep(results, "Streaming password strengthening (existing weak password forced up)", StreamingPasswordStrengthening);
|
||||
RunStep(results, "Password strength rules (gate + derivation refusal)", PasswordRules);
|
||||
RunStep(results, "Relay address-proof echo (AddrCheck round-trip)", RelayAddrCheckEcho);
|
||||
|
||||
@@ -2455,6 +2457,76 @@ internal static class SelfTest
|
||||
return "round-trip + tamper + wrong-key + garbage all correct; on-disk private key matches the embedded public key";
|
||||
}
|
||||
|
||||
/// <summary>The updater's signature ENFORCEMENT control flow (2026-07-27) — the piece
|
||||
/// ReleaseSigning's crypto test can't see: that the updater actually REFUSES a release with no
|
||||
/// signature and one whose signature doesn't verify, and only proceeds on a genuine one. Guards
|
||||
/// the hijacked-release-stream threat the signing was built for.</summary>
|
||||
private static string? UpdaterRefusesUnsignedRelease()
|
||||
{
|
||||
var zip = new byte[8192];
|
||||
new Random(99).NextBytes(zip);
|
||||
|
||||
// No signature asset at all → refused (a genuine 5.6+ release always ships one).
|
||||
Check(!RemSoundUpdater.VerifyStagedRelease(zip, signatureUrl: null, signatureBase64: null, log: null),
|
||||
"a release with NO signature asset must be refused");
|
||||
Check(!RemSoundUpdater.VerifyStagedRelease(zip, signatureUrl: "https://x/RemSound-v9.9.zip.sig", signatureBase64: null, log: null),
|
||||
"a signature URL that fetched nothing must be refused");
|
||||
|
||||
// A signature by a DIFFERENT key (an attacker's, or a tampered download) → refused.
|
||||
using (var attacker = System.Security.Cryptography.ECDsa.Create(System.Security.Cryptography.ECCurve.NamedCurves.nistP256))
|
||||
{
|
||||
var forged = UpdateSignature.SignWithKey(zip, attacker.ExportECPrivateKeyPem());
|
||||
Check(!RemSoundUpdater.VerifyStagedRelease(zip, "https://x/z.sig", forged, null),
|
||||
"a release signed by a NON-release key must be refused (the hijack case)");
|
||||
}
|
||||
Check(!RemSoundUpdater.VerifyStagedRelease(zip, "https://x/z.sig", "not-valid-base64", null),
|
||||
"a garbage signature must be refused, not throw");
|
||||
|
||||
// A genuine signature by the embedded release key → accepted. Only producible with the
|
||||
// on-disk private key (Ed's box / this session); elsewhere the accept branch is noted skipped.
|
||||
var realKeyPath = Environment.GetEnvironmentVariable("REMSOUND_SIGNING_KEY") ?? @"D:\Dropbox\proj\rsound key\remsound-signing-key.pem";
|
||||
if (!File.Exists(realKeyPath))
|
||||
return "no-sig + wrong-key + garbage all refused (genuine-accept branch needs the publisher key — noted)";
|
||||
var good = UpdateSignature.SignWithKey(zip, File.ReadAllText(realKeyPath));
|
||||
Check(RemSoundUpdater.VerifyStagedRelease(zip, "https://x/RemSound-v9.9.zip.sig", good, null),
|
||||
"a release genuinely signed by the release key must be accepted");
|
||||
// And the SAME good signature over TAMPERED bytes must be refused (integrity, end to end).
|
||||
var tamperedZip = (byte[])zip.Clone();
|
||||
tamperedZip[0] ^= 0xFF;
|
||||
Check(!RemSoundUpdater.VerifyStagedRelease(tamperedZip, "https://x/z.sig", good, null),
|
||||
"a valid signature over DIFFERENT bytes must be refused (download tamper)");
|
||||
return "no-sig + wrong-key + garbage + tamper all refused; a genuine release accepted";
|
||||
}
|
||||
|
||||
/// <summary>The streaming password-strengthening walk-through (2026-07-27): an EXISTING weak
|
||||
/// password must be forced up before audio flows — the load-bearing bit is that the streaming
|
||||
/// prompt runs with requireStrong, which DISABLES the "unchanged password is exempt" rule, so
|
||||
/// re-entering the same weak password is refused. Pins the dialog decision the pure Critique
|
||||
/// test can't see.</summary>
|
||||
private static string? StreamingPasswordStrengthening()
|
||||
{
|
||||
// Streaming mode (requireStrong: true) — the exemption is DISABLED, so re-entering the same
|
||||
// weak password is refused and only a strong replacement is accepted. This is the bit that,
|
||||
// if it regressed to the casual rule, would let "Games" keep streaming and defeat the whole
|
||||
// 5.6 password raise.
|
||||
Check(ProfilePasswordDialog.RejectionAdviceFor("Games", current: "Games", requireStrong: true) is not null,
|
||||
"streaming mode must REFUSE re-entering the same weak password (no unchanged-exemption)");
|
||||
Check(ProfilePasswordDialog.RejectionAdviceFor("kettle9tiger42moon", current: "Games", requireStrong: true) is null,
|
||||
"a strong replacement must be accepted in streaming mode");
|
||||
|
||||
// Casual mode (requireStrong: false) — an UNCHANGED existing password is grandfathered (a
|
||||
// visit that doesn't touch it must not trap the user behind the new rule)...
|
||||
Check(ProfilePasswordDialog.RejectionAdviceFor("Games", current: "Games", requireStrong: false) is null,
|
||||
"casual mode must let an UNCHANGED existing password through");
|
||||
// ...but a NEW weak password is still refused, and trailing whitespace doesn't fool the
|
||||
// unchanged comparison (both sides trimmed — the App-review inconsistency is gone).
|
||||
Check(ProfilePasswordDialog.RejectionAdviceFor("Games", current: "kettle9tiger42moon", requireStrong: false) is not null,
|
||||
"casual mode must still block a NEW weak password");
|
||||
Check(ProfilePasswordDialog.RejectionAdviceFor(" Games ", current: "Games", requireStrong: false) is null,
|
||||
"the unchanged-exemption must compare trimmed (whitespace-only edit is still 'unchanged')");
|
||||
return "requireStrong refuses an unchanged weak password; casual grandfathers unchanged but blocks new-weak; trim-safe";
|
||||
}
|
||||
|
||||
/// <summary>The 5.6 password rules: the strength critique (what the dialogs enforce and
|
||||
/// explain) and the derivation choke-point refusing weak passwords outright, so NO path —
|
||||
/// tick, startup auto-connect, headless service — streams on a guessable password.</summary>
|
||||
|
||||
Reference in New Issue
Block a user