From 88a0d26f9ee77ce44b4fd7362a14e8ca6d0c5d8d Mon Sep 17 00:00:00 2001 From: Tom Petr Date: Fri, 21 Jul 2023 10:26:25 -0400 Subject: [PATCH 1/3] log allowlist rejects --- pkg/config.go | 3 ++- pkg/inbound_proxy.go | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/config.go b/pkg/config.go index e227fdf..87860b1 100644 --- a/pkg/config.go +++ b/pkg/config.go @@ -180,7 +180,8 @@ type AllowlistItem struct { type Allowlist []AllowlistItem type LoggingConfig struct { - SkipPaths []string `mapstructure:"skipPaths"` + SkipPaths []string `mapstructure:"skipPaths"` + LogResponses bool `mapstructure:"logResponses" json:"logResponses"` } type HeartbeatConfig struct { diff --git a/pkg/inbound_proxy.go b/pkg/inbound_proxy.go index 0ebbf53..8ca327e 100644 --- a/pkg/inbound_proxy.go +++ b/pkg/inbound_proxy.go @@ -54,6 +54,7 @@ func (config *InboundProxyConfig) Start(tnet *netstack.Net) error { if !exists { c.Header(errorResponseHeader, "1") c.JSON(http.StatusForbidden, gin.H{"error": "url is not in allowlist"}) + log.Warnf("url is not in allowlist: %s %s", c.Request.Method, destinationUrl) return } From a65bd119ef72215074045ec746f007e52e217483 Mon Sep 17 00:00:00 2001 From: Tom Petr Date: Mon, 24 Jul 2023 11:56:15 -0400 Subject: [PATCH 2/3] check for allowlist match using unescaped path --- it/e2e_test.go | 8 ++++++++ pkg/allowlist.go | 2 +- pkg/allowlist_test.go | 7 +++++++ pkg/inbound_proxy.go | 8 +++++++- 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/it/e2e_test.go b/it/e2e_test.go index 91fd88f..0656f81 100644 --- a/it/e2e_test.go +++ b/it/e2e_test.go @@ -148,6 +148,10 @@ func TestWireguardInboundProxy(t *testing.T) { URL: internalServer.URL + "/allowed-post", Methods: pkg.ParseHttpMethods([]string{"POST"}), }, + { + URL: internalServer.URL + "/allowed-path/:path", + Methods: pkg.ParseHttpMethods([]string{"POST"}), + }, }, Heartbeat: pkg.HeartbeatConfig{ URL: fmt.Sprintf("http://[%v]/ping", gatewayWireguardAddress), @@ -177,6 +181,10 @@ func TestWireguardInboundProxy(t *testing.T) { // it should proxy requests that match the allowlist remoteHttpClient.AssertStatusCode(t, "GET", fmt.Sprintf("http://[%v]/proxy/%v/allowed-get", clientWireguardAddress, internalServer.URL), 200) remoteHttpClient.AssertStatusCode(t, "POST", fmt.Sprintf("http://[%v]/proxy/%v/allowed-post", clientWireguardAddress, internalServer.URL), 200) + remoteHttpClient.AssertStatusCode(t, "POST", fmt.Sprintf("http://[%v]/proxy/%v/allowed-path/foobar", clientWireguardAddress, internalServer.URL), 200) + + // it shouldnt decode urlencoded characters + remoteHttpClient.AssertStatusCode(t, "POST", fmt.Sprintf("http://[%v]/proxy/%v/allowed-path/%s", clientWireguardAddress, internalServer.URL, "foobar%2Fbla"), 200) // it should reject requests that don't match the allowlist remoteHttpClient.AssertStatusCode(t, "POST", fmt.Sprintf("http://[%v]/proxy/%v/allowed-get", clientWireguardAddress, internalServer.URL), 403) diff --git a/pkg/allowlist.go b/pkg/allowlist.go index 3601892..7705bad 100644 --- a/pkg/allowlist.go +++ b/pkg/allowlist.go @@ -19,7 +19,7 @@ func (config AllowlistItem) Matches(method string, url *url.URL) bool { } matcher := urlpath.New(parsedUrl.Path) - if _, matches := matcher.Match(url.Path); matches { + if _, matches := matcher.Match(url.EscapedPath()); matches { return true } diff --git a/pkg/allowlist_test.go b/pkg/allowlist_test.go index e527ace..fec47dd 100644 --- a/pkg/allowlist_test.go +++ b/pkg/allowlist_test.go @@ -99,6 +99,10 @@ func TestAllowlistPathMatch(t *testing.T) { URL: "https://foo.com/variable-path/:variable", Methods: ParseHttpMethods([]string{"GET"}), }, + AllowlistItem{ + URL: "https://foo.com/variable-path/:variable/suffix", + Methods: ParseHttpMethods([]string{"GET"}), + }, } // test path matching @@ -108,4 +112,7 @@ func TestAllowlistPathMatch(t *testing.T) { assertAllowlistMatch(t, allowlist, "GET", "https://foo.com/variable-path/a/b", false) assertAllowlistMatch(t, allowlist, "GET", "https://foo.com/hardcoded-path", true) assertAllowlistMatch(t, allowlist, "GET", "https://foo.com/hardcoded-path/bla", false) + + assertAllowlistMatch(t, allowlist, "GET", "https://foo.com/variable-path/bla%2Fbla/suffix", true) + assertAllowlistMatch(t, allowlist, "GET", "https://foo.com/variable-path/bla/bla/suffix", false) } diff --git a/pkg/inbound_proxy.go b/pkg/inbound_proxy.go index 8ca327e..8a2e723 100644 --- a/pkg/inbound_proxy.go +++ b/pkg/inbound_proxy.go @@ -29,7 +29,11 @@ func (config *InboundProxyConfig) Start(tnet *netstack.Net) error { // setup http server gin.SetMode(gin.ReleaseMode) r := gin.New() - r.UseRawPath = true // we want this proxy to be transparent, so don't un-escape characters in the URL + + // we want this proxy to be transparent, so don't un-escape characters in the URL + r.UseRawPath = true + r.UnescapePathValues = false + r.Use(gin.LoggerWithConfig(gin.LoggerConfig{ SkipPaths: config.Logging.SkipPaths, }), gin.Recovery()) @@ -59,6 +63,8 @@ func (config *InboundProxyConfig) Start(tnet *netstack.Net) error { } log.Infof("Proxying request: %s %s", c.Request.Method, destinationUrl) + log.Infof("Matched allowlist entry: %v", allowlistMatch) + proxy := httputil.ReverseProxy{ Director: func(req *http.Request) { req.URL = destinationUrl From 5a4e84a8681eebd06f58df5dc5483303982682e6 Mon Sep 17 00:00:00 2001 From: Tom Petr Date: Mon, 24 Jul 2023 11:57:21 -0400 Subject: [PATCH 3/3] remove unused field --- pkg/config.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/config.go b/pkg/config.go index 87860b1..e227fdf 100644 --- a/pkg/config.go +++ b/pkg/config.go @@ -180,8 +180,7 @@ type AllowlistItem struct { type Allowlist []AllowlistItem type LoggingConfig struct { - SkipPaths []string `mapstructure:"skipPaths"` - LogResponses bool `mapstructure:"logResponses" json:"logResponses"` + SkipPaths []string `mapstructure:"skipPaths"` } type HeartbeatConfig struct {