kiloparsec/REVIEW-237d404.md

Review of authenticated protocol 2

Reviewed commit: 237d404bcd4d52e015b5e0d7cd4feaec4bd848ff — Add authenticated protocol 2 and TypeScript WASM client.

Review date: 2026-10-21. Baseline: parent commit 88ceeb2.

This document records findings for separate discussion and repair. Issue numbers are stable. Implementation changes and their validation are recorded under closed issues. Proposed remedies for open issues are discussion options, not an agreed redesign.

Assessment

The reviewed commit fixed real authentication gaps, particularly unprotected RPC response correlation, but introduced availability regressions. The findings below preserve the original analysis. I1–I5 are now fixed for release 0.8.0; I6–I7 were closed as non-defects. No review issues remain open.

The nonce itself was already present in protocol 1. Receiver-side replay tracking, the fixed window, and counter-exhaustion checks are new. These changes should be evaluated separately from authenticated RPC metadata and the server-finished acknowledgement.

Reason for the original change

The committed assessment says the TypeScript port exposed two protocol-1 weaknesses. Inspection of the parent code supports both mechanisms:

1. Encrypted results had inner forId = 0; their actual request correlation was only in the plaintext outer transport ID. An intermediary could swap two outer IDs and associate authentic answers with the wrong concurrent calls when result codecs were compatible. Protocol 2 authenticates kind, correlation and priority inside encryption.
2. L0ClientId returned plaintext Unit. An intermediary could forge that completion without the server accepting the identity message. Protocol 2 returns an encrypted server-finished record echoing the exact accepted identity bytes.

Other changes include separate client/server signature domains, fresh server exchange keys per connection, repeated-handshake rejection, nonce-state preservation during identity completion, and counter-exhaustion checks. These are distinct design decisions and need not be discussed as one indivisible feature.

What server-finished means

It is the response to the existing L0ClientId RPC, not an additional network round trip or an acknowledgement of every UDP packet.

Previously: client sends encrypted identity; server replies with plaintext Unit. Now: client sends encrypted identity; server replies with encrypted ProtectedMessage(SERVER_FINISHED, 0, 0, identityBytes).

The client decrypts it and checks the kind, zero correlation/priority, and exact identity payload. With an honest authenticated server, this establishes that the session-key holder processed and accepted that identity message before the client announces connection completion. It does not establish application authorization, authenticate an otherwise untrusted server key, or acknowledge subsequent RPC execution.

The server completes its local established state before sending this response. Losing the response can therefore leave the server established while the client is still waiting. This is an acknowledgement, not an atomic agreement or a delivery-recovery mechanism.

Existing nonces versus new replay checks

Protocol 1 already contained Package(nonce: ULong, encryptedMessage), private var nonce = 0UL, and val n = protectedOp.invoke { nonce++ }. It derived the cipher nonce from the session token and that counter. Each sending direction has its own encryption key/counter context.

The encryption nonce selects a unique encryption instance under a key. It is not secret, is not the RPC ID, and does not require packets to arrive in order. Reusing a nonce for distinct plaintexts under the same key is unsafe. Retransmitting an already-produced ciphertext is a different question from encrypting new plaintext with a reused nonce.

Protocol 1 decrypted using the counter supplied with each packet, without remembering which counters it had already seen. Consequently, a captured valid encrypted call could be replayed and reach application execution again.

Protocol 2 adds receiver memory: highest authenticated nonce plus the set of seen nonces in a 4,096-position window. This recognizes repeats and bounds memory, at the cost of rejecting sufficiently old unseen packets. Only successfully authenticated packets advance the window; a forged large counter cannot move it forward.

The tasks are distinct: nonce uniqueness protects encryption; replay tracking prevents repeated acceptance; authenticated RPC correlation binds an answer to its request. Neither correlation nor server-finished intrinsically requires this particular replay-window size or session-closing policy.

Repair validation (2026-10-21)

