Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
b1c9acb
Set extra proxy headers in all `tsh` HTTP requests
Dec 30, 2022
8606338
Update http round tripper docs
Jan 4, 2023
ff9c6a7
Inline config definition
Jan 4, 2023
0107a14
Add `NewInsecureWebClient` back
Jan 4, 2023
1fd3b59
Merge branch 'master' into vitor/extra-proxy-headers
Jan 4, 2023
57c40d8
Fix TestProxyAwareRoundTripper
Jan 5, 2023
512d7ac
Merge branch 'vitor/extra-proxy-headers' of https://github.com/gravit…
Jan 5, 2023
94e612b
Merge branch 'master' into vitor/extra-proxy-headers
Jan 5, 2023
c1514a4
Don't change config state
Jan 5, 2023
90d6707
Remove unnecessary path var in TestPlainHttpFallback
Jan 5, 2023
de5dc4d
Remove ClientConfig struct
Jan 5, 2023
745e967
Merge branch 'master' into vitor/extra-proxy-headers
Jan 5, 2023
cbf721e
Rename tests
Jan 5, 2023
1e188c3
Simplify TestPlainHttpFallback
Jan 5, 2023
9641dd1
Add http roundtripper tests
Jan 6, 2023
97b40b0
Merge branch 'master' into vitor/extra-proxy-headers
Jan 6, 2023
6441f5b
Improve comment
Jan 6, 2023
0c3a5dd
Remove optimization in RoundTrip
Jan 7, 2023
aa639b1
Improve TestHttpRoundTripperDowngrade
Jan 7, 2023
451646b
Move TestHttpRoundTripper* tests to api/client/proxy
Jan 8, 2023
61b091a
Merge branch 'master' into vitor/extra-proxy-headers
Jan 9, 2023
92b31c6
Merge branch 'master' into vitor/extra-proxy-headers
Jan 9, 2023
d56c381
Merge branch 'master' into vitor/extra-proxy-headers
Jan 9, 2023
77c274e
Merge branch 'master' into vitor/extra-proxy-headers
Jan 10, 2023
17ffc0b
Don't use roundtrip in roundtripper tests
Jan 10, 2023
bb2420c
Fix lint-api
Jan 10, 2023
a584b5c
Consistent comments
Jan 10, 2023
0bde2cf
Merge branch 'master' into vitor/extra-proxy-headers
Jan 10, 2023
c348d8f
Merge branch 'master' into vitor/extra-proxy-headers
Jan 10, 2023
ebc0245
Merge branch 'master' into vitor/extra-proxy-headers
Jan 11, 2023
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
31 changes: 19 additions & 12 deletions api/client/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,32 +71,39 @@ func parse(addr string) (*url.URL, error) {
return addrURL, nil
}

