Support Proxy protocol

[zeripath]

This PR adds functionality to allow Gitea to sit behind an
HAProxy and Proxy protocolled connections directly.

Fix #7508

Signed-off-by: Andrew Thornton art27@cantab.net

[zeripath]

This PR is currently a WIP simply because I have not tested it with a HAProxy as yet and therefore it may not work!

Tested and works.

[silverwind]

PROXY Protocol is a standard that is also implemented by Nginx and to my knowledge primarily serves to communicate client ip to a upstream service in non-transparent TCP proxy mode.

I would not call it "HAProxy protcol" as that is just confusing. Or maybe you're implementing something different after all 😉

[codecov-commenter]

Codecov Report

❗ No coverage uploaded for pull request base (main@6784a70). Click here to learn what that means.
The diff coverage is 3.44%.

@@           Coverage Diff           @@
##             main   #12527   +/-   ##
=======================================
  Coverage        ?   46.94%           
=======================================
  Files           ?      993           
  Lines           ?   136742           
  Branches        ?        0           
=======================================
  Hits            ?    64199           
  Misses          ?    64666           
  Partials        ?     7877           

Impacted Files	Coverage Δ
cmd/web.go	0.00% <0.00%> (ø)
cmd/web_acme.go	0.00% <0.00%> (ø)
cmd/web_graceful.go	0.00% <0.00%> (ø)
cmd/web_https.go	0.00% <0.00%> (ø)
modules/graceful/server_http.go	0.00% <0.00%> (ø)
modules/private/internal.go	29.16% <0.00%> (ø)
modules/proxyprotocol/conn.go	0.00% <0.00%> (ø)
modules/proxyprotocol/errors.go	0.00% <0.00%> (ø)
modules/proxyprotocol/listener.go	0.00% <0.00%> (ø)
modules/proxyprotocol/util.go	0.00% <0.00%> (ø)
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[zeripath]

Fine. I've renamed and relabelled everything to make it clear it is the PROXY protocol. I'm sorry if it was unclear before.

[silverwind]

Any specific reason to not use one of these?

• https://github.com/pires/go-proxyproto (v1 and v2)
• https://github.com/armon/go-proxyproto (v1 only)

Also I'm wondering why this needs so much configuration. I thought proxy protocol is pretty much zero-conf as it's detectable by the static byte signature.

[zeripath]

https://github.com/armon/go-proxyproto was the only one I found and hitching our stars to a proxy v1 version seems silly.

I didn't find the https://github.com/pires/go-proxyproto one.

[silverwind]

Notable TODO I've seen.

[lunny]

I think @silverwind is right. You can not configure that. i.e. gitea.com have a geo domain system. Some locations you will visit the servers directly, other locations, you will visit the haproxy machines. So how to config that?

[zeripath]

@lunny In that case you can't use the PROXY protocol. The protocol is pretty strict the PROXY header must always be there - allowing direct access allows spoofing of PROXY headers and hence IP addresses.

If you want to allow direct communication to the Gitea server you cannot use the PROXY protocol. Any server that is running the PROXY protocol must be behind a proxy all the time.

@silverwind go-proxyproto doesn't handle unix addresses - which should be fine as you wouldn't really want to proxy unix addresses like this - but AFAICS doesn't properly handle reject on missing PROXY header. The protocol is very strict - if you expect a PROXY header it must be there. go-proxyproto allows you to set Require and will error on the first read if it doesn't happen but doesn't close the connection and relies on the upstream user to do this. Similarly AFAICS the PROXY protocol says you must read the header before you send data - which this doesn't appear to do.

[silverwind]

So if I understand that protocol correctly, we need to check the first bytes of the request for those static proxy protocol strings and if present, strip the header from the request. This type of checking will incur a performance penalty for non-proxy protocol users, so it'd be best to have it behind a preference. As for the other 5 options this PR adds, I'm not sure they are warranted, thought.

Regarding unix sockets: It'd certainly be possible to combine unix sockets with proxy protocol but it's a rather obscure combination, so I'm happy if we don't support that initially.

