From 76773fbdec72c87ab28659a40ddc55fc763b21c4 Mon Sep 17 00:00:00 2001 From: Chris Date: Tue, 6 Oct 2026 11:38:11 -0500 Subject: [PATCH 1/2] fix: ignore reserved binary ids 2 and 3 instead of closing The 1.0.0-rc1 spec lists only an orig_type of 1 as a malformed fragment sequence; ids 2 and 3 are "reserved for future use", and a receiver MUST ignore binary messages whose id it does not implement, fragmented or not. Drop them at dispatch so a top-level frame and a reassembled message are both ignored and the connection stays open. --- .../Connection/Noise/NoiseWireFraming.cs | 21 +++---- .../FragmentationConformanceTests.cs | 56 +++++++++++++------ 2 files changed, 51 insertions(+), 26 deletions(-) diff --git a/src/Sendspin.SDK/Connection/Noise/NoiseWireFraming.cs b/src/Sendspin.SDK/Connection/Noise/NoiseWireFraming.cs index 9533beae..e1e97ad8 100644 --- a/src/Sendspin.SDK/Connection/Noise/NoiseWireFraming.cs +++ b/src/Sendspin.SDK/Connection/Noise/NoiseWireFraming.cs @@ -457,12 +457,6 @@ private InboundFrameResult HandleTransportFrame(WireFrame frame) if (_reassemblyBuffer is not null) return Fail("non-fragment frame received while a fragmented message is in flight"); - // IDs 2 and 3 were the pre-1.0 fragment types and are now reserved (spec messaging.md). - // A reserved ID is not a valid application binary type, so it is a silent failure like a - // malformed fragment rather than something surfaced to BinaryMessageParser as a message. - if (type is 2 or 3) - return Fail($"reserved binary message id {type}"); - return DispatchMessage(type, plainBuf.AsMemory(1, plainLen - 1)); } @@ -477,10 +471,11 @@ private InboundFrameResult HandleFragment(ReadOnlyMemory plaintext, bool f if (plaintext.Length < 3) return Fail("opening fragment missing orig_type"); _reassemblyOrigType = plaintext.Span[2]; - // orig_type must be a real message type: the fragment id (1) and the reserved ids - // (2, 3) are malformed here, the same as receiving them as a top-level frame. - if (_reassemblyOrigType is NoiseConstants.MessageTypeFragment or 2 or 3) - return Fail($"orig_type is a reserved id {_reassemblyOrigType}"); + // An orig_type of 1 is the one malformed value (messaging.md § Fragmentation). The + // reserved ids 2 and 3 are merely unimplemented: they reassemble and are dropped at + // dispatch, so the fragment sequence is still tracked. + if (_reassemblyOrigType == NoiseConstants.MessageTypeFragment) + return Fail("orig_type is the fragment id 1"); _reassemblyBuffer = new MemoryStream(); data = plaintext[3..]; } @@ -532,6 +527,12 @@ private InboundFrameResult HandleRehandshakeMessage(JsonElement root) private InboundFrameResult DispatchMessage(byte type, ReadOnlyMemory payload) { + // IDs 2 and 3 were the pre-1.0 fragment types and are now reserved (messaging.md). The + // SDK does not implement them, so they are ignored ("MUST also ignore binary messages + // whose ID they do not implement") rather than surfaced to BinaryMessageParser. + if (type is 2 or 3) + return InboundFrameResult.None; + if (type == NoiseConstants.MessageTypeJsonBody) { string json = Encoding.UTF8.GetString(payload.Span); diff --git a/tests/Sendspin.SDK.Tests/Connection/FragmentationConformanceTests.cs b/tests/Sendspin.SDK.Tests/Connection/FragmentationConformanceTests.cs index f00d0edf..6e819616 100644 --- a/tests/Sendspin.SDK.Tests/Connection/FragmentationConformanceTests.cs +++ b/tests/Sendspin.SDK.Tests/Connection/FragmentationConformanceTests.cs @@ -50,20 +50,17 @@ public void NonFragmentFrame_MidReassembly_IsFatal_AndReassemblyDoesNotSurvive() Assert.Equal(new byte[] { 8, 0x01, 0x02, 0x03 }, reassembled.Binary!.Value.ToArray()); } - [Theory] - [InlineData((byte)1)] // the fragment id itself - [InlineData((byte)2)] // reserved - [InlineData((byte)3)] // reserved - public void OpeningFragment_WithReservedOrigType_IsFatal_AndSurfacesNothing(byte origType) + [Fact] + public void OpeningFragment_WithOrigTypeOfTheFragmentId_IsFatal_AndSurfacesNothing() { var identity = SendspinIdentity.Generate(); var framing = new NoiseWireFraming(identity); var server = CompleteHandshake(framing, identity); - // Spec: orig_type must be a real message type; the fragment id (1) and the reserved ids - // (2, 3) are malformed and MUST close the connection without surfacing anything. + // Spec: an orig_type of 1 is a malformed sequence and MUST close the connection + // without surfacing anything. var opening = Feed(framing, server, - [NoiseConstants.MessageTypeFragment, NoiseConstants.FragmentFlagFirst, origType, 0xAA]); + [NoiseConstants.MessageTypeFragment, NoiseConstants.FragmentFlagFirst, NoiseConstants.MessageTypeFragment, 0xAA]); Assert.NotNull(opening.FatalReason); Assert.Null(opening.Text); Assert.Null(opening.Binary); @@ -76,6 +73,28 @@ public void OpeningFragment_WithReservedOrigType_IsFatal_AndSurfacesNothing(byte Assert.Null(end.Binary); } + [Theory] + [InlineData((byte)2)] + [InlineData((byte)3)] + public void FragmentedMessage_WithReservedOrigType_IsIgnored_AndConnectionStaysOpen(byte origType) + { + var identity = SendspinIdentity.Generate(); + var framing = new NoiseWireFraming(identity); + var server = CompleteHandshake(framing, identity); + + // Spec: "The ignore rules also apply to fragmented messages." Only an orig_type of 1 is + // malformed; a reserved id is one the SDK does not implement, so the sequence is tracked + // to its last fragment and the message is not dispatched. + var opening = Feed(framing, server, + [NoiseConstants.MessageTypeFragment, NoiseConstants.FragmentFlagFirst, origType, 0xAA]); + AssertIgnored(opening); + var end = Feed(framing, server, [NoiseConstants.MessageTypeFragment, NoiseConstants.FragmentFlagLast, 0xBB]); + AssertIgnored(end); + + // The last fragment cleared the sequence state: an ordinary message still gets through. + Assert.Equal(HelloJson, Feed(framing, server, [NoiseConstants.MessageTypeJsonBody, .. Encoding.UTF8.GetBytes(HelloJson)]).Text); + } + [Fact] public void FragmentEnd_WithNothingInFlight_IsFatal() { @@ -119,20 +138,18 @@ public void Fragment_WithReservedFlagBits_IsFatal() [Theory] [InlineData((byte)2)] [InlineData((byte)3)] - public void ReservedBinaryId_IsSilentlyDiscarded_NotSurfaced(byte reservedId) + public void ReservedBinaryId_IsIgnored_AndConnectionStaysOpen(byte reservedId) { var identity = SendspinIdentity.Generate(); var framing = new NoiseWireFraming(identity); var server = CompleteHandshake(framing, identity); - // IDs 2 and 3 were the pre-1.0 fragment types; now reserved. A reserved ID is a silent - // failure like a malformed fragment — the connection closes and nothing is surfaced to - // BinaryMessageReceived, rather than being dispatched as an application binary message. - var result = Feed(framing, server, [reservedId, 0xAA, 0xBB]); + // IDs 2 and 3 were the pre-1.0 fragment types; now reserved. Spec: "They MUST also + // ignore binary messages whose ID they do not implement" — nothing is surfaced to + // BinaryMessageReceived and the connection is not closed. + AssertIgnored(Feed(framing, server, [reservedId, 0xAA, 0xBB])); - Assert.NotNull(result.FatalReason); - Assert.Null(result.Text); - Assert.Null(result.Binary); + Assert.Equal(HelloJson, Feed(framing, server, [NoiseConstants.MessageTypeJsonBody, .. Encoding.UTF8.GetBytes(HelloJson)]).Text); } [Fact] @@ -325,6 +342,13 @@ private static TestNoiseServer CompleteHandshake(NoiseWireFraming framing, Sends private static InboundFrameResult Feed(NoiseWireFraming framing, TestNoiseServer server, byte[] plaintext) => framing.ProcessInbound(new WireFrame(WireFrameKind.Binary, server.EncryptFrame(plaintext))); + private static void AssertIgnored(InboundFrameResult result) + { + Assert.Null(result.FatalReason); + Assert.Null(result.Text); + Assert.Null(result.Binary); + } + private static void SurfaceFirstApplicationMessage(NoiseWireFraming framing, TestNoiseServer server) { var result = Feed(framing, server, [NoiseConstants.MessageTypeJsonBody, .. Encoding.UTF8.GetBytes(HelloJson)]); From 4dc8e12765b5923f895516c7342aeb2d54715e4f Mon Sep 17 00:00:00 2001 From: Chris Date: Tue, 6 Oct 2026 11:38:12 -0500 Subject: [PATCH 2/2] protocol: honour psk_category in Noise message 1 Message 1 carries a psk_category (lt | pr | sn) alongside psk_id. Read it, treat a code outside the three as a malformed inner payload (silent failure), and resolve only among PSKs of the declared category, so a psk_id held under a different category is a lookup miss. The test servers now declare the category of the PSK they initiate with. --- src/Sendspin.SDK/Connection/Noise/NoisePsk.cs | 14 ++-- .../Connection/Noise/NoiseWireFraming.cs | 19 ++++- .../Connection/Noise/PairingRecordStore.cs | 6 +- tests/Sendspin.SDK.Tests/Client/FakeServer.cs | 7 +- .../SendspinClientServicePairingTests.cs | 2 +- .../SendspinHostServiceArbitrationTests.cs | 4 +- .../DisabledPairingMethodPskTests.cs | 14 ++-- .../Connection/NoiseWireFramingTests.cs | 83 ++++++++++++++++++- .../Connection/TestNoiseServer.cs | 22 +++-- 9 files changed, 139 insertions(+), 32 deletions(-) diff --git a/src/Sendspin.SDK/Connection/Noise/NoisePsk.cs b/src/Sendspin.SDK/Connection/Noise/NoisePsk.cs index ae73f47b..a6cce456 100644 --- a/src/Sendspin.SDK/Connection/Noise/NoisePsk.cs +++ b/src/Sendspin.SDK/Connection/Noise/NoisePsk.cs @@ -23,12 +23,16 @@ public enum PskCategory internal sealed record NoisePsk(ReadOnlyMemory Key, PskCategory Category, string? ServerId = null); /// -/// Resolves the psk_id received in Noise handshake message 1 to a PSK candidate. +/// Resolves the psk_id and psk_category received in Noise handshake message 1 to a PSK candidate. /// internal interface INoisePskResolver { - /// Returns the PSK whose derived psk_id matches, or null for a lookup miss. - NoisePsk? Resolve(string pskId); + /// + /// Returns the PSK of the declared category whose derived psk_id matches, or null for a + /// lookup miss. A psk_id held only under a different category is a miss (connection.md + /// § Pre-Shared Key). + /// + NoisePsk? Resolve(string pskId, PskCategory category); } /// @@ -40,8 +44,8 @@ internal sealed class SentinelPskResolver : INoisePskResolver public static SentinelPskResolver Instance { get; } = new(); /// - public NoisePsk? Resolve(string pskId) => - pskId == NoiseConstants.SentinelPskId + public NoisePsk? Resolve(string pskId, PskCategory category) => + category == PskCategory.Sentinel && pskId == NoiseConstants.SentinelPskId ? new NoisePsk(NoiseConstants.SentinelPsk.ToArray(), PskCategory.Sentinel) : null; } diff --git a/src/Sendspin.SDK/Connection/Noise/NoiseWireFraming.cs b/src/Sendspin.SDK/Connection/Noise/NoiseWireFraming.cs index e1e97ad8..0ee40fd9 100644 --- a/src/Sendspin.SDK/Connection/Noise/NoiseWireFraming.cs +++ b/src/Sendspin.SDK/Connection/Noise/NoiseWireFraming.cs @@ -309,6 +309,7 @@ private InboundFrameResult RunResponderExchange(byte[] msg1, byte[] prologue, bo // a malformed message 1, an unresolvable psk_id, a psk_id bound to another server -- // otherwise leaves a live Curve25519 private key on the heap until GC (#102). string pskId; + PskCategory pskCategory; byte[] probeKey = _identity.PrivateKey.ToArray(); try { @@ -322,18 +323,30 @@ private InboundFrameResult RunResponderExchange(byte[] msg1, byte[] prologue, bo using var payloadDoc = JsonDocument.Parse(Encoding.UTF8.GetString(probeBuf, 0, probeLen)); pskId = payloadDoc.RootElement.GetProperty("psk_id").GetString() ?? throw new FormatException("psk_id missing"); + + // messaging.md § noise/handshake: a psk_category outside the three defined codes is + // a malformed inner payload, hence a silent failure like any other throw in here. + string? categoryCode = payloadDoc.RootElement.GetProperty("psk_category").GetString(); + pskCategory = categoryCode switch + { + "lt" => PskCategory.LongTerm, + "pr" => PskCategory.Pairing, + "sn" => PskCategory.Sentinel, + _ => throw new FormatException($"unknown psk_category {categoryCode}"), + }; } finally { CryptographicOperations.ZeroMemory(probeKey); } - var resolved = _pskResolver.Resolve(pskId); + var resolved = _pskResolver.Resolve(pskId, pskCategory); if (resolved is null) { // Sentinel Fallback (connection.md § Sentinel Fallback): a psk_id that matches // nothing means the server referenced a credential this client cannot use -- a lost - // pairing record, an interrupted pairing finalize, or a PSK for a pairing method the + // pairing record, an interrupted pairing finalize, a PSK held under a different + // category than the declared psk_category, or a PSK for a pairing method the // client has disabled. On the INITIAL handshake the client completes message 2 with // the published Sentinel PSK instead of failing; the server recognises that as an // authenticated credential-mismatch signal and the session proceeds as an ordinary @@ -348,7 +361,7 @@ private InboundFrameResult RunResponderExchange(byte[] msg1, byte[] prologue, bo if (!isInitialHandshake) return Fail($"no PSK matches psk_id {pskId}"); - resolved = SentinelPskResolver.Instance.Resolve(NoiseConstants.SentinelPskId)!; + resolved = SentinelPskResolver.Instance.Resolve(NoiseConstants.SentinelPskId, PskCategory.Sentinel)!; } // A misbinding, not a miss: the psk_id DID match a stored-pubkey record, but that record diff --git a/src/Sendspin.SDK/Connection/Noise/PairingRecordStore.cs b/src/Sendspin.SDK/Connection/Noise/PairingRecordStore.cs index c3908a66..8c0c2005 100644 --- a/src/Sendspin.SDK/Connection/Noise/PairingRecordStore.cs +++ b/src/Sendspin.SDK/Connection/Noise/PairingRecordStore.cs @@ -521,17 +521,17 @@ public RecordPskResolver(IPairingRecordStore store, Func? isPairingPskEnab /// marked used by the client once a decrypted message proves the session /// authenticated. /// - public NoisePsk? Resolve(string pskId) + public NoisePsk? Resolve(string pskId, PskCategory category) { foreach (var record in _store.List()) { - if (record.PskId == pskId && IsCandidate(record)) + if (record.PskId == pskId && record.Category == category && IsCandidate(record)) { return new NoisePsk(record.Psk, record.Category, record.ServerId); } } - return SentinelPskResolver.Instance.Resolve(pskId); + return SentinelPskResolver.Instance.Resolve(pskId, category); } /// diff --git a/tests/Sendspin.SDK.Tests/Client/FakeServer.cs b/tests/Sendspin.SDK.Tests/Client/FakeServer.cs index 17244dc3..3d9c5c07 100644 --- a/tests/Sendspin.SDK.Tests/Client/FakeServer.cs +++ b/tests/Sendspin.SDK.Tests/Client/FakeServer.cs @@ -23,6 +23,7 @@ internal class FakeServer : IAsyncDisposable private readonly ClientWebSocket _ws = new(); private readonly CancellationTokenSource _cts = new(); private readonly byte[] _psk; + private readonly string? _pskCategory; private readonly IReadOnlyList _activities; private readonly KeyPair _keys; private readonly TaskCompletionSource _goodbye = @@ -38,9 +39,11 @@ internal class FakeServer : IAsyncDisposable /// Must match a PSK the host's pairing record store resolves. /// server/activate activities: ["playback"], or empty for discovery. /// Pass an existing pair so two instances share one server_id. - internal FakeServer(byte[] psk, IReadOnlyList activities, KeyPair? keys = null) + /// The psk_category code to declare; see for the default. + internal FakeServer(byte[] psk, IReadOnlyList activities, KeyPair? keys = null, string? pskCategory = null) { _psk = psk; + _pskCategory = pskCategory; _activities = activities; _keys = keys ?? KeyPair.Generate(); @@ -190,7 +193,7 @@ private async Task HandleHandshakeTextAsync(string json) { using var doc = JsonDocument.Parse(json); string clientId = doc.RootElement.GetProperty("payload").GetProperty("client_id").GetString()!; - _noise = new TestNoiseServer(SendspinIdentity.DecodePeerId(clientId), _psk, _keys); + _noise = new TestNoiseServer(SendspinIdentity.DecodePeerId(clientId), _psk, _keys, pskCategory: _pskCategory); // Both types derive a server id from the same key pair independently. They agree // only because _keys is threaded through, and nothing else pins that: a change to diff --git a/tests/Sendspin.SDK.Tests/Client/SendspinClientServicePairingTests.cs b/tests/Sendspin.SDK.Tests/Client/SendspinClientServicePairingTests.cs index e36dd01e..68e2c483 100644 --- a/tests/Sendspin.SDK.Tests/Client/SendspinClientServicePairingTests.cs +++ b/tests/Sendspin.SDK.Tests/Client/SendspinClientServicePairingTests.cs @@ -407,7 +407,7 @@ public void Resolve_DoesNotMutateTheStore() store.Upsert(new PairingRecord(psk, PskCategory.LongTerm, "srv-1")); var resolver = new RecordPskResolver(store); - var resolved = resolver.Resolve(NoiseConstants.DerivePskId(psk)); + var resolved = resolver.Resolve(NoiseConstants.DerivePskId(psk), PskCategory.LongTerm); Assert.NotNull(resolved); Assert.Null(Assert.Single(store.List()).LastUsedUtc); diff --git a/tests/Sendspin.SDK.Tests/Client/SendspinHostServiceArbitrationTests.cs b/tests/Sendspin.SDK.Tests/Client/SendspinHostServiceArbitrationTests.cs index 6d1418c0..6475b275 100644 --- a/tests/Sendspin.SDK.Tests/Client/SendspinHostServiceArbitrationTests.cs +++ b/tests/Sendspin.SDK.Tests/Client/SendspinHostServiceArbitrationTests.cs @@ -154,7 +154,7 @@ public async Task DisplacedPairingConnection_GetsPairAbortNotGoodbye() // own server reconnecting, which drops the previous socket as stale. await using var host = await StartHostAsync(); var keys = KeyPair.Generate(); - await using var pairing = new FakeServer(PairingPsk, ["pairing"], keys); + await using var pairing = new FakeServer(PairingPsk, ["pairing"], keys, pskCategory: "pr"); await pairing.ConnectAsync(host.ListeningPort); await WaitForServerConnectedAsync(host, pairing.ServerId); @@ -178,7 +178,7 @@ public async Task RejectedIncomingPairing_GetsPairAbortNotGoodbye() await existing.ConnectAsync(host.ListeningPort); await WaitForServerConnectedAsync(host, existing.ServerId); - await using var incoming = new FakeServer(PairingPsk, ["pairing"]); + await using var incoming = new FakeServer(PairingPsk, ["pairing"], pskCategory: "pr"); await incoming.ConnectAsync(host.ListeningPort); Assert.True(await incoming.WaitForPairAbortAsync("concurrent_attempt", Timeout)); diff --git a/tests/Sendspin.SDK.Tests/Connection/DisabledPairingMethodPskTests.cs b/tests/Sendspin.SDK.Tests/Connection/DisabledPairingMethodPskTests.cs index 2fc94e74..c0c3479c 100644 --- a/tests/Sendspin.SDK.Tests/Connection/DisabledPairingMethodPskTests.cs +++ b/tests/Sendspin.SDK.Tests/Connection/DisabledPairingMethodPskTests.cs @@ -28,7 +28,7 @@ public void DisabledPairingPsk_IsExcludedFromTheCandidateSet() store.Upsert(new PairingRecord(psk, PskCategory.Pairing)); var resolver = new RecordPskResolver(store, () => false); - var result = Handshake(SendspinIdentity.Generate(), resolver, psk, out var framing); + var result = Handshake(SendspinIdentity.Generate(), resolver, psk, out var framing, pskCategory: "pr"); // A lookup miss, not a late refusal: the excluded record never keys the session, so // there is no Pairing-trust channel to refuse a pairing activation on later. @@ -49,11 +49,11 @@ public void PairingPskEnablement_IsReadAtResolveTime() var resolver = new RecordPskResolver(store, () => enabled); var identity = SendspinIdentity.Generate(); - Handshake(identity, resolver, psk, out var disabled); + Handshake(identity, resolver, psk, out var disabled, pskCategory: "pr"); Assert.Equal(PskCategory.Sentinel, disabled.MatchedPsk!.Category); enabled = true; - var result = Handshake(identity, resolver, psk, out var framing); + var result = Handshake(identity, resolver, psk, out var framing, pskCategory: "pr"); Assert.Null(result.FatalReason); Assert.Equal(PskCategory.Pairing, framing.MatchedPsk!.Category); @@ -87,7 +87,7 @@ public void UnfilteredResolver_StillResolvesPairingRecords() var store = new InMemoryPairingRecordStore(); store.Upsert(new PairingRecord(psk, PskCategory.Pairing)); - var resolved = new RecordPskResolver(store).Resolve(NoiseConstants.DerivePskId(psk)); + var resolved = new RecordPskResolver(store).Resolve(NoiseConstants.DerivePskId(psk), PskCategory.Pairing); Assert.Equal(PskCategory.Pairing, resolved!.Category); } @@ -116,7 +116,7 @@ public async Task CreateForDial_WiresTheClientsPairingConfigIntoTheResolver() await using (client) { var framing = (NoiseWireFraming)client.Session; - var server = new TestNoiseServer(identity.PublicKey, psk); + var server = new TestNoiseServer(identity.PublicKey, psk, pskCategory: "pr"); var clientInit = Assert.Single(framing.Start()); var (serverInit, msg1) = server.Respond(clientInit.PayloadAsText()); framing.ProcessInbound(WireFrame.FromText(serverInit)); @@ -136,10 +136,10 @@ public async Task CreateForDial_WiresTheClientsPairingConfigIntoTheResolver() /// message whose psk_id the resolver is asked about. /// private static InboundFrameResult Handshake( - SendspinIdentity identity, INoisePskResolver resolver, byte[] serverPsk, out NoiseWireFraming framing) + SendspinIdentity identity, INoisePskResolver resolver, byte[] serverPsk, out NoiseWireFraming framing, string? pskCategory = null) { framing = new NoiseWireFraming(identity, resolver); - var server = new TestNoiseServer(identity.PublicKey, serverPsk); + var server = new TestNoiseServer(identity.PublicKey, serverPsk, pskCategory: pskCategory); var clientInit = Assert.Single(framing.Start()); var (serverInit, msg1) = server.Respond(clientInit.PayloadAsText()); framing.ProcessInbound(WireFrame.FromText(serverInit)); diff --git a/tests/Sendspin.SDK.Tests/Connection/NoiseWireFramingTests.cs b/tests/Sendspin.SDK.Tests/Connection/NoiseWireFramingTests.cs index acf7452b..d01c65f5 100644 --- a/tests/Sendspin.SDK.Tests/Connection/NoiseWireFramingTests.cs +++ b/tests/Sendspin.SDK.Tests/Connection/NoiseWireFramingTests.cs @@ -119,6 +119,87 @@ public void InitialHandshake_UnknownPskId_FallsBackToTheSentinel() Assert.Equal(PskCategory.Sentinel, framing.MatchedPsk!.Category); } + /// + /// messaging.md § noise/handshake: a psk_category outside the three defined codes is a + /// malformed inner payload — a silent failure, so the client sends no message 2. + /// + [Fact] + public void InitialHandshake_UnknownPskCategory_IsFatal_AndSendsNothing() + { + var identity = SendspinIdentity.Generate(); + var framing = new NoiseWireFraming(identity); + var server = new TestNoiseServer( + identity.PublicKey, NoiseConstants.SentinelPsk.ToArray(), pskCategory: "xx"); + + var clientInit = Assert.Single(framing.Start()); + var (serverInit, msg1) = server.Respond(clientInit.PayloadAsText()); + framing.ProcessInbound(WireFrame.FromText(serverInit)); + var result = framing.ProcessInbound(WireFrame.FromText(msg1)); + + Assert.Contains("unknown psk_category xx", result.FatalReason); + Assert.Null(result.FatalKind); + Assert.Null(result.Replies); + Assert.False(framing.IsTransportReady); + } + + /// + /// connection.md § Sentinel Fallback: holding the referenced PSK "under a different category + /// than the declared psk_category" is a lookup miss, so the initial handshake falls + /// back to the Sentinel instead of keying the session with the mis-categorised record. + /// + [Fact] + public void InitialHandshake_PskHeldUnderDifferentCategory_FallsBackToTheSentinel() + { + var identity = SendspinIdentity.Generate(); + byte[] psk = RandomNumberGenerator.GetBytes(32); + var store = new InMemoryPairingRecordStore(); + store.Upsert(new PairingRecord(psk, PskCategory.Pairing)); + var framing = new NoiseWireFraming(identity, new RecordPskResolver(store)); + + // The server names the client's pairing PSK but declares it long-term, then verifies + // message 2 against the Sentinel as the spec has it do on a mismatch. + var server = new TestNoiseServer( + identity.PublicKey, + NoiseConstants.SentinelPsk.ToArray(), + advertisedPskId: NoiseConstants.DerivePskId(psk), + pskCategory: "lt"); + + var clientInit = Assert.Single(framing.Start()); + var (serverInit, msg1) = server.Respond(clientInit.PayloadAsText()); + framing.ProcessInbound(WireFrame.FromText(serverInit)); + var result = framing.ProcessInbound(WireFrame.FromText(msg1)); + + Assert.Null(result.FatalReason); + server.CompleteHandshake(Assert.Single(result.Replies!).PayloadAsText()); + Assert.Equal(PskCategory.Sentinel, framing.MatchedPsk!.Category); + } + + /// + /// The same category miss during a re-handshake fails, as any other miss there does. + /// + [Fact] + public void Rehandshake_PskHeldUnderDifferentCategory_IsFatal() + { + var identity = SendspinIdentity.Generate(); + byte[] psk = RandomNumberGenerator.GetBytes(32); + var store = new InMemoryPairingRecordStore(); + store.Upsert(new PairingRecord(psk, PskCategory.LongTerm)); + var framing = new NoiseWireFraming(identity, new RecordPskResolver(store)); + var server = new TestNoiseServer(identity.PublicKey, NoiseConstants.SentinelPsk.ToArray()); + + var clientInit = Assert.Single(framing.Start()); + var (serverInit, msg1) = server.Respond(clientInit.PayloadAsText()); + framing.ProcessInbound(WireFrame.FromText(serverInit)); + var hs = framing.ProcessInbound(WireFrame.FromText(msg1)); + server.CompleteHandshake(Assert.Single(hs.Replies!).PayloadAsText()); + + var result = framing.ProcessInbound( + new WireFrame(WireFrameKind.Binary, server.StartRehandshake(psk, pskCategory: "pr"))); + + Assert.Contains($"no PSK matches psk_id {NoiseConstants.DerivePskId(psk)}", result.FatalReason); + Assert.False(framing.IsTransportReady); + } + /// /// The fallback is scoped to the initial handshake: connection.md § Sentinel Fallback has a /// miss during a re-handshake fail as before. The channel is already authenticated by then, @@ -622,7 +703,7 @@ [.. store.List() private sealed class BoundResolver(string serverId) : INoisePskResolver { - public NoisePsk? Resolve(string pskId) => + public NoisePsk? Resolve(string pskId, PskCategory category) => new(NoiseConstants.SentinelPsk.ToArray(), PskCategory.LongTerm, serverId); } } diff --git a/tests/Sendspin.SDK.Tests/Connection/TestNoiseServer.cs b/tests/Sendspin.SDK.Tests/Connection/TestNoiseServer.cs index 6c31ed39..ca596034 100644 --- a/tests/Sendspin.SDK.Tests/Connection/TestNoiseServer.cs +++ b/tests/Sendspin.SDK.Tests/Connection/TestNoiseServer.cs @@ -27,6 +27,7 @@ internal sealed class TestNoiseServer private readonly ReadOnlyMemory _clientPublicKey; private readonly byte[] _psk; private readonly string _advertisedPskId; + private readonly string _pskCategory; private readonly string _protocolName; private HandshakeState? _state; private Transport? _transport; @@ -42,17 +43,24 @@ internal sealed class TestNoiseServer /// one fails, so a server exercising that path names a credential the client cannot match /// while running its own state on the Sentinel — exactly this pair of arguments. /// + /// + /// The psk_category code (lt|pr|sn) to declare in Noise message 1. This server holds only + /// raw PSK bytes and cannot know where they came from, so it defaults to "sn" when it names + /// the Sentinel's psk_id and to "lt", the ordinary paired case, otherwise. + /// internal TestNoiseServer( ReadOnlyMemory clientPublicKey, byte[] psk, KeyPair? keys = null, NoiseCipherSuite suite = NoiseCipherSuite.ChaChaPoly, - string? advertisedPskId = null) + string? advertisedPskId = null, + string? pskCategory = null) { _keys = keys ?? KeyPair.Generate(); _clientPublicKey = clientPublicKey; _psk = psk; _advertisedPskId = advertisedPskId ?? NoiseConstants.DerivePskId(psk); + _pskCategory = pskCategory ?? (_advertisedPskId == NoiseConstants.SentinelPskId ? "sn" : "lt"); _protocolName = suite.ToProtocolName(); ServerId = TestBase64Url.EncodeToString(_keys.PublicKey); } @@ -77,14 +85,12 @@ internal TestNoiseServer( rs: _clientPublicKey.ToArray(), psks: [_psk]); - // psk_category (lt|pr|sn) is where the PSK came from; this server holds only raw PSK - // bytes and cannot know it, so "lt" stands in for the ordinary paired case. x-unknown - // is a deliberately unrecognised member: it pins NoiseWireFraming's tolerance of extra + // x-unknown is a deliberately unrecognised member: it pins NoiseWireFraming's tolerance of extra // message-1 members (#279) so a strict reader fails the suite here, not every handshake. string msg1Payload = JsonSerializer.Serialize(new Dictionary { ["psk_id"] = _advertisedPskId, - ["psk_category"] = "lt", + ["psk_category"] = _pskCategory, ["x-unknown"] = "1", }); var buf = new byte[NoiseProtocol.MaxMessageLength]; @@ -129,7 +135,7 @@ internal string RespondWithPrologue(byte[] prologue) string msg1Payload = JsonSerializer.Serialize(new Dictionary { ["psk_id"] = _advertisedPskId, - ["psk_category"] = "lt", // alternate initial handshake; see Respond + ["psk_category"] = _pskCategory, ["x-unknown"] = "1", }); var buf = new byte[NoiseProtocol.MaxMessageLength]; @@ -143,7 +149,7 @@ internal string RespondWithPrologue(byte[] prologue) } /// Initiates an in-band re-handshake to a new PSK; returns the encrypted msg1 frame. - internal byte[] StartRehandshake(byte[] newPsk) + internal byte[] StartRehandshake(byte[] newPsk, string pskCategory = "lt") { var protocol = NoiseProtocol.Parse(_protocolName.AsSpan()); _state = protocol.Create( @@ -154,7 +160,7 @@ internal byte[] StartRehandshake(byte[] newPsk) string payload = JsonSerializer.Serialize(new Dictionary { ["psk_id"] = NoiseConstants.DerivePskId(newPsk), - ["psk_category"] = "lt", // re-handshake message 1; see Respond + ["psk_category"] = pskCategory, ["x-unknown"] = "1", }); var buf = new byte[NoiseProtocol.MaxMessageLength];