Skip to content

Commit

Permalink
feat: frontend middleware to block matching urls (#3963)
Browse files Browse the repository at this point in the history
* add new frontend config to blacklist URLs

* changelog

* rename as deny list and panic on wrong regex

* match the complete url

* better error message
  • Loading branch information
javiermolinar authored Aug 16, 2024
1 parent 7dffd0e commit 3e20eb2
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 1 deletion.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions docs/sources/tempo/configuration/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,9 @@ query_frontend:
# (default: 0)
[api_timeout: <duration>]

# A list of regular expressions for refusing matching requests, these will apply for every request regardless of the endpoint.
[url_deny_list: <list of strings> | default = <empty list>]]

search:

# The number of concurrent jobs to execute when searching the backend.
Expand Down
3 changes: 3 additions & 0 deletions modules/frontend/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 8 additions & 0 deletions modules/frontend/frontend.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
},
Expand All @@ -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),
},
Expand All @@ -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),
},
Expand All @@ -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),
},
Expand All @@ -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},
Expand All @@ -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),
},
Expand Down
52 changes: 52 additions & 0 deletions modules/frontend/pipeline/deny_list_middleware.go
Original file line number Diff line number Diff line change
@@ -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
}
55 changes: 55 additions & 0 deletions modules/frontend/pipeline/deny_list_middleware_test.go
Original file line number Diff line number Diff line change
@@ -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&param2=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
}

0 comments on commit 3e20eb2

Please sign in to comment.