From a7865994161d10fa182b6c523ba727e91032993a Mon Sep 17 00:00:00 2001 From: gamer147 Date: Sun, 31 May 2026 21:31:22 -0400 Subject: [PATCH] fix(battle-node): clarify NodeCrypto.GenerateKey contract + add fixed-vector regression test Replace inaccurate GenerateKey docstring (it claimed to port Cryptographer.generateKeyString directly but the input shape differs: server uses one hex digit per call, client uses Random.Next(0,65535) per call). New doc is honest about the difference and explains why it's safe. Add EncryptForNode_FixedVector_ProducesStableOutput: a pinned AES-CBC vector that catches encoding/IV/padding regressions that would slip past the roundtrip test. Co-Authored-By: Claude Sonnet 4.6 --- SVSim.BattleNode/Wire/NodeCrypto.cs | 14 ++++++++++++-- SVSim.UnitTests/BattleNode/Wire/NodeCryptoTests.cs | 14 ++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/SVSim.BattleNode/Wire/NodeCrypto.cs b/SVSim.BattleNode/Wire/NodeCrypto.cs index 481e98a..af749ed 100644 --- a/SVSim.BattleNode/Wire/NodeCrypto.cs +++ b/SVSim.BattleNode/Wire/NodeCrypto.cs @@ -11,9 +11,19 @@ namespace SVSim.BattleNode.Wire; public static class NodeCrypto { /// - /// Generate a fresh 32-char key. 32 random hex digits, base64-encoded, truncated to 32 chars. - /// Port of Cryptographer.generateKeyString. + /// Generate a fresh 32-char key for server-initiated encryption. + /// Calls 32 times expecting a value in [0,15], + /// formats each as a single hex char, then base64-encodes the resulting 32-char ASCII + /// string and truncates to 32 chars. /// + /// + /// Differs from the client's Cryptographer.generateKeyString in input shape: + /// the client uses Random.Next(0, 65535).ToString("x") per iteration (1–4 hex + /// chars each). The output distribution is therefore different, but both produce a + /// valid 32-char UTF-8 AES-256 key — and the client never validates the server's key + /// since the server is decrypt-only in practice. Server-initiated encryption (e.g. + /// for synchronize pushes) uses this method. + /// public static string GenerateKey(Func randHexDigit) { var sb = new StringBuilder(32); diff --git a/SVSim.UnitTests/BattleNode/Wire/NodeCryptoTests.cs b/SVSim.UnitTests/BattleNode/Wire/NodeCryptoTests.cs index 169e93b..202ae9d 100644 --- a/SVSim.UnitTests/BattleNode/Wire/NodeCryptoTests.cs +++ b/SVSim.UnitTests/BattleNode/Wire/NodeCryptoTests.cs @@ -41,4 +41,18 @@ public class NodeCryptoTests { Assert.Throws(() => NodeCrypto.DecryptForNode("tooshort")); } + + [Test] + public void EncryptForNode_FixedVector_ProducesStableOutput() + { + // Pinned wire-format regression: any change to encoding/padding/IV derivation + // that drifts in both directions would still pass the roundtrip test but break + // this hardcoded vector — and break interop with the real client. + const string plaintext = "hello, node!"; + const string key = "01234567890123456789012345678901"; + const string expected = "012345678901234567890123456789015mEezM5MgR7UUEkmx5OzPQ=="; + + Assert.That(NodeCrypto.EncryptForNode(plaintext, key), Is.EqualTo(expected)); + Assert.That(NodeCrypto.DecryptForNode(expected), Is.EqualTo(plaintext)); + } }