Merge pull request #581 from fabiolb/issue-524-tcp-proxy

address concerns raised while troubleshooting #524

config/config.go

@@ -33,15 +33,17 @@ type CertSource struct {
 }
 
 type Listen struct {
-	Addr          string
-	Proto         string
-	ReadTimeout   time.Duration
-	WriteTimeout  time.Duration
-	CertSource    CertSource
-	StrictMatch   bool
-	TLSMinVersion uint16
-	TLSMaxVersion uint16
-	TLSCiphers    []uint16
+	Addr               string
+	Proto              string
+	ReadTimeout        time.Duration
+	WriteTimeout       time.Duration
+	CertSource         CertSource
+	StrictMatch        bool
+	TLSMinVersion      uint16
+	TLSMaxVersion      uint16
+	TLSCiphers         []uint16
+	ProxyProto         bool
+	ProxyHeaderTimeout time.Duration
 }
 
 type UI struct {

config/load.go

@@ -382,6 +382,14 @@ func parseListen(cfg map[string]string, cs map[string]CertSource, readTimeout, w
 				return Listen{}, err
 			}
 			l.TLSCiphers = c
+		case "pxyproto":
+			l.ProxyProto = (v == "true")
+		case "pxytimeout":
+			d, err := time.ParseDuration(v)
+			if err != nil {
+				return Listen{}, err
+			}
+			l.ProxyHeaderTimeout = d
 		}
 	}
 
@@ -403,7 +411,11 @@ func parseListen(cfg map[string]string, cs map[string]CertSource, readTimeout, w
 		log.Printf("[INFO] vault-pki requires strictmatch; enabling strictmatch for listener %s", l.Addr)
 		l.StrictMatch = true
 	}
-
+	if l.ProxyProto && l.ProxyHeaderTimeout == 0 {
+		// We should define a safe default if proxy-protocol was enabled but no header timeout was set.
+		// See https://github.com/fabiolb/fabio/issues/524 for more information.
+		l.ProxyHeaderTimeout = 250 * time.Millisecond
+	}
 	return
 }
 

go.mod

