From 1da377c8c86caa99264102a09f43009cea17fc19 Mon Sep 17 00:00:00 2001 From: Andrew Burke <31974658+atburke@users.noreply.github.com> Date: Wed, 23 Mar 2022 12:58:19 -0700 Subject: [PATCH] Respect HTTP_PROXY/HTTPS_PROXY (#10209) This change allows tsh to use HTTP proxies when HTTP_PROXY/HTTPS_PROXY is set in the environment. --- api/client/client.go | 2 +- api/client/contextdialer.go | 18 +- {lib/utils/proxy => api/client}/noproxy.go | 6 +- api/client/proxy.go | 150 ++++++++++++ {lib/utils/proxy => api/client}/proxy_test.go | 11 +- api/client/webclient/webclient.go | 4 + api/client/webclient/webclient_test.go | 33 +++ api/constants/constants.go | 12 + constants.go | 12 - lib/auth/clt.go | 2 +- lib/client/api.go | 40 ++-- lib/client/https_client.go | 7 + lib/client/https_client_test.go | 73 ++++++ lib/reversetunnel/agent.go | 6 +- lib/reversetunnel/transport.go | 6 +- lib/utils/proxy/proxy.go | 219 ++++++------------ 16 files changed, 392 insertions(+), 209 deletions(-) rename {lib/utils/proxy => api/client}/noproxy.go (91%) create mode 100644 api/client/proxy.go rename {lib/utils/proxy => api/client}/proxy_test.go (93%) create mode 100644 lib/client/https_client_test.go diff --git a/api/client/client.go b/api/client/client.go index dc73f2c8a63a3..e936e8887abc4 100644 --- a/api/client/client.go +++ b/api/client/client.go @@ -288,7 +288,7 @@ type ( // authConnect connects to the Teleport Auth Server directly. func authConnect(ctx context.Context, params connectParams) (*Client, error) { - dialer := NewDirectDialer(params.cfg.KeepAlivePeriod, params.cfg.DialTimeout) + dialer := NewDialer(params.cfg.KeepAlivePeriod, params.cfg.DialTimeout) clt := newClient(params.cfg, dialer, params.tlsConfig) if err := clt.dialGRPC(ctx, params.addr); err != nil { return nil, trace.Wrap(err, "failed to connect to addr %v as an auth server", params.addr) diff --git a/api/client/contextdialer.go b/api/client/contextdialer.go index 2c2d08e29e894..b07af84097e9a 100644 --- a/api/client/contextdialer.go +++ b/api/client/contextdialer.go @@ -44,14 +44,26 @@ func (f ContextDialerFunc) DialContext(ctx context.Context, network, addr string return f(ctx, network, addr) } -// NewDirectDialer makes a new dialer to connect directly to an Auth server. -func NewDirectDialer(keepAlivePeriod, dialTimeout time.Duration) ContextDialer { +// newDirectDialer makes a new dialer to connect directly to an Auth server. +func newDirectDialer(keepAlivePeriod, dialTimeout time.Duration) ContextDialer { return &net.Dialer{ Timeout: dialTimeout, KeepAlive: keepAlivePeriod, } } +// NewDialer makes a new dialer that connects to an Auth server either directly or via an HTTP proxy, depending +// on the environment. +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) + } + return dialer.DialContext(ctx, network, addr) + }) +} + // NewProxyDialer makes a dialer to connect to an Auth server through the SSH reverse tunnel on the proxy. // The dialer will ping the web client to discover the tunnel proxy address on each dial. func NewProxyDialer(ssh ssh.ClientConfig, keepAlivePeriod, dialTimeout time.Duration, discoveryAddr string, insecure bool) ContextDialer { @@ -72,7 +84,7 @@ func NewProxyDialer(ssh ssh.ClientConfig, keepAlivePeriod, dialTimeout time.Dura // newTunnelDialer makes a dialer to connect to an Auth server through the SSH reverse tunnel on the proxy. func newTunnelDialer(ssh ssh.ClientConfig, keepAlivePeriod, dialTimeout time.Duration) ContextDialer { - dialer := NewDirectDialer(keepAlivePeriod, dialTimeout) + dialer := newDirectDialer(keepAlivePeriod, dialTimeout) return ContextDialerFunc(func(ctx context.Context, network, addr string) (conn net.Conn, err error) { conn, err = dialer.DialContext(ctx, network, addr) if err != nil { diff --git a/lib/utils/proxy/noproxy.go b/api/client/noproxy.go similarity index 91% rename from lib/utils/proxy/noproxy.go rename to api/client/noproxy.go index d9dcdb5f3d8a9..a6115d20c6a5a 100644 --- a/lib/utils/proxy/noproxy.go +++ b/api/client/noproxy.go @@ -7,13 +7,13 @@ // This is the low-level Transport implementation of RoundTripper. // The high-level interface is in client.go. -package proxy +package client import ( "os" "strings" - "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/constants" ) // useProxy reports whether requests to addr should use a proxy, @@ -24,7 +24,7 @@ func useProxy(addr string) bool { return true } var noProxy string - for _, env := range []string{teleport.NoProxy, strings.ToLower(teleport.NoProxy)} { + for _, env := range []string{constants.NoProxy, strings.ToLower(constants.NoProxy)} { noProxy = os.Getenv(env) if noProxy != "" { break diff --git a/api/client/proxy.go b/api/client/proxy.go new file mode 100644 index 0000000000000..9d33a1ab23730 --- /dev/null +++ b/api/client/proxy.go @@ -0,0 +1,150 @@ +/* +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 client + +import ( + "bufio" + "context" + "net" + "net/http" + "net/url" + "os" + "strings" + + "github.com/gravitational/teleport/api/constants" + "github.com/gravitational/trace" + log "github.com/sirupsen/logrus" +) + +// DialProxy creates a connection to a server via an HTTP Proxy. +func DialProxy(ctx context.Context, proxyAddr, addr string) (net.Conn, error) { + return DialProxyWithDialer(ctx, proxyAddr, addr, &net.Dialer{}) +} + +// DialProxyWithDialer creates a connection to a server via an HTTP Proxy using a specified dialer. +func DialProxyWithDialer(ctx context.Context, proxyAddr, addr string, dialer ContextDialer) (net.Conn, error) { + conn, err := dialer.DialContext(ctx, "tcp", proxyAddr) + if err != nil { + log.Warnf("Unable to dial to proxy: %v: %v.", proxyAddr, err) + return nil, trace.ConvertSystemError(err) + } + + connectReq := &http.Request{ + Method: http.MethodConnect, + URL: &url.URL{Opaque: addr}, + Host: addr, + Header: make(http.Header), + } + + if err := connectReq.Write(conn); err != nil { + log.Warnf("Unable to write to proxy: %v.", err) + return nil, trace.Wrap(err) + } + + // Read in the response. http.ReadResponse will read in the status line, mime + // headers, and potentially part of the response body. the body itself will + // not be read, but kept around so it can be read later. + br := bufio.NewReader(conn) + // Per the above comment, we're only using ReadResponse to check the status + // and then hand off the underlying connection to the caller. + // resp.Body.Close() would drain conn and close it, we don't need to do it + // here. Disabling bodyclose linter for this edge case. + //nolint:bodyclose + resp, err := http.ReadResponse(br, connectReq) + if err != nil { + conn.Close() + log.Warnf("Unable to read response: %v.", err) + return nil, trace.Wrap(err) + } + if resp.StatusCode != http.StatusOK { + conn.Close() + return nil, trace.BadParameter("unable to proxy connection: %v", resp.Status) + } + + // Return a bufferedConn that wraps a net.Conn and a *bufio.Reader. this + // needs to be done because http.ReadResponse will buffer part of the + // response body in the *bufio.Reader that was passed in. reads must first + // come from anything buffered, then from the underlying connection otherwise + // data will be lost. + return &bufferedConn{ + Conn: conn, + reader: br, + }, 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 +// underlying connection. +type bufferedConn struct { + net.Conn + reader *bufio.Reader +} + +// Read first reads from the *bufio.Reader any data that has already been +// buffered. Once all buffered data has been read, reads go to the net.Conn. +func (bc *bufferedConn) Read(b []byte) (n int, err error) { + if bc.reader.Buffered() > 0 { + return bc.reader.Read(b) + } + 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/lib/utils/proxy/proxy_test.go b/api/client/proxy_test.go similarity index 93% rename from lib/utils/proxy/proxy_test.go rename to api/client/proxy_test.go index a5c986acd89c5..29cffc12e1b4f 100644 --- a/lib/utils/proxy/proxy_test.go +++ b/api/client/proxy_test.go @@ -14,22 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. */ -package proxy +package client import ( "fmt" - "os" "testing" - "github.com/gravitational/teleport/lib/utils" "github.com/stretchr/testify/require" ) -func TestMain(m *testing.M) { - utils.InitLoggerForTests() - os.Exit(m.Run()) -} - func TestGetProxyAddress(t *testing.T) { type env struct { name string @@ -96,7 +89,7 @@ func TestGetProxyAddress(t *testing.T) { for _, env := range tt.env { t.Setenv(env.name, env.val) } - p := getProxyAddress(tt.targetAddr) + 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 b82e16f7ac9fe..626c0cffaf029 100644 --- a/api/client/webclient/webclient.go +++ b/api/client/webclient/webclient.go @@ -33,6 +33,7 @@ import ( "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/utils" + "golang.org/x/net/http/httpproxy" "github.com/gravitational/trace" log "github.com/sirupsen/logrus" @@ -46,6 +47,9 @@ func newWebClient(insecure bool, pool *x509.CertPool) *http.Client { RootCAs: pool, InsecureSkipVerify: insecure, }, + Proxy: func(req *http.Request) (*url.URL, error) { + return httpproxy.FromEnvironment().ProxyFunc()(req.URL) + }, }, } } diff --git a/api/client/webclient/webclient_test.go b/api/client/webclient/webclient_test.go index 556d6cab96478..413be48d96abc 100644 --- a/api/client/webclient/webclient_test.go +++ b/api/client/webclient/webclient_test.go @@ -289,3 +289,36 @@ func TestExtract(t *testing.T) { }) } } + +func TestNewWebClientRespectHTTPProxy(t *testing.T) { + t.Setenv("HTTPS_PROXY", "fakeproxy.example.com:9999") + client, err := newWebClient(&Config{ + Context: context.Background(), + ProxyAddr: "localhost:3080", + }) + require.NoError(t, err) + // resp should be nil, so there will be no body to close. + //nolint:bodyclose + resp, err := client.Get("https://fakedomain.example.com") + // Client should try to proxy through nonexistent server at localhost. + require.Error(t, err, "GET unexpectedly succeeded: %+v", resp) + require.Contains(t, err.Error(), "proxyconnect") + require.Contains(t, err.Error(), "lookup fakeproxy.example.com") + require.Contains(t, err.Error(), "no such host") +} + +func TestNewWebClientNoProxy(t *testing.T) { + t.Setenv("HTTPS_PROXY", "fakeproxy.example.com:9999") + t.Setenv("NO_PROXY", "fakedomain.example.com") + client, err := newWebClient(&Config{ + Context: context.Background(), + ProxyAddr: "localhost:3080", + }) + require.NoError(t, err) + //nolint:bodyclose + resp, err := client.Get("https://fakedomain.example.com") + require.Error(t, err, "GET unexpectedly succeeded: %+v", resp) + require.NotContains(t, err.Error(), "proxyconnect") + require.Contains(t, err.Error(), "lookup fakedomain.example.com") + require.Contains(t, err.Error(), "no such host") +} diff --git a/api/constants/constants.go b/api/constants/constants.go index e19a6e3af2505..4b7edfd8b539c 100644 --- a/api/constants/constants.go +++ b/api/constants/constants.go @@ -188,3 +188,15 @@ const ( // KubeTeleportProxyALPNPrefix is a SNI Kubernetes prefix used for distinguishing the Kubernetes HTTP traffic. KubeTeleportProxyALPNPrefix = "kube-teleport-proxy-alpn." ) + +const ( + // HTTPSProxy is an environment variable pointing to a HTTPS proxy. + HTTPSProxy = "HTTPS_PROXY" + + // HTTPProxy is an environment variable pointing to a HTTP proxy. + HTTPProxy = "HTTP_PROXY" + + // NoProxy is an environment variable matching the cases + // when HTTPS_PROXY or HTTP_PROXY is ignored + NoProxy = "NO_PROXY" +) diff --git a/constants.go b/constants.go index 77a6b9417078e..1ab18552fc5ab 100644 --- a/constants.go +++ b/constants.go @@ -61,18 +61,6 @@ const ( HTTPNextProtoTLS = "http/1.1" ) -const ( - // HTTPSProxy is an environment variable pointing to a HTTPS proxy. - HTTPSProxy = "HTTPS_PROXY" - - // HTTPProxy is an environment variable pointing to a HTTP proxy. - HTTPProxy = "HTTP_PROXY" - - // NoProxy is an environment variable matching the cases - // when HTTPS_PROXY or HTTP_PROXY is ignored - NoProxy = "NO_PROXY" -) - const ( // TOTPValidityPeriod is the number of seconds a TOTP token is valid. TOTPValidityPeriod uint = 30 diff --git a/lib/auth/clt.go b/lib/auth/clt.go index d4964e65187fc..a2cfb7e6e2339 100644 --- a/lib/auth/clt.go +++ b/lib/auth/clt.go @@ -129,7 +129,7 @@ func NewHTTPClient(cfg client.Config, tls *tls.Config, params ...roundtrip.Clien if len(cfg.Addrs) == 0 { return nil, trace.BadParameter("no addresses to dial") } - contextDialer := client.NewDirectDialer(cfg.KeepAlivePeriod, cfg.DialTimeout) + contextDialer := client.NewDialer(cfg.KeepAlivePeriod, cfg.DialTimeout) dialer = client.ContextDialerFunc(func(ctx context.Context, network, _ string) (conn net.Conn, err error) { for _, addr := range cfg.Addrs { conn, err = contextDialer.DialContext(ctx, network, addr) diff --git a/lib/client/api.go b/lib/client/api.go index 5c96b27a6eac7..ac5eacea05149 100644 --- a/lib/client/api.go +++ b/lib/client/api.go @@ -69,6 +69,7 @@ import ( "github.com/gravitational/teleport/lib/tlsca" "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/teleport/lib/utils/agentconn" + "github.com/gravitational/teleport/lib/utils/proxy" "github.com/gravitational/trace" @@ -2280,37 +2281,26 @@ func (tc *TeleportClient) connectToProxy(ctx context.Context) (*ProxyClient, err }, nil } -func makeProxySSHClientWithTLSWrapper(tc *TeleportClient, sshConfig *ssh.ClientConfig) (*ssh.Client, error) { - cfg := tc.Config - clientTLSConf, err := tc.loadTLSConfig() - if err != nil { - return nil, trace.Wrap(err) - } - - clientTLSConf.NextProtos = []string{string(alpncommon.ProtocolProxySSH)} - clientTLSConf.InsecureSkipVerify = cfg.InsecureSkipVerify - - tlsConn, err := tls.Dial("tcp", cfg.WebProxyAddr, clientTLSConf) - if err != nil { - return nil, trace.Wrap(err, "failed to dial tls %v", cfg.WebProxyAddr) - } - c, chans, reqs, err := ssh.NewClientConn(tlsConn, cfg.WebProxyAddr, sshConfig) - if err != nil { - // tlsConn is closed inside ssh.NewClientConn function - return nil, trace.Wrap(err, "failed to authenticate with proxy %v", cfg.WebProxyAddr) - } - return ssh.NewClient(c, chans, reqs), nil -} - func makeProxySSHClient(tc *TeleportClient, sshConfig *ssh.ClientConfig) (*ssh.Client, error) { if tc.Config.TLSRoutingEnabled { return makeProxySSHClientWithTLSWrapper(tc, sshConfig) } - client, err := ssh.Dial("tcp", tc.Config.SSHProxyAddr, sshConfig) + return makeProxySSHClientDirect(tc, sshConfig) +} + +func makeProxySSHClientDirect(tc *TeleportClient, sshConfig *ssh.ClientConfig) (*ssh.Client, error) { + dialer := proxy.DialerFromEnvironment(tc.Config.SSHProxyAddr) + return dialer.Dial("tcp", tc.Config.SSHProxyAddr, sshConfig) +} + +func makeProxySSHClientWithTLSWrapper(tc *TeleportClient, sshConfig *ssh.ClientConfig) (*ssh.Client, error) { + tlsConfig, err := tc.loadTLSConfig() if err != nil { - return nil, trace.Wrap(err, "failed to authenticate with proxy %v", tc.Config.SSHProxyAddr) + return nil, trace.Wrap(err) } - return client, nil + tlsConfig.NextProtos = []string{string(alpncommon.ProtocolProxySSH)} + dialer := proxy.DialerFromEnvironment(tc.Config.WebProxyAddr, proxy.WithALPNDialer(tlsConfig)) + return dialer.Dial("tcp", tc.Config.WebProxyAddr, sshConfig) } func (tc *TeleportClient) rootClusterName() (string, error) { diff --git a/lib/client/https_client.go b/lib/client/https_client.go index 4d1c94807dcec..c9f01aead2cb0 100644 --- a/lib/client/https_client.go +++ b/lib/client/https_client.go @@ -27,6 +27,7 @@ import ( apiutils "github.com/gravitational/teleport/api/utils" "github.com/gravitational/teleport/lib/httplib" "github.com/gravitational/teleport/lib/utils" + "golang.org/x/net/http/httpproxy" "github.com/gravitational/roundtrip" "github.com/gravitational/trace" @@ -41,6 +42,9 @@ func NewInsecureWebClient() *http.Client { return &http.Client{ Transport: &http.Transport{ TLSClientConfig: tlsConfig, + Proxy: func(req *http.Request) (*url.URL, error) { + return httpproxy.FromEnvironment().ProxyFunc()(req.URL) + }, }, } } @@ -54,6 +58,9 @@ func newClientWithPool(pool *x509.CertPool) *http.Client { return &http.Client{ Transport: &http.Transport{ TLSClientConfig: tlsConfig, + Proxy: func(req *http.Request) (*url.URL, error) { + return httpproxy.FromEnvironment().ProxyFunc()(req.URL) + }, }, } } diff --git a/lib/client/https_client_test.go b/lib/client/https_client_test.go new file mode 100644 index 0000000000000..66adb430ef035 --- /dev/null +++ b/lib/client/https_client_test.go @@ -0,0 +1,73 @@ +/* +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 client + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestNewInsecureWebClientHTTPProxy(t *testing.T) { + t.Setenv("HTTPS_PROXY", "fakeproxy.example.com:9999") + client := NewInsecureWebClient() + // resp should be nil, so there will be no body to close. + //nolint:bodyclose + resp, err := client.Get("https://fakedomain.example.com") + // Client should try to proxy through nonexistent server at localhost. + require.Error(t, err, "GET unexpectedly succeeded: %+v", resp) + require.Contains(t, err.Error(), "proxyconnect") + require.Contains(t, err.Error(), "lookup fakeproxy.example.com") + require.Contains(t, err.Error(), "no such host") +} + +func TestNewInsecureWebClientNoProxy(t *testing.T) { + t.Setenv("HTTPS_PROXY", "fakeproxy.example.com:9999") + t.Setenv("NO_PROXY", "fakedomain.example.com") + client := NewInsecureWebClient() + //nolint:bodyclose + resp, err := client.Get("https://fakedomain.example.com") + require.Error(t, err, "GET unexpectedly succeeded: %+v", resp) + require.NotContains(t, err.Error(), "proxyconnect") + require.Contains(t, err.Error(), "lookup fakedomain.example.com") + require.Contains(t, err.Error(), "no such host") +} + +func TestNewClientWithPoolHTTPProxy(t *testing.T) { + t.Setenv("HTTPS_PROXY", "fakeproxy.example.com:9999") + client := newClientWithPool(nil) + // resp should be nil, so there will be no body to close. + //nolint:bodyclose + resp, err := client.Get("https://fakedomain.example.com") + // Client should try to proxy through nonexistent server at localhost. + require.Error(t, err, "GET unexpectedly succeeded: %+v", resp) + require.Contains(t, err.Error(), "proxyconnect") + require.Contains(t, err.Error(), "lookup fakeproxy.example.com") + require.Contains(t, err.Error(), "no such host") +} + +func TestNewClientWithPoolNoProxy(t *testing.T) { + t.Setenv("HTTPS_PROXY", "fakeproxy.example.com:9999") + t.Setenv("NO_PROXY", "fakedomain.example.com") + client := newClientWithPool(nil) + //nolint:bodyclose + resp, err := client.Get("https://fakedomain.example.com") + require.Error(t, err, "GET unexpectedly succeeded: %+v", resp) + require.NotContains(t, err.Error(), "proxyconnect") + require.Contains(t, err.Error(), "lookup fakedomain.example.com") + require.Contains(t, err.Error(), "no such host") +} diff --git a/lib/reversetunnel/agent.go b/lib/reversetunnel/agent.go index 19cb6bb499f6e..9251dd1e0632d 100644 --- a/lib/reversetunnel/agent.go +++ b/lib/reversetunnel/agent.go @@ -22,6 +22,7 @@ package reversetunnel import ( "context" + "crypto/tls" "fmt" "sync" "time" @@ -35,6 +36,7 @@ import ( "github.com/gravitational/teleport/lib" "github.com/gravitational/teleport/lib/auth" "github.com/gravitational/teleport/lib/reversetunnel/track" + alpncommon "github.com/gravitational/teleport/lib/srv/alpnproxy/common" "github.com/gravitational/teleport/lib/sshutils" "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/teleport/lib/utils/proxy" @@ -283,7 +285,9 @@ func (a *Agent) connect() (conn *ssh.Client, err error) { } if a.reverseTunnelDetails != nil && a.reverseTunnelDetails.TLSRoutingEnabled { - opts = append(opts, proxy.WithALPNDialer()) + opts = append(opts, proxy.WithALPNDialer(&tls.Config{ + NextProtos: []string{string(alpncommon.ProtocolReverseTunnel)}, + })) } for _, authMethod := range a.authMethods { diff --git a/lib/reversetunnel/transport.go b/lib/reversetunnel/transport.go index 1e3d3fedd7c77..7125005d650a8 100644 --- a/lib/reversetunnel/transport.go +++ b/lib/reversetunnel/transport.go @@ -18,6 +18,7 @@ package reversetunnel import ( "context" + "crypto/tls" "encoding/json" "fmt" "io" @@ -33,6 +34,7 @@ import ( "github.com/gravitational/teleport/api/utils/sshutils" "github.com/gravitational/teleport/lib/auth" "github.com/gravitational/teleport/lib/events" + alpncommon "github.com/gravitational/teleport/lib/srv/alpnproxy/common" "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/teleport/lib/utils/proxy" @@ -96,7 +98,9 @@ func (t *TunnelAuthDialer) DialContext(ctx context.Context, _, _ string) (net.Co // address thus the ping call will always fail. t.Log.Debugf("Failed to ping web proxy %q addr: %v", addr.Addr, err) } else if resp.Proxy.TLSRoutingEnabled { - opts = append(opts, proxy.WithALPNDialer()) + opts = append(opts, proxy.WithALPNDialer(&tls.Config{ + NextProtos: []string{string(alpncommon.ProtocolReverseTunnel)}, + })) } dialer := proxy.DialerFromEnvironment(addr.Addr, opts...) diff --git a/lib/utils/proxy/proxy.go b/lib/utils/proxy/proxy.go index 9c934cc0277fa..90c8226cced13 100644 --- a/lib/utils/proxy/proxy.go +++ b/lib/utils/proxy/proxy.go @@ -16,21 +16,16 @@ limitations under the License. package proxy import ( - "bufio" "context" "crypto/tls" "net" - "net/http" - "net/url" - "os" - "strings" "time" "github.com/gravitational/trace" "github.com/gravitational/teleport" + apiclient "github.com/gravitational/teleport/api/client" "github.com/gravitational/teleport/api/utils/sshutils" - alpncommon "github.com/gravitational/teleport/lib/srv/alpnproxy/common" "github.com/gravitational/teleport/lib/utils" "golang.org/x/crypto/ssh" @@ -56,7 +51,7 @@ func dialWithDeadline(network string, addr string, config *ssh.ClientConfig) (*s // dialALPNWithDeadline allows connecting to Teleport in single-port mode. SSH protocol is wrapped into // TLS connection where TLS ALPN protocol is set to ProtocolReverseTunnel allowing ALPN Proxy to route the // incoming connection to ReverseTunnel proxy service. -func dialALPNWithDeadline(network string, addr string, config *ssh.ClientConfig, insecure bool) (*ssh.Client, error) { +func (d directDial) dialALPNWithDeadline(network string, addr string, config *ssh.ClientConfig) (*ssh.Client, error) { dialer := &net.Dialer{ Timeout: config.Timeout, } @@ -64,11 +59,11 @@ func dialALPNWithDeadline(network string, addr string, config *ssh.ClientConfig, if err != nil { return nil, trace.Wrap(err) } - tlsConn, err := tls.DialWithDialer(dialer, network, addr, &tls.Config{ - NextProtos: []string{string(alpncommon.ProtocolReverseTunnel)}, - InsecureSkipVerify: insecure, - ServerName: address.Host(), - }) + conf, err := d.getTLSConfig(address) + if err != nil { + return nil, trace.Wrap(err) + } + tlsConn, err := tls.DialWithDialer(dialer, network, addr, conf) if err != nil { return nil, trace.Wrap(err) } @@ -85,16 +80,29 @@ type Dialer interface { } type directDial struct { - // tlsRoutingEnabled indicates that proxy is running in TLSRouting mode. - tlsRoutingEnabled bool // insecure is whether to skip certificate validation. insecure bool + // tlsRoutingEnabled indicates that proxy is running in TLSRouting mode. + tlsRoutingEnabled bool + // tlsConfig is the TLS config to use. + tlsConfig *tls.Config +} + +// getTLSConfig configures the dialers TLS config for a specified address. +func (d directDial) getTLSConfig(addr *utils.NetAddr) (*tls.Config, error) { + if d.tlsConfig == nil { + return nil, trace.BadParameter("TLS config was nil") + } + tlsConfig := d.tlsConfig.Clone() + tlsConfig.ServerName = addr.Host() + tlsConfig.InsecureSkipVerify = d.insecure + return tlsConfig, nil } // Dial calls ssh.Dial directly. func (d directDial) Dial(network string, addr string, config *ssh.ClientConfig) (*ssh.Client, error) { if d.tlsRoutingEnabled { - client, err := dialALPNWithDeadline(network, addr, config, d.insecure) + client, err := d.dialALPNWithDeadline(network, addr, config) if err != nil { return nil, trace.Wrap(err) } @@ -114,11 +122,11 @@ func (d directDial) DialTimeout(network, address string, timeout time.Duration) if err != nil { return nil, trace.Wrap(err) } - tlsConn, err := tls.Dial("tcp", address, &tls.Config{ - NextProtos: []string{string(alpncommon.ProtocolReverseTunnel)}, - InsecureSkipVerify: d.insecure, - ServerName: addr.Host(), - }) + conf, err := d.getTLSConfig(addr) + if err != nil { + return nil, trace.Wrap(err) + } + tlsConn, err := tls.Dial("tcp", address, conf) if err != nil { return nil, trace.Wrap(err) } @@ -134,10 +142,23 @@ func (d directDial) DialTimeout(network, address string, timeout time.Duration) type proxyDial struct { // proxyHost is the HTTPS proxy address. proxyHost string - // tlsRoutingEnabled indicates that proxy is running in TLSRouting mode. - tlsRoutingEnabled bool // insecure is whether to skip certificate validation. insecure bool + // tlsRoutingEnabled indicates that proxy is running in TLSRouting mode. + tlsRoutingEnabled bool + // tlsConfig is the TLS config to use. + tlsConfig *tls.Config +} + +// getTLSConfig configures the dialers TLS config for a specified address. +func (d proxyDial) getTLSConfig(addr *utils.NetAddr) (*tls.Config, error) { + if d.tlsConfig == nil { + return nil, trace.BadParameter("TLS config was nil") + } + tlsConfig := d.tlsConfig.Clone() + tlsConfig.ServerName = addr.Host() + tlsConfig.InsecureSkipVerify = d.insecure + return tlsConfig, nil } // DialTimeout acts like Dial but takes a timeout. @@ -149,7 +170,7 @@ func (d proxyDial) DialTimeout(network, address string, timeout time.Duration) ( defer cancel() ctx = timeoutCtx } - conn, err := dialProxy(ctx, d.proxyHost, address) + conn, err := apiclient.DialProxy(ctx, d.proxyHost, address) if err != nil { return nil, trace.Wrap(err) } @@ -158,11 +179,11 @@ func (d proxyDial) DialTimeout(network, address string, timeout time.Duration) ( if err != nil { return nil, trace.Wrap(err) } - conn = tls.Client(conn, &tls.Config{ - NextProtos: []string{string(alpncommon.ProtocolReverseTunnel)}, - InsecureSkipVerify: d.insecure, - ServerName: address.Host(), - }) + conf, err := d.getTLSConfig(address) + if err != nil { + return nil, trace.Wrap(err) + } + conn = tls.Client(conn, conf) } return conn, nil } @@ -171,7 +192,7 @@ func (d proxyDial) DialTimeout(network, address string, timeout time.Duration) ( // SSH connection. func (d proxyDial) Dial(network string, addr string, config *ssh.ClientConfig) (*ssh.Client, error) { // Build a proxy connection first. - pconn, err := dialProxy(context.Background(), d.proxyHost, addr) + pconn, err := apiclient.DialProxy(context.Background(), d.proxyHost, addr) if err != nil { return nil, trace.Wrap(err) } @@ -183,11 +204,11 @@ func (d proxyDial) Dial(network string, addr string, config *ssh.ClientConfig) ( if err != nil { return nil, trace.Wrap(err) } - pconn = tls.Client(pconn, &tls.Config{ - NextProtos: []string{string(alpncommon.ProtocolReverseTunnel)}, - InsecureSkipVerify: d.insecure, - ServerName: address.Host(), - }) + conf, err := d.getTLSConfig(address) + if err != nil { + return nil, trace.Wrap(err) + } + pconn = tls.Client(pconn, conf) } // Do the same as ssh.Dial but pass in proxy connection. @@ -202,19 +223,22 @@ func (d proxyDial) Dial(network string, addr string, config *ssh.ClientConfig) ( } type dialerOptions struct { - // tlsRoutingEnabled indicates that proxy is running in TLSRouting mode. - tlsRoutingEnabled bool // insecureSkipTLSVerify is whether to skip certificate validation. insecureSkipTLSVerify bool + // tlsRoutingEnabled indicates that proxy is running in TLSRouting mode. + tlsRoutingEnabled bool + // tlsConfig is the TLS config to use for TLS routing. + tlsConfig *tls.Config } // DialerOptionFunc allows setting options as functional arguments to DialerFromEnvironment type DialerOptionFunc func(options *dialerOptions) // WithALPNDialer creates a dialer that allows to Teleport running in single-port mode. -func WithALPNDialer() DialerOptionFunc { +func WithALPNDialer(tlsConfig *tls.Config) DialerOptionFunc { return func(options *dialerOptions) { options.tlsRoutingEnabled = true + options.tlsConfig = tlsConfig } } @@ -231,7 +255,7 @@ func WithInsecureSkipTLSVerify(insecure bool) DialerOptionFunc { // server directly. func DialerFromEnvironment(addr string, opts ...DialerOptionFunc) Dialer { // Try and get proxy addr from the environment. - proxyAddr := getProxyAddress(addr) + proxyAddr := apiclient.GetProxyAddress(addr) var options dialerOptions for _, opt := range opts { @@ -243,129 +267,18 @@ func DialerFromEnvironment(addr string, opts ...DialerOptionFunc) Dialer { if proxyAddr == "" { log.Debugf("No proxy set in environment, returning direct dialer.") return directDial{ - tlsRoutingEnabled: options.tlsRoutingEnabled, insecure: options.insecureSkipTLSVerify, + tlsRoutingEnabled: options.tlsRoutingEnabled, + tlsConfig: options.tlsConfig, } } log.Debugf("Found proxy %q in environment, returning proxy dialer.", proxyAddr) return proxyDial{ proxyHost: proxyAddr, - tlsRoutingEnabled: options.tlsRoutingEnabled, insecure: options.insecureSkipTLSVerify, + tlsRoutingEnabled: options.tlsRoutingEnabled, + tlsConfig: options.tlsConfig, } } type DirectDialerOptFunc func(dial *directDial) - -func dialProxy(ctx context.Context, proxyAddr string, addr string) (net.Conn, error) { - var d net.Dialer - conn, err := d.DialContext(ctx, "tcp", proxyAddr) - if err != nil { - log.Warnf("Unable to dial to proxy: %v: %v.", proxyAddr, err) - return nil, trace.ConvertSystemError(err) - } - - connectReq := &http.Request{ - Method: http.MethodConnect, - URL: &url.URL{Opaque: addr}, - Host: addr, - Header: make(http.Header), - } - err = connectReq.Write(conn) - if err != nil { - log.Warnf("Unable to write to proxy: %v.", err) - return nil, trace.Wrap(err) - } - - // Read in the response. http.ReadResponse will read in the status line, mime - // headers, and potentially part of the response body. the body itself will - // not be read, but kept around so it can be read later. - br := bufio.NewReader(conn) - // Per the above comment, we're only using ReadResponse to check the status - // and then hand off the underlying connection to the caller. - // resp.Body.Close() would drain conn and close it, we don't need to do it - // here. Disabling bodyclose linter for this edge case. - //nolint:bodyclose - resp, err := http.ReadResponse(br, connectReq) - if err != nil { - conn.Close() - log.Warnf("Unable to read response: %v.", err) - return nil, trace.Wrap(err) - } - if resp.StatusCode != http.StatusOK { - conn.Close() - return nil, trace.BadParameter("unable to proxy connection: %v", resp.Status) - } - - // Return a bufferedConn that wraps a net.Conn and a *bufio.Reader. this - // needs to be done because http.ReadResponse will buffer part of the - // response body in the *bufio.Reader that was passed in. reads must first - // come from anything buffered, then from the underlying connection otherwise - // data will be lost. - return &bufferedConn{ - Conn: conn, - reader: br, - }, nil -} - -func getProxyAddress(addr string) string { - envs := []string{ - teleport.HTTPSProxy, - strings.ToLower(teleport.HTTPSProxy), - teleport.HTTPProxy, - strings.ToLower(teleport.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 -// underlying connection. -type bufferedConn struct { - net.Conn - reader *bufio.Reader -} - -// Read first reads from the *bufio.Reader any data that has already been -// buffered. Once all buffered data has been read, reads go to the net.Conn. -func (bc *bufferedConn) Read(b []byte) (n int, err error) { - if bc.reader.Buffered() > 0 { - return bc.reader.Read(b) - } - 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 -}