feat(server): add open session core #74

Merged
Lochnair merged 3 commits from feat/server-core-open into main 2026-08-11 18:12:42 +00:00
Lochnair commented 2026-08-11 17:50:17 +00:00 (Migrated from github.com)

Add irtt-server with the deterministic protocol and session engine the
upcoming Tokio runtime will call. ServerCore::handle_datagram takes a source
endpoint and the bytes of one datagram and returns the bytes to send back, or
nothing. It performs no I/O and keeps no clock, so every decision is a pure
function of its state and that input apart from drawing session tokens.

Rejection is silence. The protocol defines no error reply, no reset and no
NACK, so malformed, unauthenticated and policy-refused datagrams are discarded
without a response and without disturbing any live session; a client tells
"rejected" from "lost" only by timing out. ServerError is reserved for
internal failures on the server's own side, so an inbound ProtoError never
surfaces as a server failure.

Open handling authenticates before decoding parameters, so a packet that fails
authentication is discarded without parsing untrusted data and no reply
behavior reveals which stage rejected it. Duration and Interval are accepted
when absent and rejected when explicitly present and not positive, which is
what the presence-aware parameter decode is for. Negotiation currently rewrites
only the protocol version; the restriction knobs the specification describes
are deliberately left for their own slices, with one obvious place to add them.

Sessions are bounded to a finite default. A single unauthenticated datagram
creates one and opens are never deduplicated, so an unbounded table is a
liability rather than a compatibility feature; upstream's absence of session
bounds is explicitly not a target. A full table drops new opens silently and
evicts nothing. No-test opens keep working at capacity because they create no
state.

Session tokens come from the operating system's random source, since a token is
the only thing standing between an off-path attacker and a usable session on an
unauthenticated server. A drawn value that is zero or collides with a live
session is redrawn within a finite budget; there is no predictable fallback,
and a colliding token never overwrites the session that holds it. The reply is
encoded before the session is inserted, so an internal failure cannot leave a
half-created session behind.

Echo and close requests pass admission and are then deliberately ignored: this
slice creates sessions, and lifecycle and receive semantics belong to the
slices that implement them. Tests pin that so a half-implemented lifecycle
cannot appear by accident.

Add `irtt-server` with the deterministic protocol and session engine the upcoming Tokio runtime will call. `ServerCore::handle_datagram` takes a source endpoint and the bytes of one datagram and returns the bytes to send back, or nothing. It performs no I/O and keeps no clock, so every decision is a pure function of its state and that input apart from drawing session tokens. Rejection is silence. The protocol defines no error reply, no reset and no NACK, so malformed, unauthenticated and policy-refused datagrams are discarded without a response and without disturbing any live session; a client tells "rejected" from "lost" only by timing out. `ServerError` is reserved for internal failures on the server's own side, so an inbound `ProtoError` never surfaces as a server failure. Open handling authenticates before decoding parameters, so a packet that fails authentication is discarded without parsing untrusted data and no reply behavior reveals which stage rejected it. Duration and Interval are accepted when absent and rejected when explicitly present and not positive, which is what the presence-aware parameter decode is for. Negotiation currently rewrites only the protocol version; the restriction knobs the specification describes are deliberately left for their own slices, with one obvious place to add them. Sessions are bounded to a finite default. A single unauthenticated datagram creates one and opens are never deduplicated, so an unbounded table is a liability rather than a compatibility feature; upstream's absence of session bounds is explicitly not a target. A full table drops new opens silently and evicts nothing. No-test opens keep working at capacity because they create no state. Session tokens come from the operating system's random source, since a token is the only thing standing between an off-path attacker and a usable session on an unauthenticated server. A drawn value that is zero or collides with a live session is redrawn within a finite budget; there is no predictable fallback, and a colliding token never overwrites the session that holds it. The reply is encoded before the session is inserted, so an internal failure cannot leave a half-created session behind. Echo and close requests pass admission and are then deliberately ignored: this slice creates sessions, and lifecycle and receive semantics belong to the slices that implement them. Tests pin that so a half-implemented lifecycle cannot appear by accident.
chatgpt-codex-connector[bot] (Migrated from github.com) reviewed 2026-08-11 17:54:37 +00:00
chatgpt-codex-connector[bot] (Migrated from github.com) left a comment

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 59f6e88005

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

### 💡 Codex Review Here are some automated review suggestions for this pull request. **Reviewed commit:** `59f6e88005` <details> <summary>ℹ️ About Codex in GitHub</summary> <br/> [Your team has set up Codex to review pull requests in this repo](https://chatgpt.com/codex/cloud/settings/general). Reviews are triggered when you - Open a pull request for review - Mark a draft as ready - Comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". </details>
chatgpt-codex-connector[bot] (Migrated from github.com) commented 2026-08-11 17:54:37 +00:00

P2 Badge Preserve Clock::Both as the public default

For downstream callers constructing values such as Params { stamp_at: StampAt::Both, ..Params::default() }, changing the derived default from Both to Unspecified silently changes encoded OPEN packets by omitting the Clock tag and makes echo layout calculations omit all timestamp fields. The decoder needs Unspecified for an absent wire tag, but it can initialize that value explicitly without changing the established public Default behavior.

AGENTS.md reference: AGENTS.md:L74-L76

Useful? React with 👍 / 👎.

**<sub><sub>![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat)</sub></sub> Preserve Clock::Both as the public default** For downstream callers constructing values such as `Params { stamp_at: StampAt::Both, ..Params::default() }`, changing the derived default from `Both` to `Unspecified` silently changes encoded OPEN packets by omitting the Clock tag and makes echo layout calculations omit all timestamp fields. The decoder needs `Unspecified` for an absent wire tag, but it can initialize that value explicitly without changing the established public `Default` behavior. AGENTS.md reference: [AGENTS.md:L74-L76](https://github.com/Lochnair/irtt-rs/blob/59f6e8800576d5a9b1546a632cdfdcbcb34a6347/AGENTS.md#L74-L76) Useful? React with 👍 / 👎.
Lochnair (Migrated from github.com) reviewed 2026-08-11 18:11:13 +00:00
Lochnair (Migrated from github.com) commented 2026-08-11 18:11:13 +00:00

Not taking this one — the change is the point of the refactor. Params is now defined as the low-level wire representation, so Params { stamp_at: StampAt::Both, ..Params::default() } is meant to read as "StampAt=Both, Clock absent" rather than silently inheriting Clock=Both from a client-oriented default that had leaked into the protocol type. Normal clients do not regress: ClientConfig::default() sets Clock::Both explicitly and client config validation rejects Clock::Unspecified.

Not taking this one — the change is the point of the refactor. `Params` is now defined as the low-level wire representation, so `Params { stamp_at: StampAt::Both, ..Params::default() }` is meant to read as "StampAt=Both, Clock absent" rather than silently inheriting `Clock=Both` from a client-oriented default that had leaked into the protocol type. Normal clients do not regress: `ClientConfig::default()` sets `Clock::Both` explicitly and client config validation rejects `Clock::Unspecified`.
Sign in to join this conversation.