-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add support for per-request HTTPS traffic Proxying #23
Changes from 4 commits
def7612
30c6c09
ef7d8ce
8682538
22108c1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,11 +33,12 @@ const ( | |
) | ||
|
||
var ( | ||
OkConnect = &ConnectAction{Action: ConnectAccept, TLSConfig: TLSConfigFromCA(&GoproxyCa)} | ||
MitmConnect = &ConnectAction{Action: ConnectMitm, TLSConfig: TLSConfigFromCA(&GoproxyCa)} | ||
HTTPMitmConnect = &ConnectAction{Action: ConnectHTTPMitm, TLSConfig: TLSConfigFromCA(&GoproxyCa)} | ||
RejectConnect = &ConnectAction{Action: ConnectReject, TLSConfig: TLSConfigFromCA(&GoproxyCa)} | ||
httpsRegexp = regexp.MustCompile(`^https:\/\/`) | ||
OkConnect = &ConnectAction{Action: ConnectAccept, TLSConfig: TLSConfigFromCA(&GoproxyCa)} | ||
MitmConnect = &ConnectAction{Action: ConnectMitm, TLSConfig: TLSConfigFromCA(&GoproxyCa)} | ||
HTTPMitmConnect = &ConnectAction{Action: ConnectHTTPMitm, TLSConfig: TLSConfigFromCA(&GoproxyCa)} | ||
RejectConnect = &ConnectAction{Action: ConnectReject, TLSConfig: TLSConfigFromCA(&GoproxyCa)} | ||
httpsRegexp = regexp.MustCompile(`^https:\/\/`) | ||
PerRequestHTTPSProxyHeaderKey = "X-Https-Proxy" | ||
) | ||
|
||
type ConnectAction struct { | ||
|
@@ -84,8 +85,8 @@ func (proxy *ProxyHttpServer) connectDialContext(ctx *ProxyCtx, network, addr st | |
} | ||
|
||
func (proxy *ProxyHttpServer) handleHttps(w http.ResponseWriter, r *http.Request) { | ||
ctx := &ProxyCtx{Req: r, Session: atomic.AddInt64(&proxy.sess, 1), proxy: proxy} | ||
|
||
ctx := &ProxyCtx{Req: r, Session: atomic.AddInt64(&proxy.sess, 1), proxy: proxy} | ||
hij, ok := w.(http.Hijacker) | ||
if !ok { | ||
panic("httpserver does not support hijacking") | ||
|
@@ -118,7 +119,12 @@ func (proxy *ProxyHttpServer) handleHttps(w http.ResponseWriter, r *http.Request | |
host += ":80" | ||
} | ||
|
||
httpsProxy, err := httpsProxyAddr(r.URL, proxy.HttpsProxyAddr) | ||
var httpsProxyURL string = proxy.HttpsProxyAddr | ||
if r.Header.Get(PerRequestHTTPSProxyHeaderKey) != "" { | ||
httpsProxyURL = r.Header.Get(PerRequestHTTPSProxyHeaderKey) | ||
} | ||
|
||
httpsProxy, err := httpsProxyAddr(r.URL, httpsProxyURL) | ||
if err != nil { | ||
ctx.Warnf("Error configuring HTTPS proxy err=%q url=%q", err, r.URL.String()) | ||
} | ||
|
@@ -565,7 +571,7 @@ func (proxy *ProxyHttpServer) connectDialProxyWithContext(ctx *ProxyCtx, proxyHo | |
return c, nil | ||
} | ||
|
||
// httpsProxyAddr function uses the address in httpsProxy parameter. | ||
// 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 | ||
|
@@ -587,6 +593,10 @@ func httpsProxyAddr(reqURL *url.URL, httpsProxy string) (string, error) { | |
reqSchemeURL.Scheme = "https" | ||
|
||
proxyURL, err := cfg.ProxyFunc()(reqSchemeURL) | ||
if err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you mean to add this twice? |
||
return "", err | ||
} | ||
|
||
if err != nil { | ||
return "", err | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -722,7 +722,7 @@ func TestHttpProxyAddrsFromEnv(t *testing.T) { | |
os.Setenv("http_proxy", l.URL) | ||
os.Setenv("https_proxy", l.URL) | ||
proxy2 := goproxy.NewProxyHttpServer() | ||
|
||
client, l2 := oneShotProxy(proxy2, t) | ||
defer l2.Close() | ||
if r := string(getOrFail(https.URL+"/bobo", client, t)); r != "bobo bobo" { | ||
|
@@ -733,6 +733,91 @@ func TestHttpProxyAddrsFromEnv(t *testing.T) { | |
os.Unsetenv("https_proxy") | ||
} | ||
|
||
func TestOverrideHttpsProxyAddrsFromEnvWithRequest(t *testing.T) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a great test! Thanks for getting this working. |
||
// The request essentially does: | ||
// Client -> FakeStripeEgressProxy -> FakeExternalProxy -> FinalDestination | ||
finalDestinationUrl := "https://httpbin.org/get" | ||
|
||
// We'll use this counter to mark whether FakeStripeEgressProxy has been called | ||
c := 0 | ||
|
||
// TODO(pspieker): figure out why this doesn't work - for now, this is fine, | ||
// but we should fix this in the medium term before a wider launch | ||
// | ||
// We should set the env vars here to ensure that our per-request config overrides these | ||
// os.Setenv("http_proxy", "http://incorrectproxy.com") | ||
// os.Setenv("https_proxy", "http://incorrectproxy.com") | ||
|
||
fakeExternalProxy := goproxy.NewProxyHttpServer() | ||
fakeExternalProxyTestStruct := httptest.NewServer(fakeExternalProxy) | ||
fakeExternalProxy.OnRequest().HandleConnect(goproxy.AlwaysMitm) | ||
tagExternalProxyPassthrough := func(resp *http.Response, ctx *goproxy.ProxyCtx) *http.Response { | ||
b, err := ioutil.ReadAll(resp.Body) | ||
panicOnErr(err, "readAll resp") | ||
resp.Body = ioutil.NopCloser(bytes.NewBufferString(string(b) + "-externalproxy")) | ||
return resp | ||
} | ||
fakeExternalProxy.OnResponse().DoFunc(tagExternalProxyPassthrough) | ||
|
||
fakeStripeEgressProxy := goproxy.NewProxyHttpServer() | ||
// We set the CONNECT response handler function to increment our counter such that we can tell | ||
// if our FakeStripeEgressProxy was actually called | ||
fakeStripeEgressProxy.ConnectRespHandler = func(ctx *goproxy.ProxyCtx, resp *http.Response) error { | ||
c += 1 | ||
return nil | ||
} | ||
fakeStripeEgressProxyTestStruct := httptest.NewServer(fakeStripeEgressProxy) | ||
|
||
// Next, we construct the client that we'll be using to talk to our 2 proxies | ||
egressProxyUrl, _ := url.Parse(fakeStripeEgressProxyTestStruct.URL) | ||
tr := &http.Transport{ | ||
TLSClientConfig: acceptAllCerts, | ||
Proxy: http.ProxyURL(egressProxyUrl), | ||
ProxyConnectHeader: map[string][]string{ | ||
"X-Https-Proxy": {fakeExternalProxyTestStruct.URL}, | ||
}, | ||
} | ||
client := &http.Client{Transport: tr} | ||
|
||
// r := string(getOrFail(finalDestinationUrl, client, t)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we delete this? |
||
req, err := http.NewRequest("GET", finalDestinationUrl, nil) | ||
if err != nil { | ||
t.Fatal("Unable to construct request!") | ||
} | ||
|
||
req.Header.Set("X-Test-Header-Key", "Test-Header-Value") | ||
|
||
res, err := client.Do(req) | ||
if err != nil { | ||
t.Fatal("Unable to make the request!") | ||
} | ||
|
||
bodyBytes, err := io.ReadAll(res.Body) | ||
if err != nil { | ||
t.Fatal("Unable to parse the response bytes!") | ||
} | ||
resBody := string(bodyBytes) | ||
|
||
// Making sure we received the response we expected from the final destination | ||
if !strings.Contains(resBody, "\"X-Test-Header-Key\": \"Test-Header-Value\"") { | ||
t.Error("Expected the passed request headers to be present in the response body!") | ||
} | ||
|
||
// Ensuring the external proxy was routed through | ||
if resBody[len(resBody)-14:] != "-externalproxy" { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd probably prefer something less brittle like another |
||
t.Error("Expected the request have been passed through the external proxy on the way to the final destination!") | ||
} | ||
|
||
// Ensuring the "internal" egress proxy was routed through | ||
if c != 1 { | ||
t.Error("Expected the internal egress proxy to have been passed through!") | ||
} | ||
|
||
// TODO(pspieker): see comment above | ||
// os.Unsetenv("http_proxy") | ||
// os.Unsetenv("https_proxy") | ||
} | ||
|
||
func TestCustomHttpProxyAddrs(t *testing.T) { | ||
proxy := goproxy.NewProxyHttpServer() | ||
doubleString := func(resp *http.Response, ctx *goproxy.ProxyCtx) *http.Response { | ||
|
@@ -748,7 +833,7 @@ func TestCustomHttpProxyAddrs(t *testing.T) { | |
defer l.Close() | ||
|
||
proxy2 := goproxy.NewProxyHttpServer(goproxy.WithHttpProxyAddr(l.URL), goproxy.WithHttpsProxyAddr(l.URL)) | ||
|
||
client, l2 := oneShotProxy(proxy2, t) | ||
defer l2.Close() | ||
if r := string(getOrFail(https.URL+"/bobo", client, t)); r != "bobo bobo" { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a small nit, but I think we should be more explicit. Can we change this to
X-Upstream-Https-Proxy
?