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

@@ -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

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)
 

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
 }