pacing_rate had been computed on every ACK since the controller was written
and never read by anything: admission was decided purely by cwnd. But cwnd
bounds how much may be UNACKNOWLEDGED, not how fast it reaches the wire, so a
full window went out back-to-back. On a bottleneck with a deep buffer that
burst is absorbed rather than dropped, and it lands as standing queue — the
mechanism behind the multi-second RTT this protocol has been showing on
mobile. It is also why BBR could not simply be dropped in: BBR's whole model
is "send at the estimated bottleneck rate", which is meaningless without a
pacer underneath it.
Adds a token bucket to CongestionController, charged in on_send so every byte
that reaches the wire pays exactly once — retransmits included, since those
are precisely what must not bypass the limit and pile into a full queue.
Burst allowance is 10ms-at-rate rather than one packet. Pacing intervals here
are fractions of a millisecond, so strict per-packet release would need a
sub-millisecond timer per packet; sizing the burst to the loop's existing
~10ms wakeup lets the configured rate still be saturated. There is a floor of
4 MTU so a cold or collapsed rate estimate can never wedge sending entirely.
Wired into both directions. The client gates its proxy-event branch on it,
and the server's per-session backpressure snapshot reports zero headroom when
the bucket is empty. The download path matters most here — that is the one
carrying video — and it was also still clamped to the old 16384-packet
ceiling (~20 MB outstanding), now aligned with MAX_CWND_PACKETS.
Tested that the bucket denies once drained and refills over time; the second
is what keeps a stalled bucket from wedging sending permanently.
Reported symptom: on mobile the reported RTT jumps to 15-20s (worst case
~150s), video stops loading, and it takes ~5 minutes to recover — or doesn't,
until the user reconnects.
Nothing on the network takes 150 seconds. That delay was our own queue. The
controller only ever treated LOSS as congestion, and mobile carrier buffers
are deep enough to absorb a burst rather than drop it, so the loss signal
never arrived and three things compounded:
- slow start grew cwnd unbounded, with the client permitting up to 16384
packets in flight (~20 MB — minutes of queue on a mobile uplink);
- the resulting standing queue inflated RTT samples, which raised SRTT,
which raised the adaptive RTO, so retransmits piled into the same queue;
- backoff multiplied that already-inflated RTO by up to 64x. With RTO_MAX
at 16s a frame could sit unretransmitted for ~17 minutes.
Reconnecting "fixed" it only because a fresh session resets cwnd to 32.
Three bounds, matching how delay-based controllers handle this:
- Treat sustained RTT inflation as congestion: leave slow start at 2x the
observed path floor, actively halve cwnd at 4x. This is the part that
works where loss never comes.
- Hard cwnd ceiling of 1024 packets (~1.2 MB), well above any real BDP here
but far below a queue measured in seconds. The client's in-flight gate is
lowered to match.
- Cap the post-backoff retransmit timer at 8s.
Tested for the no-loss inflation case and the ceiling. Both encode the bug
directly, since neither is observable from loss-driven tests.
Not addressed here: pacing is still computed and unused, so sending remains
bursty. That is the next lever and wants real-link validation.
Both produce the same reported symptom - traffic stops dead, the session
itself looks fine, and only a manual reconnect recovers it.
1. protocol.rs: a retry could be charged to a frame that was never sent.
The retransmit loop is budget-limited per tick, but it bumped `retries`
and reset `last_sent` for every due frame regardless of whether the
budget actually allowed a send. The budget is smallest exactly when loss
is heaviest (it is derived from cwnd, which collapses under loss), so
under real packet loss frames accumulated "phantom retries" they never
received - measured at 40 retries charged for 8 frames actually sent in
one tick. After max_retries+2 such rounds the zombie eviction dropped
them as dead. That data was never delivered and never would be: the
stream stalls permanently while pings keep flowing, so nothing upstream
notices anything is wrong. Retries/timers are now only charged on an
actual transmit, and the loop stops scanning once the budget is spent
(sent_history is in send order, so this also keeps retransmit priority
oldest-first). Covered by a new test that asserts retries charged ==
datagrams emitted; verified it fails against the old code.
2. bridge.rs: the stall detector was reset by datagrams that never
validated. `last_valid_recv` - "last VALID recv" - was assigned before
decryption, so a datagram that failed to decrypt still refreshed it on
its way to the error return. Anything landing on that port kept the
client convinced the tunnel was healthy: frames from a session the
server had already evicted, stale retransmits, or plain garbage from an
off-path source that knows the ip:port. The 25s background reconnect in
handle_keepalive therefore never fired. It also made the UI health
indicator report a dead tunnel as fine, and gave any off-path sender a
trivial way to pin a client in a dead session indefinitely. Now set only
after the datagram authenticates and decrypts.
The freeze users hit every few minutes: traffic drops to 0 B/s, the RTT
readout sticks at its last value, and only a manual reconnect clears it.
Delivery is gated on expected_recv_nonce, so one missing frame holds back
every frame behind it. That is correct only while the sender can still
retransmit — but the sender drops a frame from sent_history once it passes
max_retries + 2 attempts (zombie eviction in handle_tick). Past that point
the frame no longer exists anywhere and both sides deadlock: the receiver
buffers indefinitely and NACKs a nonce nobody can resend.
The watchdog could not save it, which is why it froze rather than
reconnecting. Retransmits, ACKs and NACKs keep arriving throughout, so the
client's last_valid_recv keeps refreshing and its 25s stall detector never
fires. The frozen RTT has the same cause: Pong travels in a Data frame,
stuck behind the very gap it would have reported.
The machinery for this was half-built: last_recv_advance was declared,
initialised and written on every advance, and its doc comment describes
exactly this recovery — but nothing ever read it, and a warning elsewhere
already referred to "gap recovery" that did not exist.
So implement it. Once the sequence has been stuck longer than the sender's
retransmit budget could plausibly last (8x the live RTO, clamped to 2..10s
so fast links do not discard merely-late frames and slow ones still
unblock), skip to the lowest buffered nonce, drain, and mark an ACK
pending so the peer stops retransmitting into a void.
This runs on the inbound path, not on Tick, for two reasons: both tick
handlers discard DeliverApp actions (client bridge.rs and server
dispatcher.rs match only SendDatagram/Multiple), and inbound frames keep
flowing all through the stall, so the path is reliably reached.
Skipping the hole drops one frame's payload — one RelayMessage, a chunk of
a single stream. That is a real cost, paid only when the data was already
lost for good, against a tunnel that otherwise stays dead until the user
intervenes.
Both tests were confirmed to fail without the fix (0 frames released
instead of 2), so they pin the deadlock rather than just the happy path.
- Karn's algorithm: drop_acked_frames no longer samples RTT from frames
that were retransmitted (last_sent is bumped on each retransmit, so an
ACK for the original transmission would measure a spuriously small RTT
and drag SRTT/RTO down). Added CongestionController::on_ack_no_rtt for
the case where every acked frame was ambiguous, so the window still
advances without polluting the RTT estimator. Refactored the shared
window-growth into grow_window.
- Frame decode: header+payload+pad length now uses checked_add. payload_len
is a u32 from the header and on 32-bit targets (MIPS/ARMv7 routers are
supported) the sum could wrap usize and slip past the truncation check.
- Replay cache: a full cache used to reject ALL new handshakes globally
until the next tick, letting one flooding key-holder deny service to
everyone. Now it reclaims expired entries and, if still full, evicts the
single oldest — new handshakes always get in. Fixed the mislabelled
"100000" log (cap is 50000) and named it REPLAY_CACHE_MAX.
The resumption module (SessionTicket/TicketValidator) was never wired into
the client or server — nothing issued or validated tickets, and no Resume
frame was ever sent. But it "encrypted" tickets by XOR-ing them with a
single static keystream SHA256(psk || const) and had no MAC (despite a doc
comment claiming HMAC): a textbook many-time-pad, trivially broken from a
couple of captured tickets, and malleable. Leaving it in-tree invited
someone to wire up a broken 0-RTT path later.
Removed the module, its FrameKind::Resume wire variant, and the protocol
handler for it. 0-RTT can be reintroduced later on a real AEAD-sealed
ticket if desired.
Two classes of issue:
- Hot-path/attacker-triggerable events logged at info/error with internal
detail: a per-handshake info! byte dump (raw_vec[0..6]) and a per-packet
error! on session-id mismatch that dumped expected/got session ids.
Both are log-flood + info-leak surfaces; downgraded to debug and
stripped of the sensitive detail. Close/Resume frame handling likewise
moved from info to debug.
- The access key (a shared secret) was written to logs verbatim in three
places (session drop, key-created UI event, API create-user) and as an
8-char prefix in one. Added key_fp() — a short SHA-256 fingerprint — and
routed all key logging through it so operators can still correlate
events without the secret ever hitting the log.
CRITICAL forward-secrecy fix. Session transport keys were derived as
SHA256(get_handshake_hash() || label). The Noise handshake hash `h` only
ever absorbs PUBLIC transcript data (ephemeral pubkeys + on-wire
ciphertexts, via MixHash); the ephemeral ee DH result is mixed via MixKey
into the chaining key `ck` ONLY, never into `h` (confirmed in snow 0.9.6
symmetricstate.rs). So the data-transport keys depended on the PSK and the
public transcript but NOT on the DH secret, meaning:
- zero forward secrecy: anyone who later learns the access-key PSK can
decrypt all recorded past sessions from the observed handshake alone;
- any PSK holder can passively decrypt any other session on that key;
- the ephemeral Diffie-Hellman was cryptographically wasted.
Fix: take the two directional keys from Noise's Split() over the final `ck`
via snow's dangerously_get_raw_split (risky-raw-split feature). These keys
depend on ee, restoring forward secrecy. The custom out-of-order AEAD,
explicit nonces, session_id AAD, framing and reordering are all unchanged
- only the key SOURCE moved. The dead into_transport()/handshake_hash()
paths and the unreachable NoiseSession::Transport variant are removed.
Wire-breaking: PROTOCOL_VERSION 4 -> 5 so pre-fix peers derive different
keys and cannot interop (version gate is invisible on the wire).
Added noise unit tests for the .0/.1 -> send/recv role mapping and the
not-finished guard.
In the Closing state the old code force-transitioned to Closed after a
SINGLE inbound packet, so any data/ACKs the peer still had in flight when we
initiated Close were dropped (Closed returns Noop for everything). Stay in
Closing and process inbound normally; handle_inbound already owns the
Close->Closed transition when it actually receives the peer's Close frame.
Also handle Tick in Closing so our own Close frame is retransmitted until
acknowledged.
Ported surgically from 47d44fa — only the Closing-state correctness fix, NOT
that commit's bundled RFC-6298 RTO / congestion rewrite (a behavioural change
to the working base) or the sent_history BTreeMap perf swap (broad hot-path
change for a perf-only gain). cargo test -p ostp-core: 36/36 incl.
test_close_sequence.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Boxed HandshakeState in NoiseSession to reduce enum variant sizes
- Used is_ok() instead of let Ok(_) pattern
- Applied automatic clippy fixes for minor warnings
8 new integration tests in ostp-core::protocol::tests:
- test_full_handshake: Noise handshake -> Established state
- test_data_exchange_client_to_server: encrypt/decrypt data frame C->S
- test_data_exchange_server_to_client: encrypt/decrypt data frame S->C
- test_close_sequence: Close frame -> Closed state
- test_wrong_psk_handshake_fails: bad PSK rejected, never reaches Established
- test_congestion_controller_after_handshake: CC budget >= 2 in SlowStart
- test_multiple_data_frames: 10 sequential frames, payload integrity verified
- test_tick_no_crash: Tick event stable on both sides
Total: 43 tests, 0 failures
Applied Kerckhoffs's principle: the protocol's security and obfuscation
now depend SOLELY on the access key. An adversary who reverse-engineers
the binary cannot build a DPI filter without knowing the key.
Changes:
- Replaced hardcoded salt string ('-ostp-psk-salt') with HKDF-SHA256.
The salt is now derived from the key hash itself — no protocol-specific
strings remain in the binary.
- Unified all secret derivation into derive_all_secrets() which produces
PSK, obfuscation key, and handshake padding range from a single HKDF
invocation.
- Handshake padding range is now key-derived: different access keys
produce different size distributions (min: 16-79, max: +48..+175).
A universal size-based filter is impossible without the key.
- HKDF-SHA256 (RFC 5869) implemented inline using existing hmac+sha2
dependencies — no new crate required.
What remains identifiable in the binary:
- 'Noise_NNpsk0_25519_ChaChaPoly_BLAKE2s' — standard Noise pattern
string, shared with many other projects, NOT OSTP-specific.
- Generic HMAC/SHA-256/ChaCha20-Poly1305 code — standard crypto
primitives used by millions of applications.
The previous commit added random padding after Noise handshake payloads
but the receiver passed the entire raw buffer (including padding) to
snow::read_handshake(), which cannot handle trailing bytes.
New wire format:
[session_id:4][noise_len:2][noise_payload:N][random_padding:32-128]
Changes:
- wrap_datagram_handshake: puts noise_len (u16 BE) at bytes [4..6]
before the Noise payload, followed by 32-128 random padding bytes
- handle_inbound: reads noise_len from [4..6], passes only
raw_vec[6..6+noise_len] to snow, ignoring trailing padding
- obfuscation: handshake mask extended from 4 to 6 bytes to also
cover the noise_len field (prevents DPI from seeing constant u16)
- dispatcher: key-trial loop updated to deobfuscate 6-byte header
Both client and server now produce/consume the same padded format.
DPI/TSPU resistance:
- Handshake packets now padded with 32-128 random bytes (prevents size
fingerprinting — previously every handshake was exactly 52 bytes)
- Frame header reserved bytes randomized instead of always 0 (prevents
known-plaintext oracle inside encrypted payload)
- Padding jitter cap increased from 96 to 256 bytes for better traffic
pattern masking
GUI Windows app (tunnel/proxy not starting):
- CRITICAL: Added CREATE_NO_WINDOW flag to all reg.exe calls in sysproxy.rs.
In Tauri GUI context (no console window), Command::new('reg') was silently
failing because there was no attached console. This prevented the Windows
system proxy from being enabled.
- Added ProxyOverride bypass list (localhost;127.*;10.*;192.168.*;<local>)
to prevent proxy loop for local traffic
- Added comprehensive logging for all registry operations
- Set initial connection_state to 1 (connecting) instead of 0 — prevents
UI polling from immediately flipping back to 'disconnected' before the
handshake has a chance to begin
Code quality:
- Fixed log file paths: log_to_core_file() and log_to_file() now write next
to the executable instead of CWD. In GUI context, CWD could be
C:\Windows\System32, causing write failures or misplaced log files.
Critical fixes (6):
- protocol.rs: in_flight_count() now counts only retransmittable Data frames,
not Ack/Nack control frames — eliminates false backpressure under load
- protocol.rs: NACK is now rate-limited to once per 30ms — prevents
retransmission storm during normal UDP jitter
- protocol.rs: zombie frames exceeding max_retries+4 are evicted each tick —
prevents unbounded memory growth and stale retransmits
- protocol.rs: Closing state now processes final in-flight packets instead
of silently dropping them — prevents data loss at session teardown
- server/lib.rs: stream_tx changed from bounded(10000) to unbounded_channel —
prevents TCP-reader collapse during Speedtest with 50+ streams
- bridge.rs: liveness timeout raised from 30s to 60s — prevents false
reconnect during heavy Speedtest load
Medium fixes (8):
- protocol.rs: ACK range truncation preserves cumulative range (index 0)
- bridge.rs: Ping now uses send_datagram() for correct TURN wrapping
- dispatcher.rs: replay_cache hard-capped at 100k entries (DoS protection)
- dispatcher.rs: old addr cleaned from addr_to_session on roaming
- server/lib.rs: TCP connect_target() now has 10s timeout
- config.rs: TURN section parsed during hot-reload
- proxy.rs: HTTP header parsing uses 512-byte chunks instead of 1-byte reads
- proxy.rs: stream_id wrap-around skips active IDs to prevent collision
- runner.rs: is_essential_log matches actual log strings from bridge.rs
Other:
- kex.rs: clearly marked as dead PQ stub (not used by protocol)
- README.md + README.ru.md: complete rewrite with architecture diagram
- docs/en/specification.md: updated ARQ section with all new semantics
ospab
ospab