// HTTPFallbackRoundTripper is a wrapper for http.Transport that downgrades requests
// to plain HTTP when using a plain HTTP proxy at localhost.
type HTTPFallbackRoundTripper struct {
// HTTPRoundTripper is a wrapper for http.Transport that
Comment thread
ibeckermayer marked this conversation as resolved.
// - adds extra HTTP headers to all requests, and
// - downgrades requests to plain HTTP when proxy is at localhost and the wrapped http.Transport has TLSClientConfig.InsecureSkipVerify set to true.
type HTTPRoundTripper struct {
*http.Transport
// extraHeaders is a map of extra HTTP headers to be included in requests.
extraHeaders map[string]string
// isProxyHTTPLocalhost indicates that the HTTP_PROXY is at "http://localhost"
isProxyHTTPLocalhost bool
}

// NewHTTPFallbackRoundTripper creates a new initialized HTTP fallback roundtripper.
func NewHTTPFallbackRoundTripper(transport *http.Transport, insecure bool) *HTTPFallbackRoundTripper {
// NewHTTPRoundTripper creates a new initialized HTTP roundtripper.
func NewHTTPRoundTripper(transport *http.Transport, extraHeaders map[string]string) *HTTPRoundTripper {
proxyConfig := httpproxy.FromEnvironment()
rt := HTTPFallbackRoundTripper{
return &HTTPRoundTripper{
Transport: transport,
extraHeaders: extraHeaders,
isProxyHTTPLocalhost: strings.HasPrefix(proxyConfig.HTTPProxy, "http://localhost"),
}
if rt.TLSClientConfig != nil {
rt.TLSClientConfig.InsecureSkipVerify = insecure
}
return &rt
}

// RoundTrip executes a single HTTP transaction. Part of the RoundTripper interface.
func (rt *HTTPFallbackRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
tlsConfig := rt.Transport.TLSClientConfig
func (rt *HTTPRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
// Add extra HTTP headers.
for header, v := range rt.extraHeaders {
req.Header.Add(header, v)
}

// Use plain HTTP if proxying via http://localhost in insecure mode.
tlsConfig := rt.Transport.TLSClientConfig
if rt.isProxyHTTPLocalhost && tlsConfig != nil && tlsConfig.InsecureSkipVerify {
req.URL.Scheme = "http"
}

return rt.Transport.RoundTrip(req)
}
192 changes: 190 additions & 2 deletions api/client/proxy/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ package proxy
import (
"crypto/tls"
"fmt"
"net"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"
Expand Down Expand Up @@ -182,12 +184,14 @@ func buildProxyAddr(addr, user, pass string) (string, error) {
func TestProxyAwareRoundTripper(t *testing.T) {
t.Setenv("HTTP_PROXY", "http://localhost:8888")
transport := &http.Transport{
TLSClientConfig: &tls.Config{},
TLSClientConfig: &tls.Config{
InsecureSkipVerify: true,
},
Proxy: func(req *http.Request) (*url.URL, error) {
return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
},
}
rt := NewHTTPFallbackRoundTripper(transport, true)
rt := NewHTTPRoundTripper(transport, nil)
req, err := http.NewRequest(http.MethodGet, "https://localhost:9999", nil)
require.NoError(t, err)
// Don't care about response, only if the scheme changed.
Expand All @@ -197,6 +201,190 @@ func TestProxyAwareRoundTripper(t *testing.T) {
require.Equal(t, "http", req.URL.Scheme)
}

// TestHttpRoundTripperDowngrade tests that the round tripper downgrades https requests to http
// when HTTP_PROXY is set to "http://localhost:*" (i.e. there's an http proxy running on localhost).
func TestHttpRoundTripperDowngrade(t *testing.T) {
testCases := []struct {
desc string
setHTTPProxy bool
shouldHitProxy bool
}{
{
desc: "hits http proxy if insecure and localhost http proxy is set",
setHTTPProxy: true,
shouldHitProxy: true,
},
{
desc: "does not hit http proxy if insecure and localhost http proxy is not set",
setHTTPProxy: false,
shouldHitProxy: false,
},
}

for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
newHandler := func(runningAtProxy bool, wasHit *bool) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
*wasHit = true
if tc.shouldHitProxy {
// If the request should hit the proxy, then:
// - this handler is running at the proxy, and
// - the scheme should be http.
require.True(t, runningAtProxy)
require.Equal(t, "http", r.URL.Scheme)
}
w.WriteHeader(http.StatusOK)
}
}

// Start localhost http proxy.
runningAtProxy := true
loopback := true
https := false
httpProxyWasHit := false
httpProxy, err := newServer(newHandler(runningAtProxy, &httpProxyWasHit), loopback, https)
require.NoError(t, err)
defer httpProxy.Close()

// Start non-localhost https server.
runningAtProxy = false
loopback = false
https = true
httpsSrvWasHit := false
httpsSrv, err := newServer(newHandler(runningAtProxy, &httpsSrvWasHit), loopback, https)
require.NoError(t, err)
defer httpsSrv.Close()

if tc.setHTTPProxy {
// url.Parse won't correctly parse an absolute URL without a scheme.
u, err := url.Parse("http://" + httpProxy.Listener.Addr().String())
require.NoError(t, err)
_, port, err := net.SplitHostPort(u.Host)
require.NoError(t, err)

// Set HTTP_PROXY to "http://localhost:*".
t.Setenv("HTTP_PROXY", fmt.Sprintf("http://localhost:%s", port))
}

clt := newClient(t, nil)

// Perform any request.
// Set addr to the https server. If HTTP_PROXY was set above,
// the http proxy should be hit regardless.
addr := httpsSrv.Listener.Addr().String()
request(t, clt, addr)

// Validate that the correct server was hit.
require.Equal(t, tc.shouldHitProxy, httpProxyWasHit)
require.Equal(t, !tc.shouldHitProxy, httpsSrvWasHit)
})
}
}

// TestHttpRoundTripperExtraHeaders tests that the round tripper adds the extra headers set.
func TestHttpRoundTripperExtraHeaders(t *testing.T) {
testCases := []struct {
desc string
extraHeaders map[string]string
expectHeaders func(*testing.T, http.Header)
}{
{
desc: "extra headers are added",
extraHeaders: map[string]string{
"header1": "value1",
"header2": "value2",
},
expectHeaders: func(t *testing.T, headers http.Header) {
require.Equal(t, []string{"value1"}, headers.Values("header1"))
require.Equal(t, []string{"value2"}, headers.Values("header2"))
},
},
{
desc: "extra headers do not overwrite existing headers",
extraHeaders: map[string]string{
"header1": "value1",
"Content-Type": "value2",
},
expectHeaders: func(t *testing.T, headers http.Header) {
require.Equal(t, []string{"value1"}, headers.Values("header1"))
require.Equal(t, []string{"application/json", "value2"}, headers.Values("Content-Type"))
},
},
}

for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
var handler http.HandlerFunc = func(w http.ResponseWriter, r *http.Request) {
tc.expectHeaders(t, r.Header)
w.WriteHeader(http.StatusOK)
}

// Start localhost https server.
loopback := true
tls := true
httpsSrv, err := newServer(handler, loopback, tls)
require.NoError(t, err)
defer httpsSrv.Close()

clt := newClient(t, tc.extraHeaders)

// Perform any request.
// Set the address to the localhost https server.
addr := httpsSrv.Listener.Addr().String()
request(t, clt, addr)
})
}
}

