From 76e8e61705e1d2f9f6e0c09d86eb4e9e7d45d0bc Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 2 Mar 2026 01:10:26 -0800 Subject: [PATCH] fix #2227 (#2334) * fix #2227 Instead of returning an error from Accept(), force the caller to process errors from trying to read and parse the PROXY protocol. The advantage here is that we don't have to rely on (net.Error).Temporary or incur timed backoff from net/http when hitting these errors. However, we still risk stalling processing of new incoming connections if someone opens a connection to the proxy listener and doesn't send anything. This is hard to fix while maintaining the net.Listener abstraction in cooperation with http.Server. * reduce proxy deadline to 5 seconds --- irc/config.go | 2 +- irc/listeners.go | 16 ++++++++++++-- irc/utils/proxy.go | 53 +++++++++++++++++----------------------------- 3 files changed, 34 insertions(+), 37 deletions(-) diff --git a/irc/config.go b/irc/config.go index f8e2ed70..f56d4833 100644 --- a/irc/config.go +++ b/irc/config.go @@ -46,7 +46,7 @@ import ( ) const ( - defaultProxyDeadline = time.Minute + defaultProxyDeadline = 5 * time.Second ) // here's how this works: exported (capitalized) members of the config structs diff --git a/irc/listeners.go b/irc/listeners.go index a1f7b988..c69c46d3 100644 --- a/irc/listeners.go +++ b/irc/listeners.go @@ -99,8 +99,13 @@ func (nl *NetListener) serve() { // hand off the connection wConn, ok := conn.(*utils.WrappedConn) if ok { - confirmProxyData(wConn, "", "", "", nl.server.Config()) - go nl.server.RunClient(NewIRCStreamConn(wConn)) + if wConn.ProxyError == nil { + confirmProxyData(wConn, "", "", "", nl.server.Config()) + go nl.server.RunClient(NewIRCStreamConn(wConn)) + } else { + nl.server.logger.Error("internal", "PROXY protocol error", nl.addr, wConn.ProxyError.Error()) + conn.Close() + } } else { nl.server.logger.Error("internal", "invalid connection type", nl.addr) } @@ -185,6 +190,13 @@ func (wl *WSListener) handle(w http.ResponseWriter, r *http.Request) { conn.Close() return } + if wConn.ProxyError != nil { + // actually the connection is likely corrupted, so probably Upgrade() + // would have already failed + wl.server.logger.Error("internal", "PROXY protocol error on websocket", wl.addr, wConn.ProxyError.Error()) + conn.Close() + return + } confirmProxyData(wConn, remoteAddr, xff, xfp, config) diff --git a/irc/utils/proxy.go b/irc/utils/proxy.go index d31a4ff8..3650e778 100644 --- a/irc/utils/proxy.go +++ b/irc/utils/proxy.go @@ -6,6 +6,7 @@ package utils import ( "crypto/tls" "encoding/binary" + "errors" "io" "net" "strings" @@ -20,24 +21,8 @@ const ( maxProxyLineLenV1 = 107 ) -// XXX implement net.Error with a Temporary() method that returns true; -// otherwise, ErrBadProxyLine will cause (*http.Server).Serve() to exit -type proxyLineError struct{} - -func (p *proxyLineError) Error() string { - return "invalid PROXY line" -} - -func (p *proxyLineError) Timeout() bool { - return false -} - -func (p *proxyLineError) Temporary() bool { - return true -} - var ( - ErrBadProxyLine error = &proxyLineError{} + ErrBadProxyLine = errors.New("invalid PROXY protocol line") ) // ListenerConfig is all the information about how to process @@ -208,12 +193,13 @@ func parseProxyLineV2(line []byte) (ip net.IP, err error) { // configuration. type WrappedConn struct { net.Conn - ProxiedIP net.IP - TLS bool - Tor bool - STSOnly bool - WebSocket bool - HideSTS bool + ProxiedIP net.IP + ProxyError error + TLS bool + Tor bool + STSOnly bool + WebSocket bool + HideSTS bool // Secure indicates whether we believe the connection between us and the client // was secure against interception and modification (including all proxies): Secure bool @@ -256,6 +242,7 @@ func (rl *ReloadableListener) Accept() (conn net.Conn, err error) { } var proxiedIP net.IP + var proxyError error if config.RequireProxy { // this will occur synchronously on the goroutine calling Accept(), // but that's OK because this listener *requires* a PROXY line, @@ -265,10 +252,7 @@ func (rl *ReloadableListener) Accept() (conn net.Conn, err error) { if err == nil { proxiedIP, err = ParseProxyLine(proxyLine) } - if err != nil { - conn.Close() - return nil, err - } + proxyError = err } if config.TLSConfig != nil { @@ -276,13 +260,14 @@ func (rl *ReloadableListener) Accept() (conn net.Conn, err error) { } return &WrappedConn{ - Conn: conn, - ProxiedIP: proxiedIP, - TLS: config.TLSConfig != nil, - Tor: config.Tor, - STSOnly: config.STSOnly, - WebSocket: config.WebSocket, - HideSTS: config.HideSTS, + Conn: conn, + ProxiedIP: proxiedIP, + ProxyError: proxyError, + TLS: config.TLSConfig != nil, + Tor: config.Tor, + STSOnly: config.STSOnly, + WebSocket: config.WebSocket, + HideSTS: config.HideSTS, // Secure will be set later by client code }, nil }