tsp-tcp: bound response size and handle short writes #4

New issue

Closed

opened 2026-07-21 17:20:16 +02:00 by heiko · 1 comment

heiko commented

2026-07-21 17:20:16 +02:00

Owner

Problem

internal/tsp-tcp/tcp.go sends the request with one conn.Write and reads the response using unbounded io.ReadAll(conn).

Risks:

A TCP peer can stream indefinitely and exhaust process memory.
Although net.Conn.Write should return a non-nil error for a short write, robust code must handle n != len(request) and return io.ErrShortWrite even if the error is nil.
The current error text says “short write” only when Write itself reports an error.

This endpoint is remote and untrusted. Context cancellation bounds time, but not allocation rate before the deadline.

Checklist: #81 (network-client timeout/resource hardening) and #53 (complete I/O error handling).

Proposed fix

Define a justified maximum RFC 3161 TCP response size.
Read at most max+1 bytes using io.LimitReader/LimitedReader; reject any response exceeding the maximum.
Use a full-write helper or explicitly reject n != len(request) with io.ErrShortWrite.
Keep context cancellation able to unblock both operations.
Consider setting connection deadlines from the context as a simpler alternative to a watcher goroutine, while preserving cancellation behavior.

Acceptance criteria

A response exactly at the limit is accepted.
An oversized or endless response is rejected without unbounded memory growth.
A test connection that simulates a short write returns io.ErrShortWrite.
Existing local timestamp round-trip and context cancellation tests pass.

Reviewed against 948b7a9 on master (Go 1.26.2).

## Problem `internal/tsp-tcp/tcp.go` sends the request with one `conn.Write` and reads the response using unbounded `io.ReadAll(conn)`. Risks: - A TCP peer can stream indefinitely and exhaust process memory. - Although `net.Conn.Write` should return a non-nil error for a short write, robust code must handle `n != len(request)` and return `io.ErrShortWrite` even if the error is nil. - The current error text says “short write” only when `Write` itself reports an error. This endpoint is remote and untrusted. Context cancellation bounds time, but not allocation rate before the deadline. Checklist: #81 (network-client timeout/resource hardening) and #53 (complete I/O error handling). ## Proposed fix - Define a justified maximum RFC 3161 TCP response size. - Read at most `max+1` bytes using `io.LimitReader`/`LimitedReader`; reject any response exceeding the maximum. - Use a full-write helper or explicitly reject `n != len(request)` with `io.ErrShortWrite`. - Keep context cancellation able to unblock both operations. - Consider setting connection deadlines from the context as a simpler alternative to a watcher goroutine, while preserving cancellation behavior. ## Acceptance criteria - A response exactly at the limit is accepted. - An oversized or endless response is rejected without unbounded memory growth. - A test connection that simulates a short write returns `io.ErrShortWrite`. - Existing local timestamp round-trip and context cancellation tests pass. Reviewed against `948b7a9` on `master` (Go 1.26.2).

heiko commented

2026-07-21 23:07:49 +02:00

Author

Owner

Response-size bounds, exact-limit behavior, short-write handling, and regression coverage are complete.

Closing commit: b5c194322a89

Response-size bounds, exact-limit behavior, short-write handling, and regression coverage are complete. Closing commit: [`b5c194322a89`](https://forgejo.schlittermann.de/heiko/mail-seal/commit/b5c194322a8904230e4771c9aa55d20065f30913)

heiko closed this issue

2026-07-21 23:07:50 +02:00

heiko referenced this issue from a commit

2026-08-01 12:03:04 +02:00

fix(verify): version-exact canon registry, strict FieldSet, corpus/regression guards ai:claude-sonnet-4-5

heiko referenced this issue from a commit

2026-08-01 12:03:04 +02:00

fix(verify): document strict FieldSet, add fuzz seeds, clarify error ai:claude-opus-4-8