Implements missing HTTPS proxy functionality

[seans3]

What type of PR is this? (check all applicable)

• Feature

Description

• Implements missing HTTPS Proxy functionality.
• Adds both client/proxy/server tests and unit tests.

Related Tickets & Documents

• Closes [feature] Support for proxying websocket through https proxy #739
• Closes Is https not supported?  #794

Added/updated tests?

• Yes

Run verifications and test

$ go test -race -cover
PASS
coverage: 84.2% of statements
ok  	github.com/gorilla/websocket	4.103s

[seans3]

/cc @adrianosela

[seans3]

/assign @liggitt
/assign @aojea

[adrianosela]

Thanks for this @seans3 🚀

[aojea]

IIUIC the same certificate is used for both the proxy and the backend, don't we need to handle the case when both are different? what happens if the proxy requires a different certificate than the backend?

EDIT

I see, the TLSConfig can be used for that, also @seans3 sent me this https://github.com/adrianosela/https-proxy/tree/main?tab=readme-ov-file#https-proxy

[aojea]

IIUIC this change the behavior, before if the returned proxyURL was nil meant "this connections should not be proxied" and now it will error , I think this should be just a noop

Suggested change

	if proxyURL == nil {
	return nil, errors.New("generated proxy url is nil")
	}
	// this request should not be proxied and use the original dialer
	if proxyURL == nil {
	return netDial, nil
	}

[seans3]

Fixed.

[aojea]

This does not match the logic in

	var netDial netDialerFunc
	switch {
	case u.Scheme == "https" && d.NetDialTLSContext != nil:
		netDial = d.NetDialTLSContext
	case d.NetDialContext != nil:
		netDial = d.NetDialContext
	case d.NetDial != nil:
		netDial = func(ctx context.Context, net, addr string) (net.Conn, error) {
			return d.NetDial(net, addr)
		}
	default:
		netDial = (&net.Dialer{}).DialContext
	}

that seems to indicate that NetDialTLSContext is ONLY used if the scheme is https , but ignored otherwise

[aojea]

it seems we can remove this check or just shortcut and

Suggested change

	if proxyURL.Scheme == "http" && d.NetDialTLSContext != nil {
	return nil, errors.New("dial TLS function set for dialing non-HTTPS proxy")
	}
	if proxyURL.Scheme == "http" {
	return proxyFromURL(proxyURL, netDial)
	}

[aojea]

note for self:

• url is http and proxy-url is https, we replace the dialer ... users can specify a custom tls dialer just for the proxy.
• url is https and proxy-url is https, the same tls dialer is used for both

[seans3]

• url is https and proxy-url is https, the same tls dialer is used for both

I do not think this is correct. There is only one "dial". It is either to the "proxy" or to the "upstream" server. If there is a proxy, the dial is to the "proxy" (and the TLS handshake must occur in the dial). The "proxy" server is the place that dials the "upstream" in this case. The second TLS handshake (not dial) to the "upstream" occurs over the previously established connection, and it MUST use the TLSClientConfig. So the NetDialTLSContext is only used for the proxy connection if it is set and the Proxy function returns non-nil, HTTPS proxy URL.

[aojea]

is it there other possibility? doesn't this mean, we always use the httpProxyDialer ?

[seans3]

Yes--This will always use the httpProxyDialer. The difference is the forwardDial, which now will also perform the TLS handshake if necessary.

[seans3]

IIUIC the same certificate is used for both the proxy and the backend, don't we need to handle the case when both are different? what happens if the proxy requires a different certificate than the backend?

The client is responsible for configuring the root CA certificates. The client TLS config contains a cert pool, which can contain multiple root CA certificates. So for TLS to the proxy and the upstream, the TLS config root CA cert pool must contain CA's which have signed both the proxy and the upstream.