diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ebb747d0..87245685 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -23,6 +23,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..cd7a46a8 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, "--transport-http-proxy-addr="+httpProxyAddr) + args = append(args, "--transport-https-proxy-addr="+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..42173be8 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: "transport-http-proxy-addr", + Value: "", + Usage: "Set the smokescreen's HTTP transport proxy address", + }, + cli.StringFlag{ + Name: "transport-https-proxy-addr", + Value: "", + Usage: "Set the smokescreen's HTTPS transport 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("transport-http-proxy-addr") { + conf.TransportHttpProxyAddr = c.String("transport-http-proxy-addr") + } + + if c.IsSet("transport-https-proxy-addr") { + conf.TransportHttpsProxyAddr = c.String("transport-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/pkg/smokescreen/config.go b/pkg/smokescreen/config.go index 250911a7..5af48fb1 100644 --- a/pkg/smokescreen/config.go +++ b/pkg/smokescreen/config.go @@ -73,6 +73,10 @@ type Config struct { TransportMaxIdleConns int TransportMaxIdleConnsPerHost int + // There are the http and https address for the transport proxy + TransportHttpProxyAddr string + TransportHttpsProxyAddr string + // Used for logging connection time TimeConnect bool diff --git a/pkg/smokescreen/smokescreen.go b/pkg/smokescreen/smokescreen.go index b20239f2..a5cecb4b 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.TransportHttpProxyAddr), goproxy.WithHttpsProxyAddr(config.TransportHttpsProxyAddr)) proxy.Verbose = false configureTransport(proxy.Tr, config)