Parsing v2 UNSPEC and v1 PROXY UNKNOWN not implemented

[bohanwood]

Hi! I found that this library is missing the ability to parse these two kinds of header, which are required by the protocol and mandatory to implement, while trying to resolve XTLS/Xray-core#166

v2 UNSPEC

When this header is received:

0d 0a 0d 0a 00 0d 0a 51 55 49 54 0a 20 00 00 00

which is + LOCAL command (\x20) + UNSPEC address family UNSPEC protocol (\x00)
ErrUnsupportedAddressFamilyAndProtocol is returned

go-proxyproto/v2.go

Lines 85 to 87 in adbbabe

	if _, ok := supportedTransportProtocol[header.TransportProtocol]; !ok {
	return nil, ErrUnsupportedAddressFamilyAndProtocol
	}

This is because UNSPEC is absent in supportedTransportProtocol

go-proxyproto/addr_proto.go

Lines 6 to 23 in adbbabe

	const (
	UNSPEC AddressFamilyAndProtocol = '\x00'
	TCPv4 AddressFamilyAndProtocol = '\x11'
	UDPv4 AddressFamilyAndProtocol = '\x12'
	TCPv6 AddressFamilyAndProtocol = '\x21'
	UDPv6 AddressFamilyAndProtocol = '\x22'
	UnixStream AddressFamilyAndProtocol = '\x31'
	UnixDatagram AddressFamilyAndProtocol = '\x32'
	)
	var supportedTransportProtocol = map[AddressFamilyAndProtocol]bool{
	TCPv4: true,
	UDPv4: true,
	TCPv6: true,
	UDPv6: true,
	UnixStream: true,
	UnixDatagram: true,
	}

This UNSPEC header is used when HAProxy doing health checks, or the proxied client is requesting via a unix socket.
Also the protocol spec says:

Only the UNSPEC protocol byte (\x00) is mandatory to implement on the receiver.
A receiver is not required to implement other ones, provided that it
automatically falls back to the UNSPEC mode for the valid combinations above
that it does not support.

v1 PROXY UNKNOWN

When header PROXY UNKNOWN\r\n is received,

go-proxyproto/v1.go

Lines 25 to 33 in adbbabe

	// Make sure we have a v1 header
	line, err := reader.ReadString('\n')
	if !strings.HasSuffix(line, crlf) {
	return nil, ErrCantReadProtocolVersionAndCommand
	}
	tokens := strings.Split(line[:len(line)-2], separator)
	if len(tokens) < 6 {
	return nil, ErrCantReadProtocolVersionAndCommand
	}

returns ErrCantReadProtocolVersionAndCommand.
This is because in this case, there's only 2 parts separated by spaces, instead of 6 parts as PROXY <PROTO> <SRC_ADDR> <DEST_ADDR> <SRC_PORT> <DEST_PORT>
The protocol spec says:

If the announced transport protocol is "UNKNOWN", then the receiver knows that
the sender speaks the correct PROXY protocol with the appropriate version, and
SHOULD accept the connection and use the real connection's parameters as if
there were no PROXY protocol header on the wire.

Also, this kind of header can be formatted by the library.

go-proxyproto/v1.go

Lines 76 to 88 in adbbabe

	func (header *Header) formatVersion1() ([]byte, error) {
	// As of version 1, only "TCP4" ( \x54 \x43 \x50 \x34 ) for TCP over IPv4,
	// and "TCP6" ( \x54 \x43 \x50 \x36 ) for TCP over IPv6 are allowed.
	var proto string
	switch header.TransportProtocol {
	case TCPv4:
	proto = "TCP4"
	case TCPv6:
	proto = "TCP6"
	default:
	// Unknown connection (short form)
	return []byte("PROXY UNKNOWN\r\n"), nil
	}

Although I really don't know much about Golang, I'll try my best to support you to resolve this issue. Thanks!

[pires]

Thank you for the detailed issue description.

I think I can do this, however the spec seems to have changed under our feet, ie

Note that an earlier version of this specification suggested to use this when sending health checks,
but this causes issues with servers that reject the "UNKNOWN" keyword. Thus is it
now recommended not to send "UNKNOWN" when the connection is expected to
be accepted, but only when it is not possible to correctly fill the PROXY line.

[bohanwood]

Thank you. I admit that UNKNOWN is not something recommended, but there ARE cases that "it is not possible to correctly fill the PROXY line", for example when using unix socket or even abstract domain docket with HAProxy. IMO we should implement it as this is a generic library.

[pires]

For v2, I'll add support for UNSPEC but only when LOCAL is set. The spec is clear the receiver can reject the unsupported stuff. Do you have a different interpretation?

[bohanwood]

Yeah, it's OK to restrict UNSPEC to LOCAL only.

HAProxy only sends 3 kinds of header:

• v1: UNKNOWN, TCP4, TCP6
• v2: LOCAL UNSPEC, PROXY AF_INET STREAM, PROXY AF_INET6 STREAM

ref: https://github.com/haproxy/haproxy/blob/6ca89162dc881df8fecd7713ca1fe5dbaa66b315/src/connection.c#L1009

[pires]

PR to fix this is open. Will wait a few days for feedback, if that's ok.

[pires]

Are you able to try it @bohanyang or provide me with reproducible steps?