@@ -9,7 +9,7 @@ require (
 	github.com/Shopify/toxiproxy v2.1.3+incompatible // indirect
 	github.com/apache/thrift v0.0.0-20181028152738-da1169d75b15 // indirect
 	github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da // indirect
-	github.com/armon/go-proxyproto v0.0.0-20150207025855-609d6338d3a7
+	github.com/armon/go-proxyproto v0.0.0-20180202201750-5b7edb60ff5f
 	github.com/armon/go-radix v1.0.0 // indirect
 	github.com/aws/aws-sdk-go v1.15.76 // indirect
 	github.com/boltdb/bolt v1.3.1 // indirect

go.sum

@@ -19,6 +19,8 @@ github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da h1:8GUt8eRujhVEGZ
 github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmVTwzkszR9V5SSuryQ31EELlFMUz1kKyl939pY=
 github.com/armon/go-proxyproto v0.0.0-20150207025855-609d6338d3a7 h1:AuTQOaYRBRtfDwY9ksr2sLXBgNvI5Mi6nhPdKdLE8Uw=
 github.com/armon/go-proxyproto v0.0.0-20150207025855-609d6338d3a7/go.mod h1:QmP9hvJ91BbJmGVGSbutW19IC0Q9phDCLGaomwTJbgU=
+github.com/armon/go-proxyproto v0.0.0-20180202201750-5b7edb60ff5f h1:SaJ6yqg936TshyeFZqQE+N+9hYkIeL9AMr7S4voCl10=
+github.com/armon/go-proxyproto v0.0.0-20180202201750-5b7edb60ff5f/go.mod h1:QmP9hvJ91BbJmGVGSbutW19IC0Q9phDCLGaomwTJbgU=
 github.com/armon/go-radix v1.0.0 h1:F4z6KzEeeQIMeLFa97iZU6vupzoecKdU5TX24SNppXI=
 github.com/armon/go-radix v1.0.0/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8=
 github.com/aws/aws-sdk-go v1.15.76 h1:AZB4clNWIk13YJaTm07kqyrHkj7gZYBQCgyTh/v4Sec=

proxy/listen.go

@@ -3,16 +3,17 @@ package proxy
 import (
 	"crypto/tls"
 	"fmt"
+	"github.com/fabiolb/fabio/config"
 	"net"
 	"time"
 
 	proxyproto "github.com/armon/go-proxyproto"
 )
 
-func ListenTCP(laddr string, cfg *tls.Config) (net.Listener, error) {
-	addr, err := net.ResolveTCPAddr("tcp", laddr)
+func ListenTCP(l config.Listen, cfg *tls.Config) (net.Listener, error) {
+	addr, err := net.ResolveTCPAddr("tcp", l.Addr)
 	if err != nil {
-		return nil, fmt.Errorf("listen: Fail to resolve tcp addr. %s", laddr)
+		return nil, fmt.Errorf("listen: Fail to resolve tcp addr. %s", l.Addr)
 	}
 
 	var ln net.Listener
@@ -25,7 +26,12 @@ func ListenTCP(laddr string, cfg *tls.Config) (net.Listener, error) {
 	ln = tcpKeepAliveListener{ln.(*net.TCPListener)}
 
 	// enable PROXY protocol support
-	ln = &proxyproto.Listener{Listener: ln}
+	if l.ProxyProto {
+		ln = &proxyproto.Listener{
+			Listener:           ln,
+			ProxyHeaderTimeout: l.ProxyHeaderTimeout,
+		}
+	}
 
 	// enable TLS
 	if cfg != nil {

proxy/serve.go

@@ -55,7 +55,7 @@ func Shutdown(timeout time.Duration) {
 }
 
 func ListenAndServeHTTP(l config.Listen, h http.Handler, cfg *tls.Config) error {
-	ln, err := ListenTCP(l.Addr, cfg)
+	ln, err := ListenTCP(l, cfg)
 	if err != nil {
 		return err
 	}
@@ -70,7 +70,7 @@ func ListenAndServeHTTP(l config.Listen, h http.Handler, cfg *tls.Config) error
 }
 
 func ListenAndServeTCP(l config.Listen, h tcp.Handler, cfg *tls.Config) error {
-	ln, err := ListenTCP(l.Addr, cfg)
+	ln, err := ListenTCP(l, cfg)
 	if err != nil {
 		return err
 	}

route/access_rules.go

@@ -16,16 +16,20 @@ const (
 
 // AccessDeniedHTTP checks rules on the target for HTTP proxy routes.
 func (t *Target) AccessDeniedHTTP(r *http.Request) bool {
-	var ip net.IP
-	host, _, err := net.SplitHostPort(r.RemoteAddr)
+	// No rules ... skip checks
+	if len(t.accessRules) == 0 {
+		return false
+	}
 
+	host, _, err := net.SplitHostPort(r.RemoteAddr)
 	if err != nil {
 		log.Printf("[ERROR] failed to get host from remote header %s: %s",
 			r.RemoteAddr, err.Error())
 		return false
 	}
 
-	if ip = net.ParseIP(host); ip == nil {
+	ip := net.ParseIP(host)
+	if ip == nil {
 		log.Printf("[WARN] failed to parse remote address %s", host)
 	}
 
@@ -64,16 +68,21 @@ func (t *Target) AccessDeniedHTTP(r *http.Request) bool {
 
 // AccessDeniedTCP checks rules on the target for TCP proxy routes.
 func (t *Target) AccessDeniedTCP(c net.Conn) bool {
-	var addr *net.TCPAddr
-	var ok bool
+	// Calling RemoteAddr on a proxy-protocol enabled connection may block.
+	// Therefore we explicitly check and bail out early if there are no
+	// rules defined for the target.
+	// See https://github.com/fabiolb/fabio/issues/524 for background.
+	if len(t.accessRules) == 0 {
+		return false
+	}
 	// validate remote address assertion
-	if addr, ok = c.RemoteAddr().(*net.TCPAddr); !ok {
+	if addr, ok := c.RemoteAddr().(*net.TCPAddr); !ok {
 		log.Printf("[ERROR] failed to assert remote connection address for %s", t.Service)
 		return false
-	}
-	// check remote connection address
-	if t.denyByIP(addr.IP) {
+		// check remote connection address
+		if t.denyByIP(addr.IP) {
 			return true
+		}
 	}
 	// default allow
 	return false

vendor/modules.txt

@@ -2,7 +2,7 @@
 github.com/Shopify/sarama
 # github.com/apache/thrift v0.0.0-20181028152738-da1169d75b15
 github.com/apache/thrift/lib/go/thrift
-# github.com/armon/go-proxyproto v0.0.0-20150207025855-609d6338d3a7
+# github.com/armon/go-proxyproto v0.0.0-20180202201750-5b7edb60ff5f
 github.com/armon/go-proxyproto
 # github.com/circonus-labs/circonus-gometrics v2.2.4+incompatible
 github.com/circonus-labs/circonus-gometrics