• 42 focused JVM tests passed, including existing security, priority and transport tests and the new replay/queue/admission regressions.
• TypeScript: 38 passed; the normally skipped interop fixture was run separately and passed against this checkout's JVM server.
• Release validation: all 64 JVM and 47 Kotlin/JS Node tests passed. Real loopback UDP exercised reordered and duplicated packets and concurrent bidirectional RPCs; real TCP exercised concurrent bidirectional RPCs and priority. The new carrier test cancels its collector before closing the listener.
• All 13 Maven publications and Dokka built successfully. A standalone consumer resolved staged 0.8.0 artifacts without source substitution and passed the TypeScript interoperability suite (authentication, duplex, pushes, errors, concurrent calls and reconnect).
• Kotlin Wasm compilation passed. Browser, Android and Apple runtime qualification was not performed. Apple KLIB publication does not qualify final Apple application linking or runtime.
• Protocol-2 wire encoding remains unchanged; older protocol-2 peers still retain their own previous replay-disconnect behavior until upgraded.
• The encrypted backlog test validates all 4098 messages, including an old low-priority message, decrypt successfully after scheduling. Concurrent real JVM preparation exercises both directions and calls, replies, pushes and handshake messages.
• A temporary preparation benchmark (source retained at /tmp/PreparationBenchmarkTest.kt) ran concurrent 1 MiB and 1 KiB payloads under the preparation mutex. Measured medians including lock waiting were 86.86 ms and 86.60 ms, with maxima 117.40 ms and 117.65 ms. This is a local JVM observation including serialization and allocation; large-message preparation can delay small messages before queue scheduling. The benchmark source was removed from the checkout.
• Historical probe/test counts below describe the original review, not the final repair run.

Original review validation and limits

• 14 focused JVM tests passed: 7 existing authenticated-envelope tests, 3 security-boundary tests, and 4 temporary review probes.
• The review probes checked replay-window boundaries, adjacent reordering survival, deep reordering session closure, and local send failure session closure.
• Follow-up I5 check: 3 additional focused JVM probes passed, reproducing ignored client/server initialization rejection and confirming rejection through the application execute gate. Probe source saved at /tmp/ReviewI5Test.kt and removed from the checkout.
• TypeScript tests: 35 passed, 1 interoperability test skipped.
• No live UDP impairment, browser qualification, full cryptographic audit, or sustained-load benchmark was performed.
• Temporary probe source was saved at /tmp/Review237d404Test.kt and removed from the checkout. This path is ephemeral.
• Existing .idea/misc.xml changes and untracked kotlin-js-store/ were left untouched.

All review issues are closed: I1–I5 are included in release 0.8.0; I6–I7 require no remediation.

Closed issues

Issue 1 — Duplicate or stale packets terminate the session

Resolution (2026-10-21): Done — duplicate/stale encrypted packets are silently discarded on Kotlin and TypeScript. Incoming calls/pushes are not executed again; response authentication/replay checks run before consuming the pending waiter. A valid later reply can still complete it. Authentication/routing failures remain fatal. No automatic retry was added.

Original finding (before repair):

Priority: High for UDP. Status: Confirmed regression; reproduced.

KiloParams.decrypt rejects a previously received nonce, or an unseen nonce at least 4,096 positions behind the highest authenticated nonce. KiloL0Interface.executeTransportBlock turns rejection into SecurityException, a subclass of BreakConnectionException, and the transport closes.

Observed behavior:

Delivery	Result
Adjacent packets reversed	Both accepted
Unseen nonce 4,095 positions behind highest	Accepted
Unseen nonce 4,096 positions behind highest	Rejected
Duplicate incoming call or push	Rejected; session closes

Duplicate responses are different: the outer transport normally ignores responses whose pending request was already removed. A stale response for a still-pending call reaches authentication and can close the session.

A controlled test held an encrypted push, delivered 4,096 newer pushes, then delivered the original. The valid original packet caused session closure. An adjacent-reordering test survived. The existing duplicate-call regression also explicitly expects connection termination.

Impact: ordinary datagram duplication or sufficiently deep reordering becomes a session-wide failure, affecting unrelated calls. This is an availability issue, not evidence of broken encryption.

Discussion direction: distinguish duplicate/stale packet disposition from fatal errors; discard without repeating application execution. If reliable delivery is desired, separately specify timeouts, retransmission and response caching. Discarding duplicates alone does not implement reliable RPC.

