From 3e20eb2352ed1f67bcd58d39f46125365af7908c Mon Sep 17 00:00:00 2001 From: Javier Molina Reyes Date: Fri, 16 Aug 2024 15:48:35 +0200 Subject: [PATCH] feat: frontend middleware to block matching urls (#3963) * add new frontend config to blacklist URLs * changelog * rename as deny list and panic on wrong regex * match the complete url * better error message --- CHANGELOG.md | 2 +- docs/sources/tempo/configuration/_index.md | 3 + modules/frontend/config.go | 3 + modules/frontend/frontend.go | 8 +++ .../frontend/pipeline/deny_list_middleware.go | 52 ++++++++++++++++++ .../pipeline/deny_list_middleware_test.go | 55 +++++++++++++++++++ 6 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 modules/frontend/pipeline/deny_list_middleware.go create mode 100644 modules/frontend/pipeline/deny_list_middleware_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 70cf7b27312..b1b5270238f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,8 +50,8 @@ * [ENHANCEMENT] Update to the latest dskit [#3915](https://github.com/grafana/tempo/pull/3915) (@andreasgerstmayr) * [ENHANCEMENT] Reduce allocs building queriers sharded requests [#3932](https://github.com/grafana/tempo/pull/3932) (@javiermolinar) * [ENHANCEMENT] Allow compaction disablement per-tenant [#3965](https://github.com/grafana/tempo/pull/3965) (@zalegrala) - * [ENHANCEMENT] Implement polling tenants concurrently [#3647](https://github.com/grafana/tempo/pull/3647) (@zalegrala) +* [ENHANCEMENT] Added new middleware to block matching urls [#3963](https://github.com/grafana/tempo/pull/3963) (@javiermolinar) * [BUGFIX] Fix panic in certain metrics queries using `rate()` with `by` [#3847](https://github.com/grafana/tempo/pull/3847) (@stoewer) * [BUGFIX] Fix double appending the primary iterator on second pass with event iterator [#3903](https://github.com/grafana/tempo/pull/3903) (@ie-pham) * [BUGFIX] Fix metrics queries when grouping by attributes that may not exist [#3734](https://github.com/grafana/tempo/pull/3734) (@mdisibio) diff --git a/docs/sources/tempo/configuration/_index.md b/docs/sources/tempo/configuration/_index.md index e2b9d08f2d0..4f5342a6d47 100644 --- a/docs/sources/tempo/configuration/_index.md +++ b/docs/sources/tempo/configuration/_index.md @@ -561,6 +561,9 @@ query_frontend: # (default: 0) [api_timeout: ] + # A list of regular expressions for refusing matching requests, these will apply for every request regardless of the endpoint. + [url_deny_list: | default = ]] + search: # The number of concurrent jobs to execute when searching the backend. diff --git a/modules/frontend/config.go b/modules/frontend/config.go index a6326ff29d0..84e96a33b0d 100644 --- a/modules/frontend/config.go +++ b/modules/frontend/config.go @@ -29,6 +29,9 @@ type Config struct { // traceql, tag search, tag value search, trace by id and all streaming gRPC endpoints. // 0 disables APITimeout time.Duration `yaml:"api_timeout,omitempty"` + + // A list of regexes for black listing requests, these will apply for every request regardless the endpoint + URLDenyList []string `yaml:"url_deny_list,omitempty"` } type SearchConfig struct { diff --git a/modules/frontend/frontend.go b/modules/frontend/frontend.go index cb2e4ddd427..0c67e135f97 100644 --- a/modules/frontend/frontend.go +++ b/modules/frontend/frontend.go @@ -88,12 +88,15 @@ func New(cfg Config, next http.RoundTripper, o overrides.Interface, reader tempo } retryWare := pipeline.NewRetryWare(cfg.MaxRetries, registerer) + cacheWare := pipeline.NewCachingWare(cacheProvider, cache.RoleFrontendSearch, logger) statusCodeWare := pipeline.NewStatusCodeAdjustWare() traceIDStatusCodeWare := pipeline.NewStatusCodeAdjustWareWithAllowedCode(http.StatusNotFound) + urlDenyListWare := pipeline.NewURLDenyListWare(cfg.URLDenyList) tracePipeline := pipeline.Build( []pipeline.AsyncMiddleware[combiner.PipelineResponse]{ + urlDenyListWare, multiTenantMiddleware(cfg, logger), newAsyncTraceIDSharder(&cfg.TraceByID, logger), }, @@ -102,6 +105,7 @@ func New(cfg Config, next http.RoundTripper, o overrides.Interface, reader tempo searchPipeline := pipeline.Build( []pipeline.AsyncMiddleware[combiner.PipelineResponse]{ + urlDenyListWare, multiTenantMiddleware(cfg, logger), newAsyncSearchSharder(reader, o, cfg.Search.Sharder, logger), }, @@ -110,6 +114,7 @@ func New(cfg Config, next http.RoundTripper, o overrides.Interface, reader tempo searchTagsPipeline := pipeline.Build( []pipeline.AsyncMiddleware[combiner.PipelineResponse]{ + urlDenyListWare, multiTenantMiddleware(cfg, logger), newAsyncTagSharder(reader, o, cfg.Search.Sharder, parseTagsRequest, logger), }, @@ -118,6 +123,7 @@ func New(cfg Config, next http.RoundTripper, o overrides.Interface, reader tempo searchTagValuesPipeline := pipeline.Build( []pipeline.AsyncMiddleware[combiner.PipelineResponse]{ + urlDenyListWare, multiTenantMiddleware(cfg, logger), newAsyncTagSharder(reader, o, cfg.Search.Sharder, parseTagValuesRequest, logger), }, @@ -127,6 +133,7 @@ func New(cfg Config, next http.RoundTripper, o overrides.Interface, reader tempo // metrics summary metricsPipeline := pipeline.Build( []pipeline.AsyncMiddleware[combiner.PipelineResponse]{ + urlDenyListWare, multiTenantUnsupportedMiddleware(cfg, logger), }, []pipeline.Middleware{statusCodeWare, retryWare}, @@ -135,6 +142,7 @@ func New(cfg Config, next http.RoundTripper, o overrides.Interface, reader tempo // traceql metrics queryRangePipeline := pipeline.Build( []pipeline.AsyncMiddleware[combiner.PipelineResponse]{ + urlDenyListWare, multiTenantMiddleware(cfg, logger), newAsyncQueryRangeSharder(reader, o, cfg.Metrics.Sharder, logger), }, diff --git a/modules/frontend/pipeline/deny_list_middleware.go b/modules/frontend/pipeline/deny_list_middleware.go new file mode 100644 index 00000000000..cd475fe5df3 --- /dev/null +++ b/modules/frontend/pipeline/deny_list_middleware.go @@ -0,0 +1,52 @@ +package pipeline + +import ( + "fmt" + "regexp" + + "github.com/grafana/tempo/modules/frontend/combiner" +) + +type urlDenylistWare struct { + denyList []*regexp.Regexp + next AsyncRoundTripper[combiner.PipelineResponse] +} + +func NewURLDenyListWare(denyList []string) AsyncMiddleware[combiner.PipelineResponse] { + compiledDenylist := make([]*regexp.Regexp, 0) + for _, v := range denyList { + r, err := regexp.Compile(v) + if err == nil { + compiledDenylist = append(compiledDenylist, r) + } else { + panic(fmt.Sprintf("error compiling query frontend deny list regex: %s", err)) + } + } + + return AsyncMiddlewareFunc[combiner.PipelineResponse](func(next AsyncRoundTripper[combiner.PipelineResponse]) AsyncRoundTripper[combiner.PipelineResponse] { + return &urlDenylistWare{ + next: next, + denyList: compiledDenylist, + } + }) +} + +func (c urlDenylistWare) RoundTrip(req Request) (Responses[combiner.PipelineResponse], error) { + if len(c.denyList) != 0 { + err := c.validateRequest(req.HTTPRequest().URL.String()) + if err != nil { + return NewBadRequest(err), nil + } + } + + return c.next.RoundTrip(req) +} + +func (c urlDenylistWare) validateRequest(url string) error { + for _, v := range c.denyList { + if v.MatchString(url) { + return fmt.Errorf("Invalid request %s. This query has been identified as one that destabilizes our system. Contact your system administrator for more information", url) + } + } + return nil +} diff --git a/modules/frontend/pipeline/deny_list_middleware_test.go b/modules/frontend/pipeline/deny_list_middleware_test.go new file mode 100644 index 00000000000..9445aafdf18 --- /dev/null +++ b/modules/frontend/pipeline/deny_list_middleware_test.go @@ -0,0 +1,55 @@ +package pipeline + +import ( + "bytes" + "context" + "io" + "net/http" + "testing" + + "github.com/grafana/tempo/modules/frontend/combiner" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +var next = AsyncRoundTripperFunc[combiner.PipelineResponse](func(_ Request) (Responses[combiner.PipelineResponse], error) { + return NewHTTPToAsyncResponse(&http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader([]byte{})), + }), nil +}) + +func TestURLBlackListMiddlewareForEmptyBlackList(t *testing.T) { + regexes := []string{} + roundTrip := NewURLDenyListWare(regexes).Wrap(next) + statusCode := DoRequest(t, "http://localhost:8080/api/v2/traces/123345", roundTrip) + assert.Equal(t, 200, statusCode) +} + +func TestURLBlackListMiddlewarePanicsOnSyntacticallyIncorrectRegex(t *testing.T) { + regexes := []string{"qr/^(.*\\.traces\\/[a-f0-9]{32}$/"} + assert.Panics(t, func() { + NewURLDenyListWare(regexes).Wrap(next) + }) +} + +func TestURLBlackListMiddleware(t *testing.T) { + regexes := []string{ + "^.*v2.*", + } + roundTrip := NewURLDenyListWare(regexes).Wrap(next) + statusCode := DoRequest(t, "http://localhost:9000?param1=a¶m2=b", roundTrip) + assert.Equal(t, 200, statusCode) + + // Blacklisted url + statusCode = DoRequest(t, "http://localhost:8080/api/v2/traces/123345", roundTrip) + assert.Equal(t, 400, statusCode) +} + +func DoRequest(t *testing.T, url string, rt AsyncRoundTripper[combiner.PipelineResponse]) int { + req, _ := http.NewRequest(http.MethodGet, url, nil) + resp, _ := rt.RoundTrip(NewHTTPRequest(req)) + httpResponse, _, err := resp.Next(context.Background()) + require.NoError(t, err) + return httpResponse.HTTPResponse().StatusCode +}