Skip to content

Conversation

@emersion
Copy link
Contributor

@emersion emersion commented Feb 5, 2026

Alternative to #144 and #150. Based on #154.

See individual commits.

Copy link
Owner

@pires pires left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am conceding on increasing the max V2 header size to 4KB as it seems to fix the issues observed by @clementnuss and others.

Up until now we were using Peek to ensure (1) the client sent at
least the number of bytes announced in the length field and (2) the
client doesn't exceed a 256 byte limit. This wasn't very explicit,
and forces each connection to pay an up-front memory cost of the
largest header size we want to accept.

Instead, add an explicit check for the max header size (to avoid
DoS), and check that all bytes have been read from the
io.LimitedReader.
PP2_SUBTYPE_SSL_CLIENT_CERT is typically around 1 or 2KiB.
Add a test with a 2KiB TLV, which is typical for
PP2_SUBTYPE_SSL_CLIENT_CERT.
This is important to check for security purposes: all TLVs are
buffered in memory, and we don't want clients to be able to cause
a DoS due to ENOMEM.
@pires pires merged commit 1d477a1 into pires:main Feb 5, 2026
8 checks passed
@emersion emersion deleted the large-tlv branch February 5, 2026 15:40
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.

2 participants