From 99b5c1986a6b079e52b0efa61da915f7a0e92bc5 Mon Sep 17 00:00:00 2001 From: Aaron Hurt Date: Fri, 7 Dec 2018 10:27:41 -0600 Subject: [PATCH] allow toggling of proxy-protocol per listener - see #524 --- config/config.go | 20 ++--- config/load.go | 14 +++- go.mod | 2 +- go.sum | 2 + proxy/listen.go | 14 +++- proxy/serve.go | 4 +- .../github.com/armon/go-proxyproto/README.md | 2 +- .../armon/go-proxyproto/protocol.go | 75 ++++++++++++++++--- vendor/modules.txt | 2 +- 9 files changed, 104 insertions(+), 31 deletions(-) diff --git a/config/config.go b/config/config.go index 22ff1e153..3b1c04d2d 100644 --- a/config/config.go +++ b/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 { diff --git a/config/load.go b/config/load.go index 60d039acf..7b3c091a0 100644 --- a/config/load.go +++ b/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 } diff --git a/go.mod b/go.mod index 2b4354a51..287ef33e7 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 27847a38d..60e89d361 100644 --- a/go.sum +++ b/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= diff --git a/proxy/listen.go b/proxy/listen.go index 41ad9ddbb..1d4b5cea7 100644 --- a/proxy/listen.go +++ b/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 { diff --git a/proxy/serve.go b/proxy/serve.go index c1859bb4f..424ea79a1 100644 --- a/proxy/serve.go +++ b/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 } diff --git a/vendor/github.com/armon/go-proxyproto/README.md b/vendor/github.com/armon/go-proxyproto/README.md index 25a779cca..47e971885 100644 --- a/vendor/github.com/armon/go-proxyproto/README.md +++ b/vendor/github.com/armon/go-proxyproto/README.md @@ -28,7 +28,7 @@ Using the library is very simple: list, err := net.Listen("tcp", "...") // Wrap listener in a proxyproto listener -proxyList := &proxyproto.Listener{list} +proxyList := &proxyproto.Listener{Listener: list} conn, err :=proxyList.Accept() ... diff --git a/vendor/github.com/armon/go-proxyproto/protocol.go b/vendor/github.com/armon/go-proxyproto/protocol.go index 2fc1dfc01..38bb6cbb1 100644 --- a/vendor/github.com/armon/go-proxyproto/protocol.go +++ b/vendor/github.com/armon/go-proxyproto/protocol.go @@ -3,6 +3,7 @@ package proxyproto import ( "bufio" "bytes" + "errors" "fmt" "io" "log" @@ -18,25 +19,49 @@ var ( // to check if this connection is using the proxy protocol prefix = []byte("PROXY ") prefixLen = len(prefix) + + ErrInvalidUpstream = errors.New("upstream connection address not trusted for PROXY information") ) +// SourceChecker can be used to decide whether to trust the PROXY info or pass +// the original connection address through. If set, the connecting address is +// passed in as an argument. If the function returns an error due to the source +// being disallowed, it should return ErrInvalidUpstream. +// +// If error is not nil, the call to Accept() will fail. If the reason for +// triggering this failure is due to a disallowed source, it should return +// ErrInvalidUpstream. +// +// If bool is true, the PROXY-set address is used. +// +// If bool is false, the connection's remote address is used, rather than the +// address claimed in the PROXY info. +type SourceChecker func(net.Addr) (bool, error) + // Listener is used to wrap an underlying listener, // whose connections may be using the HAProxy Proxy Protocol (version 1). // If the connection is using the protocol, the RemoteAddr() will return // the correct client address. +// +// Optionally define ProxyHeaderTimeout to set a maximum time to +// receive the Proxy Protocol Header. Zero means no timeout. type Listener struct { - Listener net.Listener + Listener net.Listener + ProxyHeaderTimeout time.Duration + SourceCheck SourceChecker } // Conn is used to wrap and underlying connection which // may be speaking the Proxy Protocol. If it is, the RemoteAddr() will // return the address of the client instead of the proxy address. type Conn struct { - bufReader *bufio.Reader - conn net.Conn - dstAddr *net.TCPAddr - srcAddr *net.TCPAddr - once sync.Once + bufReader *bufio.Reader + conn net.Conn + dstAddr *net.TCPAddr + srcAddr *net.TCPAddr + useConnRemoteAddr bool + once sync.Once + proxyHeaderTimeout time.Duration } // Accept waits for and returns the next connection to the listener. @@ -46,7 +71,19 @@ func (p *Listener) Accept() (net.Conn, error) { if err != nil { return nil, err } - return NewConn(conn), nil + var useConnRemoteAddr bool + if p.SourceCheck != nil { + allowed, err := p.SourceCheck(conn.RemoteAddr()) + if err != nil { + return nil, err + } + if !allowed { + useConnRemoteAddr = true + } + } + newConn := NewConn(conn, p.ProxyHeaderTimeout) + newConn.useConnRemoteAddr = useConnRemoteAddr + return newConn, nil } // Close closes the underlying listener. @@ -61,10 +98,11 @@ func (p *Listener) Addr() net.Addr { // NewConn is used to wrap a net.Conn that may be speaking // the proxy protocol into a proxyproto.Conn -func NewConn(conn net.Conn) *Conn { +func NewConn(conn net.Conn, timeout time.Duration) *Conn { pConn := &Conn{ - bufReader: bufio.NewReader(conn), - conn: conn, + bufReader: bufio.NewReader(conn), + conn: conn, + proxyHeaderTimeout: timeout, } return pConn } @@ -104,9 +142,11 @@ func (p *Conn) RemoteAddr() net.Addr { p.once.Do(func() { if err := p.checkPrefix(); err != nil && err != io.EOF { log.Printf("[ERR] Failed to read proxy prefix: %v", err) + p.Close() + p.bufReader = bufio.NewReader(p.conn) } }) - if p.srcAddr != nil { + if p.srcAddr != nil && !p.useConnRemoteAddr { return p.srcAddr } return p.conn.RemoteAddr() @@ -125,11 +165,22 @@ func (p *Conn) SetWriteDeadline(t time.Time) error { } func (p *Conn) checkPrefix() error { + if p.proxyHeaderTimeout != 0 { + readDeadLine := time.Now().Add(p.proxyHeaderTimeout) + p.conn.SetReadDeadline(readDeadLine) + defer p.conn.SetReadDeadline(time.Time{}) + } + // Incrementally check each byte of the prefix for i := 1; i <= prefixLen; i++ { inp, err := p.bufReader.Peek(i) + if err != nil { - return err + if neterr, ok := err.(net.Error); ok && neterr.Timeout() { + return nil + } else { + return err + } } // Check for a prefix mis-match, quit early diff --git a/vendor/modules.txt b/vendor/modules.txt index e0687c971..dce0de3ed 100644 --- a/vendor/modules.txt +++ b/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