IP range AC for data sources: compare the base of the URL only (#83305)

* compare the base of the URL and ignore the path

* change the logic to compare scheme and host explicitly

* fix the test
This commit is contained in:
Ieva 2024-02-23 16:13:21 +00:00 committed by GitHub
parent 65534e62a6
commit 19b1e71fee
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 33 additions and 15 deletions

View File

@ -5,7 +5,7 @@ import (
"crypto/hmac"
"crypto/sha256"
"encoding/hex"
"path"
"net/url"
"github.com/google/uuid"
@ -49,17 +49,22 @@ func (m *HostedGrafanaACHeaderMiddleware) applyGrafanaRequestIDHeader(ctx contex
}
// Check if the request is for a datasource that is allowed to have the header
target := pCtx.DataSourceInstanceSettings.URL
dsURL := pCtx.DataSourceInstanceSettings.URL
dsBaseURL, err := url.Parse(dsURL)
if err != nil {
m.log.Debug("Failed to parse data source URL", "error", err)
return
}
foundMatch := false
for _, allowedURL := range m.cfg.IPRangeACAllowedURLs {
if path.Clean(allowedURL) == path.Clean(target) {
// Only look at the scheme and host, ignore the path
if allowedURL.Host == dsBaseURL.Host && allowedURL.Scheme == dsBaseURL.Scheme {
foundMatch = true
break
}
}
if !foundMatch {
m.log.Debug("Data source URL not among the allow-listed URLs", "url", target)
m.log.Debug("Data source URL not among the allow-listed URLs", "url", dsBaseURL.String())
return
}

View File

@ -6,6 +6,7 @@ import (
"crypto/sha256"
"encoding/hex"
"net/http"
"net/url"
"testing"
"github.com/stretchr/testify/require"
@ -22,7 +23,8 @@ import (
func Test_HostedGrafanaACHeaderMiddleware(t *testing.T) {
t.Run("Should set Grafana request ID headers if the data source URL is in the allow list", func(t *testing.T) {
cfg := setting.NewCfg()
cfg.IPRangeACAllowedURLs = []string{"https://logs.grafana.net"}
allowedURL := &url.URL{Scheme: "https", Host: "logs.grafana.net"}
cfg.IPRangeACAllowedURLs = []*url.URL{allowedURL}
cfg.IPRangeACSecretKey = "secret"
cdt := clienttest.NewClientDecoratorTest(t, clienttest.WithMiddlewares(NewHostedGrafanaACHeaderMiddleware(cfg)))
@ -63,7 +65,8 @@ func Test_HostedGrafanaACHeaderMiddleware(t *testing.T) {
t.Run("Should not set Grafana request ID headers if the data source URL is not in the allow list", func(t *testing.T) {
cfg := setting.NewCfg()
cfg.IPRangeACAllowedURLs = []string{"https://logs.grafana.net"}
allowedURL := &url.URL{Scheme: "https", Host: "logs.grafana.net"}
cfg.IPRangeACAllowedURLs = []*url.URL{allowedURL}
cfg.IPRangeACSecretKey = "secret"
cdt := clienttest.NewClientDecoratorTest(t, clienttest.WithMiddlewares(NewHostedGrafanaACHeaderMiddleware(cfg)))
@ -85,9 +88,10 @@ func Test_HostedGrafanaACHeaderMiddleware(t *testing.T) {
require.Len(t, cdt.CallResourceReq.Headers[GrafanaSignedRequestID], 0)
})
t.Run("Should set Grafana request ID headers if a sanitized data source URL is in the allow list", func(t *testing.T) {
t.Run("Should set Grafana request ID headers if URL scheme and host match a URL from the allow list", func(t *testing.T) {
cfg := setting.NewCfg()
cfg.IPRangeACAllowedURLs = []string{"https://logs.GRAFANA.net/"}
allowedURL := &url.URL{Scheme: "https", Host: "logs.grafana.net"}
cfg.IPRangeACAllowedURLs = []*url.URL{allowedURL}
cfg.IPRangeACSecretKey = "secret"
cdt := clienttest.NewClientDecoratorTest(t, clienttest.WithMiddlewares(NewHostedGrafanaACHeaderMiddleware(cfg)))
@ -99,19 +103,20 @@ func Test_HostedGrafanaACHeaderMiddleware(t *testing.T) {
err := cdt.Decorator.CallResource(ctx, &backend.CallResourceRequest{
PluginContext: backend.PluginContext{
DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{
URL: "https://logs.grafana.net/abc/../",
URL: "https://logs.grafana.net/abc/../some/path",
},
},
}, nopCallResourceSender)
require.NoError(t, err)
require.Len(t, cdt.CallResourceReq.Headers[GrafanaRequestID], 0)
require.Len(t, cdt.CallResourceReq.Headers[GrafanaSignedRequestID], 0)
require.Len(t, cdt.CallResourceReq.Headers[GrafanaRequestID], 1)
require.Len(t, cdt.CallResourceReq.Headers[GrafanaSignedRequestID], 1)
})
t.Run("Should set Grafana internal request header if the request is internal (doesn't have X-Real-IP header set)", func(t *testing.T) {
cfg := setting.NewCfg()
cfg.IPRangeACAllowedURLs = []string{"https://logs.grafana.net"}
allowedURL := &url.URL{Scheme: "https", Host: "logs.grafana.net"}
cfg.IPRangeACAllowedURLs = []*url.URL{allowedURL}
cfg.IPRangeACSecretKey = "secret"
cdt := clienttest.NewClientDecoratorTest(t, clienttest.WithMiddlewares(NewHostedGrafanaACHeaderMiddleware(cfg)))

View File

@ -339,7 +339,7 @@ type Cfg struct {
// IP range access control
IPRangeACEnabled bool
IPRangeACAllowedURLs []string
IPRangeACAllowedURLs []*url.URL
IPRangeACSecretKey string
// SQL Data sources
@ -1949,7 +1949,15 @@ func (cfg *Cfg) readDataSourceSecuritySettings() {
cfg.IPRangeACEnabled = datasources.Key("enabled").MustBool(false)
cfg.IPRangeACSecretKey = datasources.Key("secret_key").MustString("")
allowedURLString := datasources.Key("allow_list").MustString("")
cfg.IPRangeACAllowedURLs = util.SplitString(allowedURLString)
for _, urlString := range util.SplitString(allowedURLString) {
allowedURL, err := url.Parse(urlString)
if err != nil {
cfg.Logger.Error("Error parsing allowed URL for IP range access control", "error", err)
continue
} else {
cfg.IPRangeACAllowedURLs = append(cfg.IPRangeACAllowedURLs, allowedURL)
}
}
}
func (cfg *Cfg) readSqlDataSourceSettings() {