// newServer starts a new server that:
// - runs TLS if `https`
// - uses a loopback listener if `loopback`
func newServer(handler http.HandlerFunc, loopback bool, https bool) (*httptest.Server, error) {
srv := httptest.NewUnstartedServer(handler)

if !loopback {
// Replace the test-supplied loopback listener with the first available
// non-loopback address.
srv.Listener.Close()
l, err := net.Listen("tcp", "0.0.0.0:0")
if err != nil {
return nil, err
}
srv.Listener = l
}

if https {
srv.StartTLS()
} else {
srv.Start()
}
return srv, nil
}

// newClient creates a new https roundtrip client.
func newClient(t *testing.T, extraHeaders map[string]string) *http.Client {
transport := &http.Transport{
TLSClientConfig: &tls.Config{
// Setting insecure ensures that https requests succeed.
InsecureSkipVerify: true,
},
Proxy: func(req *http.Request) (*url.URL, error) {
return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
},
}
return &http.Client{
Transport: NewHTTPRoundTripper(transport, extraHeaders),
}
}

// request perform a POST request.
func request(t *testing.T, clt *http.Client, addr string) {
url := "https://" + addr + "/v1/content"
resp, err := clt.Post(url, "application/json", nil)
require.NoError(t, err)
require.NoError(t, resp.Body.Close())
}