References: KiloParams.kt, KiloL0Interface.kt, Transport.kt, AuthenticatedEnvelopeTest.kt.

Issue 2 — Priority scheduling can exceed the replay window locally

Resolution (2026-10-21): Done — OutQueue enforces the 2730-position allowance; one preparation mutex covers nonce allocation, sealing, encoding and admission. The transport owns the nonce-aware writer and submits serially to the existing adapter, preserving its selected order. Encryption stays in secure-layer callbacks. Full queues reject immediately rather than wait under the mutex.

Original finding (before repair):

Priority: High under sustained priority traffic. Status: Code-established interaction; actual scheduler saturation not reproduced.

Encryption allocates a nonce before PriorityBlockWriter selects transmission order. A low-priority encrypted packet can remain queued while newer high-priority packets overtake it. There is no fairness or maximum-overtaking guarantee. The 128-item queue bound limits simultaneous occupancy, not the total number of packets that can overtake one waiting packet.

If overtaking spans 4,096 nonce positions, eventual delivery triggers issue 1. This can occur even with ordered TCP/WebSocket delivery: the reordering happens before the carrier sends the bytes. Calls, replies and pushes share one nonce sequence per sending direction.

Agreed design (2026-10-21): introduce a specific OutQueue with push(message, nonce, priority) and pop(). Preserve encryption in its existing layer. Pass nonce metadata explicitly to the queue rather than decoding encrypted-package structures in the scheduler.

For receiver window W = 4096, use a local overtaking allowance B = floor(2 * W / 3) = 2730:

pop():
    oldest = minimum queued nonce
    eligible = messages with nonce - oldest <= B
    select highest priority among eligible
    break priority ties by smallest nonce

Use subtraction after establishing the minimum to avoid addition overflow. The allowance is based on nonce distance, not the number of successful sends. This accommodates gaps from failed or cancelled messages and prevents starvation by indefinitely newer high-priority traffic.

One mutex per sending direction covers nonce allocation, encryption/encoding, and queue insertion as one ordered preparation/admission operation. Queue insertion and selection must themselves be synchronized. This prevents an older allocated nonce from remaining outside the queue while newer packets are transmitted. All encrypted message paths sharing that sending key must participate, including calls, replies, pushes, and handshake ciphertexts.

Wait for transmission completion outside the mutex. Queue-capacity waiting must not prevent the writer from draining the queue; coordinate capacity reservation/admission accordingly. Failed preparation may consume a nonce, which must never be reused. Cancellation and close must release reservations and completion waiters.

The queue and writer must preserve the selected transmission order. This bounds scheduler-induced reordering while leaving roughly one third of the receiver window for subsequent network/receive-processing reordering. It does not guarantee acceptance after arbitrarily deep external reordering; issue 1 remains a separate fix.

Validation targets: priority selection inside the permitted range; oldest-message progress under sustained higher-priority traffic; exact window boundary; nonce gaps; concurrent preparation/admission; cancellation, capacity and close behavior; and an integration test showing a delayed low-priority encrypted message arrives without becoming stale. Measure preparation-lock contention with large messages rather than assuming encryption cost is negligible.

Implementation status: implemented and verified. A 4098-message encrypted backlog, concurrent duplex admission, queue capacity/cancellation/close and existing saturated-link responsiveness tests pass. Network reordering beyond the receiver window remains discardable, without closing the session.

References: KiloParams.kt, PriorityBlockWriter.kt.

Issue 3 — Local send failures are misclassified as authentication failures

Resolution (2026-10-21): Done — plaintext remote errors are explicitly distinguished at the receive boundary. Local preparation/admission/write failures retain their original errors and do not abort the healthy session. A local reply-send failure is logged without fabricating a plaintext remote error; the caller retains its timeout/cancellation policy.

Original finding (before repair):

Priority: Medium. Status: Confirmed regression; reproduced.

KiloRemoteInterface.call catches almost every exception from transport.callWithId, closes the transport, and throws SecurityException("unauthenticated outer error"). That operation includes local encryption and sending, not only receipt of a plaintext remote error.

