Skip to content

Commit

Permalink
Fix hostname parsing for square brackets
Browse files Browse the repository at this point in the history
  • Loading branch information
jjiang-stripe committed Apr 28, 2022
1 parent e589a2f commit dea7b3c
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 18 deletions.
4 changes: 2 additions & 2 deletions pkg/smokescreen/smokescreen.go
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ func BuildProxy(config *Config) *goproxy.ProxyHttpServer {
pctx.RoundTripper = rtFn

// Build an address parsable by net.ResolveTCPAddr
remoteHost := req.Host
remoteHost := req.URL.Hostname()
if strings.LastIndex(remoteHost, ":") <= strings.LastIndex(remoteHost, "]") {
switch req.URL.Scheme {
case "http":
Expand Down Expand Up @@ -600,7 +600,7 @@ func handleConnect(config *Config, pctx *goproxy.ProxyCtx) error {
sctx := pctx.UserData.(*smokescreenContext)

// Check if requesting role is allowed to talk to remote
sctx.decision, sctx.lookupTime, pctx.Error = checkIfRequestShouldBeProxied(config, pctx.Req, pctx.Req.Host)
sctx.decision, sctx.lookupTime, pctx.Error = checkIfRequestShouldBeProxied(config, pctx.Req, pctx.Req.URL.Hostname())
if pctx.Error != nil {
return pctx.Error
}
Expand Down
51 changes: 48 additions & 3 deletions pkg/smokescreen/smokescreen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,11 @@ func TestClassifyAddr(t *testing.T) {
}
}

func TestUnsafeAllowPrivateRanges (t *testing.T) {
func TestUnsafeAllowPrivateRanges(t *testing.T) {
a := assert.New(t)

conf := NewConfig()
a.NoError(conf.SetDenyRanges([]string {"192.168.0.0/24", "10.0.0.0/8"}))
a.NoError(conf.SetDenyRanges([]string{"192.168.0.0/24", "10.0.0.0/8"}))
conf.ConnectTimeout = 10 * time.Second
conf.ExitTimeout = 10 * time.Second
conf.AdditionalErrorMessageOnDeny = "Proxy denied"
Expand Down Expand Up @@ -160,7 +160,6 @@ func TestUnsafeAllowPrivateRanges (t *testing.T) {
}
}


}

// TestClearsErrors tests that we are correctly preserving/removing the X-Smokescreen-Error header.
Expand Down Expand Up @@ -443,6 +442,52 @@ func TestInvalidHost(t *testing.T) {
}
}

var hostSquareBracketsCases = []struct {
scheme string
proxyType string
}{
{"http", "http"},
{"https", "connect"},
}

func TestHostSquareBrackets(t *testing.T) {
for _, testCase := range hostSquareBracketsCases {
t.Run(testCase.scheme, func(t *testing.T) {
a := assert.New(t)
r := require.New(t)

cfg, err := testConfig("test-open-srv")
require.NoError(t, err)
logHook := proxyLogHook(cfg)

proxySrv := proxyServer(cfg)
defer proxySrv.Close()

// Create a http.Client that uses our proxy
client, err := proxyClient(proxySrv.URL)
r.NoError(err)

resp, err := client.Get(fmt.Sprintf("%s://[stripe.com]", testCase.scheme))
if err != nil {
r.Contains(err.Error(), "Request rejected by proxy")
} else {
r.Equal(http.StatusProxyAuthRequired, resp.StatusCode)
}

entry := findCanonicalProxyDecision(logHook.AllEntries())
r.NotNil(entry)

if a.Contains(entry.Data, "allow") {
a.Equal(false, entry.Data["allow"])
a.Equal("host matched rule in global deny list", entry.Data["decision_reason"])
}
if a.Contains(entry.Data, "proxy_type") {
a.Contains(entry.Data["proxy_type"], testCase.proxyType)
}
})
}
}

func TestErrorHeader(t *testing.T) {
a := assert.New(t)
r := require.New(t)
Expand Down
32 changes: 19 additions & 13 deletions pkg/smokescreen/testdata/acl.yaml
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
---
version: v1
services:
- name: test-trusted-srv
project: security
action: enforce
allowed_domains:
- notarealhost.test
- httpbin.org
- name: test-local-srv
project: security
action: open
allowed_domains:
- 127.0.0.1
version: v1
services:
- name: test-trusted-srv
project: security
action: enforce
allowed_domains:
- notarealhost.test
- httpbin.org
- name: test-local-srv
project: security
action: open
allowed_domains:
- 127.0.0.1
- name: test-open-srv
project: security
action: open

global_deny_list:
- stripe.com

0 comments on commit dea7b3c

Please sign in to comment.