armon / go-proxyproto Goto Github PK

HAProxy and others, such as Varnish, already support proxy protocol version 2.

Is it possible to do a small change on the error in protocol.go:

log.Printf("[ERR] Failed to read proxy prefix: %v", err)
to
log.Printf("[ERROR] Failed to read proxy prefix: %v", err)

At present there are portion of Vault logs where this does not align:

… vault1 vault[11406]: … [ERROR] core: unlocking HA lock failed: error="cannot find peer"
… vault1 vault[11406]: … [ERROR] storage.raft.raft-net: failed to accept connection: error="Raft RPC layer closed"
… vault1 vault[11406]: … [ERR] Failed to read proxy prefix: read tcp4 …:8200->…:36236: read: connection reset by peer 

I can do a PR around this but I'm not sure if it could potentially impact other forks / builds using this else where?

@armon This project has been incredibly useful - would you consider tagging a release for this project? (at v1.0 or otherwise) I'd love to be able to more reliably reference a specific version of this library in my projects.

This isn't exactly a bug with this package, but we learned the hard way in production that this code (or anything implementing this interface) isn't suitable for use with http.Server.Serve. Might be worth mentioning in docs.

Specifically: http.Server.Serve looks like:

// Serve accepts incoming connections on the Listener l, creating a
// new service goroutine for each.  The service goroutines read requests and
// then call srv.Handler to reply to them.
func (srv *Server) Serve(l net.Listener) error {
    defer l.Close()
    var tempDelay time.Duration // how long to sleep on accept failure
    for {
        rw, e := l.Accept()
        if e != nil {
            if ne, ok := e.(net.Error); ok && ne.Temporary() {
                if tempDelay == 0 {
                    tempDelay = 5 * time.Millisecond
                } else {
                    tempDelay *= 2
                }
                if max := 1 * time.Second; tempDelay > max {
                    tempDelay = max
                }
                srv.logf("http: Accept error: %v; retrying in %v", e, tempDelay)
                time.Sleep(tempDelay)
                continue
            }
            return e
        }
        tempDelay = 0
        c, err := srv.newConn(rw)
        if err != nil {
            continue
        }
        c.setState(c.rwc, StateNew) // before Serve can return
        go c.serve()
    }
}

srv.newConn calls rw.RemoteAddr(), and if that's a proxyproto listener, it can block if the incoming connection doesn't actually send over any data. And this is before go c.serve(), so it's in the main http.Server.Serve goroutine!

Workarounds for this seem pretty complex, unfortunately...

Per the spec the automatic detection of whether the PROXY protocol is in use is a "MUST not", because it allows a "major security breach".

The receiver MUST NOT start processing the connection before it receives a
complete and valid PROXY protocol header. This is particularly important for
protocols where the receiver is expected to speak first (eg: SMTP, FTP or SSH).
The receiver may apply a short timeout and decide to abort the connection if
the protocol header is not seen within a few seconds (at least 3 seconds to
cover a TCP retransmit).

The receiver MUST be configured to only receive the protocol described in this
specification and MUST not try to guess whether the protocol header is present
or not. This means that the protocol explicitly prevents port sharing between
public and private access. Otherwise it would open a major security breach by
allowing untrusted parties to spoof their connection addresses. The receiver
SHOULD ensure proper access filtering so that only trusted proxies are allowed
to use this protocol.