Chore: Remove sensitive information from presigned URLs prior to logging (#87035)

Co-authored-by: Will Browne <wbrowne@users.noreply.github.com>
Co-authored-by: Dan Cech <dcech@grafana.com>
Co-authored-by: Andres Martinez Gotor <andres.martinez@grafana.com>
This commit is contained in:
Marcus Andersson 2024-06-24 14:53:42 +02:00 committed by GitHub
parent 96fda0d6ea
commit 04f39457cf
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 138 additions and 64 deletions

View File

@ -480,6 +480,7 @@ github.com/alecthomas/kingpin/v2 v2.4.0 h1:f48lwail6p8zpO1bC4TxtqACaGqHYA22qkHjH
github.com/alecthomas/kong v0.2.11 h1:RKeJXXWfg9N47RYfMm0+igkxBCTF4bzbneAxaqid0c4=
github.com/alecthomas/kong v0.2.11/go.mod h1:kQOmtJgV+Lb4aj+I2LEn40cbtawdWJ9Y8QLq+lElKxE=
github.com/alecthomas/participle/v2 v2.1.0 h1:z7dElHRrOEEq45F2TG5cbQihMtNTv8vwldytDj7Wrz4=
github.com/alecthomas/participle/v2 v2.1.0/go.mod h1:Y1+hAs8DHPmc3YUFzqllV+eSQ9ljPTk0ZkPMtEdAx2c=
github.com/alecthomas/repr v0.2.0 h1:HAzS41CIzNW5syS8Mf9UwXhNH1J9aix/BvDRf1Ml2Yk=
github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751 h1:JYp7IbQjafoB+tBA3gMyHYHrpOtNuDiK/uB5uXxq5wM=
github.com/alexflint/go-arg v1.4.2/go.mod h1:9iRbDxne7LcR/GSvEr7ma++GLpdIU1zrghf2y2768kM=
@ -886,6 +887,7 @@ github.com/gobwas/pool v0.2.1 h1:xfeeEhW7pwmX8nuLVlqbzVc7udMDrwetjEv+TZIz1og=
github.com/gobwas/ws v1.2.1 h1:F2aeBZrm2NDsc7vbovKrWSogd4wvfAxg0FQ89/iqOTk=
github.com/gobwas/ws v1.3.0/go.mod h1:hRKAFb8wOxFROYNsT1bqfWnhX+b5MFeJM9r2ZSwg/KY=
github.com/goccy/go-yaml v1.11.0 h1:n7Z+zx8S9f9KgzG6KtQKf+kwqXZlLNR2F6018Dgau54=
github.com/goccy/go-yaml v1.11.0/go.mod h1:H+mJrWtjPTJAHvRbV09MCK9xYwODM+wRTVFFTWckfng=
github.com/gocql/gocql v0.0.0-20190301043612-f6df8288f9b4 h1:vF83LI8tAakwEwvWZtrIEx7pOySacl2TOxx6eXk4ePo=
github.com/godbus/dbus v0.0.0-20151105175453-c7fdd8b5cd55/go.mod h1:/YcGZj5zSblfDWMMoOzV4fas9FZnQYTkDnsGvmh2Grw=
github.com/godbus/dbus v0.0.0-20180201030542-885f9cc04c9c/go.mod h1:/YcGZj5zSblfDWMMoOzV4fas9FZnQYTkDnsGvmh2Grw=

View File

@ -373,13 +373,18 @@ func (proxy *DataSourceProxy) logRequest() {
panelPluginId := proxy.ctx.Req.Header.Get("X-Panel-Plugin-Id")
uri, err := util.SanitizeURI(proxy.ctx.Req.RequestURI)
if err == nil {
proxy.ctx.Logger.Error("Could not sanitize RequestURI", "error", err)
}
ctxLogger := logger.FromContext(proxy.ctx.Req.Context())
ctxLogger.Info("Proxying incoming request",
"userid", proxy.ctx.UserID,
"orgid", proxy.ctx.OrgID,
"username", proxy.ctx.Login,
"datasource", proxy.ds.Type,
"uri", proxy.ctx.Req.RequestURI,
"uri", uri,
"method", proxy.ctx.Req.Method,
"panelPluginId", panelPluginId,
"body", body)

View File

@ -19,11 +19,11 @@ import (
"errors"
"fmt"
"net/http"
"net/url"
"time"
"github.com/grafana/grafana/pkg/middleware"
"github.com/grafana/grafana/pkg/middleware/requestmeta"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/apimachinery/errutil"
@ -113,7 +113,7 @@ func (l *loggerImpl) prepareLogParams(c *contextmodel.ReqContext, duration time.
"size", rw.Size(),
}
referer, err := SanitizeURL(r.Referer())
referer, err := util.SanitizeURI(r.Referer())
// We add an empty referer when there's a parsing error, hence this is before the err check.
logParams = append(logParams, "referer", referer)
if err != nil {
@ -153,27 +153,3 @@ func errorLogParams(err error) []any {
"error", gfErr.LogMessage,
}
}
var sensitiveQueryStrings = [...]string{
"auth_token",
}
func SanitizeURL(s string) (string, error) {
if s == "" {
return s, nil
}
u, err := url.ParseRequestURI(s)
if err != nil {
return "", fmt.Errorf("failed to sanitize URL")
}
// strip out sensitive query strings
values := u.Query()
for _, query := range sensitiveQueryStrings {
values.Del(query)
}
u.RawQuery = values.Encode()
return u.String(), nil
}

View File

@ -16,43 +16,6 @@ import (
"github.com/grafana/grafana/pkg/web"
)
func Test_sanitizeURL(t *testing.T) {
tests := []struct {
name string
input string
want string
expectError bool
}{
{
name: "Receiving empty string should return it",
input: "",
want: "",
},
{
name: "Receiving valid URL string should return it parsed",
input: "https://grafana.com/",
want: "https://grafana.com/",
},
{
name: "Receiving invalid URL string should return empty string",
input: "this is not a valid URL",
want: "",
expectError: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
url, err := SanitizeURL(tt.input)
if tt.expectError {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}
assert.Equalf(t, tt.want, url, "SanitizeURL(%v)", tt.input)
})
}
}
func Test_prepareLog(t *testing.T) {
type opts struct {
Features []any

54
pkg/util/uri_sanitize.go Normal file
View File

@ -0,0 +1,54 @@
package util
import (
"fmt"
"net/url"
"strings"
)
const masking = "hidden"
var sensitiveQueryChecks = map[string]func(key string, urlValues url.Values) bool{
"auth_token": func(key string, urlValues url.Values) bool {
return true
},
"x-amz-signature": func(key string, urlValues url.Values) bool {
return true
},
"x-goog-signature": func(key string, urlValues url.Values) bool {
return true
},
"sig": func(key string, urlValues url.Values) bool {
for k := range urlValues {
if strings.ToLower(k) == "sv" {
return true
}
}
return false
},
}
func SanitizeURI(s string) (string, error) {
if s == "" {
return s, nil
}
u, err := url.ParseRequestURI(s)
if err != nil {
return "", fmt.Errorf("failed to sanitize URL")
}
// strip out sensitive query strings
urlValues := u.Query()
for key := range urlValues {
lk := strings.ToLower(key)
if checker, ok := sensitiveQueryChecks[lk]; ok {
if checker(key, urlValues) {
urlValues.Set(key, masking)
}
}
}
u.RawQuery = urlValues.Encode()
return u.String(), nil
}

View File

@ -0,0 +1,74 @@
package util
import (
"testing"
"github.com/stretchr/testify/assert"
)
func Test_sanitizeURI(t *testing.T) {
tests := []struct {
name string
input string
want string
expectError bool
}{
{
name: "Receiving empty string should return it",
input: "",
want: "",
},
{
name: "Receiving URL with auth_token should remove it",
input: "https://grafana.com/?auth_token=secret-token&q=1234",
want: "https://grafana.com/?auth_token=hidden&q=1234",
},
{
name: "Receiving presigned URL from AWS should remove signature",
input: "https://s3.amazonaws.com/finance-department-bucket/2022/tax-certificate.pdf?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIA3SGQVQG7FGA6KKA6%2F20221104%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20221104T140227Z&X-Amz-Expires=3600&X-Amz-Signature=b22&X-Amz-SignedHeaders=host",
want: "https://s3.amazonaws.com/finance-department-bucket/2022/tax-certificate.pdf?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIA3SGQVQG7FGA6KKA6%2F20221104%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20221104T140227Z&X-Amz-Expires=3600&X-Amz-Signature=hidden&X-Amz-SignedHeaders=host",
},
{
name: "Receiving presigned URL from GCP should remove signature",
input: "https://storage.googleapis.com/example-bucket/cat.jpeg?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=example%40example-project.iam.gserviceaccount.com%2F20181026%2Fus-central1%2Fstorage%2Fgoog4_request&X-Goog-Date=20181026T181309Z&X-Goog-Expires=900&X-Goog-Signature=247a&X-Goog-SignedHeaders=host",
want: "https://storage.googleapis.com/example-bucket/cat.jpeg?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=example%40example-project.iam.gserviceaccount.com%2F20181026%2Fus-central1%2Fstorage%2Fgoog4_request&X-Goog-Date=20181026T181309Z&X-Goog-Expires=900&X-Goog-Signature=hidden&X-Goog-SignedHeaders=host",
},
{
name: "Receiving presigned URL with lower case query params from GCP should remove signature",
input: "https://storage.googleapis.com/example-bucket/cat.jpeg?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=example%40example-project.iam.gserviceaccount.com%2F20181026%2Fus-central1%2Fstorage%2Fgoog4_request&X-Goog-Date=20181026T181309Z&X-Goog-Expires=900&x-goog-signature=247a&X-Goog-SignedHeaders=host",
want: "https://storage.googleapis.com/example-bucket/cat.jpeg?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=example%40example-project.iam.gserviceaccount.com%2F20181026%2Fus-central1%2Fstorage%2Fgoog4_request&X-Goog-Date=20181026T181309Z&X-Goog-Expires=900&X-Goog-SignedHeaders=host&x-goog-signature=hidden",
},
{
name: "Receiving presigned URL from Azure should remove signature",
input: "https://myaccount.queue.core.windows.net/myqueue/messages?se=2015-07-02T08%3A49Z&si=YWJjZGVmZw%3D%3D&sig=jDrr6cna7JPwIaxWfdH0tT5v9dc%3d&sp=p&st=2015-07-01T08%3A49Z&sv=2015-02-21&visibilitytimeout=120",
want: "https://myaccount.queue.core.windows.net/myqueue/messages?se=2015-07-02T08%3A49Z&si=YWJjZGVmZw%3D%3D&sig=hidden&sp=p&st=2015-07-01T08%3A49Z&sv=2015-02-21&visibilitytimeout=120",
},
{
name: "Receiving presigned URL from Azure with upper case query values should remove signature",
input: "https://myaccount.queue.core.windows.net/myqueue/messages?se=2015-07-02T08%3A49Z&si=YWJjZGVmZw%3D%3D&SIG=jDrr6cna7JPwIaxWfdH0tT5v9dc%3d&sp=p&st=2015-07-01T08%3A49Z&SV=2015-02-21&visibilitytimeout=120",
want: "https://myaccount.queue.core.windows.net/myqueue/messages?SIG=hidden&SV=2015-02-21&se=2015-07-02T08%3A49Z&si=YWJjZGVmZw%3D%3D&sp=p&st=2015-07-01T08%3A49Z&visibilitytimeout=120",
},
{
name: "Receiving valid URL string should return it parsed",
input: "https://grafana.com/?sig=testing-a-generic-parameter",
want: "https://grafana.com/?sig=testing-a-generic-parameter",
},
{
name: "Receiving invalid URL string should return empty string",
input: "this is not a valid URL",
want: "",
expectError: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
url, err := SanitizeURI(tt.input)
if tt.expectError {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}
assert.Equalf(t, tt.want, url, "SanitizeURI(%v)", tt.input)
})
}
}