Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore HTTP_PROXY for multi-port mode #13048

Merged
merged 6 commits into from
Jun 7, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions api/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,12 +289,7 @@ type (

// authConnect connects to the Teleport Auth Server directly.
func authConnect(ctx context.Context, params connectParams) (*Client, error) {
var dialer ContextDialer
if params.cfg.IgnoreHTTPProxy {
dialer = NewDirectDialer(params.cfg.KeepAlivePeriod, params.cfg.DialTimeout)
} else {
dialer = NewDialer(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)
Expand Down Expand Up @@ -474,8 +469,6 @@ type Config struct {
// ALPNSNIAuthDialClusterName if present the client will include ALPN SNI routing information in TLS Hello message
// allowing to dial auth service through Teleport Proxy directly without using SSH Tunnels.
ALPNSNIAuthDialClusterName string
// IgnoreHTTPProxy disables support for HTTP proxying when true.
IgnoreHTTPProxy bool
}

// CheckAndSetDefaults checks and sets default config values.
Expand Down
8 changes: 4 additions & 4 deletions api/client/contextdialer.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ 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, ignoring any HTTP proxies.
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,
Expand All @@ -58,7 +58,7 @@ func NewDirectDialer(keepAlivePeriod, dialTimeout time.Duration) ContextDialer {
// 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)
dialer := newDirectDialer(keepAlivePeriod, dialTimeout)
if proxyAddr := proxy.GetProxyAddress(addr); proxyAddr != nil {
return DialProxyWithDialer(ctx, proxyAddr.Host, addr, dialer)
}
Expand Down Expand Up @@ -87,7 +87,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 {
Expand Down
8 changes: 2 additions & 6 deletions api/client/webclient/webclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,6 @@ type Config struct {
// ExtraHeaders is a map of extra HTTP headers to be included in
// requests.
ExtraHeaders map[string]string
// IgnoreHTTPProxy disables support for HTTP proxying when true.
IgnoreHTTPProxy bool
// Timeout is a timeout for requests.
Timeout time.Duration
}
Expand Down Expand Up @@ -91,11 +89,9 @@ func newWebClient(cfg *Config) (*http.Client, error) {
InsecureSkipVerify: cfg.Insecure,
RootCAs: cfg.Pool,
},
}
if !cfg.IgnoreHTTPProxy {
transport.Proxy = func(req *http.Request) (*url.URL, error) {
Proxy: func(req *http.Request) (*url.URL, error) {
return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
}
},
}
return &http.Client{
Transport: otelhttp.NewTransport(proxy.NewHTTPFallbackRoundTripper(&transport, cfg.Insecure)),
Expand Down
16 changes: 0 additions & 16 deletions api/client/webclient/webclient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,19 +323,3 @@ func TestNewWebClientNoProxy(t *testing.T) {
require.Contains(t, err.Error(), "lookup fakedomain.example.com")
require.Contains(t, err.Error(), "no such host")
}

func TestNewWebClientIgnoreProxy(t *testing.T) {
t.Setenv("HTTPS_PROXY", "fakeproxy.example.com:9999")
client, err := newWebClient(&Config{
Context: context.Background(),
ProxyAddr: "localhost:3080",
IgnoreHTTPProxy: true,
})
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")
}
17 changes: 13 additions & 4 deletions integration/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,11 +239,18 @@ func TestALPNSNIHTTPSProxy(t *testing.T) {
require.Greater(t, ps.Count(), 0, "proxy did not intercept any connection")
}

// TestMultiPortNoProxy tests that the reverse tunnel does NOT use http_proxy
// when not in single-port mode.
func TestMultiPortNoProxy(t *testing.T) {
// TestMultiPortHTTPSProxy tests if the reverse tunnel uses http_proxy
// on a multiple proxy port setup.
func TestMultiPortHTTPSProxy(t *testing.T) {
// start the http proxy
ps := &proxyServer{}
ts := httptest.NewServer(ps)
defer ts.Close()

// set the http_proxy environment variable
t.Setenv("http_proxy", "fakeproxy.example.com")
u, err := url.Parse(ts.URL)
require.NoError(t, err)
t.Setenv("http_proxy", u.Host)

username := mustGetCurrentUser(t).Username
// httpproxy won't proxy when target address is localhost, so use this instead.
Expand All @@ -266,6 +273,8 @@ func TestMultiPortNoProxy(t *testing.T) {
"Two clusters do not see each other: tunnels are not working.")
require.Eventually(t, waitForClusters(suite.leaf.Tunnel, 1), 10*time.Second, 1*time.Second,
"Two clusters do not see each other: tunnels are not working.")

require.Greater(t, ps.Count(), 0, "proxy did not intercept any connection")
}

// TestAlpnSniProxyKube tests Kubernetes access with custom Kube API mock where traffic is forwarded via
Expand Down
2 changes: 0 additions & 2 deletions lib/auth/authclient/authclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ func Connect(ctx context.Context, cfg *Config) (auth.ClientI, error) {
Credentials: []apiclient.Credentials{
apiclient.LoadTLS(cfg.TLS),
},
// Deliberately ignore HTTP proxies for backwards compatibility.
IgnoreHTTPProxy: true,
})
if err != nil {
return nil, trace.Wrap(err, "failed direct dial to auth server: %v", err)
Expand Down
7 changes: 1 addition & 6 deletions lib/auth/clt.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,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")
}
var contextDialer client.ContextDialer
if cfg.IgnoreHTTPProxy {
contextDialer = client.NewDirectDialer(cfg.KeepAlivePeriod, cfg.DialTimeout)
} else {
contextDialer = client.NewDialer(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)
Expand Down
2 changes: 1 addition & 1 deletion lib/reversetunnel/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ func (a *Agent) getHostCheckers() ([]ssh.PublicKey, error) {
func (a *Agent) getReverseTunnelDetails() *reverseTunnelDetails {
pd := reverseTunnelDetails{TLSRoutingEnabled: false}
resp, err := webclient.Find(
&webclient.Config{Context: a.ctx, ProxyAddr: a.Addr.Addr, Insecure: lib.IsInsecureDevMode(), IgnoreHTTPProxy: true})
&webclient.Config{Context: a.ctx, ProxyAddr: a.Addr.Addr, Insecure: lib.IsInsecureDevMode()})

if err != nil {
// If TLS Routing is disabled the address is the proxy reverse tunnel
Expand Down
3 changes: 1 addition & 2 deletions lib/reversetunnel/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,8 @@ func WebClientResolver(ctx context.Context, addrs []utils.NetAddr, insecureTLS b
for _, addr := range addrs {
// In insecure mode, any certificate is accepted. In secure mode the hosts
// CAs are used to validate the certificate on the proxy.
// Ignore HTTP proxy for backwards compatibility.
tunnelAddr, err := webclient.GetTunnelAddr(
&webclient.Config{Context: ctx, ProxyAddr: addr.String(), Insecure: insecureTLS, IgnoreHTTPProxy: true})
&webclient.Config{Context: ctx, ProxyAddr: addr.String(), Insecure: insecureTLS})

if err != nil {
errs = append(errs, err)
Expand Down
2 changes: 1 addition & 1 deletion lib/reversetunnel/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (t *TunnelAuthDialer) DialContext(ctx context.Context, _, _ string) (net.Co

// Check if t.ProxyAddr is ProxyWebPort and remote Proxy supports TLS ALPNSNIListener.
resp, err := webclient.Find(
&webclient.Config{Context: ctx, ProxyAddr: addr.Addr, Insecure: t.InsecureSkipTLSVerify, IgnoreHTTPProxy: true})
&webclient.Config{Context: ctx, ProxyAddr: addr.Addr, Insecure: t.InsecureSkipTLSVerify})
if err != nil {
// If TLS Routing is disabled the address is the proxy reverse tunnel
// address thus the ping call will always fail.
Expand Down
2 changes: 0 additions & 2 deletions lib/service/connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -974,8 +974,6 @@ func (process *TeleportProcess) newClientDirect(authServers []utils.NetAddr, tls
apiclient.LoadTLS(tlsConfig),
},
DialOpts: dialOpts,
// Deliberately ignore HTTP proxies for backwards compatibility.
IgnoreHTTPProxy: true,
}, cltParams...)
if err != nil {
return nil, trace.Wrap(err)
Expand Down