Skip to content
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

Add Support for "ProxyHeaderTimeout" #65

Closed
tecnobrat opened this issue Jan 27, 2021 · 8 comments
Closed

Add Support for "ProxyHeaderTimeout" #65

tecnobrat opened this issue Jan 27, 2021 · 8 comments

Comments

@tecnobrat
Copy link

This library seems to lean heavily on inspiration from https://github.com/armon/go-proxyproto

That library supports passing in a timeout when you define the listener:

		tln = &proxyproto.Listener{
			Listener:           tln,
			ProxyHeaderTimeout: l.ProxyHeaderTimeout,
		}

It would be nice if the same feature, or a similar feature was possible with this library.

@pires
Copy link
Owner

pires commented Mar 5, 2021

Can you please describe the requirement instead? What is is the problem you're running into?

@unmarshal
Copy link
Contributor

+1. The problem is that a malicious client can open thousands of tcp connections and not send the proxy protocol header. The server will hold those socket descriptors open indefinitely, eventually running out of resources. I'll take a crack at making a pull request.

@unmarshal
Copy link
Contributor

For your consideration: #74

Excellent library by the way. I am planning on using this in production.

unmarshal added a commit to unmarshal/go-proxyproto that referenced this issue Apr 10, 2021
Set a read deadline when waiting for the PROXY protocol header.
Fix for pires#65
unmarshal added a commit to unmarshal/go-proxyproto that referenced this issue Apr 12, 2021
Set a read deadline when waiting for the PROXY protocol header.
Fix for pires#65
@tecnobrat
Copy link
Author

Thank you @unmarshal!

Sorry I didn't see your question before @pires. My motivation is a little different, but @unmarshal is right on the technical reasoning.

My motivation is that I would like to add proxy v2 support to https://github.com/fabiolb/fabio and they already make use of ProxyHeaderTimeout, so for me to be able to swap their implementation to this library, I'd need that support.

Not great motivation for this project, but thats my personal motivation!

@unmarshal
Copy link
Contributor

Hi @tecnobrat, does that mean you would prefer it be called ProxyHeaderTimeout?

@tecnobrat
Copy link
Author

@unmarshal I don't think the naming matters, it just means I'll need to do:

		ln = &proxyproto.Listener{
			Listener:           ln,
			ReadHeaderTimeout: l.ProxyHeaderTimeout,
		}

No big deal :)

@pires
Copy link
Owner

pires commented Apr 15, 2021

Thanks for sharing, @tecnobrat.

@pires pires added this to the 0.6 milestone Apr 15, 2021
unmarshal added a commit to unmarshal/go-proxyproto that referenced this issue Apr 16, 2021
Set a read deadline when waiting for the PROXY protocol header.
Fix for pires#65
@pires
Copy link
Owner

pires commented Jul 10, 2021

This has been addressed by #74.

@pires pires closed this as completed Jul 10, 2021
rbeuque74 added a commit to loopfz/gadgeto that referenced this issue Jul 27, 2021
Following pires/go-proxyproto#65, we can now configure a
ReadHeaderTimeout on the listener, to protect ourselves from malicious
clients (CVE-2021-23409).

Signed-off-by: Romain Beuque <556072+rbeuque74@users.noreply.github.com>
csmith added a commit to csmith/soju that referenced this issue Sep 14, 2021
go-proxyproto added support for a read timeout in 0.6.0[1] and
defaulted it to 200ms. After this time if no data is read on
the socket, it is closed.

This is _really_ low if the underlying connection is a TLS
one as no data pops out the other end until the handshake is
done. It effectively limits you to TLS connections within
a 50ms RTT of your bouncer with clients that are fast enough
at responding.

It appears that HexChat on Arch is somehow slow enough at
TLS connections thant it consistently takes longer than
200ms even over localhost, meaning it outright can't connect
to soju any longer.

To make this a lot less painful, have soju pass in a read
timeout of 5 seconds. This feels like a reasonable tradeoff
between keeping (possibly malicious) connections open and
accepting the realities of network connections.

[1]: pires/go-proxyproto#65
emersion pushed a commit to emersion/soju that referenced this issue Sep 19, 2021
go-proxyproto added support for a read timeout in 0.6.0[1] and
defaulted it to 200ms. After this time if no data is read on
the socket, it is closed.

This is _really_ low if the underlying connection is a TLS
one as no data pops out the other end until the handshake is
done. It effectively limits you to TLS connections within
a 50ms RTT of your bouncer with clients that are fast enough
at responding.

It appears that HexChat on Arch is somehow slow enough at
TLS connections thant it consistently takes longer than
200ms even over localhost, meaning it outright can't connect
to soju any longer.

To make this a lot less painful, have soju pass in a read
timeout of 5 seconds. This feels like a reasonable tradeoff
between keeping (possibly malicious) connections open and
accepting the realities of network connections.

[1]: pires/go-proxyproto#65
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants