Merge pull request #1391 from slingamn/proxy.3

fix #1389

default.yaml

@@ -52,9 +52,10 @@ server:
             tls:
                 cert: fullchain.pem
                 key: privkey.pem
-                # 'proxy' should typically be false. It's only for Kubernetes-style load
+            # 'proxy' should typically be false. It's for cloud load balancers that
-                # balancing that does not terminate TLS, but sends an initial PROXY line
+            # always send PROXY headers ahead of the connection (e.g., a v1 header
-                # in plaintext.
+            # ahead of unterminated TLS, or a v2 binary header) that MUST be present
+            # and cannot be processed on an optional basis.
             proxy: false
 
         # Example of a Unix domain socket for proxying:

irc/config.go

@@ -49,12 +49,13 @@ import (
 type TLSListenConfig struct {
 	Cert  string
 	Key   string
-	Proxy bool
+	Proxy bool // XXX: legacy key: it's preferred to specify this directly in listenerConfigBlock
 }
 
 // This is the YAML-deserializable type of the value of the `Server.Listeners` map
 type listenerConfigBlock struct {
 	TLS       TLSListenConfig
+	Proxy     bool
 	Tor       bool
 	STSOnly   bool `yaml:"sts-only"`
 	WebSocket bool
@@ -829,8 +830,8 @@ func (conf *Config) prepareListeners() (err error) {
 				return err
 			}
 			lconf.TLSConfig = tlsConfig
-			lconf.RequireProxy = block.TLS.Proxy
 		}
+		lconf.RequireProxy = block.TLS.Proxy || block.Proxy
 		lconf.WebSocket = block.WebSocket
 		conf.Server.trueListeners[addr] = lconf
 	}

irc/gateways.go

@@ -121,9 +121,11 @@ func handleProxyCommand(server *Server, client *Client, session *Session, line s
 		}
 	}()
 
