docs: Complete rewrite of PROXY protocol guide #3882
Conversation
polarathene
added
service/dovecot
service/postfix
area/security
service/security/fail2ban
kind/improvement
polarathene
requested review from
cfis,
casperklein,
wernerfred and
georglauterbach
|
Documentation preview for this PR is ready! 🎉 Built with commit: deda867 |
Preview linksDirect links for comparison:
Random notesPotentially relevant snippets if anyone is interested (for config inspection or testing locally): # View the non-defaults (or explicitly configured values) of a service:
doveconf -n service/imap-login
# View full config of a service:
doveconf service/imap-login
# Example: TLS verification with SNI:
openssl s_client -connect 172.16.42.2:587 -starttls smtp -servername mail.example.testNice article on PROXY protocol from 2020: https://seriousben.com/posts/2020-02-exploring-the-proxy-protocol/ |
| When working with Layer 7 and a protocol like HTTP, it is possible to inspect a protocol header like [`Forwarded`][networking::http-header::forwarded] (_or it's predecessor: [`X-Forwarded-For`][networking::http-header::x-forwarded-for]_). At a lower level with Layer 4, that information is not available and we are routing traffic agnostic to the application protocol being proxied. | ||
|
|
||
| A proxy can prepend the [PROXY protocol][networking::spec:proxy-protocol] header to the TCP/UDP connection as it is routed to the service, which must be configured to be compatible with PROXY protocol (_often this adds a restriction that connections must provide the header, otherwise they're rejected_). | ||
|
|
There was a problem hiding this comment.
I would remove "(often this adds a restriction that connections must provide the header, otherwise they're rejected)." I don't think its often, I think its always. But that is already captured by the sentence above.
There was a problem hiding this comment.
I don't think its often, I think its always.
What source do you have to say always? My statement is valid, here is why it varies:
- Traefik entrypoints support the PROXY header as optional. I'm able to enable it and connect without PROXY protocol in use from curl.
- Whereas the equivalent for Caddy will allow normal connections without PROXY protocol, unless they're in the
allowlist in which case they're expected to only use PROXY protocol.- I can allow Traefik to connect with PROXY protocol, and in a separate container route through Traefik or alternatively connect to Caddy directly without PROXY protocol. So long as I didn't allow PROXY protocol for the curl container by configuring the entire subnet, that direct connection via curl works fine.
- Postfix and Dovecot however only accept connections using PROXY protocol for ports that enable it. They're not as flexible.
There was a problem hiding this comment.
I base my comment on this paragraph from the spec (https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt):
The receiver MUST be configured to only receive the protocol described in this
specification and MUST not try to guess whether the protocol header is present
or not. This means that the protocol explicitly prevents port sharing between
public and private access. Otherwise it would open a major security breach by
allowing untrusted parties to spoof their connection addresses. The receiver
SHOULD ensure proper access filtering so that only trusted proxies are allowed
to use this protocol.
There was a problem hiding this comment.
This means that the protocol explicitly prevents port sharing between public and private access.
Otherwise it would open a major security breach by allowing untrusted parties to spoof their connection addresses.
The receiver SHOULD ensure proper access filtering so that only trusted proxies are allowed to use this protocol.
The SHOULD making it advised rather than mandatory resolves the issue though?
If the receiver only allows a connection from a trusted host, you could require the PROXY protocol header only on those connections. But what is the risk in making the header optional when you're trusting that host regardless?
Likewise, if you reject a connection from a host providing a PROXY header when they're not trusted, what is the risk in allowing them to connect without the PROXY header present?
The upcoming Caddy 2.8 release changed their PROXY protocol dependency to the same that Traefik is using. Thus they'll also have the capability for ports to accept an optional PROXY header that's accepted when the remote host is trusted by the reverse proxy software.
The Go library used is go-proxyproto, and this flexibility is offered via the Policy feature (landed roughly a year ago). You can see the implementation for Caddy here.
There wasn't much discussion about security concerns or the spec you're quoting, but there doesn't seem to be any major concerns with that flexibility via policy since it still implements the SHOULD to ensure PROXY protocol is only valid with trusted connections.
Regardless, the spec doesn't reflect actual popular software support in the wild which our docs are noting that the restriction is not always imposed.
So agree to leave as-is?
There was a problem hiding this comment.
I think the first sentence is pretty clear:
The receiver MUST be configured to only receive the protocol described in this
specification and MUST not try to guess whether the protocol header is present
or not.
I read the SHOULD as a secondary concern, it does not impact that first sentence in anyway. Thus, I understand why Postfix and Dovecot have implemented proxy support the way they have.
I think the Caddy code you posted is clearly in violation of the spec (not that it matters what I think of course). I would guess it was implemented by someone who didn't read the spec and likely didn't consider the security implications.
Having said all that, whether Caddy is right or wrong obviously shouldn't hold up this PR. Either leave as is, or maybe add text "according to the spec the port should always ... but some implementation allow ... "
Anyway pires/go-proxyproto#95 was a good find - familiar sounding problem.
There was a problem hiding this comment.
I would guess it was implemented by someone who didn't read the spec and likely didn't consider the security implications.
What security implications?
If you only follow the MUST requirements of the spec and ignore the SHOULD, you have less security 🤷♂️
I see no security concerns with the policy approach that go-proxyproto implements.
Either leave as is, or maybe add text "according to the spec the port should always ... but some implementation allow ... "
Leaving as-is 👍
The docs are focused on PROXY protocol compatibility with different reverse proxies / software relevant to DMS. I don't see any value in the expectations of the spec when they're not well justified (specifications can have flaws or become outdated too).
I have raised an issue at HAProxy, perhaps they'll update it accordingly or weigh in with something that justifies the current wording.
There was a problem hiding this comment.
The spec has been updated 11 times in ten years...guessing its not a flaw in the spec. Excellent that you raised an issue, hope it gets a response.
|
Added a few comments, but overall looks good to me! Thanks for writing this up. |
Thanks for the feedback! 😁 I know it's quite a bit to go over, so I really appreciate it! Writing docs is an exhausting process for me to tackle 😩 I think my replies have addressed the feedback you gave, but if you feel I could improve on the content or disagree in anyway let me know :) |
|
There is an AWS specific gotcha with their load balancer with PROXY protocol that affects SMTP, not sure if it's relevant for our docs though. I've not got the time to verify if it's still accurate. https://github.com/pires/go-proxyproto#special-notes It was added in June 2020: pires/go-proxyproto#25 Presumably fixed since. (EDIT: Whoops, forgot this was protocol specific, so maybe not resolved yet 🤷♂️ ) With Caddy 2.8 migrating to the referenced |
Description
Based on the rather long discussion at #3866
The existing docs are a bit dated:
amavis/postscreen.,)Added:
Type of change
Checklist
CHANGELOG.mdCan bundle in changelog entry referencing various docs PRs 👍