[bug] Can't establish websocket connection when using `NetDialTLSContext` and `Proxy` together

[sleeyax]

Describe the bug I can't connect to a websocket server over a HTTP proxy when I set both NetDialTLSContext and Proxy fields on the websocket.Dialer struct. See the code snippet below for more details, it's best explained by code.

Versions Go version: go1.18.1 linux/amd64. Package version: v1.5.0.

Steps to Reproduce

1. Get a HTTP proxy to connect to. You could install a local HTTP debugging proxy like Burp Suite, Charles Proxy, MITMProxy, ... on your development machine for testing.
2. Execute the code snippet below and observe that no websocket connection can be established over the proxy you configured.

Expected behavior The websocket dialer should be able to establish a websocket connection over a HTTP proxy while also applying a custom TLS connection without issues.

Code Snippets The following code results in an error like websocket: bad handshake or unexpected EOF (depends on the proxy):

package main

import (
	"context"
	"crypto/tls"
	"github.com/gorilla/websocket"
	"log"
	"net"
	"net/http"
	"net/url"
)

const (
	// Local or remote HTTP proxy.
	proxyUrl = "http://127.0.0.1:8888"

	// Websocket endpoint for testing
	websocketUrl = "wss://echo.websocket.events/"
)

func main() {
	p, _ := url.Parse(proxyUrl)

	dialer := websocket.Dialer{
		Proxy:             http.ProxyURL(p),
		EnableCompression: true,
		NetDialTLSContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
			// TCP dial
			netConn, err := net.Dial(network, addr)
			if err != nil {
				return nil, err
			}

			// 'NetDialTLSContext' also gets called during the proxy CONNECT for some reason (at this point 'network' equals "TCP" and 'addr' equals "127.0.0.1:8888")
			// The HTTP proxy doesn't support HTTPS however, so I return the established TCP connection early.
			// If I don't do this check, the connection hangs forever (tested with several proxies).
			// This feels kinda hacky though, not sure if this is the correct approach...
			if p.Host == addr {
				return netConn, err
			}

			// Example TLS handshake
			tlsConn := tls.Client(netConn, &tls.Config{ServerName: "echo.websocket.events", InsecureSkipVerify: true})
			if err = tlsConn.Handshake(); err != nil {
				return nil, err
			}

			return tlsConn, nil
		},
	}

	conn, _, err := dialer.Dial(websocketUrl, nil)
	if err != nil {
		log.Fatalln(err)
	}

	log.Println(conn.LocalAddr())
	// ...
}

When I check the request in burp, it looks like this:

As highlighted in red, the host URL became http://echo.websocket.events:443/, which obviously should just be https://echo.websocket.events/. I've been trying to debug why this is happening exactly all day but can't seem to find the culprit so I need help.

If I remove NetDialTLSContext and just specify TLSClientConfig everything works as expected. Same result with the Proxy field removed. The issue only occurs when both of these fields are set.

Am I missing something obvious?

[sleeyax]

I spent hours trying to debug and understand this issue and I think I finally figured it out! Just had to learn more about the HTTP proxy CONNECT sub-protocol to get a better understanding.

In short, what should happen is basically the following:

1. Establish a TCP connection to the HTTP proxy.
2. Send a HTTP request using the CONNECT method over the connection, which at least contains the target host to connect to.
3. Establish a TLS connection to the target host through the established proxy connection (optional).

However, there's currently an issue in the implementation of this flow. See the code snippets below.

Here, conn, err := hpd.forwardDial(network, hostPort) actually calls NetDialTLSContext if it's set. But in this context it is only the first step in the flow, resulting in connection errors because the proxy doesn't understand TLS yet: https://github.com/gorilla/websocket/blob/78cf1bc733a927f673fd1988a25256b425552a8a/proxy.go#L34-L39

If we change this to a regular net.Dial, the first and second step in the flow are fixed. But we're not there yet.

We get the established proxy connection here: https://github.com/gorilla/websocket/blob/78cf1bc733a927f673fd1988a25256b425552a8a/client.go#L320

But there's currently no API to convert an existing TCP connection net.Conn into a TLS connection tls.Conn . The NetDialTLSContext method requires you to do the dial again, so that's unusable at this point. Therefore, an API change to add such method is required. For example:

// client.go
type Dialer struct {
    // ...
    ProxyTLSConnection func(ctx context.Context, proxyConn net.Conn) (net.Conn, error)
}

Now, further down in the code we can establish the TLS connection to the target host through the proxy connection by calling this new method (if it is defined):

// client.go
if u.Scheme == "https" {
	  if d.ProxyTLSConnection != nil && d.Proxy != nil {
	  		// If we are connected to a proxy, perform the TLS handshake through the existing tunnel
	  		netConn, err = d.ProxyTLSConnection(ctx, netConn)
	  } else if d.NetDialTLSContext == nil {
	  		// ...
	  }
}

Now the third and final step in the flow is fixed!

---

I hope my detailed explanation is clear and helpful to someone. I'll open a PR soon.

[mikaelthd]

What bothers me about this is the fact that in net/http's Transport type one can specify a Proxy field as well as a DialTLSContext func which is exactly what NetDialTLSContext in gorilla was supposed to mirror. I therefore suspect that it is possible to fix this without requiring an API change.

Before posting this comment, I went ahead and had a quick peek at the source code. Even the golang source code has some weird behavior if DialTLSContext and Proxy are set. Unless I'm misreading this, it turns out that they actually skip the user defined DialTLSContext if the matched proxy URL has an HTTP scheme seen here. If the target's scheme is HTTPS they will upgrade the connection using their own TLS dial function with the user defined TLSClientConfig seen here, completely skipping DialTLSContext. Interestingly, if the proxy scheme is HTTPS, then they will attempt to establish a TLS connection using DialTLSContext directly against the proxy.

In other words, they "fixed" this issue in their code by completely ignoring it.