From 265dc7f37a04fdd036f6ee798b7fb0dabb3ac902 Mon Sep 17 00:00:00 2001 From: Andrew Burke <31974658+atburke@users.noreply.github.com> Date: Tue, 12 Apr 2022 10:52:33 -0700 Subject: [PATCH] NO_PROXY port support + special case for proxying via localhost (#11403) This change updates NO_PROXY handling to allow blocking specific host:port combinations, rather than just the host. It also adds a special case for downgrading requests to plain HTTP when --insecure is true and the request goes through a plain HTTP proxy at localhost (i.e. HTTP_PROXY=http://localhost). --- api/client/contextdialer.go | 5 +- api/client/noproxy.go | 74 ------------ api/client/proxy.go | 48 -------- api/client/proxy/proxy.go | 102 +++++++++++++++++ api/client/proxy/proxy_test.go | 183 ++++++++++++++++++++++++++++++ api/client/proxy_test.go | 96 ---------------- api/client/webclient/webclient.go | 20 ++-- integration/helpers.go | 23 ++++ integration/integration_test.go | 14 ++- integration/proxy_helpers_test.go | 25 +++- integration/proxy_test.go | 16 ++- lib/client/https_client.go | 16 +-- lib/utils/proxy/proxy.go | 7 +- 13 files changed, 379 insertions(+), 250 deletions(-) delete mode 100644 api/client/noproxy.go create mode 100644 api/client/proxy/proxy.go create mode 100644 api/client/proxy/proxy_test.go delete mode 100644 api/client/proxy_test.go diff --git a/api/client/contextdialer.go b/api/client/contextdialer.go index b0d19be7961a0..52c8efb2b2a23 100644 --- a/api/client/contextdialer.go +++ b/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) }) diff --git a/api/client/noproxy.go b/api/client/noproxy.go deleted file mode 100644 index a6115d20c6a5a..0000000000000 --- a/api/client/noproxy.go +++ /dev/null @@ -1,74 +0,0 @@ -// Copyright 2011 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// HTTP client implementation. See RFC 2616. -// -// This is the low-level Transport implementation of RoundTripper. -// The high-level interface is in client.go. - -package client - -import ( - "os" - "strings" - - "github.com/gravitational/teleport/api/constants" -) - -// useProxy reports whether requests to addr should use a proxy, -// according to the NO_PROXY or no_proxy environment variable. -// addr is always a canonicalAddr with a host and port. -func useProxy(addr string) bool { - if len(addr) == 0 { - return true - } - var noProxy string - for _, env := range []string{constants.NoProxy, strings.ToLower(constants.NoProxy)} { - noProxy = os.Getenv(env) - if noProxy != "" { - break - } - } - - if noProxy == "" { - return true - } - if noProxy == "*" { - return false - } - - addr = strings.ToLower(strings.TrimSpace(addr)) - if hasPort(addr) { - addr = addr[:strings.LastIndex(addr, ":")] - } - for _, p := range strings.Split(noProxy, ",") { - p = strings.ToLower(strings.TrimSpace(p)) - if len(p) == 0 { - continue - } - if hasPort(p) { - p = p[:strings.LastIndex(p, ":")] - } - if addr == p { - return false - } - if len(p) == 0 { - // There is no host part, likely the entry is malformed; ignore. - continue - } - if p[0] == '.' && (strings.HasSuffix(addr, p) || addr == p[1:]) { - // no_proxy ".foo.com" matches "bar.foo.com" or "foo.com" - return false - } - if p[0] != '.' && strings.HasSuffix(addr, p) && addr[len(addr)-len(p)-1] == '.' { - // no_proxy "foo.com" matches "bar.foo.com" - return false - } - } - return true -} - -// Given a string of the form "host", "host:port", or "[ipv6::address]:port", -// return true if the string includes a port. -func hasPort(s string) bool { return strings.LastIndex(s, ":") > strings.LastIndex(s, "]") } diff --git a/api/client/proxy.go b/api/client/proxy.go index 9d33a1ab23730..e6c61164ffbff 100644 --- a/api/client/proxy.go +++ b/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 -} diff --git a/api/client/proxy/proxy.go b/api/client/proxy/proxy.go new file mode 100644 index 0000000000000..017833ac37108 --- /dev/null +++ b/api/client/proxy/proxy.go @@ -0,0 +1,102 @@ +/* +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" + + "github.com/gravitational/trace" + "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 || addrURL == 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 an absolute URL. Unlike url.Parse, absolute URLs without a scheme are allowed. +func parse(addr string) (*url.URL, error) { + if addr == "" { + return nil, nil + } + addrURL, err := url.Parse(addr) + if err == nil && addrURL.Host != "" { + return addrURL, nil + } + + // url.Parse won't correctly parse an absolute URL without a scheme, so try again with a scheme. + addrURL, err2 := url.Parse("http://" + addr) + if err2 != nil { + return nil, trace.NewAggregate(err, err2) + } + addrURL.Scheme = "" + return addrURL, 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 +} + +// NewHTTPFallbackRoundTripper creates a new initialized HTTP fallback roundtripper. +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) +} diff --git a/api/client/proxy/proxy_test.go b/api/client/proxy/proxy_test.go new file mode 100644 index 0000000000000..89d30b077e242 --- /dev/null +++ b/api/client/proxy/proxy_test.go @@ -0,0 +1,183 @@ +/* +Copyright 2017 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 ( + "crypto/tls" + "fmt" + "net/http" + "net/url" + "testing" + + "github.com/stretchr/testify/require" + "golang.org/x/net/http/httpproxy" +) + +func TestGetProxyAddress(t *testing.T) { + type env struct { + name string + val string + } + var tests = []struct { + info string + env []env + targetAddr string + proxyAddr string + }{ + { + info: "valid, can be raw host:port", + env: []env{{name: "http_proxy", val: "proxy:1234"}}, + proxyAddr: "proxy:1234", + targetAddr: "192.168.1.1:3030", + }, + { + info: "valid, raw host:port works for https", + env: []env{{name: "HTTPS_PROXY", val: "proxy:1234"}}, + proxyAddr: "proxy:1234", + targetAddr: "192.168.1.1:3030", + }, + { + info: "valid, correct full url", + env: []env{{name: "https_proxy", val: "https://proxy:1234"}}, + proxyAddr: "proxy:1234", + targetAddr: "192.168.1.1:3030", + }, + { + info: "valid, http endpoint can be set in https_proxy", + env: []env{{name: "https_proxy", val: "http://proxy:1234"}}, + proxyAddr: "proxy:1234", + targetAddr: "192.168.1.1:3030", + }, + { + info: "valid, http endpoint can be set in https_proxy, but no_proxy override matches domain", + env: []env{ + {name: "https_proxy", val: "http://proxy:1234"}, + {name: "no_proxy", val: "proxy"}}, + proxyAddr: "", + targetAddr: "proxy:1234", + }, + { + info: "valid, http endpoint can be set in https_proxy, but no_proxy override matches ip", + env: []env{ + {name: "https_proxy", val: "http://proxy:1234"}, + {name: "no_proxy", val: "192.168.1.1"}}, + proxyAddr: "", + targetAddr: "192.168.1.1:1234", + }, + { + info: "valid, http endpoint can be set in https_proxy, but no_proxy override matches subdomain", + env: []env{ + {name: "https_proxy", val: "http://proxy:1234"}, + {name: "no_proxy", val: ".example.com"}}, + 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 { + t.Run(fmt.Sprintf("%v: %v", i, tt.info), func(t *testing.T) { + for _, env := range tt.env { + t.Setenv(env.name, env.val) + } + p := GetProxyAddress(tt.targetAddr) + if tt.proxyAddr == "" { + require.Nil(t, p) + } else { + require.NotNil(t, p) + 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. + //nolint:bodyclose + _, err = rt.RoundTrip(req) + require.Error(t, err) + require.Equal(t, "http", req.URL.Scheme) +} + +func TestParse(t *testing.T) { + successTests := []struct { + name, addr, scheme, host, path string + }{ + {name: "scheme-host-port", addr: "http://example.com:8080", scheme: "http", host: "example.com:8080", path: ""}, + {name: "host-port", addr: "example.com:8080", scheme: "", host: "example.com:8080", path: ""}, + {name: "scheme-ip4-port", addr: "http://127.0.0.1:8080", scheme: "http", host: "127.0.0.1:8080", path: ""}, + {name: "ip4-port", addr: "127.0.0.1:8080", scheme: "", host: "127.0.0.1:8080", path: ""}, + {name: "scheme-ip6-port", addr: "http://[::1]:8080", scheme: "http", host: "[::1]:8080", path: ""}, + {name: "ip6-port", addr: "[::1]:8080", scheme: "", host: "[::1]:8080"}, + {name: "host/path", addr: "example.com/path/to/somewhere", scheme: "", host: "example.com", path: "/path/to/somewhere"}, + } + for _, tc := range successTests { + t.Run(fmt.Sprintf("should parse: %s", tc.name), func(t *testing.T) { + u, err := parse(tc.addr) + require.NoError(t, err) + errMsg := fmt.Sprintf("(%v, %v, %v)", u.Scheme, u.Host, u.Path) + require.Equal(t, tc.scheme, u.Scheme, errMsg) + require.Equal(t, tc.host, u.Host, errMsg) + require.Equal(t, tc.path, u.Path) + }) + } + + failTests := []struct { + name, addr string + }{ + {name: "invalid char in host without scheme", addr: "bad addr"}, + } + for _, tc := range failTests { + t.Run(fmt.Sprintf("should not parse: %s", tc.name), func(t *testing.T) { + u, err := parse(tc.addr) + require.Error(t, err, u) + }) + } + + t.Run("empty addr", func(t *testing.T) { + u, err := parse("") + require.NoError(t, err) + require.Nil(t, u) + }) +} diff --git a/api/client/proxy_test.go b/api/client/proxy_test.go deleted file mode 100644 index 29cffc12e1b4f..0000000000000 --- a/api/client/proxy_test.go +++ /dev/null @@ -1,96 +0,0 @@ -/* -Copyright 2017 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 client - -import ( - "fmt" - "testing" - - "github.com/stretchr/testify/require" -) - -func TestGetProxyAddress(t *testing.T) { - type env struct { - name string - val string - } - var tests = []struct { - info string - env []env - targetAddr string - proxyAddr string - }{ - { - info: "valid, can be raw host:port", - env: []env{{name: "http_proxy", val: "proxy:1234"}}, - proxyAddr: "proxy:1234", - targetAddr: "192.168.1.1:3030", - }, - { - info: "valid, raw host:port works for https", - env: []env{{name: "HTTPS_PROXY", val: "proxy:1234"}}, - proxyAddr: "proxy:1234", - targetAddr: "192.168.1.1:3030", - }, - { - info: "valid, correct full url", - env: []env{{name: "https_proxy", val: "https://proxy:1234"}}, - proxyAddr: "proxy:1234", - targetAddr: "192.168.1.1:3030", - }, - { - info: "valid, http endpoint can be set in https_proxy", - env: []env{{name: "https_proxy", val: "http://proxy:1234"}}, - proxyAddr: "proxy:1234", - targetAddr: "192.168.1.1:3030", - }, - { - info: "valid, http endpoint can be set in https_proxy, but no_proxy override matches domain", - env: []env{ - {name: "https_proxy", val: "http://proxy:1234"}, - {name: "no_proxy", val: "proxy"}}, - proxyAddr: "", - targetAddr: "proxy:1234", - }, - { - info: "valid, http endpoint can be set in https_proxy, but no_proxy override matches ip", - env: []env{ - {name: "https_proxy", val: "http://proxy:1234"}, - {name: "no_proxy", val: "192.168.1.1"}}, - proxyAddr: "", - targetAddr: "192.168.1.1:1234", - }, - { - info: "valid, http endpoint can be set in https_proxy, but no_proxy override matches subdomain", - env: []env{ - {name: "https_proxy", val: "http://proxy:1234"}, - {name: "no_proxy", val: ".example.com"}}, - proxyAddr: "", - targetAddr: "bla.example.com:1234", - }, - } - - for i, tt := range tests { - t.Run(fmt.Sprintf("%v: %v", i, tt.info), func(t *testing.T) { - for _, env := range tt.env { - t.Setenv(env.name, env.val) - } - p := GetProxyAddress(tt.targetAddr) - require.Equal(t, tt.proxyAddr, p) - }) - } -} diff --git a/api/client/webclient/webclient.go b/api/client/webclient/webclient.go index f0b6f4920e50a..6e7157a622f7c 100644 --- a/api/client/webclient/webclient.go +++ b/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,17 @@ 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{ + InsecureSkipVerify: cfg.Insecure, + 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 } diff --git a/integration/helpers.go b/integration/helpers.go index eea278af97910..c41a36774cb10 100644 --- a/integration/helpers.go +++ b/integration/helpers.go @@ -1745,3 +1745,26 @@ func genUserKey() (*client.Key, error) { }}, }, nil } + +// getLocalIP gets the non-loopback IP address of this host. +func getLocalIP() (string, error) { + addrs, err := net.InterfaceAddrs() + if err != nil { + return "", trace.Wrap(err) + } + for _, addr := range addrs { + var ip net.IP + switch v := addr.(type) { + case *net.IPNet: + ip = v.IP + case *net.IPAddr: + ip = v.IP + default: + continue + } + if !ip.IsLoopback() { + return ip.String(), nil + } + } + return "", trace.NotFound("No non-loopback local IP address found") +} diff --git a/integration/integration_test.go b/integration/integration_test.go index 46ab70e2d6846..c61366ac32405 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -1641,8 +1641,12 @@ func testTwoClustersProxy(t *testing.T, suite *integrationTestSuite) { username := suite.me.Username - a := suite.newNamedTeleportInstance(t, "site-A") - b := suite.newNamedTeleportInstance(t, "site-B") + // httpproxy doesn't allow proxying when the target is localhost, so use + // this address instead. + addr, err := getLocalIP() + require.NoError(t, err) + a := suite.newNamedTeleportInstance(t, "site-A", WithNodeName(addr)) + b := suite.newNamedTeleportInstance(t, "site-B", WithNodeName(addr)) a.AddUser(username, []string{username}) b.AddUser(username, []string{username}) @@ -5673,6 +5677,12 @@ func (s *integrationTestSuite) newNamedTeleportInstance(t *testing.T, clusterNam return NewInstance(cfg) } +func WithNodeName(nodeName string) InstanceConfigOption { + return func(config *InstanceConfig) { + config.NodeName = nodeName + } +} + func (s *integrationTestSuite) defaultServiceConfig() *service.Config { cfg := service.MakeDefaultConfig() cfg.Console = nil diff --git a/integration/proxy_helpers_test.go b/integration/proxy_helpers_test.go index 09c4d335f9804..8e3e55acdffe9 100644 --- a/integration/proxy_helpers_test.go +++ b/integration/proxy_helpers_test.go @@ -64,6 +64,9 @@ type proxySuiteOptions struct { rootConfigModFunc []func(config *service.Config) leafConfigModFunc []func(config *service.Config) + rootClusterNodeName string + leafClusterNodeName string + rootClusterPorts *InstancePorts leafClusterPorts *InstancePorts @@ -79,8 +82,10 @@ type proxySuiteOptions struct { func newProxySuite(t *testing.T, opts ...proxySuiteOptionsFunc) *ProxySuite { options := proxySuiteOptions{ - rootClusterPorts: singleProxyPortSetup(), - leafClusterPorts: singleProxyPortSetup(), + rootClusterNodeName: Host, + leafClusterNodeName: Host, + rootClusterPorts: singleProxyPortSetup(), + leafClusterPorts: singleProxyPortSetup(), } for _, opt := range opts { opt(&options) @@ -89,7 +94,7 @@ func newProxySuite(t *testing.T, opts ...proxySuiteOptionsFunc) *ProxySuite { rc := NewInstance(InstanceConfig{ ClusterName: "root.example.com", HostID: uuid.New().String(), - NodeName: Host, + NodeName: options.rootClusterNodeName, log: utils.NewLoggerForTests(), Ports: options.rootClusterPorts, }) @@ -98,7 +103,7 @@ func newProxySuite(t *testing.T, opts ...proxySuiteOptionsFunc) *ProxySuite { lc := NewInstance(InstanceConfig{ ClusterName: "leaf.example.com", HostID: uuid.New().String(), - NodeName: Host, + NodeName: options.leafClusterNodeName, Priv: rc.Secrets.PrivKey, Pub: rc.Secrets.PubKey, log: utils.NewLoggerForTests(), @@ -264,6 +269,18 @@ func withRootAndLeafTrustedClusterReset() proxySuiteOptionsFunc { } } +func withRootClusterNodeName(nodeName string) proxySuiteOptionsFunc { + return func(options *proxySuiteOptions) { + options.rootClusterNodeName = nodeName + } +} + +func withLeafClusterNodeName(nodeName string) proxySuiteOptionsFunc { + return func(options *proxySuiteOptions) { + options.leafClusterNodeName = nodeName + } +} + func withRootClusterPorts(ports *InstancePorts) proxySuiteOptionsFunc { return func(options *proxySuiteOptions) { options.rootClusterPorts = ports diff --git a/integration/proxy_test.go b/integration/proxy_test.go index e45126e01b684..b1793d55730ee 100644 --- a/integration/proxy_test.go +++ b/integration/proxy_test.go @@ -215,10 +215,15 @@ func TestALPNSNIHTTPSProxy(t *testing.T) { t.Setenv("http_proxy", u.Host) username := mustGetCurrentUser(t).Username + // httpproxy won't proxy when target address is localhost, so use this instead. + addr, err := getLocalIP() + require.NoError(t, err) suite := newProxySuite(t, withRootClusterConfig(rootClusterStandardConfig(t)), withLeafClusterConfig(leafClusterStandardConfig(t)), + withRootClusterNodeName(addr), + withLeafClusterNodeName(addr), withRootClusterPorts(singleProxyPortSetup()), withLeafClusterPorts(singleProxyPortSetup()), withRootAndLeafClusterRoles(createTestRole(username)), @@ -697,10 +702,13 @@ func TestALPNProxyHTTPProxyNoProxyDial(t *testing.T) { lib.SetInsecureDevMode(true) defer lib.SetInsecureDevMode(false) + addr, err := getLocalIP() + require.NoError(t, err) + rc := NewInstance(InstanceConfig{ ClusterName: "root.example.com", HostID: uuid.New().String(), - NodeName: Loopback, + NodeName: addr, log: utils.NewLoggerForTests(), Ports: singleProxyPortSetup(), }) @@ -716,7 +724,7 @@ func TestALPNProxyHTTPProxyNoProxyDial(t *testing.T) { rcConf.Proxy.DisableWebInterface = true rcConf.SSH.Enabled = false - err := rc.CreateEx(t, nil, rcConf) + err = rc.CreateEx(t, nil, rcConf) require.NoError(t, err) err = rc.Start() @@ -732,9 +740,9 @@ func TestALPNProxyHTTPProxyNoProxyDial(t *testing.T) { require.NoError(t, err) t.Setenv("http_proxy", u.Host) - t.Setenv("no_proxy", "127.0.0.1") + t.Setenv("no_proxy", addr) - rcProxyAddr := net.JoinHostPort(Loopback, rc.GetPortWeb()) + rcProxyAddr := net.JoinHostPort(addr, rc.GetPortWeb()) // Start the node, due to no_proxy=127.0.0.1 env variable the connection established // to the proxy should not go through the http_proxy server. diff --git a/lib/client/https_client.go b/lib/client/https_client.go index c9f01aead2cb0..9bac84d006a6e 100644 --- a/lib/client/https_client.go +++ b/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 /* insecure */), + } } func newClientWithPool(pool *x509.CertPool) *http.Client { diff --git a/lib/utils/proxy/proxy.go b/lib/utils/proxy/proxy.go index 90c8226cced13..12731056dcc1c 100644 --- a/lib/utils/proxy/proxy.go +++ b/lib/utils/proxy/proxy.go @@ -25,6 +25,7 @@ import ( "github.com/gravitational/teleport" apiclient "github.com/gravitational/teleport/api/client" + apiproxy "github.com/gravitational/teleport/api/client/proxy" "github.com/gravitational/teleport/api/utils/sshutils" "github.com/gravitational/teleport/lib/utils" @@ -255,7 +256,7 @@ func WithInsecureSkipTLSVerify(insecure bool) DialerOptionFunc { // server directly. func DialerFromEnvironment(addr string, opts ...DialerOptionFunc) Dialer { // Try and get proxy addr from the environment. - proxyAddr := apiclient.GetProxyAddress(addr) + proxyAddr := apiproxy.GetProxyAddress(addr) var options dialerOptions for _, opt := range opts { @@ -264,7 +265,7 @@ func DialerFromEnvironment(addr string, opts ...DialerOptionFunc) Dialer { // If no proxy settings are in environment return regular ssh dialer, // otherwise return a proxy dialer. - if proxyAddr == "" { + if proxyAddr == nil { log.Debugf("No proxy set in environment, returning direct dialer.") return directDial{ insecure: options.insecureSkipTLSVerify, @@ -274,7 +275,7 @@ func DialerFromEnvironment(addr string, opts ...DialerOptionFunc) Dialer { } log.Debugf("Found proxy %q in environment, returning proxy dialer.", proxyAddr) return proxyDial{ - proxyHost: proxyAddr, + proxyHost: proxyAddr.Host, insecure: options.insecureSkipTLSVerify, tlsRoutingEnabled: options.tlsRoutingEnabled, tlsConfig: options.tlsConfig,