Change DialTLS signature

Make it look closer to standard Transport.DialTLS, because it's good to be
like standard things.
This also allows usage of thrid party TLS stacks, enabling
https://github.com/refraction-networking/utls.Roller integration.

common_test.go

@@ -17,6 +17,7 @@ import (
 )
 
 var credentialsEmpty = ""
+var credentialsCorrectPlain = "test:pass"
 var credentialsCorrect = "Basic dGVzdDpwYXNz"                                 // test:pass
 var credentialsUpstreamCorrect = "basic dXBzdHJlYW10ZXN0OnVwc3RyZWFtcGFzcw==" // upstreamtest:upstreampass
 var credentialsWrong = []string{
@@ -62,6 +63,7 @@ type caddyTestServer struct {
 var (
 	caddyForwardProxy            caddyTestServer
 	caddyForwardProxyAuth        caddyTestServer // requires auth
+	caddyHTTPForwardProxyAuth    caddyTestServer // requires auth, does not use TLS
 	caddyForwardProxyProbeResist caddyTestServer // requires auth, and has probing resistance on
 	caddyDummyProbeResist        caddyTestServer // same as caddyForwardProxyProbeResist, but w/o forwardproxy
 
@@ -140,6 +142,12 @@ func TestMain(m *testing.M) {
 			"acl {\nallow all\n}"}}
 	caddyForwardProxyAuth.StartTestServer()
 
+	caddyHTTPForwardProxyAuth = caddyTestServer{addr: "127.0.0.2:6973", root: "./test/forwardproxy",
+		directives:   []string{"tls off"},
+		proxyEnabled: true, proxyDirectives: []string{"basicauth test pass",
+			"acl {\nallow all\n}"}}
+	caddyHTTPForwardProxyAuth.StartTestServer()
+
 	caddyForwardProxyProbeResist = caddyTestServer{addr: "127.0.0.2:8888", root: "./test/forwardproxy",
 		directives: []string{"tls self_signed"}, HTTPRedirectPort: "8880",
 		proxyEnabled: true, proxyDirectives: []string{"basicauth test pass",
@@ -192,6 +200,7 @@ func TestMain(m *testing.M) {
 
 	caddyForwardProxy.Stop()
 	caddyForwardProxyAuth.Stop()
+	caddyHTTPForwardProxyAuth.Stop()
 	caddyForwardProxyProbeResist.Stop()
 	caddyDummyProbeResist.Stop()
 	caddyTestTarget.Stop()

httpclient/httpclient.go

@@ -43,9 +43,9 @@ type HTTPConnectDialer struct {
 
 	Dialer net.Dialer // overridden dialer allow to control establishment of TCP connection
 
-	// overridden DialTLS allows user to control establishment of TLS connection,
-	// given TCP connection. returns established TLS connection, ALPN and error
-	DialTLS func(net.Conn) (net.Conn, string, error)
+	// overridden DialTLS allows user to control establishment of TLS connection
+	// MUST return connection with completed Handshake, and NegotiatedProtocol
+	DialTLS func(network string, address string) (net.Conn, string, error)
 
 	EnableH2ConnReuse  bool
 	cacheH2Mu          sync.Mutex
@@ -183,34 +183,37 @@ func (c *HTTPConnectDialer) DialContext(ctx context.Context, network, address st
 		c.cacheH2Mu.Unlock()
 	}
 
-	rawConn, err := c.Dialer.DialContext(ctx, network, c.ProxyUrl.Host)
-	if err != nil {
-		return nil, err
-	}
-
+	var err error
+	var rawConn net.Conn
 	negotiatedProtocol := ""
 	switch c.ProxyUrl.Scheme {
 	case "http":
+		rawConn, err = c.Dialer.DialContext(ctx, network, c.ProxyUrl.Host)
+		if err != nil {
+			return nil, err
+		}
 	case "https":
 		if c.DialTLS != nil {
-			rawConn, negotiatedProtocol, err = c.DialTLS(rawConn)
+			rawConn, negotiatedProtocol, err = c.DialTLS(network, c.ProxyUrl.Host)
 			if err != nil {
 				return nil, err
 			}
-			break
-		}
-		tlsConf := tls.Config{
-			NextProtos: []string{"h2", "http/1.1"},
-			ServerName: c.ProxyUrl.Hostname(),
-		}
-
-		tlsConn := tls.Client(rawConn, &tlsConf)
-		err = tlsConn.Handshake()
-		if err != nil {
-			return nil, err
+		} else {
+			tlsConf := tls.Config{
+				NextProtos: []string{"h2", "http/1.1"},
+				ServerName: c.ProxyUrl.Hostname(),
+			}
+			tlsConn, err := tls.Dial(network, c.ProxyUrl.Host, &tlsConf)
+			if err != nil {
+				return nil, err
+			}
+			err = tlsConn.Handshake()
+			if err != nil {
+				return nil, err
+			}
+			negotiatedProtocol = tlsConn.ConnectionState().NegotiatedProtocol
+			rawConn = tlsConn
 		}
-		negotiatedProtocol = tlsConn.ConnectionState().NegotiatedProtocol
-		rawConn = tlsConn
 	default:
 		return nil, errors.New("scheme " + c.ProxyUrl.Scheme + " is not supported")
 	}

httpclient_test.go

@@ -0,0 +1,96 @@
+// tests ./httpclient/ but is in root as it needs access to test files in root
+package forwardproxy
+
+import (
+	"crypto/tls"
+	"net"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/caddyserver/forwardproxy/httpclient"
+)
+
+func TestHttpClient(t *testing.T) {
+	_test := func(proxyUrl string) {
+		for _, httpProxyVer := range testHttpProxyVersions {
+			for _, httpTargetVer := range testHttpTargetVersions {
+				for _, resource := range testResources {
+					dialer, err := httpclient.NewHTTPConnectDialer(proxyUrl)
+					if err != nil {
+						t.Fatal(err)
+					}
+					dialer.DialTLS = func(network string, address string) (net.Conn, string, error) {
+						conn, err := tls.Dial(network, address, &tls.Config{InsecureSkipVerify: true,
+							NextProtos: []string{httpVersionToAlpn[httpProxyVer]}})
+						if err != nil {
+							return nil, "", err
+						}
+						return conn, conn.ConnectionState().NegotiatedProtocol, nil
+					}
+					conn, err := dialer.Dial("tcp", caddyTestTarget.addr)
+					if err != nil {
+						t.Fatal(err)
+					}
+					response, err := getResourceViaProxyConn(conn, caddyTestTarget.addr, resource, httpTargetVer, credentialsCorrect)
+					if err != nil {
+						t.Fatal(httpProxyVer, httpTargetVer, err)
+					} else if err = responseExpected(response, caddyTestTarget.contents[resource]); err != nil {
+						t.Fatal(httpProxyVer, httpTargetVer, err)
+					}
+				}
+			}
+		}
+	}
+
+	_test("https://" + credentialsCorrectPlain + "@" + caddyForwardProxyAuth.addr)
+	_test("http://" + credentialsCorrectPlain + "@" + caddyHTTPForwardProxyAuth.addr)
+}
+
+func TestHttpClientH2Multiplexing(t *testing.T) {
+	// doesn't actually confirm that it is multiplexed, just that it doesn't break things
+	// but it was manually inspected in Wireshark when this code was committed
+	httpProxyVer := "HTTP/2.0"
+	httpTargetVer := "HTTP/1.1"
+
+	dialer, err := httpclient.NewHTTPConnectDialer("https://" + credentialsCorrectPlain + "@" + caddyForwardProxyAuth.addr)
+	if err != nil {
+		t.Fatal(err)
+	}
+	dialer.DialTLS = func(network string, address string) (net.Conn, string, error) {
+		conn, err := tls.Dial(network, address, &tls.Config{InsecureSkipVerify: true,
+			NextProtos: []string{httpVersionToAlpn[httpProxyVer]}})
+		if err != nil {
+			return nil, "", err
+		}
+		return conn, conn.ConnectionState().NegotiatedProtocol, nil
+	}
+
+	retries := 20
+	sleepInterval := time.Millisecond * 100
+
+	var wg sync.WaitGroup
+	wg.Add(retries + 1) // + for one serial launch
+	_test := func() {
+		defer wg.Done()
+		for _, resource := range testResources {
+			conn, err := dialer.Dial("tcp", caddyTestTarget.addr)
+			if err != nil {
+				t.Fatal(err)
+			}
+			response, err := getResourceViaProxyConn(conn, caddyTestTarget.addr, resource, httpTargetVer, credentialsCorrect)
+			if err != nil {
+				t.Fatal(httpProxyVer, httpTargetVer, err)
+			} else if err = responseExpected(response, caddyTestTarget.contents[resource]); err != nil {
+				t.Fatal(httpProxyVer, httpTargetVer, err)
+			}
+		}
+	}
+
+	_test() // do serially at least once
+
+	for i := 0; i < retries; i++ {
+		go _test()
+		time.Sleep(sleepInterval)
+	}
+}

setup.go

@@ -284,13 +284,12 @@ func setup(c *caddy.Controller) error {
 				// disabling verification helps with testing the package and setups
 				// either way, it's impossible to have a legit TLS certificate for "127.0.0.1"
 				log.Println("Localhost upstream detected, disabling verification of TLS certificate")
-				d.DialTLS = func(conn net.Conn) (net.Conn, string, error) {
-					cl := tls.Client(conn, &tls.Config{InsecureSkipVerify: true})
-					err := cl.Handshake()
+				d.DialTLS = func(network string, address string) (net.Conn, string, error) {
+					conn, err := tls.Dial(network, address, &tls.Config{InsecureSkipVerify: true})
 					if err != nil {
 						return nil, "", err
 					}
-					return cl, cl.ConnectionState().NegotiatedProtocol, err
+					return conn, conn.ConnectionState().NegotiatedProtocol, nil
 				}
 			}
 			return d, nil