[zeripath]

The protocol is very clear:

The receiver MUST NOT start processing the connection before it receives a
complete and valid PROXY protocol header. This is particularly important for
protocols where the receiver is expected to speak first (eg: SMTP, FTP or SSH).
The receiver may apply a short timeout and decide to abort the connection if
the protocol header is not seen within a few seconds (at least 3 seconds to
cover a TCP retransmit).

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.

I've implemented the unix sockets already.

The options are the absolute minimum required for this to work. We listen in 3 places:

• Main socket (HTTP/HTTPS/Unix)
• HTTP Redirector
• SSH

Any solution that does not allow you to choose to place which ones behind the Proxy will end up having to implement that in future.

For HTTPS the question is whether the PROXY protocol occurs before or after TLS negotiation - therefore that needs an option (AFAIU there is no equivalent of TLS bridging for SSH as it doesn't make sense IMO)

There's an option to provide the timeout for the header - I can't understand why that is not a reasonable thing.

There's an option whether to accept UNKNOWN headers - which is explicitly recommended in the Spec.

Finally there is an option regarding whether when gitea speaks to itself "internally" it needs to use the PROXY protocol. It is not hard to imagine a situation whereby the locally running gitea manager/gitea serv/gitea hook might need to be redirected through the proxy rather than going directly - this option provides a way to do that.

[lafriks]

Conflict needs to be resolved but otherwise lgtm

[zeripath]

I think we should move this to 1.14 unless there's definitely someone who really wants it in 1.13

[silverwind]

I plan to test this against Nginx's PROXY protocol implementation later.

[wxiaoguang]

All of the options are needed.
See #12527 (comment)

Why not internal SSH only? HTTP/HTTPS can use X-Real-IP header, that's more straight, and all reverse-proxies support it (including haproxy). And I read the issue #7508 again, the author also said that they can handle HTTP/HTTPS, the only need is internal SSH server. Usually no HTTP/HTTPS server is running behind a PROXY protocol.

I can not imagine a real use case where the users must use PROXY protocol for the HTTP/HTTPS server. It's easy to add code in the code base but it's hard to remove them, so maybe we can be more careful about adding new features (especially for rare cases). We can only make internal SSH support PROXY protocol and it does resolve #7508 . If there is really further requirements for HTTP/HTTPS, we can evaluate why X-Real-IP can not be used and then make another PR.

[terribleplan]

I would love to see at least PROXY protocol v1 support for the internal SSH server added soon. Many hosting configurations (including in k8s) make the use of a LB essential even if only running one app server instance. This means that without such support all incoming connections appear to come from the same LB IPs which is far from ideal.

[silverwind]

Needs a rebase.

[zeripath]

conflicts resolved.

Should be re-reviewed prior to merging though.

[zeripath]

OK I think I've fixed the mismerges.

[silverwind]

All of the options are needed.
See #12527 (comment)

Why not internal SSH only? HTTP/HTTPS can use X-Real-IP header, that's more straight, and all reverse-proxies support it (including haproxy). And I read the issue #7508 again, the author also said that they can handle HTTP/HTTPS, the only need is internal SSH server. Usually no HTTP/HTTPS server is running behind a PROXY protocol.

I can not imagine a real use case where the users must use PROXY protocol for the HTTP/HTTPS server. It's easy to add code in the code base but it's hard to remove them, so maybe we can be more careful about adding new features (especially for rare cases). We can only make internal SSH support PROXY protocol and it does resolve #7508 . If there is really further requirements for HTTP/HTTPS, we can evaluate why X-Real-IP can not be used and then make another PR.

I think it's a pretty common scenario when you want complete end-to-end encryption with the gitea web server listening on HTTPS with a load balancer before that only does TCP proxying. In such a case, metadata like real client ip can not be communicated to gitea by the proxy (unless LB spoofs the source ip). I think this is the primary reason why proxy protocol was invented in first place.

[silverwind]

LGTM. I still think some of the settings are probably unnecessary and their behaviour can be done automatically, but I guess setting removal can be a future improvement.