-	ip, err := utils.ParseProxyLine(line)
+	ip, err := utils.ParseProxyLineV1(line)
 	if err != nil {
 		return err
+	} else if ip == nil {
+		return nil
 	}
 
 	if utils.IPInNets(client.realIP, server.Config().Server.proxyAllowedFromNets) {

irc/server.go

@@ -747,7 +747,7 @@ func (server *Server) loadFromDatastore(config *Config) (err error) {
 func (server *Server) setupListeners(config *Config) (err error) {
 	logListener := func(addr string, config utils.ListenerConfig) {
 		server.logger.Info("listeners",
-			fmt.Sprintf("now listening on %s, tls=%t, tlsproxy=%t, tor=%t, websocket=%t.", addr, (config.TLSConfig != nil), config.RequireProxy, config.Tor, config.WebSocket),
+			fmt.Sprintf("now listening on %s, tls=%t, proxy=%t, tor=%t, websocket=%t.", addr, (config.TLSConfig != nil), config.RequireProxy, config.Tor, config.WebSocket),
 		)
 	}
 

irc/utils/proxy.go

@@ -5,7 +5,9 @@ package utils
 
 import (
 	"crypto/tls"
+	"encoding/binary"
 	"errors"
+	"io"
 	"net"
 	"strings"
 	"sync"
@@ -18,7 +20,7 @@ const (
 	// https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt
 	// "a 108-byte buffer is always enough to store all the line and a trailing zero
 	// for string processing."
-	maxProxyLineLen = 107
+	maxProxyLineLenV1 = 107
 )
 
 // XXX implement net.Error with a Temporary() method that returns true;
@@ -56,41 +58,88 @@ type ListenerConfig struct {
 	WebSocket bool
 }
 
-// read a PROXY line one byte at a time, to ensure we don't read anything beyond
+// read a PROXY header (either v1 or v2), ensuring we don't read anything beyond
-// that into a buffer, which would break the TLS handshake
+// the header into a buffer (this would break the TLS handshake)
-func readRawProxyLine(conn net.Conn, deadline time.Duration) (result string) {
+func readRawProxyLine(conn net.Conn, deadline time.Duration) (result []byte, err error) {
 	// normally this is covered by ping timeouts, but we're doing this outside
 	// of the normal client goroutine:
 	conn.SetDeadline(time.Now().Add(deadline))
 	defer conn.SetDeadline(time.Time{})
 
-	var buf [maxProxyLineLen]byte
+	// read the first 16 bytes of the proxy header
-	oneByte := make([]byte, 1)
+	buf := make([]byte, 16, maxProxyLineLenV1)
-	i := 0
+	_, err = io.ReadFull(conn, buf)
-	for i < maxProxyLineLen {
-		n, err := conn.Read(oneByte)
 	if err != nil {
 		return
-		} else if n == 1 {
-			buf[i] = oneByte[0]
-			if buf[i] == '\n' {
-				candidate := string(buf[0 : i+1])
-				if strings.HasPrefix(candidate, "PROXY") {
-					return candidate
-				} else {
-					return
-				}
-			}
-			i += 1
-		}
 	}
 
-	// no \r\n, fail out
+	switch buf[0] {
-	return
+	case 'P':
+		// PROXY v1: starts with "PROXY"
+		return readRawProxyLineV1(conn, buf)
+	case '\r':
+		// PROXY v2: starts with "\r\n\r\n"
+		return readRawProxyLineV2(conn, buf)
+	default:
+		return nil, ErrBadProxyLine
+	}
 }
 
-// ParseProxyLine parses a PROXY protocol (v1) line and returns the remote IP.
+func readRawProxyLineV1(conn net.Conn, buf []byte) (result []byte, err error) {
-func ParseProxyLine(line string) (ip net.IP, err error) {
+	for {
+		i := len(buf)
+		if i >= maxProxyLineLenV1 {
+			return nil, ErrBadProxyLine // did not find \r\n, fail
+		}
+		// prepare a single byte of free space, then read into it
+		buf = buf[0 : i+1]
+		_, err = io.ReadFull(conn, buf[i:])
+		if err != nil {
+			return nil, err
+		}
+		if buf[i] == '\n' {
+			return buf, nil
+		}
+	}
+}
+
+func readRawProxyLineV2(conn net.Conn, buf []byte) (result []byte, err error) {
+	// "The 15th and 16th bytes is the address length in bytes in network endian order."
+	addrLen := int(binary.BigEndian.Uint16(buf[14:16]))
+	if addrLen == 0 {
+		return buf[0:16], nil
+	} else if addrLen <= cap(buf)-16 {
+		buf = buf[0 : 16+addrLen]
+	} else {
+		// proxy source is unix domain, we don't really handle this
+		buf2 := make([]byte, 16+addrLen)
+		copy(buf2[0:16], buf[0:16])
+		buf = buf2
+	}
+	_, err = io.ReadFull(conn, buf[16:16+addrLen])
+	if err != nil {
+		return
+	}
+	return buf[0 : 16+addrLen], nil
+}
+
+// ParseProxyLine parses a PROXY protocol (v1 or v2) line and returns the remote IP.
+func ParseProxyLine(line []byte) (ip net.IP, err error) {
+	if len(line) == 0 {
+		return nil, ErrBadProxyLine
+	}
+	switch line[0] {
+	case 'P':
+		return ParseProxyLineV1(string(line))
+	case '\r':
+		return parseProxyLineV2(line)
+	default:
+		return nil, ErrBadProxyLine
+	}
+}
+
+// ParseProxyLineV1 parses a PROXY protocol (v1) line and returns the remote IP.
+func ParseProxyLineV1(line string) (ip net.IP, err error) {
 	params := strings.Fields(line)
 	if len(params) != 6 || params[0] != "PROXY" {
 		return nil, ErrBadProxyLine
@@ -102,6 +151,62 @@ func ParseProxyLine(line string) (ip net.IP, err error) {
 	return ip.To16(), nil
 }
 
+func parseProxyLineV2(line []byte) (ip net.IP, err error) {
+	if len(line) < 16 {
+		return nil, ErrBadProxyLine
+	}
+	// this doesn't allocate
+	if string(line[:12]) != "\x0d\x0a\x0d\x0a\x00\x0d\x0a\x51\x55\x49\x54\x0a" {
+		return nil, ErrBadProxyLine
+	}
+	// "The next byte (the 13th one) is the protocol version and command."
+	versionCmd := line[12]
+	// "The highest four bits contains the version [....] it must always be sent as \x2"
+	if (versionCmd >> 4) != 2 {
+		return nil, ErrBadProxyLine
+	}
+	// "The lowest four bits represents the command"
+	switch versionCmd & 0x0f {
+	case 0:
+		return nil, nil // LOCAL command
+	case 1:
+		// PROXY command, continue below
+	default:
+		// "Receivers must drop connections presenting unexpected values here"
+		return nil, ErrBadProxyLine
+	}
+
+	var addrLen int
+	// "The 14th byte contains the transport protocol and address family."
+	protoAddr := line[13]
+	// "The highest 4 bits contain the address family"
+	switch protoAddr >> 4 {
+	case 1:
+		addrLen = 4 // AF_INET
+	case 2:
+		addrLen = 16 // AF_INET6
+	default:
+		return nil, nil // AF_UNSPEC or AF_UNIX, either way there's no IP address
+	}
+
+	// header, source and destination address, two 16-bit port numbers:
+	expectedLen := 16 + 2*addrLen + 4
+	if len(line) < expectedLen {
+		return nil, ErrBadProxyLine
+	}
+
+	// "Starting from the 17th byte, addresses are presented in network byte order.
+	//  The address order is always the same :
+	//    - source layer 3 address in network byte order [...]"
+	if addrLen == 4 {
+		ip = net.IP(line[16 : 16+addrLen]).To16()
+	} else {
+		ip = make(net.IP, addrLen)
+		copy(ip, line[16:16+addrLen])
+	}
+	return ip, nil
+}
+
 /// WrappedConn is a net.Conn with some additional data stapled to it;
 // the proxied IP, if one was read via the PROXY protocol, and the listener
 // configuration.
@@ -161,8 +266,10 @@ func (rl *ReloadableListener) Accept() (conn net.Conn, err error) {
 		// but that's OK because this listener *requires* a PROXY line,
 		// therefore it must be used with proxies that always send the line
 		// and we won't get slowloris'ed waiting for the client response
-		proxyLine := readRawProxyLine(conn, config.ProxyDeadline)
+		proxyLine, err := readRawProxyLine(conn, config.ProxyDeadline)
+		if err == nil {
 			proxiedIP, err = ParseProxyLine(proxyLine)
+		}
 		if err != nil {
 			conn.Close()
 			return nil, err

traditional.yaml

@@ -26,9 +26,10 @@ server:
             tls:
                 cert: fullchain.pem
                 key: privkey.pem
-                # 'proxy' should typically be false. It's only for Kubernetes-style load
+            # 'proxy' should typically be false. It's for cloud load balancers that
-                # balancing that does not terminate TLS, but sends an initial PROXY line
+            # always send PROXY headers ahead of the connection (e.g., a v1 header
-                # in plaintext.
+            # ahead of unterminated TLS, or a v2 binary header) that MUST be present
+            # and cannot be processed on an optional basis.
             proxy: false
 
         # Example of a Unix domain socket for proxying: