Skip to content

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

Merged
chrisuthe merged 2 commits into
mainfrom
chrisuthe/task/ignore-reserved-binary-ids-and-honour-psk
Oct 6, 2026
Merged

chrisuthe merged 2 commits into
mainfrom
chrisuthe/task/ignore-reserved-binary-ids-and-honour-psk

Conversation

@chrisuthe

@chrisuthe chrisuthe commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Two "ignore, do not fail" rules from Sendspin/spec at 1.0.0-rc1 (671a34d). Both audit rows were re-read against the tag and the code before changing anything; both were real gaps.

Reserved binary IDs 2 and 3 are ignored (Sendspin/spec#289)

They MUST also ignore binary messages whose ID they do not implement.
— messaging.md, "Communication"

The ignore rules also apply to fragmented messages. […] Malformed sequences are protocol errors; the receiver MUST close the connection. They are: a first fragment received while a fragmented message is in flight, a non-first fragment received with none in flight, a non-fragment binary message received while a fragmented message is in flight, a nonzero reserved flag bit, and an orig_type of 1.
— messaging.md, "Fragmentation"

dce5302 and 3033587 (both in #294) closed the connection on a top-level ID 2/3 and on a fragment whose orig_type is 2/3. The tag asks for neither: IDs 2–3 are "reserved for future use", and the malformed list names only orig_type 1.

  • NoiseWireFraming.DispatchMessage now drops types 2 and 3. Single frames and reassembled messages both go through it, so one check covers both, and nothing reaches BinaryMessageReceived.
  • HandleFragment rejects only orig_type 1. A reserved orig_type reassembles under the existing size caps and is dropped at its last fragment, so the sequence is still tracked and the malformed-sequence rules still apply.

psk_category is honoured (Sendspin/spec#284)

psk_category: 'lt' | 'pr' | 'sn' […] A psk_id the client holds only under a different category is a lookup miss

A malformed inner handshake payload (not valid UTF-8 JSON of the shape above, including a psk_category outside the three defined codes) is a silent failure and closes the WebSocket.
— messaging.md, noise/handshake

The client […] compares the included psk_id to the hash of each candidate PSK of the declared psk_category
— connection.md, "Pre-Shared Key"

The client read only psk_id. It now reads psk_category too, fails silently on a missing or unknown code, and passes the category to INoisePskResolver.Resolve, which matches only records of that category. A category miss takes the existing miss path: Sentinel Fallback on the initial handshake, fatal on a re-handshake.

Breaking behaviour

  • A server that omits psk_category from Noise message 1, or sends a code other than lt/pr/sn, no longer completes a handshake. aiosendspin 10.0.0 sends it (noise/driver.py), so the shipped server is unaffected.
  • A server that names the Sentinel psk_id under a non-sn category now gets a miss (fallback on the initial handshake, fatal on a re-handshake) rather than a match.
  • INoisePskResolver is internal; no public API changes.

Notes

  • The spec scopes the ignore rules to "after … the client receives the initial server/activate". The framing layer does not know about activation and drops IDs 2/3 at any point in transport mode; the server is not permitted to send them before activation anyway.
  • The spec lets a receiver discard a reserved orig_type's fragment data without buffering (a MAY). This buffers and drops, reusing the existing reassembly path and its caps.
  • MIGRATION-10.0.0.md still says IDs 2 and 3 are "no longer sent or accepted". Left as is: they are still never surfaced.
  • Test servers (TestNoiseServer, FakeServer) now declare a category: sn when naming the Sentinel psk_id, lt otherwise, with an override the Pairing PSK tests set to pr.

Verification

dotnet build: 0 errors.

Framework Baseline (a5a2701) This branch
net8.0 Passed: 1186 Passed: 1189, Failed: 0
net10.0 Passed: 1188 Passed: 1191, Failed: 0

The +3 are the new psk_category tests. The reserved-ID tests were rewritten in place (same count).

Interop against aiosendspin[server]==10.0.0 (tools/interop/run.sh): unpaired, pairing and static-pin all PASS. Between them they cover sn, pr, and the lt re-handshake. They were run on ports other than the script's 8931, which another local run was holding. player and the other scenarios were not run.

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.
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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The implementation consistently enforces category-aware PSK lookup and reserved-ID handling with appropriate regression coverage.

Review effort: Balanced
Findings: None

What changed in this PR

Updates Noise transport handling to conform to Sendspin 1.0 reserved-message and PSK-category rules.

Changes:

  • Ignores reserved binary IDs 2–3, including fragmented messages.
  • Parses and enforces psk_category during PSK resolution.
  • Updates test servers and coverage for category mismatches and reserved IDs.
File Description
src/​Sendspin.SDK/​Connection/​Noise/​NoiseWireFraming.cs Handles reserved IDs and PSK categories.
src/​Sendspin.SDK/​Connection/​Noise/​NoisePsk.cs Extends resolver contract with category matching.
src/​Sendspin.SDK/​Connection/​Noise/​PairingRecordStore.cs Filters PSK records by category.
tests/​Sendspin.SDK.Tests/​Connection/​TestNoiseServer.cs Emits configurable PSK categories.
tests/​Sendspin.SDK.Tests/​Connection/​NoiseWireFramingTests.cs Tests category validation and mismatches.
tests/​Sendspin.SDK.Tests/​Connection/​FragmentationConformanceTests.cs Tests ignored reserved IDs and fragments.
tests/​Sendspin.SDK.Tests/​Connection/​DisabledPairingMethodPskTests.cs Updates pairing-category test handshakes.
tests/​Sendspin.SDK.Tests/​Client/​FakeServer.cs Propagates PSK category in test connections.
tests/​Sendspin.SDK.Tests/​Client/​SendspinHostServiceArbitrationTests.cs Declares pairing credentials correctly.
tests/​Sendspin.SDK.Tests/​Client/​SendspinClientServicePairingTests.cs Updates resolver invocation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@chrisuthe
chrisuthe merged commit f3830b7 into main Oct 6, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants