Ignore reserved binary IDs 2-3 and honour psk_category (spec #289, #284)

src/Sendspin.SDK/Connection/Noise/NoisePsk.cs

@@ -23,12 +23,16 @@ public enum PskCategory
 internal sealed record NoisePsk(ReadOnlyMemory<byte> Key, PskCategory Category, string? ServerId = null);
 
 /// <summary>
-/// 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.
 /// </summary>
 internal interface INoisePskResolver
 {
-    /// <summary>Returns the PSK whose derived psk_id matches, or null for a lookup miss.</summary>
-    NoisePsk? Resolve(string pskId);
+    /// <summary>
+    /// 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).
+    /// </summary>
+    NoisePsk? Resolve(string pskId, PskCategory category);
 }
 
 /// <summary>
@@ -40,8 +44,8 @@ internal sealed class SentinelPskResolver : INoisePskResolver
     public static SentinelPskResolver Instance { get; } = new();
 
     /// <inheritdoc/>
-    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;
 }

src/Sendspin.SDK/Connection/Noise/NoiseWireFraming.cs

@@ -293,7 +293,7 @@
     /// The Sentinel Fallback below applies only to the former, so the caller says which this is
     /// rather than the method inferring it.
     /// </param>
     private InboundFrameResult RunResponderExchange(byte[] msg1, byte[] prologue, bool isInitialHandshake)
     {
         byte[] serverPub = SendspinIdentity.DecodePeerId(_serverId!);
         var protocol = NoiseProtocol.Parse(_suite.ToProtocolName().AsSpan());
@@ -309,6 +309,7 @@
         // 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 @@
             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 @@
             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
@@ -457,12 +470,6 @@
         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 +484,11 @@
             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 +540,12 @@
 
     private InboundFrameResult DispatchMessage(byte type, ReadOnlyMemory<byte> 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);

src/Sendspin.SDK/Connection/Noise/PairingRecordStore.cs

@@ -521,17 +521,17 @@ public RecordPskResolver(IPairingRecordStore store, Func<bool>? isPairingPskEnab
     /// marked used by the client once a decrypted message proves the session
     /// authenticated.
     /// </remarks>
-    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);
     }
 
     /// <summary>

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<string> _activities;
     private readonly KeyPair _keys;
     private readonly TaskCompletionSource<string> _goodbye =
@@ -38,9 +39,11 @@ internal class FakeServer : IAsyncDisposable
     /// <param name="psk">Must match a PSK the host's pairing record store resolves.</param>
     /// <param name="activities">server/activate activities: ["playback"], or empty for discovery.</param>
     /// <param name="keys">Pass an existing pair so two instances share one server_id.</param>
-    internal FakeServer(byte[] psk, IReadOnlyList<string> activities, KeyPair? keys = null)
+    /// <param name="pskCategory">The psk_category code to declare; see <see cref="TestNoiseServer"/> for the default.</param>
+    internal FakeServer(byte[] psk, IReadOnlyList<string> 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

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);

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));

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.
     /// </summary>
     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));

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)]);