diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ebb747d0..821b5a53 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,5 +1,9 @@ on: [push, pull_request] name: Test +# Workaround for coveralls error "Can't add a job to a build that is already closed" +# See https://github.com/lemurheavy/coveralls-public/issues/1716 +env: + COVERALLS_SERVICE_NUMBER: ${{ github.run_id }}-${{ github.run_attempt }} jobs: test: strategy: @@ -23,6 +27,7 @@ jobs: go test -race -v -timeout 2m -failfast -covermode atomic -coverprofile=.covprofile ./... -tags=nointegration # Run integration tests hermetically to avoid nondeterministic races on environment variables go test -race -v -timeout 2m -failfast ./cmd/... -run TestSmokescreenIntegration + go test -race -v -timeout 2m -failfast ./cmd/... -run TestInvalidUpstreamProxyConfiguratedFromEnv go test -race -v -timeout 2m -failfast ./cmd/... -run TestInvalidUpstreamProxyConfiguration go test -race -v -timeout 2m -failfast ./cmd/... -run TestClientHalfCloseConnection - name: Install goveralls diff --git a/cmd/integration_test.go b/cmd/integration_test.go index ed1320cb..cfa53cb1 100644 --- a/cmd/integration_test.go +++ b/cmd/integration_test.go @@ -279,7 +279,7 @@ func TestSmokescreenIntegration(t *testing.T) { for _, useTLS := range []bool{true, false} { // Smokescreen instances - _, proxyServer, err := startSmokescreen(t, useTLS, &logHook) + _, proxyServer, err := startSmokescreen(t, useTLS, &logHook, "") require.NoError(t, err) defer proxyServer.Close() proxyServers[useTLS] = proxyServer @@ -449,13 +449,13 @@ func validateProxyResponseWithUpstream(t *testing.T, test *TestCase, resp *http. // This test must be run with a separate test command as the environment variables // required can race with the test above. -func TestInvalidUpstreamProxyConfiguration(t *testing.T) { +func TestInvalidUpstreamProxyConfiguratedFromEnv(t *testing.T) { var logHook logrustest.Hook servers := map[bool]*httptest.Server{} // Create TLS and non-TLS instances of Smokescreen for _, useTLS := range []bool{true, false} { - _, server, err := startSmokescreen(t, useTLS, &logHook) + _, server, err := startSmokescreen(t, useTLS, &logHook, "") require.NoError(t, err) defer server.Close() servers[useTLS] = server @@ -500,6 +500,58 @@ func TestInvalidUpstreamProxyConfiguration(t *testing.T) { } } +func TestInvalidUpstreamProxyConfiguration(t *testing.T) { + var logHook logrustest.Hook + servers := map[bool]*httptest.Server{} + + // Create TLS and non-TLS instances of Smokescreen + for _, useTLS := range []bool{true, false} { + var httpProxyAddr string + if useTLS { + httpProxyAddr = "https://notaproxy.prxy.svc:443" + } else { + httpProxyAddr = "http://notaproxy.prxy.svc:80" + } + _, server, err := startSmokescreen(t, useTLS, &logHook, httpProxyAddr) + require.NoError(t, err) + defer server.Close() + servers[useTLS] = server + } + + // Passing an illegal upstream proxy value is not designed to be an especially well + // handled error so it would fail many of the checks in our other tests. We really + // only care to ensure that these requests never succeed. + for _, overConnect := range []bool{true, false} { + t.Run(fmt.Sprintf("illegal proxy with CONNECT %t", overConnect), func(t *testing.T) { + var proxyTarget string + var upstreamProxy string + + // These proxy targets don't actually matter as the requests won't be sent. + // because the resolution of the upstream proxy will fail. + if overConnect { + upstreamProxy = "https://notaproxy.prxy.svc:443" + proxyTarget = "https://api.stripe.com:443" + } else { + upstreamProxy = "http://notaproxy.prxy.svc:80" + proxyTarget = "http://checkip.amazonaws.com:80" + } + + testCase := &TestCase{ + OverConnect: overConnect, + OverTLS: overConnect, + ProxyURL: servers[overConnect].URL, + TargetURL: proxyTarget, + UpstreamProxy: upstreamProxy, + RoleName: generateRoleForPolicy(acl.Open), + ExpectStatus: http.StatusBadGateway, + } + resp, err := executeRequestForTest(t, testCase, &logHook) + validateProxyResponseWithUpstream(t, testCase, resp, err, logHook.AllEntries()) + + }) + } +} + func TestClientHalfCloseConnection(t *testing.T) { a := assert.New(t) @@ -510,7 +562,7 @@ func TestClientHalfCloseConnection(t *testing.T) { var logHook logrustest.Hook - conf, server, err := startSmokescreen(t, false, &logHook) + conf, server, err := startSmokescreen(t, false, &logHook, "") require.NoError(t, err) defer server.Close() @@ -577,7 +629,7 @@ func findLogEntry(entries []*logrus.Entry, msg string) *logrus.Entry { return nil } -func startSmokescreen(t *testing.T, useTLS bool, logHook logrus.Hook) (*smokescreen.Config, *httptest.Server, error) { +func startSmokescreen(t *testing.T, useTLS bool, logHook logrus.Hook, httpProxyAddr string) (*smokescreen.Config, *httptest.Server, error) { args := []string{ "smokescreen", "--listen-ip=127.0.0.1", @@ -596,6 +648,11 @@ func startSmokescreen(t *testing.T, useTLS bool, logHook logrus.Hook) (*smokescr ) } + if httpProxyAddr != ""{ + args = append(args, fmt.Sprintf("--upstream-http-proxy-addr=%s", httpProxyAddr)) + args = append(args, fmt.Sprintf("--upstream-https-proxy-addr=%s", httpProxyAddr)) + } + conf, err := NewConfiguration(args, nil) if err != nil { t.Fatalf("Failed to create configuration: %v", err) diff --git a/cmd/smokescreen.go b/cmd/smokescreen.go index 9089f129..be3a75f8 100644 --- a/cmd/smokescreen.go +++ b/cmd/smokescreen.go @@ -94,11 +94,11 @@ func NewConfiguration(args []string, logger *log.Logger) (*smokescreen.Config, e Value: "/metrics", Usage: "Expose prometheus metrics on `ENDPOINT`. Requires --expose-prometheus-metrics to be set. Defaults to \"/metrics\"", }, - cli.StringFlag{ - Name: "prometheus-listen-ip", - Value: "0.0.0.0", - Usage: "Listen for prometheus metrics on interface with address IP. Requires --expose-prometheus-metrics to be set. Defaults to \"0.0.0.0\"", - }, + cli.StringFlag{ + Name: "prometheus-listen-ip", + Value: "0.0.0.0", + Usage: "Listen for prometheus metrics on interface with address IP. Requires --expose-prometheus-metrics to be set. Defaults to \"0.0.0.0\"", + }, cli.StringFlag{ Name: "prometheus-port", Value: "9810", @@ -146,6 +146,16 @@ func NewConfiguration(args []string, logger *log.Logger) (*smokescreen.Config, e Name: "unsafe-allow-private-ranges", Usage: "Allow private ip ranges by default", }, + cli.StringFlag{ + Name: "upstream-http-proxy-addr", + Value: "", + Usage: "Set Smokescreen's upstream HTTP proxy address", + }, + cli.StringFlag{ + Name: "upstream-https-proxy-addr", + Value: "", + Usage: "Set Smokescreen's upstream HTTPS proxy address", + }, } app.Action = func(c *cli.Context) error { @@ -287,6 +297,14 @@ func NewConfiguration(args []string, logger *log.Logger) (*smokescreen.Config, e } } + if c.IsSet("upstream-http-proxy-addr") { + conf.UpstreamHttpProxyAddr = c.String("upstream-http-proxy-addr") + } + + if c.IsSet("upstream-https-proxy-addr") { + conf.UpstreamHttpsProxyAddr = c.String("upstream-https-proxy-addr") + } + // Setup the connection tracker if there is not yet one in the config if conf.ConnTracker == nil { conf.ConnTracker = conntrack.NewTracker(conf.IdleTimeout, conf.MetricsClient, conf.Log, conf.ShuttingDown, nil) diff --git a/go.mod b/go.mod index 5838ef24..9d827cf1 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/rs/xid v1.2.1 github.com/sirupsen/logrus v1.9.0 github.com/stretchr/testify v1.8.0 - github.com/stripe/goproxy v0.0.0-20230801191332-fabc3ecb7251 + github.com/stripe/goproxy v0.0.0-20231113215313-dbbdf2f6d709 golang.org/x/net v0.17.0 gopkg.in/urfave/cli.v1 v1.20.0 gopkg.in/yaml.v2 v2.4.0 diff --git a/go.sum b/go.sum index a4d1a5a2..9b9026be 100644 --- a/go.sum +++ b/go.sum @@ -218,6 +218,8 @@ github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PK github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stripe/goproxy v0.0.0-20230801191332-fabc3ecb7251 h1:wR1exp7OglR0ctk8yWPVp1oTOuyaLUlJv3/Wlbvbw64= github.com/stripe/goproxy v0.0.0-20230801191332-fabc3ecb7251/go.mod h1:hF2CVgH4++5ijZiy9grGVP8Fsi4u+SMOtbnIKYbMUjY= +github.com/stripe/goproxy v0.0.0-20231113215313-dbbdf2f6d709 h1:b0AttHAJ5f9rIK2frq9Q4WEeeBNQccr1j+cjQCmOl6s= +github.com/stripe/goproxy v0.0.0-20231113215313-dbbdf2f6d709/go.mod h1:hF2CVgH4++5ijZiy9grGVP8Fsi4u+SMOtbnIKYbMUjY= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= diff --git a/pkg/smokescreen/config.go b/pkg/smokescreen/config.go index 250911a7..bf60aa4a 100644 --- a/pkg/smokescreen/config.go +++ b/pkg/smokescreen/config.go @@ -73,6 +73,10 @@ type Config struct { TransportMaxIdleConns int TransportMaxIdleConnsPerHost int + // These are the http and https address for the upstream proxy + UpstreamHttpProxyAddr string + UpstreamHttpsProxyAddr string + // Used for logging connection time TimeConnect bool diff --git a/pkg/smokescreen/smokescreen.go b/pkg/smokescreen/smokescreen.go index b20239f2..2832315c 100644 --- a/pkg/smokescreen/smokescreen.go +++ b/pkg/smokescreen/smokescreen.go @@ -433,7 +433,7 @@ func newContext(cfg *Config, proxyType string, req *http.Request) *SmokescreenCo } func BuildProxy(config *Config) *goproxy.ProxyHttpServer { - proxy := goproxy.NewProxyHttpServer() + proxy := goproxy.NewProxyHttpServer(goproxy.WithHttpProxyAddr(config.UpstreamHttpProxyAddr), goproxy.WithHttpsProxyAddr(config.UpstreamHttpsProxyAddr)) proxy.Verbose = false configureTransport(proxy.Tr, config) diff --git a/vendor/github.com/stripe/goproxy/https.go b/vendor/github.com/stripe/goproxy/https.go index 0eea3660..3908e580 100644 --- a/vendor/github.com/stripe/goproxy/https.go +++ b/vendor/github.com/stripe/goproxy/https.go @@ -118,7 +118,7 @@ func (proxy *ProxyHttpServer) handleHttps(w http.ResponseWriter, r *http.Request host += ":80" } - httpsProxy, err := httpsProxyFromEnv(r.URL) + httpsProxy, err := httpsProxyAddr(r.URL, proxy.HttpsProxyAddr) if err != nil { ctx.Warnf("Error configuring HTTPS proxy err=%q url=%q", err, r.URL.String()) } @@ -398,14 +398,20 @@ func copyAndClose(ctx *ProxyCtx, dst, src *net.TCPConn) { src.CloseRead() } -func dialerFromEnv(proxy *ProxyHttpServer) func(network, addr string) (net.Conn, error) { - https_proxy := os.Getenv("HTTPS_PROXY") +// dialerFromProxy gets the HttpsProxyAddr from proxy to create a dialer. +// When the HttpsProxyAddr from proxy is empty, use the HTTPS_PROXY, https_proxy from environment variables. +func dialerFromProxy(proxy *ProxyHttpServer) func(network, addr string) (net.Conn, error) { + https_proxy := proxy.HttpsProxyAddr if https_proxy == "" { - https_proxy = os.Getenv("https_proxy") - } - if https_proxy == "" { - return nil + https_proxy = os.Getenv("HTTPS_PROXY") + if https_proxy == "" { + https_proxy = os.Getenv("https_proxy") + } + if https_proxy == "" { + return nil + } } + return proxy.NewConnectDialToProxy(https_proxy) } @@ -559,10 +565,17 @@ func (proxy *ProxyHttpServer) connectDialProxyWithContext(ctx *ProxyCtx, proxyHo return c, nil } -// httpsProxyFromEnv allows goproxy to respect no_proxy env vars +// httpsProxyAddr function uses the address in httpsProxy parameter. +// When the httpProxyAddr parameter is empty, uses the HTTPS_PROXY, https_proxy from environment variables. +// httpsProxyAddr function allows goproxy to respect no_proxy env vars // https://github.com/stripe/goproxy/pull/5 -func httpsProxyFromEnv(reqURL *url.URL) (string, error) { +func httpsProxyAddr(reqURL *url.URL, httpsProxy string) (string, error) { cfg := httpproxy.FromEnvironment() + + if httpsProxy != "" { + cfg.HTTPSProxy = httpsProxy + } + // We only use this codepath for HTTPS CONNECT proxies so we shouldn't // return anything from HTTPProxy cfg.HTTPProxy = "" diff --git a/vendor/github.com/stripe/goproxy/proxy.go b/vendor/github.com/stripe/goproxy/proxy.go index d189b48f..c4d34385 100644 --- a/vendor/github.com/stripe/goproxy/proxy.go +++ b/vendor/github.com/stripe/goproxy/proxy.go @@ -6,9 +6,12 @@ import ( "log" "net" "net/http" + "net/url" "os" "regexp" "sync/atomic" + + "golang.org/x/net/http/httpproxy" ) // The basic proxy type. Implements http.Handler. @@ -54,6 +57,10 @@ type ProxyHttpServer struct { // ConnectRespHandler allows users to mutate the response to the CONNECT request before it // is returned to the client. ConnectRespHandler func(ctx *ProxyCtx, resp *http.Response) error + + // HTTP and HTTPS proxy addresses + HttpProxyAddr string + HttpsProxyAddr string } var hasPort = regexp.MustCompile(`:\d+$`) @@ -179,8 +186,40 @@ func (proxy *ProxyHttpServer) ServeHTTP(w http.ResponseWriter, r *http.Request) } } +type options struct { + httpProxyAddr string + httpsProxyAddr string +} +type fnOption func(*options) + +func (fn fnOption) apply(opts *options) { fn(opts) } + +type ProxyHttpServerOptions interface { + apply(*options) +} + +func WithHttpProxyAddr(httpProxyAddr string) ProxyHttpServerOptions { + return fnOption(func(opts *options) { + opts.httpProxyAddr = httpProxyAddr + }) +} + +func WithHttpsProxyAddr(httpsProxyAddr string) ProxyHttpServerOptions { + return fnOption(func(opts *options) { + opts.httpsProxyAddr = httpsProxyAddr + }) +} + // NewProxyHttpServer creates and returns a proxy server, logging to stderr by default -func NewProxyHttpServer() *ProxyHttpServer { +func NewProxyHttpServer(opts ...ProxyHttpServerOptions) *ProxyHttpServer { + appliedOpts := &options{ + httpProxyAddr: "", + httpsProxyAddr: "", + } + for _, opt := range opts { + opt.apply(appliedOpts) + } + proxy := ProxyHttpServer{ Logger: log.New(os.Stderr, "", log.LstdFlags), reqHandlers: []ReqHandler{}, @@ -191,7 +230,27 @@ func NewProxyHttpServer() *ProxyHttpServer { }), Tr: &http.Transport{TLSClientConfig: tlsClientSkipVerify, Proxy: http.ProxyFromEnvironment}, } - proxy.ConnectDial = dialerFromEnv(&proxy) + + // httpProxyCfg holds configuration for HTTP proxy settings. See FromEnvironment for details. + httpProxyCfg := httpproxy.FromEnvironment() + + if appliedOpts.httpProxyAddr != "" { + proxy.HttpProxyAddr = appliedOpts.httpProxyAddr + httpProxyCfg.HTTPProxy = appliedOpts.httpProxyAddr + } + + if appliedOpts.httpsProxyAddr != "" { + proxy.HttpsProxyAddr = appliedOpts.httpsProxyAddr + httpProxyCfg.HTTPSProxy = appliedOpts.httpsProxyAddr + } + + proxy.ConnectDial = dialerFromProxy(&proxy) + + if appliedOpts.httpProxyAddr != "" || appliedOpts.httpsProxyAddr != "" { + proxy.Tr.Proxy = func(req *http.Request) (*url.URL, error) { + return httpProxyCfg.ProxyFunc()(req.URL) + } + } return &proxy } diff --git a/vendor/modules.txt b/vendor/modules.txt index a9b82a9f..496fab7d 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -69,7 +69,7 @@ github.com/sirupsen/logrus/hooks/test ## explicit; go 1.13 github.com/stretchr/testify/assert github.com/stretchr/testify/require -# github.com/stripe/goproxy v0.0.0-20230801191332-fabc3ecb7251 +# github.com/stripe/goproxy v0.0.0-20231113215313-dbbdf2f6d709 ## explicit; go 1.13 github.com/stripe/goproxy # golang.org/x/mod v0.8.0