From 4587d68a5fe4e48af2ea5f65471e78a377839615 Mon Sep 17 00:00:00 2001 From: cmoresco-stripe <106690468+cmoresco-stripe@users.noreply.github.com> Date: Thu, 3 Aug 2023 14:16:17 -0400 Subject: [PATCH] Use AcceptResponseHandler in goproxy https CONNECT hook (#199) * pipe AcceptResponseHandler into new goproxy hook * update comment * go mod vendor * unit test * use smokescreenctx in acceptresponsehandler * fix unit tests --- go.mod | 2 +- go.sum | 2 ++ pkg/smokescreen/config.go | 4 +-- pkg/smokescreen/smokescreen.go | 15 +++++++++++- pkg/smokescreen/smokescreen_test.go | 3 ++- vendor/github.com/stripe/goproxy/https.go | 30 ++++++++++++++++++++++- vendor/github.com/stripe/goproxy/proxy.go | 4 +++ vendor/modules.txt | 2 +- 8 files changed, 55 insertions(+), 7 deletions(-) diff --git a/go.mod b/go.mod index b8399e80..82b199a7 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-20220308202309-3f1dfba6d1a4 + github.com/stripe/goproxy v0.0.0-20230801191332-fabc3ecb7251 golang.org/x/net v0.7.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 3a1e8c4f..208f45cb 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-20220308202309-3f1dfba6d1a4 h1:6lqpRCXAhigpiMu6aZYTHryDmLrhIND0H4iUKEntN1M= github.com/stripe/goproxy v0.0.0-20220308202309-3f1dfba6d1a4/go.mod h1:hF2CVgH4++5ijZiy9grGVP8Fsi4u+SMOtbnIKYbMUjY= +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/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 6eb23697..cd3642d5 100644 --- a/pkg/smokescreen/config.go +++ b/pkg/smokescreen/config.go @@ -82,8 +82,8 @@ type Config struct { // Custom handler to allow clients to modify reject responses RejectResponseHandler func(*http.Response) - // Custom handler to allow clients to modify accept responses - AcceptResponseHandler func(*http.Response) + // Custom handler to allow clients to modify successful CONNECT responses + AcceptResponseHandler func(*smokescreenContext, *http.Response) error // UnsafeAllowPrivateRanges inverts the default behavior, telling smokescreen to allow private IP // ranges by default (exempting loopback and unicast ranges) diff --git a/pkg/smokescreen/smokescreen.go b/pkg/smokescreen/smokescreen.go index 6a073c1a..520d1728 100644 --- a/pkg/smokescreen/smokescreen.go +++ b/pkg/smokescreen/smokescreen.go @@ -546,7 +546,7 @@ func BuildProxy(config *Config) *goproxy.ProxyHttpServer { resp.Header.Del(errorHeader) } if sctx.cfg.AcceptResponseHandler != nil { - sctx.cfg.AcceptResponseHandler(resp) + sctx.cfg.AcceptResponseHandler(sctx, resp) } } @@ -559,6 +559,19 @@ func BuildProxy(config *Config) *goproxy.ProxyHttpServer { logProxy(config, pctx) return resp }) + + // This function will be called on the response to a successful https CONNECT request. + // The goproxy OnResponse() function above is only called for non-https responses. + if config.AcceptResponseHandler != nil { + proxy.ConnectRespHandler = func(pctx *goproxy.ProxyCtx, resp *http.Response) error { + sctx, ok := pctx.UserData.(*smokescreenContext) + if !ok { + return fmt.Errorf("goproxy ProxyContext missing required UserData *smokescreenContext") + } + return config.AcceptResponseHandler(sctx, resp) + } + } + return proxy } diff --git a/pkg/smokescreen/smokescreen_test.go b/pkg/smokescreen/smokescreen_test.go index 8fe192b7..9ba44114 100644 --- a/pkg/smokescreen/smokescreen_test.go +++ b/pkg/smokescreen/smokescreen_test.go @@ -1073,8 +1073,9 @@ func TestAcceptResponseHandler(t *testing.T) { cfg, err := testConfig("test-local-srv") // set a custom AcceptResponseHandler that will set a header on every reject response - cfg.AcceptResponseHandler = func(resp *http.Response) { + cfg.AcceptResponseHandler = func(_ *smokescreenContext, resp *http.Response) error { resp.Header.Set(testHeader, "This header is added by the AcceptResponseHandler") + return nil } r.NoError(err) diff --git a/vendor/github.com/stripe/goproxy/https.go b/vendor/github.com/stripe/goproxy/https.go index ac193a8c..0eea3660 100644 --- a/vendor/github.com/stripe/goproxy/https.go +++ b/vendor/github.com/stripe/goproxy/https.go @@ -2,6 +2,7 @@ package goproxy import ( "bufio" + "bytes" "context" "crypto/tls" "errors" @@ -134,7 +135,18 @@ func (proxy *ProxyHttpServer) handleHttps(w http.ResponseWriter, r *http.Request } ctx.Logf("Accepting CONNECT to %s", host) - proxyClient.Write([]byte("HTTP/1.0 200 OK\r\n\r\n")) + respBytes, err := createCustomConnectResponse(ctx) + if respBytes != nil { + // Write the custom response, if one was created + proxyClient.Write(respBytes) + } else { + // Otherwise, log any errors and fallback to the default response + if err != nil { + ctx.Warnf("Error writing custom CONNECT response: %s", err.Error()) + return + } + proxyClient.Write([]byte("HTTP/1.0 200 OK\r\n\r\n")) + } if proxy.ConnectCopyHandler != nil { go proxy.ConnectCopyHandler(ctx, proxyClient, targetSiteCon) @@ -334,6 +346,22 @@ func (proxy *ProxyHttpServer) handleHttps(w http.ResponseWriter, r *http.Request } } +func createCustomConnectResponse(ctx *ProxyCtx) ([]byte, error) { + if ctx.proxy.ConnectRespHandler == nil { + return nil, nil + } + resp := &http.Response{Status: "200 OK", StatusCode: 200, Proto: "HTTP/1.0", Header: http.Header{}} + err := ctx.proxy.ConnectRespHandler(ctx, resp) + if err != nil { + return nil, err + } + buf := &bytes.Buffer{} + if err := resp.Write(buf); err != nil { + return nil, err + } + return buf.Bytes(), nil +} + func httpError(w io.WriteCloser, ctx *ProxyCtx, err error) { if ctx.HTTPErrorHandler != nil { ctx.HTTPErrorHandler(w, ctx, err) diff --git a/vendor/github.com/stripe/goproxy/proxy.go b/vendor/github.com/stripe/goproxy/proxy.go index bc679919..d189b48f 100644 --- a/vendor/github.com/stripe/goproxy/proxy.go +++ b/vendor/github.com/stripe/goproxy/proxy.go @@ -50,6 +50,10 @@ type ProxyHttpServer struct { // the hijacked proxy client net.Conn. This is useful for wrapping the connection // to implement timeouts or additional tracing. ConnectClientConnHandler func(net.Conn) net.Conn + + // 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 } var hasPort = regexp.MustCompile(`:\d+$`) diff --git a/vendor/modules.txt b/vendor/modules.txt index fc81528a..44c38d01 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-20220308202309-3f1dfba6d1a4 +# github.com/stripe/goproxy v0.0.0-20230801191332-fabc3ecb7251 ## explicit; go 1.13 github.com/stripe/goproxy # golang.org/x/mod v0.8.0