[tor-commits] [meek/utls_2] Honor proxy even with http URLs in UTLSRoundTripper.

[dcf at torproject.org]

commit cbd1cea4c22bc9f15311219ec2a0055f07a553b7
Author: David Fifield <david at bamsoftware.com>
Date:   Wed Feb 6 22:49:57 2019 -0700

    Honor proxy even with http URLs in UTLSRoundTripper.
    
    Formerly, RoundTrip simply deferred to the global httpRoundTripper (the
    one we would use if we were not using uTLS) for any http (as opposed to
    https) requests. However, we allow setting a proxy on UTLSRoundTripper
    that can be different from the proxy set on the global httpRoundTripper.
    
    In practice, the global httpRoundTripper probably *would* have the same
    proxy setting, just because of how the PT protocol only allows a single
    proxy and how we initialize everything together. But there's nothing at
    the UTLSRoundTripper layer that enforces that, so it's better if we
    don't assume it. UTLSRoundTripper is already slightly weird because of
    the rule about not mixing http/1.1 and h2; this change means that http
    URLs don't add any additional weirdness.
---
 meek-client/utls.go      | 18 +++++++++++++++--
 meek-client/utls_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/meek-client/utls.go b/meek-client/utls.go
index 9a49317..a527d7e 100644
--- a/meek-client/utls.go
+++ b/meek-client/utls.go
@@ -120,13 +120,17 @@ type UTLSRoundTripper struct {
 	config        *utls.Config
 	proxyDialer   proxy.Dialer
 	rt            http.RoundTripper
+
+	// Transport for HTTP requests, which don't use uTLS.
+	httpRT *http.Transport
 }
 
 func (rt *UTLSRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
 	switch req.URL.Scheme {
 	case "http":
-		// If http, we don't invoke uTLS; just pass it to the global http.Transport.
-		return httpRoundTripper.RoundTrip(req)
+		// If http, we don't invoke uTLS; just pass it to an ordinary
+		// http.Transport.
+		return rt.httpRT.RoundTrip(req)
 	case "https":
 	default:
 		return nil, fmt.Errorf("unsupported URL scheme %q", req.URL.Scheme)
@@ -295,13 +299,23 @@ func NewUTLSRoundTripper(name string, cfg *utls.Config, proxyURL *url.URL) (http
 		// Special case for "none" and HelloGolang.
 		return httpRoundTripper, nil
 	}
+
 	proxyDialer, err := makeProxyDialer(proxyURL, cfg, clientHelloID)
 	if err != nil {
 		return nil, err
 	}
+
+	// This special-case RoundTripper is used for HTTP requests, which don't
+	// use uTLS but should use the specified proxy.
+	httpRT := &http.Transport{}
+	copyPublicFields(httpRT, httpRoundTripper)
+	httpRT.Proxy = http.ProxyURL(proxyURL)
+
 	return &UTLSRoundTripper{
 		clientHelloID: clientHelloID,
 		config:        cfg,
 		proxyDialer:   proxyDialer,
+		// rt will be set in the first call to RoundTrip.
+		httpRT: httpRT,
 	}, nil
 }
diff --git a/meek-client/utls_test.go b/meek-client/utls_test.go
index 2eb72af..eebb1db 100644
--- a/meek-client/utls_test.go
+++ b/meek-client/utls_test.go
@@ -230,3 +230,54 @@ func TestUTLSServerName(t *testing.T) {
 		t.Errorf("expected \"test.example\" server_name extension with given ServerName and hostname dial")
 	}
 }
+
+// Test that HTTP requests (which don't go through the uTLS code path) still use
+// any proxy that's configured on the UTLSRoundTripper.
+func TestUTLSHTTPWithProxy(t *testing.T) {
+	// Make a web server that we should *not* be able to reach.
+	server := &http.Server{
+		Handler: http.NotFoundHandler(),
+	}
+	serverLn, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		panic(err)
+	}
+	defer serverLn.Close()
+	go server.Serve(serverLn)
+
+	// Make a non-functional proxy server.
+	proxyLn, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		panic(err)
+	}
+	defer proxyLn.Close()
+	go func() {
+		for {
+			conn, err := proxyLn.Accept()
+			if err == nil {
+				conn.Close() // go away
+			}
+		}
+	}()
+
+	// Try to access the web server through the non-functional proxy.
+	for _, proxyURL := range []url.URL{
+		url.URL{Scheme: "socks5", Host: proxyLn.Addr().String()},
+		url.URL{Scheme: "http", Host: proxyLn.Addr().String()},
+		url.URL{Scheme: "https", Host: proxyLn.Addr().String()},
+	} {
+		rt, err := NewUTLSRoundTripper("HelloFirefox_63", &utls.Config{InsecureSkipVerify: true}, &proxyURL)
+		if err != nil {
+			panic(err)
+		}
+		fetchURL := url.URL{Scheme: "http", Host: serverLn.Addr().String()}
+		req, err := http.NewRequest("GET", fetchURL.String(), nil)
+		if err != nil {
+			panic(err)
+		}
+		_, err = rt.RoundTrip(req)
+		if err == nil {
+			t.Errorf("fetch of %s through %s proxy should have failed", &fetchURL, proxyURL.Scheme)
+		}
+	}
+}