From 1858aafa1569d836def5c2fdb06c0b0583777825 Mon Sep 17 00:00:00 2001 From: Gavin Frazar Date: Wed, 22 Jun 2022 17:27:29 -0700 Subject: [PATCH] Fix http proxy basic auth (#13140) * Fix http proxy basic auth * Update docs about HTTP CONNECT env var formats --- api/client/contextdialer.go | 4 +- api/client/proxy.go | 27 +++++-- api/client/proxy/proxy.go | 4 +- api/client/proxy/proxy_test.go | 71 +++++++++++++--- docs/pages/setup/reference/networking.mdx | 2 +- integration/helpers.go | 2 +- integration/helpers/proxy.go | 80 +++++++++++++++++- integration/integration_test.go | 8 +- integration/proxy_test.go | 98 ++++++++++++++++++++--- lib/client/client.go | 2 +- lib/utils/proxy/proxy.go | 15 ++-- 11 files changed, 266 insertions(+), 47 deletions(-) diff --git a/api/client/contextdialer.go b/api/client/contextdialer.go index 0fae13ae726..eaa86e77019 100644 --- a/api/client/contextdialer.go +++ b/api/client/contextdialer.go @@ -59,8 +59,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 := proxy.GetProxyAddress(addr); proxyAddr != nil { - return DialProxyWithDialer(ctx, proxyAddr.Host, addr, dialer) + if proxyURL := proxy.GetProxyURL(addr); proxyURL != nil { + return DialProxyWithDialer(ctx, proxyURL, addr, dialer) } return dialer.DialContext(ctx, network, addr) }) diff --git a/api/client/proxy.go b/api/client/proxy.go index e6c61164ffb..c0b92c550a9 100644 --- a/api/client/proxy.go +++ b/api/client/proxy.go @@ -19,6 +19,7 @@ package client import ( "bufio" "context" + "encoding/base64" "net" "net/http" "net/url" @@ -28,23 +29,37 @@ import ( ) // 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{}) +func DialProxy(ctx context.Context, proxyURL *url.URL, addr string) (net.Conn, error) { + return DialProxyWithDialer(ctx, proxyURL, 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) +func DialProxyWithDialer(ctx context.Context, proxyURL *url.URL, addr string, dialer ContextDialer) (net.Conn, error) { + if proxyURL == nil { + return nil, trace.BadParameter("missing proxy url") + } + conn, err := dialer.DialContext(ctx, "tcp", proxyURL.Host) if err != nil { - log.Warnf("Unable to dial to proxy: %v: %v.", proxyAddr, err) + log.Warnf("Unable to dial to proxy: %v: %v.", proxyURL.Host, err) return nil, trace.ConvertSystemError(err) } + header := make(http.Header) + if proxyURL.User != nil { + // dont use User.String() because it performs url encoding (rfc 1738), + // which we don't want in our header + password, _ := proxyURL.User.Password() + // empty user/pass is permitted by the spec. The minimum required is a single colon. + // see: https://datatracker.ietf.org/doc/html/rfc1945#section-11 + creds := proxyURL.User.Username() + ":" + password + basicAuth := "Basic " + base64.StdEncoding.EncodeToString([]byte(creds)) + header.Add("Proxy-Authorization", basicAuth) + } connectReq := &http.Request{ Method: http.MethodConnect, URL: &url.URL{Opaque: addr}, Host: addr, - Header: make(http.Header), + Header: header, } if err := connectReq.Write(conn); err != nil { diff --git a/api/client/proxy/proxy.go b/api/client/proxy/proxy.go index 017833ac371..7b95b3ba8a4 100644 --- a/api/client/proxy/proxy.go +++ b/api/client/proxy/proxy.go @@ -25,8 +25,8 @@ import ( "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 { +// GetProxyURL gets the HTTP proxy address to use for a given address, if any. +func GetProxyURL(dialAddr string) *url.URL { addrURL, err := parse(dialAddr) if err != nil || addrURL == nil { return nil diff --git a/api/client/proxy/proxy_test.go b/api/client/proxy/proxy_test.go index 89d30b077e2..633ce6d0c74 100644 --- a/api/client/proxy/proxy_test.go +++ b/api/client/proxy/proxy_test.go @@ -21,8 +21,10 @@ import ( "fmt" "net/http" "net/url" + "strings" "testing" + "github.com/gravitational/trace" "github.com/stretchr/testify/require" "golang.org/x/net/http/httpproxy" ) @@ -106,22 +108,71 @@ func TestGetProxyAddress(t *testing.T) { }, } + // used to augment test cases with auth credentials + authTests := []struct { + info string + user string + password string + }{ + {info: "no credentials", user: "", password: ""}, + {info: "plain password", user: "alice", password: "password"}, + {info: "special characters in password", user: "alice", password: " !@#$%^&*()_+-=[]{};:,.<>/?`~\"\\ abc123"}, + } + 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 { + for j, authTest := range authTests { + t.Run(fmt.Sprintf("%v %v: %v with %v", i, j, tt.info, authTest.info), func(t *testing.T) { + for _, env := range tt.env { + switch strings.ToLower(env.name) { + case "http_proxy", "https_proxy": + // add auth test credentials into http(s)_proxy env vars + val, err := buildProxyAddr(env.val, authTest.user, authTest.password) + require.NoError(t, err) + t.Setenv(env.name, val) + case "no_proxy": + t.Setenv(env.name, env.val) + } + } + p := GetProxyURL(tt.targetAddr) + + // is a proxy expected? + if tt.proxyAddr == "" { + require.Nil(t, p) + return + } require.NotNil(t, p) require.Equal(t, tt.proxyAddr, p.Host) - } - }) + + // are auth credentials expected? + if authTest.user == "" && authTest.password == "" { + require.Nil(t, p.User) + return + } + require.NotNil(t, p.User) + require.Equal(t, authTest.user, p.User.Username()) + password, _ := p.User.Password() + require.Equal(t, authTest.password, password) + }) + } } } +func buildProxyAddr(addr, user, pass string) (string, error) { + if user == "" && pass == "" { + return addr, nil + } + userInfo := url.UserPassword(user, pass) + if strings.HasPrefix(addr, "http") { + u, err := url.Parse(addr) + if err != nil { + return "", trace.Wrap(err) + } + u.User = userInfo + return u.String(), nil + } + return fmt.Sprintf("%v@%v", userInfo.String(), addr), nil +} + func TestProxyAwareRoundTripper(t *testing.T) { t.Setenv("HTTP_PROXY", "http://localhost:8888") transport := &http.Transport{ diff --git a/docs/pages/setup/reference/networking.mdx b/docs/pages/setup/reference/networking.mdx index bcd5514a4c8..95a2354f6b2 100644 --- a/docs/pages/setup/reference/networking.mdx +++ b/docs/pages/setup/reference/networking.mdx @@ -71,7 +71,7 @@ Environment="NO_PROXY=localhost,127.0.0.1,192.168.0.0/16,172.16.0.0/12,10.0.0.0/ When Teleport builds and establishes the reverse tunnel to the main cluster, it will funnel all traffic through the proxy. Specifically, if using the default configuration, Teleport will tunnel ports `3024` (SSH, reverse tunnel) and `3080` (HTTPS, establishing trust) through the proxy. The value of `HTTPS_PROXY` or `HTTP_PROXY` should be in the format -`scheme://host:port` where scheme is either `https` or `http` . If the value is +`scheme://[user[:password]@]host:port` where scheme is either `https` or `http` . If the value is `host:port` , Teleport will prepend `http` .