Add support for Proxy Protocol between dnsdist and the recursor

[rgacogne]

Short description

This PR implements:

• outgoing Proxy Protocol (UDP, TCP, healthchecks) in dnsdist ;
• incoming Proxy Protocol (UDP, TCP) in the recursor.

It allows dnsdist to pass the initial source and destination addresses and ports to the recursor, along with custom values.
Compared to the (ab)use of EDNS Client Subnet, this doesn't prevent preserving an existing ECS value (zeo scope, notably) and provides the ports, along with custom values.
Compared to ECS and XPF, this prevents having to parse the existing payload and inject either an EDNS option or a new record, preserving TSIG-signatures.

Based on a rebased version of #8448.

Could use a good review :)

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)

[Habbie]

Did one skim, looks good so far! I'll test the code, and will do a careful review of the tests you wrote, and post another review.

[rgacogne]

I realized that the current logic of trying to reuse TCP backend connections is completely flawed when the proxy protocol is enabled. I'm working on a fix.

[rgacogne]

It's in fact more complicated than that because of the custom values. Currently they can be set per-query which works for UDP but doesn't match the fact that they can only be sent once per backend connection over TCP. We could disable connection reuse when the proxy protocol is enabled, but that would be quite bad for performances. On the other hand, if we keep it, we at least need to ensure that the same backend connection can only be used for the same incoming client connection which is a bit better for performances but cumbersome.

[Habbie]

I realized that the current logic of trying to reuse TCP backend connections is completely flawed when the proxy protocol is enabled. I'm working on a fix.

I've been wondering if the protocol should come with some framing, but that's another rabbit hole.

[rgacogne]

Fixed by disabling TCP connection reuse in dnsdist when a backend has the proxy protocol feature enabled. We could try to still reuse the connection for the same client, as long as the TLV values are empty, but it's error-prone since in that case we should not add the proxy protocol payload for subsequent queries, except if the connection breaks at some point and we need to open a new one.

[gibson042]

I'm very excited about this, but surprised to see the asymmetry of dnsdist being able to send PROXY messages but not receive them. Do you think it could also get settings like the recursor's proxy-protocol-from and proxy-protocol-maximum-size in this PR?

[rgacogne]

I'm very excited about this, but surprised to see the asymmetry of dnsdist being able to send PROXY messages but not receive them. Do you think it could also get settings like the recursor's proxy-protocol-from and proxy-protocol-maximum-size in this PR?

Yes, we are planning on adding support for incoming proxy protocol payload! Unfortunately our time is finite so we had to make some choices. This PR will not extend beyond outgoing proxy protocol, and I'm not sure yet whether we will be able to implement incoming support before 1.5.0, but it will definitely be there in 1.6.0 at the latest.

[rgacogne]

Rebased on master to fix a conflict with #8851.

[rgacogne]

@omoerbeek I requested a review from you mostly for the recursor-related bits, although your review on the rest of the code would be nice to have as well :-)

[omoerbeek]

• It should be documented how the proxy protocol and allow-from interact. AFAKS, if a request is proxied, the from address in the proxy header is used to check against allow-from.
• I don't know if the recursor packer parsing is subject to being fuzzed, but it would be nice, especially with these additions.
• A check for interoperability with another implementation would really be nice.

More comments inline.

[rgacogne]

I have fixed most of the reported shortcomings.

It should be documented how the proxy protocol and allow-from interact. AFAKS, if a request is proxied, the from address in the proxy header is used to check against allow-from.

Done!

I don't know if the recursor packer parsing is subject to being fuzzed, but it would be nice, especially with these additions.

I'm planning on adding a fuzzing target for the proxy protocol parser itself, but if you are thinking of the TCP handling code it's a bit complicated because it's event-based, with a state machine.

A check for interoperability with another implementation would really be nice.

Agreed, I'll try to do that.

[omoerbeek]

I couldn't find any implementation to test incoming or outgoing support over UDP, Cloudflare's Spectrum is supposed to support it but it requires an enterprise plan.
So unless someone disagrees, I'll rebase this PR to fix the regression test issue and then merge it as it is.

Did you manage to do a TCP test? That is at least something...

[rgacogne]

Did you manage to do a TCP test? That is at least something...

I did, see #8874 (comment)

[omoerbeek]

Did you manage to do a TCP test? That is at least something...

I did, see #8874 (comment)

Great! I missed that comment earlier. LGTM!