Skip to content

Commit

Permalink
Fix pickDefaultAddr not respecting HTTPS_PROXY (#22490)
Browse files Browse the repository at this point in the history
This change fixes a bug where tsh would not respect HTTPS_PROXY when it
has to guess the Telport proxy's port.
  • Loading branch information
atburke committed Mar 10, 2023
1 parent a42a403 commit e829922
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 9 deletions.
4 changes: 2 additions & 2 deletions integration/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -1840,8 +1840,8 @@ func genUserKey() (*client.Key, error) {
}, nil
}

// getLocalIP gets the non-loopback IP address of this host.
func getLocalIP() (string, error) {
// 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)
Expand Down
2 changes: 1 addition & 1 deletion integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1948,7 +1948,7 @@ func testTwoClustersProxy(t *testing.T, suite *integrationTestSuite) {

// httpproxy doesn't allow proxying when the target is localhost, so use
// this address instead.
addr, err := getLocalIP()
addr, err := GetLocalIP()
require.NoError(t, err)
a := suite.newNamedTeleportInstance(t, "site-A", WithNodeName(addr))
b := suite.newNamedTeleportInstance(t, "site-B", WithNodeName(addr))
Expand Down
8 changes: 4 additions & 4 deletions integration/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func TestALPNSNIHTTPSProxy(t *testing.T) {

username := mustGetCurrentUser(t).Username
// httpproxy won't proxy when target address is localhost, so use this instead.
addr, err := getLocalIP()
addr, err := GetLocalIP()
require.NoError(t, err)

suite := newProxySuite(t,
Expand Down Expand Up @@ -258,7 +258,7 @@ func TestMultiPortHTTPSProxy(t *testing.T) {

username := mustGetCurrentUser(t).Username
// httpproxy won't proxy when target address is localhost, so use this instead.
addr, err := getLocalIP()
addr, err := GetLocalIP()
require.NoError(t, err)

suite := newProxySuite(t,
Expand Down Expand Up @@ -943,7 +943,7 @@ func TestALPNProxyHTTPProxyNoProxyDial(t *testing.T) {
lib.SetInsecureDevMode(true)
defer lib.SetInsecureDevMode(false)

addr, err := getLocalIP()
addr, err := GetLocalIP()
require.NoError(t, err)

rc := NewInstance(InstanceConfig{
Expand Down Expand Up @@ -1018,7 +1018,7 @@ func TestALPNProxyHTTPProxyBasicAuthDial(t *testing.T) {

log := utils.NewLoggerForTests()

rcAddr, err := getLocalIP()
rcAddr, err := GetLocalIP()
require.NoError(t, err)

rc := NewInstance(InstanceConfig{
Expand Down
5 changes: 5 additions & 0 deletions tool/tsh/resolve_default_addr.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@ import (
"io"
"net"
"net/http"
"net/url"
"strconv"
"sync"
"time"

"github.com/gravitational/trace"
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"

"github.com/gravitational/teleport/api/client/proxy"
"github.com/gravitational/teleport/api/observability/tracing"
)

Expand Down Expand Up @@ -119,6 +121,9 @@ func pickDefaultAddr(ctx context.Context, insecure bool, host string, ports []in
TLSClientConfig: &tls.Config{
InsecureSkipVerify: insecure,
},
Proxy: func(req *http.Request) (*url.URL, error) {
return proxy.GetProxyURL(req.URL.String()), nil
},
},
otelhttp.WithSpanNameFormatter(tracing.HTTPTransportFormatter),
),
Expand Down
46 changes: 44 additions & 2 deletions tool/tsh/resolve_default_addr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"fmt"
"io"
"net"
"net/http"
"net/http/httptest"
"net/url"
Expand All @@ -29,6 +30,9 @@ import (

"github.com/gravitational/trace"
"github.com/stretchr/testify/require"

"github.com/gravitational/teleport/integration"
"github.com/gravitational/teleport/integration/helpers"
)

var testLog = log.WithField(trace.Component, "test")
Expand Down Expand Up @@ -74,8 +78,14 @@ func mustGetCandidatePorts(servers []*httptest.Server) []int {
return result
}

func makeTestServer(t *testing.T, h http.Handler) *httptest.Server {
svr := httptest.NewTLSServer(h)
type testServerOption func(*httptest.Server)

func makeTestServer(t *testing.T, h http.Handler, opts ...testServerOption) *httptest.Server {
svr := httptest.NewUnstartedServer(h)
for _, opt := range opts {
opt(svr)
}
svr.StartTLS()
t.Cleanup(func() { svr.Close() })
return svr
}
Expand Down Expand Up @@ -259,3 +269,35 @@ func TestResolveDefaultAddrTimeoutBeforeAllRacersLaunched(t *testing.T) {
// the call timing out.
require.Equal(t, context.DeadlineExceeded, err)
}

func TestResolveDefaultAddrHTTPProxy(t *testing.T) {
proxyHandler := &helpers.ProxyHandler{}
proxyServer := httptest.NewServer(proxyHandler)
t.Cleanup(proxyServer.Close)

// Go won't proxy to localhost, so use this address instead.
localIP, err := integration.GetLocalIP()
require.NoError(t, err)

var serverAddr net.Addr
respondingHandler := newRespondingHandler()
server := makeTestServer(t, respondingHandler, func(srv *httptest.Server) {
// Replace the test server's address.
l, err := net.Listen("tcp", localIP+":0")
require.NoError(t, err)
require.NoError(t, srv.Listener.Close())
srv.Listener = l
serverAddr = l.Addr()
})

ports := mustGetCandidatePorts([]*httptest.Server{server})

// Given an http proxy address...
t.Setenv("HTTPS_PROXY", proxyServer.URL)
// When I attempt to resove an address...
addr, err := pickDefaultAddr(context.Background(), true, localIP, ports)
// Expect that pickDefaultAddr uses the http proxy.
require.NoError(t, err)
require.Equal(t, serverAddr.String(), addr)
require.Equal(t, 1, proxyHandler.Count())
}

0 comments on commit e829922

Please sign in to comment.