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

Ignore HTTP_PROXY in reverse tunnels, part 2 #12335

Merged
merged 9 commits into from
May 11, 2022
9 changes: 8 additions & 1 deletion api/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,12 @@ type (

// authConnect connects to the Teleport Auth Server directly.
func authConnect(ctx context.Context, params connectParams) (*Client, error) {
dialer := NewDialer(params.cfg.KeepAlivePeriod, params.cfg.DialTimeout)
var dialer ContextDialer
if params.cfg.IgnoreHTTPProxy {
dialer = NewDirectDialer(params.cfg.KeepAlivePeriod, params.cfg.DialTimeout)
} else {
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 @@ -469,6 +474,8 @@ 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 @@ -45,8 +45,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.
func newDirectDialer(keepAlivePeriod, dialTimeout time.Duration) ContextDialer {
// NewDirectDialer makes a new dialer to connect directly to an Auth server, ignoring any HTTP proxies.
func NewDirectDialer(keepAlivePeriod, dialTimeout time.Duration) ContextDialer {
return &net.Dialer{
Timeout: dialTimeout,
KeepAlive: keepAlivePeriod,
Expand All @@ -57,7 +57,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 @@ -86,7 +86,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
29 changes: 29 additions & 0 deletions integration/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,35 @@ 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) {
// set the http_proxy environment variable
t.Setenv("http_proxy", "fakeproxy.example.com")

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(standardPortSetup()),
withLeafClusterPorts(standardPortSetup()),
withRootAndLeafClusterRoles(createTestRole(username)),
withStandardRoleMapping(),
)

// Wait for both cluster to see each other via reverse tunnels.
require.Eventually(t, waitForClusters(suite.root.Tunnel, 1), 10*time.Second, 1*time.Second,
"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.")
}

// TestAlpnSniProxyKube tests Kubernetes access with custom Kube API mock where traffic is forwarded via
//SNI ALPN proxy service to Kubernetes service based on TLS SNI value.
func TestALPNSNIProxyKube(t *testing.T) {
Expand Down
2 changes: 2 additions & 0 deletions lib/auth/authclient/authclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ 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,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm probably missing something, but I'm still not sure why we need to ignore it in the authclient too. Isn't lib/reversetunnel/resolver enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't enough. authclient was also affected by the original HTTP_PROXY change. TestMultiPortNoProxy doesn't pass unless authclient ignores proxies.

})
if err != nil {
return nil, trace.Wrap(err, "failed direct dial to auth server: %v", err)
Expand Down
7 changes: 6 additions & 1 deletion lib/auth/clt.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,12 @@ 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.NewDialer(cfg.KeepAlivePeriod, cfg.DialTimeout)
var contextDialer client.ContextDialer
if cfg.IgnoreHTTPProxy {
contextDialer = client.NewDirectDialer(cfg.KeepAlivePeriod, cfg.DialTimeout)
} else {
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
3 changes: 2 additions & 1 deletion lib/reversetunnel/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,9 @@ 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})
&webclient.Config{Context: ctx, ProxyAddr: addr.String(), Insecure: insecureTLS, IgnoreHTTPProxy: true})
atburke marked this conversation as resolved.
Show resolved Hide resolved

if err != nil {
errs = append(errs, err)
Expand Down
2 changes: 2 additions & 0 deletions lib/service/connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -963,6 +963,8 @@ 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