Skip to content

dnsdist: Only set the proxy protocol payload size when actually added #15534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

rgacogne
Copy link
Member

@rgacogne rgacogne commented May 7, 2025

Short description

I can think of two cases where we got this wrong:

  • the query was initially assigned to a backend using the proxy protocol payload, then later restarted and assigned to a backend not using it. The proxy protocol payload size was then kept from the first assignment.
  • we failed to actually prepend the proxy protocol payload but the payload size was updated.

Both cases could cause a corrupted payload to be sent, or an exception to be raised if the size of the proxy protocol payload was larger than the size of the initial query.

Found while investigating #15529 but it might or might not be related.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

rgacogne and others added 2 commits May 9, 2025 11:18
I can think of two cases where we got this wrong:
- the query was initially assigned to a backend using the proxy protocol
  payload, then later restarted and assigned to a backend not using it.
  The proxy protocol payload size was then kept from the first assignment.
- we failed to actually prepend the proxy protocol payload but the payload
  size was updated.

Both cases could cause a corrupted payload to be sent, or an exception to
be raised if the size of the proxy protocol payload was larger than the
size of the initial query.
@rgacogne rgacogne force-pushed the ddist-fix-invalid-proxy-protocol-payload-size branch from adafc96 to a462a3b Compare May 9, 2025 09:18
@rgacogne
Copy link
Member Author

rgacogne commented May 9, 2025

Note to self: a regression test would be nice!

@rgacogne
Copy link
Member Author

Adding a regression test revealed that the issue was not fixed for TCP-based backends. Now fixed for TCP-backends as well, and with a regression test.

@rgacogne rgacogne merged commit d606622 into PowerDNS:master May 12, 2025
87 checks passed
@rgacogne rgacogne deleted the ddist-fix-invalid-proxy-protocol-payload-size branch May 12, 2025 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants