Test TLS Server code has the wrong wrapping order

[kinghrothgar]

I was using this library to build a service that terminates TLS and does IP based blocking using the proxy protocol headers. As far I can tell, wrapping the tls listener inside of proxyproto listener is the wrong order. I'm not sure why the test code works with how it is:

https://github.com/pires/go-proxyproto/blob/main/protocol_test.go#L960

When I did it exactly the way the test does it, curl --haproxy-protocol returns TLS errors and in the code I got tls: first record does not look like a TLS handshake. When I did it this way, it works:

	l, _ := net.Listen("tcp", ":8443")
	ppl := &proxyproto.Listener{
		Listener: l,
		Policy: func(upstream net.Addr) (proxyproto.Policy, error) {
			return proxyproto.REQUIRE, nil
		},
	}
	...
	listener := tls.NewListener(l, &config)

I believe this logically makes since because the proxy protocol header needs to be handled first as the tls library doesn't know how to handle it. Is there something I'm missing?

[pires]

I'm sorry for the delay in coming back to you on this one, but in between work and sickness, I just haven't been able to find the time to look into this.

[pires]

@emersion since you've been active and I think you also wrap TLS, can you maybe help here? Thanks in advance!

[kennylevinsen]

I'm not sure why the test code works with how it is

The test will work equally well for tls(proxy(tcp)) as it will for proxy(tls(tcp)). All that matters is whether or not the client and server agree, which they obviously do in the test.

In the test, a TCP connection is established, a TLS connection is built on top of that, and the PROXY header is sent over the TLS connection like any other TLS-protected data.

The behavior of curl and haproxy (following the intent of the protocol) is to establish a TCP connection, send the PROXY header, and then build a TLS connection on top of the TCP connection after our PROXY shenanigans.

[kinghrothgar]

So maybe the solution here is just adding another example? That would hopefully save future peeps from having to figure this out via debugging. I can submit a PR if y'all would like.

[kennylevinsen]

Well, changing the test to be the "proper" use should be fine. No need to test unnecessary things, especially as they have no effect on the operation of the library.

[pires]

I'm fine with changing the test flow if that improves the experience of other developers that rely on this project. So yeah @kinghrothgar, please, do submit a PR.

[shanezhiu]

I'm not sure why the test code works with how it is

The test will work equally well for tls(proxy(tcp)) as it will for proxy(tls(tcp)). All that matters is whether or not the client and server agree, which they obviously do in the test.

In the test, a TCP connection is established, a TLS connection is built on top of that, and the PROXY header is sent over the TLS connection like any other TLS-protected data.

The behavior of curl and haproxy (following the intent of the protocol) is to establish a TCP connection, send the PROXY header, and then build a TLS connection on top of the TCP connection after our PROXY shenanigans.

I got the same issue.

and I rearrange the order,

lis, err := net.Listen("tcp4", addr)
	
lis = &proxyproto.Listener{
	Listener: lis,
}
s.Listener = tls.NewListener(lis, config)

and it worked for me.