func TestParse(t *testing.T) {
successTests := []struct {
name, addr, scheme, host, path string
Expand Down
2 changes: 1 addition & 1 deletion api/client/webclient/webclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func newWebClient(cfg *Config) (*http.Client, error) {
}
return &http.Client{
Transport: otelhttp.NewTransport(
proxy.NewHTTPFallbackRoundTripper(&transport, cfg.Insecure),
proxy.NewHTTPRoundTripper(&transport, nil),
otelhttp.WithSpanNameFormatter(tracing.HTTPTransportFormatter),
),
Timeout: cfg.Timeout,
Expand Down
8 changes: 7 additions & 1 deletion lib/client/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -2787,7 +2787,12 @@ func makeProxySSHClient(ctx context.Context, tc *TeleportClient, sshConfig *ssh.
if len(tc.JumpHosts) > 0 {
sshProxyAddr = tc.JumpHosts[0].Addr.Addr
// Check if JumpHost address is a proxy web address.
resp, err := webclient.Find(&webclient.Config{Context: ctx, ProxyAddr: sshProxyAddr, Insecure: tc.InsecureSkipVerify})
resp, err := webclient.Find(&webclient.Config{
Context: ctx,
ProxyAddr: sshProxyAddr,
Insecure: tc.InsecureSkipVerify,
ExtraHeaders: tc.ExtraProxyHeaders,
})
// If JumpHost address is a proxy web port and proxy supports TLSRouting dial proxy with TLSWrapper.
if err == nil && resp.Proxy.TLSRoutingEnabled {
log.Infof("Connecting to proxy=%v login=%q using TLS Routing JumpHost", sshProxyAddr, sshConfig.User)
Expand Down Expand Up @@ -3284,6 +3289,7 @@ func (tc *TeleportClient) newSSHLogin(priv *keys.PrivateKey) (SSHLogin, error) {
RouteToCluster: tc.SiteName,
KubernetesCluster: tc.KubernetesCluster,
AttestationStatement: attestationStatement,
ExtraHeaders: tc.ExtraProxyHeaders,
}, nil
}

Expand Down
33 changes: 12 additions & 21 deletions lib/client/https_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,39 +37,30 @@ import (
)

func NewInsecureWebClient() *http.Client {
// Because Teleport clients can't be configured (yet), they take the default
// list of cipher suites from Go.
tlsConfig := utils.TLSConfig(nil)
transport := http.Transport{
TLSClientConfig: tlsConfig,
Proxy: func(req *http.Request) (*url.URL, error) {
return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
},
}
return newClient(true, nil, nil)
}

func newClient(insecure bool, pool *x509.CertPool, extraHeaders map[string]string) *http.Client {
return &http.Client{
Transport: otelhttp.NewTransport(
apiproxy.NewHTTPFallbackRoundTripper(&transport, true /* insecure */),
apiproxy.NewHTTPRoundTripper(httpTransport(insecure, pool), extraHeaders),
otelhttp.WithSpanNameFormatter(tracing.HTTPTransportFormatter),
),
}
}

func newClientWithPool(pool *x509.CertPool) *http.Client {
func httpTransport(insecure bool, pool *x509.CertPool) *http.Transport {
// Because Teleport clients can't be configured (yet), they take the default
// list of cipher suites from Go.
tlsConfig := utils.TLSConfig(nil)
tlsConfig.InsecureSkipVerify = insecure
tlsConfig.RootCAs = pool

return &http.Client{
Transport: otelhttp.NewTransport(
&http.Transport{
TLSClientConfig: tlsConfig,
Proxy: func(req *http.Request) (*url.URL, error) {
return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
},
},
otelhttp.WithSpanNameFormatter(tracing.HTTPTransportFormatter),
),
return &http.Transport{
TLSClientConfig: tlsConfig,
Proxy: func(req *http.Request) (*url.URL, error) {
return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
},
}
}

Expand Down
8 changes: 4 additions & 4 deletions lib/client/https_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ func TestNewInsecureWebClientNoProxy(t *testing.T) {
require.Contains(t, err.Error(), "no such host")
}

func TestNewClientWithPoolHTTPProxy(t *testing.T) {
func TestNewSecureWebClientHTTPProxy(t *testing.T) {
t.Setenv("HTTPS_PROXY", "fakeproxy.example.com:9999")
client := newClientWithPool(nil)
client := newClient(false, nil, nil)
//nolint:bodyclose // resp should be nil, so there will be no body to close.
resp, err := client.Get("https://fakedomain.example.com")
// Client should try to proxy through nonexistent server at localhost.
Expand All @@ -58,10 +58,10 @@ func TestNewClientWithPoolHTTPProxy(t *testing.T) {
require.Contains(t, err.Error(), "no such host")
}

func TestNewClientWithPoolNoProxy(t *testing.T) {
func TestNewSecureWebClientNoProxy(t *testing.T) {
t.Setenv("HTTPS_PROXY", "fakeproxy.example.com:9999")
t.Setenv("NO_PROXY", "fakedomain.example.com")
client := newClientWithPool(nil)
client := newClient(false, nil, nil)
//nolint:bodyclose // resp should be nil, so there will be no body to close.
resp, err := client.Get("https://fakedomain.example.com")
require.Error(t, err, "GET unexpectedly succeeded: %+v", resp)
Expand Down
2 changes: 1 addition & 1 deletion lib/client/redirect.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ type RedirectorConfig struct {

// NewRedirector returns new local web server redirector
func NewRedirector(ctx context.Context, login SSHLoginSSO, config *RedirectorConfig) (*Redirector, error) {
clt, proxyURL, err := initClient(login.ProxyAddr, login.Insecure, login.Pool)
clt, proxyURL, err := initClient(login.ProxyAddr, login.Insecure, login.Pool, login.ExtraHeaders)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down
Loading