NO_PROXY port support + special case for proxying via localhost

api/client/contextdialer.go

@@ -22,6 +22,7 @@ import (
 	"net"
 	"time"
 
+	"github.com/gravitational/teleport/api/client/proxy"
 	"github.com/gravitational/teleport/api/client/webclient"
 	"github.com/gravitational/teleport/api/constants"
 	"github.com/gravitational/teleport/api/utils/sshutils"
@@ -57,8 +58,8 @@ func newDirectDialer(keepAlivePeriod, dialTimeout time.Duration) ContextDialer {
 func NewDialer(keepAlivePeriod, dialTimeout time.Duration) ContextDialer {
 	return ContextDialerFunc(func(ctx context.Context, network, addr string) (net.Conn, error) {
 		dialer := newDirectDialer(keepAlivePeriod, dialTimeout)
-		if proxyAddr := GetProxyAddress(addr); proxyAddr != "" {
-			return DialProxyWithDialer(ctx, proxyAddr, addr, dialer)
+		if proxyAddr := proxy.GetProxyAddress(addr); proxyAddr != nil {
+			return DialProxyWithDialer(ctx, proxyAddr.Host, addr, dialer)
 		}
 		return dialer.DialContext(ctx, network, addr)
 	})

api/client/proxy.go

@@ -22,10 +22,7 @@ import (
 	"net"
 	"net/http"
 	"net/url"
-	"os"
-	"strings"
 
-	"github.com/gravitational/teleport/api/constants"
 	"github.com/gravitational/trace"
 	log "github.com/sirupsen/logrus"
 )
@@ -86,37 +83,6 @@ func DialProxyWithDialer(ctx context.Context, proxyAddr, addr string, dialer Con
 	}, nil
 }
 
-// GetProxyAddress gets the HTTP proxy address to use for a given address, if any.
-func GetProxyAddress(addr string) string {
-	envs := []string{
-		constants.HTTPSProxy,
-		strings.ToLower(constants.HTTPSProxy),
-		constants.HTTPProxy,
-		strings.ToLower(constants.HTTPProxy),
-	}
-
-	for _, v := range envs {
-		envAddr := os.Getenv(v)
-		if envAddr == "" {
-			continue
-		}
-		proxyAddr, err := parse(envAddr)
-		if err != nil {
-			log.Debugf("Unable to parse environment variable %q: %q.", v, envAddr)
-			continue
-		}
-		log.Debugf("Successfully parsed environment variable %q: %q to %q.", v, envAddr, proxyAddr)
-		if !useProxy(addr) {
-			log.Debugf("Matched NO_PROXY override for %q: %q, going to ignore proxy variable.", v, envAddr)
-			return ""
-		}
-		return proxyAddr
-	}
-
-	log.Debugf("No valid environment variables found.")
-	return ""
-}
-
 // bufferedConn is used when part of the data on a connection has already been
 // read by a *bufio.Reader. Reads will first try and read from the
 // *bufio.Reader and when everything has been read, reads will go to the
@@ -134,17 +100,3 @@ func (bc *bufferedConn) Read(b []byte) (n int, err error) {
 	}
 	return bc.Conn.Read(b)
 }
-
-// parse will extract the host:port of the proxy to dial to. If the
-// value is not prefixed by "http", then it will prepend "http" and try.
-func parse(addr string) (string, error) {
-	proxyurl, err := url.Parse(addr)
-	if err != nil || !strings.HasPrefix(proxyurl.Scheme, "http") {
-		proxyurl, err = url.Parse("http://" + addr)
-		if err != nil {
-			return "", trace.Wrap(err)
-		}
-	}
-
-	return proxyurl.Host, nil
-}

api/client/proxy/proxy.go

@@ -0,0 +1,95 @@
+/*
+Copyright 2022 Gravitational, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package proxy
+
+import (
+	"net/http"
+	"net/url"
+	"strings"
+
+	"golang.org/x/net/http/httpproxy"
+)
+
+// GetProxyAddress gets the HTTP proxy address to use for a given address, if any.
+func GetProxyAddress(dialAddr string) *url.URL {
+	addrURL, err := parse(dialAddr)
+	if err != nil {
+		return nil
+	}
+	proxyFunc := httpproxy.FromEnvironment().ProxyFunc()
+	if addrURL.Scheme != "" {
+		proxyURL, err := proxyFunc(addrURL)
+		if err != nil {
+			return nil
+		}
+		return proxyURL
+	}
+
+	for _, scheme := range []string{"https", "http"} {
+		addrURL.Scheme = scheme
+		proxyURL, err := proxyFunc(addrURL)
+		if err == nil && proxyURL != nil {
+			return proxyURL
+		}
+	}
+
+	return nil
+}
+
+// parse parses a URL. If the address does not have a scheme, it will prepend "http" and try.
+func parse(addr string) (*url.URL, error) {
+	proxyurl, err := url.Parse(addr)
+	// Some URLs will fail to parse without a scheme (for example, <ip-address>:<port>), so try
+	// to parse again with a scheme. If that fails, return the original error.
+	if err != nil || !strings.HasPrefix(proxyurl.Scheme, "http") {
+		if proxyurl, err := url.Parse("http://" + addr); err == nil {
+			proxyurl.Scheme = ""
+			return proxyurl, nil
+		}
+		return nil, err
+	}
+	return proxyurl, nil
+}
+
+// HTTPFallbackRoundTripper is a wrapper for http.Transport that downgrades requests
+// to plain HTTP when using a plain HTTP proxy at localhost.
+type HTTPFallbackRoundTripper struct {
+	*http.Transport
+	isProxyHTTPLocalhost bool
+}
+
+func NewHTTPFallbackRoundTripper(transport *http.Transport, insecure bool) *HTTPFallbackRoundTripper {
+	proxyConfig := httpproxy.FromEnvironment()
+	rt := HTTPFallbackRoundTripper{
+		Transport:            transport,
+		isProxyHTTPLocalhost: strings.HasPrefix(proxyConfig.HTTPProxy, "http://localhost"),
+	}
+	if rt.TLSClientConfig != nil {
+		rt.TLSClientConfig.InsecureSkipVerify = insecure
+	}
+	return &rt
+}
+
+// RoundTrip executes a single HTTP transaction. Part of the RoundTripper interface.
+func (rt *HTTPFallbackRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
+	tlsConfig := rt.Transport.TLSClientConfig
+	// Use plain HTTP if proxying via http://localhost in insecure mode.
+	if rt.isProxyHTTPLocalhost && tlsConfig != nil && tlsConfig.InsecureSkipVerify {
+		req.URL.Scheme = "http"
+	}
+	return rt.Transport.RoundTrip(req)
+}

api/client/proxy_test.go → api/client/proxy/proxy_test.go

@@ -14,13 +14,17 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package client
+package proxy
 
 import (
+	"crypto/tls"
 	"fmt"
+	"net/http"
+	"net/url"
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"golang.org/x/net/http/httpproxy"
 )
 
 func TestGetProxyAddress(t *testing.T) {
@@ -82,6 +86,24 @@ func TestGetProxyAddress(t *testing.T) {
 			proxyAddr:  "",
 			targetAddr: "bla.example.com:1234",
 		},
+		{
+			info: "valid, no_proxy blocks matching port",
+			env: []env{
+				{name: "https_proxy", val: "proxy:9999"},
+				{name: "no_proxy", val: "example.com:1234"},
+			},
+			proxyAddr:  "",
+			targetAddr: "example.com:1234",
+		},
+		{
+			info: "valid, no_proxy matches host but not port",
+			env: []env{
+				{name: "https_proxy", val: "proxy:9999"},
+				{name: "no_proxy", val: "example.com:1234"},
+			},
+			proxyAddr:  "proxy:9999",
+			targetAddr: "example.com:5678",
+		},
 	}
 
 	for i, tt := range tests {
@@ -90,7 +112,28 @@ func TestGetProxyAddress(t *testing.T) {
 				t.Setenv(env.name, env.val)
 			}
 			p := GetProxyAddress(tt.targetAddr)
-			require.Equal(t, tt.proxyAddr, p)
+			if tt.proxyAddr == "" {
+				require.Nil(t, p)
+			} else {
+				require.Equal(t, tt.proxyAddr, p.Host)
+			}
 		})
 	}
 }
+
+func TestProxyAwareRoundTripper(t *testing.T) {
+	t.Setenv("HTTP_PROXY", "http://localhost:8888")
+	transport := &http.Transport{
+		TLSClientConfig: &tls.Config{},
+		Proxy: func(req *http.Request) (*url.URL, error) {
+			return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
+		},
+	}
+	rt := NewHTTPFallbackRoundTripper(transport, true)
+	req, err := http.NewRequest(http.MethodGet, "https://localhost:9999", nil)
+	require.NoError(t, err)
+	// Don't care about response, only if the scheme changed.
+	_, err = rt.RoundTrip(req)
+	require.Error(t, err)
+	require.Equal(t, "http", req.URL.Scheme)
+}

api/client/webclient/webclient.go

@@ -30,6 +30,7 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/gravitational/teleport/api/client/proxy"
 	"github.com/gravitational/teleport/api/constants"
 	"github.com/gravitational/teleport/api/defaults"
 	"github.com/gravitational/teleport/api/utils"
@@ -76,16 +77,16 @@ func newWebClient(cfg *Config) (*http.Client, error) {
 	if err := cfg.CheckAndSetDefaults(); err != nil {
 		return nil, trace.Wrap(err)
 	}
-	return &http.Client{
-		Transport: &http.Transport{
-			TLSClientConfig: &tls.Config{
-				RootCAs:            cfg.Pool,
-				InsecureSkipVerify: cfg.Insecure,
-			},
-			Proxy: func(req *http.Request) (*url.URL, error) {
-				return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
-			},
+	transport := http.Transport{
+		TLSClientConfig: &tls.Config{
+			RootCAs: cfg.Pool,
+		},
+		Proxy: func(req *http.Request) (*url.URL, error) {
+			return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
 		},
+	}
+	return &http.Client{
+		Transport: proxy.NewHTTPFallbackRoundTripper(&transport, cfg.Insecure),
 	}, nil
 }
 

lib/client/https_client.go

@@ -24,6 +24,7 @@ import (
 	"net/url"
 
 	"github.com/gravitational/teleport"
+	apiproxy "github.com/gravitational/teleport/api/client/proxy"
 	apiutils "github.com/gravitational/teleport/api/utils"
 	"github.com/gravitational/teleport/lib/httplib"
 	"github.com/gravitational/teleport/lib/utils"
@@ -37,16 +38,15 @@ func NewInsecureWebClient() *http.Client {
 	// Because Teleport clients can't be configured (yet), they take the default
 	// list of cipher suites from Go.
 	tlsConfig := utils.TLSConfig(nil)
-	tlsConfig.InsecureSkipVerify = true
-
-	return &http.Client{
-		Transport: &http.Transport{
-			TLSClientConfig: tlsConfig,
-			Proxy: func(req *http.Request) (*url.URL, error) {
-				return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
-			},
+	transport := http.Transport{
+		TLSClientConfig: tlsConfig,
+		Proxy: func(req *http.Request) (*url.URL, error) {
+			return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
 		},
 	}
+	return &http.Client{
+		Transport: apiproxy.NewHTTPFallbackRoundTripper(&transport, true),
+	}
 }
 
 func newClientWithPool(pool *x509.CertPool) *http.Client {