The priority writer throws RemoteInterface.Exception("Outgoing block queue is full") on admission failure. Injecting that same local send exception reproduced an authentication error and termination of both peers.

Impact: local congestion closes healthy sessions and unrelated pending calls; diagnostics incorrectly suggest a remote security failure.

Discussion direction: explicitly distinguish plaintext remote errors at the receive boundary from local encoding, admission and send failures.

References: KiloRemoteInterface.kt, PriorityBlockWriter.kt.

Issue 4 — Replay tracking adds a full-window scan per advancing packet

Resolution (2026-10-21): Done — Kotlin and TypeScript use 4096-bit circular replay windows. Normal in-order advancement clears one bit, large jumps clear a fixed bitmap, and only successfully authenticated packets update history.

Original finding (before repair):

Priority: Medium performance concern. Status: Confirmed algorithmic cost; throughput impact not benchmarked.

Kotlin uses received.removeAll when the high watermark advances; TypeScript scans its received set. In steady in-order traffic with a populated window, this is approximately 4,096 age checks per packet. Kotlin performs this under the lock also used for nonce allocation and decryption.

Impact: bounded memory, but avoidable CPU work and contention for a high-throughput transport.

Discussion direction: if retaining windowed replay protection, use a circular bitmap or another bounded representation with inexpensive advancement.

References: KiloParams.kt, client.ts. Algorithm background: RFC 6479.

Issue 5 — Connection-init policy rejection is swallowed

Status: Done on 2026-10-21 — initialization rejection now propagates on both ends.

Design decision: preserve Kotlin's code-based remote-party authorization through the local interface and initialization handlers. Both peers receive verified identity and session data through KiloScope. No trusted-key-list constructor parameter is required; the earlier proposal to add one is withdrawn.

Original defect: the shared invokeAll helper discarded every handler exception using runCatching, including deliberate security/connection rejection. This defect predates commit 237d404. Follow-up probes reproduced successful connection and application execution despite rejection on either peer.

Fix: replace runCatching with explicit try/catch. Rethrow LocalInterface.BreakConnectionException and all subclasses, including RemoteInterface.SecurityException; continue ignoring other handler failures as before. Rejection stops subsequent handlers. The shared helper also propagates this exception family when invoked for disconnect handlers.

Validation: ConnectionInitializationTest covers client and server initialization rejection using a custom SecurityException subclass and a custom BreakConnectionException subclass. Each case verifies that the policy sees the verified remote identity, both connection runs finish, later init handlers do not run, connected state is not announced, and a subsequent client RPC fails as closed. A fifth test verifies that ordinary handler failures still allow initialization and a successful RPC. The four rejection cases failed before the fix and passed after it. All 15 focused JVM tests passed, including AuthenticatedEnvelopeTest and SecurityBoundaryTest.

Scope: this completes the requested exception-propagation repair. Parameter publication order and initialization-time RPC semantics were not changed; these tests do not establish a general concurrent-dispatch barrier before initialization completes.

References: KiloClientConnection.kt, ConnectionInitializationTest.kt.

Issue 6 — Protocol 2 requires coordinated peer upgrades

Status: Closed — intentional compatibility decision, not a defect; no remediation required.

Both peers require protocol 2, with no automatic v1 fallback. The new envelope and identity acknowledgement deliberately break wire compatibility with v1. Coordinated peer upgrades were an agreed design decision, not an outstanding issue.

The original issue number is retained for reference.

References: KiloProtocol.kt, SECURITY.md.

Issue 7 — Replay protection is not business-operation deduplication

Status: Closed on 2026-10-21 — not a protocol defect; excluded from remediation.

Kiloparsec protects against replayed encrypted packets and forged or altered protocol messages. A new valid RPC issued by application code, on the same connection or after reconnecting, is a new request. The protocol cannot determine whether repeating that operation is intentional or an application mistake.

Business-operation deduplication requires application-defined semantics, such as a durable operation identifier and an explicit idempotency contract. It belongs to business logic and should not have been classified as a Kiloparsec issue.

Resolution: no protocol change required. The original issue number is retained for reference.