Skip to content

pgwire: bound startup and pre-auth frame sizes per call site - #38809

Draft
jubrad wants to merge 1 commit into
MaterializeInc:mainfrom
jubrad:justin/clo-270-right-size-pgwire-frame-bounds
Draft

pgwire: bound startup and pre-auth frame sizes per call site#38809
jubrad wants to merge 1 commit into
MaterializeInc:mainfrom
jubrad:justin/clo-270-right-size-pgwire-frame-bounds

Conversation

@jubrad

@jubrad jubrad commented Sep 12, 2026

Copy link
Copy Markdown
Member

Issue

parse_frame_len applies one ceiling, MAX_FRAME_SIZE (64 MiB), to three call sites whose real requirements differ by orders of magnitude. The startup path inherited a limit sized for bulk COPY traffic, and decode_startup sizes its buffer from the declared length before reading any of the body:

let frame_len = parse_frame_len(&frame_len)?;   // bounded only by MAX_FRAME_SIZE
let mut buf = BytesMut::new();
buf.resize(frame_len, b'0');                    // allocate and fill, before reading
conn.read_exact(&mut buf).await?;               // then wait for a body that may never arrive

resize writes every byte, so the pages are committed rather than lazily mapped. Four bytes from an unauthenticated client, on a plaintext socket before any TLS handshake, commit 64 MiB. That is roughly 16 connections per 1 GiB balancerd pod, on a component every tenant in a region shares.

Found by a penetration test.

Linear: CLO-270, a sub-issue of CLO-267.

Solution

The ceiling becomes an argument to parse_frame_len, so each call site states the bound it actually needs and a caller added later cannot silently inherit 64 MiB.

site bound when
decode_startup MAX_STARTUP_FRAME_SIZE, 10,000 bytes always
balancerd Codec MAX_PREAUTH_FRAME_SIZE, 16 KiB always
pgwire Codec MAX_PREAUTH_FRAME_SIZE, then MAX_FRAME_SIZE raised on successful authentication

decode_startup takes 10,000 bytes, matching PostgreSQL's MAX_STARTUP_PACKET_LENGTH, so any client that can complete a PostgreSQL handshake can complete this one.

environmentd passes MAX_FORWARDED_STARTUP_FRAME_SIZE, which adds a 128 byte allowance. balancerd appends mz_connection_uuid and mz_forwarded_for to the parameters before forwarding, up to 119 bytes, so a frame that just fits the client budget arrives downstream larger than the client sent it. Without the allowance such a connection would be accepted by balancerd and then rejected behind it, surfacing as a proxy error with no obvious cause.

balancerd's Codec only ever decodes one frame, the client's credential, before copy_bidirectional splices the connection and the remaining bytes are proxied unframed. It never needs the wider bound.

pgwire's Codec serves one connection across both phases, so no single constant can be tight enough for a credential and wide enough for a large Bind parameter. The bound starts tight and FramedConn::allow_post_auth_frames widens it, called immediately before AuthenticationOk. StateMachine is constructed in exactly one place, after that call, so no authenticated path can skip the raise.

Testing

New unit tests in mz-pgwire-common:

  • test_startup_frame_over_budget_is_rejected_before_allocating asserts an oversized declaration is refused and, crucially, that no buffer was sized first. It fails against the previous bound.
  • test_startup_frame_within_budget_is_accepted guards against over-tightening.
  • test_forwarded_startup_params_fit_the_allowance builds a frame at exactly the client budget, appends the two forwarded parameters at their widest possible values, and asserts it still decodes downstream. It fails if a third forwarded parameter is added without raising the allowance.
  • test_startup_body_wait_has_no_deadline records what this change does not fix, and pins that a stalled connection is now bounded by the startup budget rather than MAX_FRAME_SIZE.

New integration tests in mz-balancerd:

  • test_pgwire_oversized_startup_frame_is_rejected drives a real balancerd listener and asserts an oversized declaration is refused while a frame at the budget is still served, so the rejection is the budget doing its job rather than balancerd refusing everything.
  • test_pgwire_startup_preauth_connections_are_held records the remaining gap.

Gotchas for the reviewer

  • This bounds what one pre-startup connection costs, not how many exist. A client can still open unlimited connections, declare a frame within budget, go silent and hold them: there is no handshake deadline and no pre-startup connection limit. That is CLO-272. The two tests noted above are written so they will fail and want flipping when it lands.
  • decode_startup still pre-allocates. At 10,000 bytes the memset is irrelevant, so this keeps the diff small. Reading incrementally removes the remaining amplification but produces no ceiling on its own, so it is follow-up polish rather than part of the fix.
  • The pre-auth bound is 16 KiB rather than ~1 KB because an authenticator may accept a JWT rather than an app password, and those run to several KB.
  • src/pgwire/src/codec.rs still has no aggregate MAX_REQUEST_SIZE guard, unlike balancerd's. The comment on environmentd/tests/server.rs says that was removed deliberately so large parameters work, and it is unchanged here.

Release notes

This release will reject pgwire startup messages larger than 10,000 bytes, matching PostgreSQL's limit, and hold pre-authentication messages to 16 KiB.

🤖 Generated with Claude Code

`parse_frame_len` applied a single 64 MiB ceiling, `MAX_FRAME_SIZE`, to
three call sites whose real requirements differ by orders of magnitude.
The startup path inherited a limit meant for bulk `COPY` traffic, so four
unauthenticated bytes on a plaintext socket made `decode_startup`
allocate and zero-fill 64 MiB before reading any of the body.

The ceiling is now an argument, so each site states the bound it needs
and a new caller cannot silently inherit 64 MiB:

* `decode_startup` takes `MAX_STARTUP_FRAME_SIZE` (10,000 bytes, matching
  PostgreSQL's `MAX_STARTUP_PACKET_LENGTH`). environmentd passes
  `MAX_FORWARDED_STARTUP_FRAME_SIZE`, which adds an allowance for the two
  parameters balancerd appends in transit. Without it a frame that just
  fits the client budget would be accepted by balancerd and then rejected
  behind it.
* balancerd's `Codec` takes `MAX_PREAUTH_FRAME_SIZE` (16 KiB). It only
  ever decodes a credential before the connection is spliced.
* pgwire's `Codec` serves one connection across both phases, so the bound
  starts at `MAX_PREAUTH_FRAME_SIZE` and `allow_post_auth_frames` widens
  it once authentication succeeds.

Closes: CLO-270

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant