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

Respect HTTP_PROXY/HTTPS_PROXY #10209

Merged
merged 42 commits into from
Mar 23, 2022
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
72459fb
Add http proxy to web clients
atburke Feb 7, 2022
05fa8b5
Add tests
atburke Feb 8, 2022
37cd786
Reduce arg ambiguity
atburke Feb 9, 2022
e2df595
Make error assertions more specific
atburke Feb 9, 2022
8281dfc
Fix linting
atburke Feb 9, 2022
59088d7
Merge branch 'master' into atburke/tsh-https-proxy
atburke Feb 9, 2022
700abc2
Add messages to assertions
atburke Feb 9, 2022
d41122a
Change fake proxy address in tests
atburke Feb 9, 2022
d225e71
Add http proxy support for ssh
atburke Feb 10, 2022
810d958
Move proxy utils to api
atburke Feb 15, 2022
dd5e232
Add proxy-aware context dialer
atburke Feb 15, 2022
7981450
Fix incorrect address in makeProxySSHClient
atburke Feb 16, 2022
726297f
Merge branch 'master' into atburke/tsh-https-proxy
atburke Feb 19, 2022
1823b90
Merge branch 'master' into atburke/tsh-https-proxy
atburke Feb 22, 2022
f57114d
Merge branch 'master' into atburke/tsh-https-proxy
atburke Mar 1, 2022
ad44ccf
Add docs
atburke Mar 1, 2022
de6c715
Add NO_PROXY tests
atburke Mar 1, 2022
df6c5e2
Simplify bodyclose workaround
atburke Mar 2, 2022
bde8fb2
Stop caching env vars
atburke Mar 3, 2022
bbc3af7
Split makeProxySSHClient
atburke Mar 3, 2022
79e3f3f
Remove leftover teleport imports from api
atburke Mar 10, 2022
fcf43ee
Make dialALPNWithDeadline a member of directDial
atburke Mar 11, 2022
e24d3d0
Add DialProxyWithDialer for custom dialers
atburke Mar 11, 2022
749d889
Make tlsConfig mandatory
atburke Mar 14, 2022
4e2a20a
Bring back tlsRoutingEnabled flag
atburke Mar 14, 2022
cad2d6c
Move tls config configuration to its own function
atburke Mar 15, 2022
0edbc1d
Merge branch 'master' into atburke/tsh-https-proxy
atburke Mar 21, 2022
df2853e
Remove siddontang logger from proxy.go
atburke Mar 22, 2022
926f7bb
Merge branch 'master' into atburke/tsh-https-proxy
atburke Mar 22, 2022
23fdd82
Address review comments
atburke Mar 22, 2022
a94ce4b
Fix error message checks in tests
atburke Mar 22, 2022
dd2cb23
Fix missed url update
atburke Mar 22, 2022
2815f4c
Fix no_proxy value in tests
atburke Mar 22, 2022
5238d5a
Merge branch 'master' into atburke/tsh-https-proxy
atburke Mar 23, 2022
4f14ec4
Merge branch 'master' into atburke/tsh-https-proxy
atburke Mar 23, 2022
b343ff8
Tweak newWebClient arg description
atburke Mar 23, 2022
0774cc4
Merge branch 'atburke/tsh-https-proxy' of github.com:gravitational/te…
atburke Mar 23, 2022
b6dff62
Fix newWebClient signature in tests
atburke Mar 23, 2022
b861f75
Merge branch 'master' into atburke/tsh-https-proxy
atburke Mar 23, 2022
f5b3b71
Merge branch 'master' into atburke/tsh-https-proxy
atburke Mar 23, 2022
e127340
Merge branch 'master' into atburke/tsh-https-proxy
atburke Mar 23, 2022
76dfcbe
Merge branch 'master' into atburke/tsh-https-proxy
atburke Mar 23, 2022
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
2 changes: 1 addition & 1 deletion api/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 15 additions & 3 deletions api/client/contextdialer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
atburke marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand All @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions lib/utils/proxy/noproxy.go → api/client/noproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down
150 changes: 150 additions & 0 deletions api/client/proxy.go
Original file line number Diff line number Diff line change
@@ -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) {
Comment on lines +34 to +39
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:
We can combined this using optional parameters but leaving it up to you.

func DialProxy(ctx context.Context, proxyAddr, addr string, opts ...dialOptions)

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 {
atburke marked this conversation as resolved.
Show resolved Hide resolved
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
}
11 changes: 2 additions & 9 deletions lib/utils/proxy/proxy_test.go → api/client/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
})
}
Expand Down
4 changes: 4 additions & 0 deletions api/client/webclient/webclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
},
Comment on lines +85 to +87
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Proxy: func(req *http.Request) (*url.URL, error) {
return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
},
Proxy: httpproxy.FromEnvironment().ProxyFunc(),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would be nice, but the function returned by ProxyFunc() expects a URL while Proxy: expects it to be a Request.

},
}
}
Expand Down
25 changes: 25 additions & 0 deletions api/client/webclient/webclient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,3 +289,28 @@ func TestExtract(t *testing.T) {
})
}
}

func TestNewWebClientRespectHTTPProxy(t *testing.T) {
atburke marked this conversation as resolved.
Show resolved Hide resolved
t.Setenv("HTTPS_PROXY", "fakeproxy.example.com:9999")
client := newWebClient(false /* insecure */, nil /* pool */)
// 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 := newWebClient(false /* insecure */, nil /* pool */)
//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")
}
12 changes: 12 additions & 0 deletions api/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,3 +189,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"
)
12 changes: 0 additions & 12 deletions constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/auth/clt.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading