filters/accesslog: Add maskAccessLogQuery filter

docs/reference/filters.md

@@ -2772,6 +2772,20 @@ unverifiedAuditLog()
 unverifiedAuditLog("azp")
 ```
 
+### maskAccessLogQuery
+
+Filter `maskAccessLogQuery` masks values of the provided query parameters in access logs by replacing them with hashes. It accepts query parameter keys as arguments.
+
+Examples:
+
+```
+maskAccessLogQuery("key_1")
+```
+
+```
+maskAccessLogQuery("key_1", "key_2")
+```
+
 ## Backend
 ### backendIsProxy
 

filters/accesslog/control.go

@@ -1,6 +1,9 @@
 package accesslog
 
-import "github.com/zalando/skipper/filters"
+import (
+	"github.com/zalando/skipper/filters"
+	"maps"
+)
 
 const (
 	// Deprecated, use filters.DisableAccessLogName instead
@@ -14,17 +17,40 @@ const (
 
 	// AccessLogAdditionalDataKey is the key used in the state bag to pass extra data to access log
 	AccessLogAdditionalDataKey = "statebag:access_log:additional"
+
+	// KeyMaskedQueryParams is the key used to store and retrieve masked query parameters
+	// from the additional data.
+	KeyMaskedQueryParams = "maskedQueryParams"
 )
 
 // AccessLogFilter stores access log state
 type AccessLogFilter struct {
-	Enable   bool
+	// Enable represents whether or not the access log is enabled.
+	Enable bool
+	// Prefixes contains the list of response code prefixes.
 	Prefixes []int
+	// MaskedQueryParams contains the set of query parameters (keys) that are masked/obfuscated in the access log.
+	MaskedQueryParams map[string]struct{}
 }
 
 func (al *AccessLogFilter) Request(ctx filters.FilterContext) {
 	bag := ctx.StateBag()
 	bag[AccessLogEnabledKey] = al
+
+	if al.MaskedQueryParams != nil {
+		additionalData, ok := bag[AccessLogAdditionalDataKey].(map[string]interface{})
+		if !ok {
+			additionalData = make(map[string]interface{})
+			bag[AccessLogAdditionalDataKey] = additionalData
+		}
+		maskedQueryParams, ok := additionalData[KeyMaskedQueryParams].(map[string]struct{})
+		if !ok {
+			maskedQueryParams = make(map[string]struct{})
+			additionalData[KeyMaskedQueryParams] = maskedQueryParams
+		}
+		maps.Copy(maskedQueryParams, al.MaskedQueryParams)
+
+	}
 }
 
 func (*AccessLogFilter) Response(filters.FilterContext) {}
@@ -87,3 +113,32 @@ func (*enableAccessLog) Name() string { return filters.EnableAccessLogName }
 func (al *enableAccessLog) CreateFilter(args []interface{}) (filters.Filter, error) {
 	return extractFilterValues(args, true)
 }
+
+type maskAccessLogQuery struct{}
+
+// NewMaskAccessLogQuery creates a filter spec to mask specific query parameters from the access log for a specific route.
+// Takes in query param keys as arguments. When provided, the value of these keys are masked (i.e., hashed).
+//
+//	maskAccessLogQuery("key_1", "key_2") to mask the value of provided keys in the access log.
+func NewMaskAccessLogQuery() filters.Spec {
+	return &maskAccessLogQuery{}
+}
+
+func (*maskAccessLogQuery) Name() string { return filters.MaskAccessLogQueryName }
+
+func (al *maskAccessLogQuery) CreateFilter(args []interface{}) (filters.Filter, error) {
+	if len(args) == 0 {
+		return nil, filters.ErrInvalidFilterParameters
+	}
+
+	keys := make(map[string]struct{}, len(args))
+	for _, arg := range args {
+		if key, ok := arg.(string); ok && key != "" {
+			keys[key] = struct{}{}
+		} else {
+			return nil, filters.ErrInvalidFilterParameters
+		}
+	}
+
+	return &AccessLogFilter{Enable: true, MaskedQueryParams: keys}, nil
+}

filters/accesslog/control_test.go

@@ -1,7 +1,8 @@
 package accesslog
 
 import (
-	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"testing"
 
 	"github.com/zalando/skipper/filters"
@@ -20,14 +21,14 @@ func TestAccessLogControl(t *testing.T) {
 			msg:     "enables-access-log",
 			state:   NewEnableAccessLog(),
 			args:    nil,
-			result:  AccessLogFilter{true, make([]int, 0)},
+			result:  AccessLogFilter{Enable: true, Prefixes: make([]int, 0)},
 			isError: false,
 		},
 		{
 			msg:     "enable-access-log-selective",
 			state:   NewEnableAccessLog(),
 			args:    []interface{}{2, 4, 300},
-			result:  AccessLogFilter{true, []int{2, 4, 300}},
+			result:  AccessLogFilter{Enable: true, Prefixes: []int{2, 4, 300}},
 			isError: false,
 		},
 		{
@@ -41,34 +42,44 @@ func TestAccessLogControl(t *testing.T) {
 			msg:     "disables-access-log",
 			state:   NewDisableAccessLog(),
 			args:    nil,
-			result:  AccessLogFilter{false, make([]int, 0)},
+			result:  AccessLogFilter{Enable: false, Prefixes: make([]int, 0)},
 			isError: false,
 		},
 		{
 			msg:     "disables-access-log-selective",
 			state:   NewDisableAccessLog(),
 			args:    []interface{}{1, 201, 30},
-			result:  AccessLogFilter{false, []int{1, 201, 30}},
+			result:  AccessLogFilter{Enable: false, Prefixes: []int{1, 201, 30}},
 			isError: false,
 		},
 		{
 			msg:     "disables-access-log-convert-float",
 			state:   NewDisableAccessLog(),
 			args:    []interface{}{1.0, 201},
-			result:  AccessLogFilter{false, []int{1, 201}},
+			result:  AccessLogFilter{Enable: false, Prefixes: []int{1, 201}},
 			isError: false,
 		},
+		{
+			msg:     "mask-access-log-query",
+			state:   NewMaskAccessLogQuery(),
+			args:    []interface{}{"key_1"},
+			result:  AccessLogFilter{Enable: true, MaskedQueryParams: map[string]struct{}{"key_1": {}}},
+			isError: false,
+		},
+		{
+			msg:     "mask-access-log-query-convert-int",
+			state:   NewMaskAccessLogQuery(),
+			args:    []interface{}{1},
+			result:  AccessLogFilter{},
+			isError: true,
+		},
 	} {
 		t.Run(ti.msg, func(t *testing.T) {
 			f, err := ti.state.CreateFilter(ti.args)
 
 			if ti.isError {
-				if err == nil {
-					t.Errorf("Unexpected error creating filter %v", err)
-					return
-				} else {
-					return
-				}
+				require.Error(t, err, "Expected error creating filter")
+				return
 			}
 
 			var ctx filtertest.Context
@@ -77,9 +88,58 @@ func TestAccessLogControl(t *testing.T) {
 			f.Request(&ctx)
 			bag := ctx.StateBag()
 			filter := bag[AccessLogEnabledKey]
-			if diff := cmp.Diff(filter, &ti.result); diff != "" {
-				t.Errorf("access log state is not equal to expected '%v' got %v", ti.result, bag[AccessLogEnabledKey])
+
+			assert.Equal(t, filter, &ti.result, "access log state is not equal to expected")
+		})
+	}
+}
+
+func TestAccessLogMaskedParametersMerging(t *testing.T) {
+	for _, ti := range []struct {
+		msg    string
+		state  filters.Spec
+		args   [][]any
+		result map[string]struct{}
+	}{
+		{
+			msg:   "should merge masked query params from multiple filters",
+			state: NewMaskAccessLogQuery(),
+			args: [][]any{
+				{"key_1"},
+				{"key_2"},
+			},
+			result: map[string]struct{}{"key_1": {}, "key_2": {}},
+		},
+		{
+			msg:   "should overwrite already masked params",
+			state: NewMaskAccessLogQuery(),
+			args: [][]any{
+				{"key_1"},
+				{"key_1"},
+				{"key_1"},
+			},
+			result: map[string]struct{}{"key_1": {}},
+		},
+	} {
+		t.Run(ti.msg, func(t *testing.T) {
+
+			filters := make([]filters.Filter, len(ti.args))
+			for i, a := range ti.args {
+				f, err := ti.state.CreateFilter(a)
+				require.NoError(t, err, "Expected no error creating filter")
+				filters[i] = f
 			}
+
+			var ctx filtertest.Context
+			ctx.FStateBag = make(map[string]any)
+
+			for _, f := range filters {
+				f.Request(&ctx)
+			}
+
+			bag := ctx.StateBag()
+			params := bag[AccessLogAdditionalDataKey].(map[string]interface{})[KeyMaskedQueryParams].(map[string]struct{})
+			assert.Equal(t, params, ti.result, "access log state is not equal to expected")
 		})
 	}
 }

filters/accesslog/disable.go

@@ -38,7 +38,7 @@ func (*accessLogDisabled) CreateFilter(args []interface{}) (filters.Filter, erro
 
 func (al *accessLogDisabled) Request(ctx filters.FilterContext) {
 	bag := ctx.StateBag()
-	bag[AccessLogEnabledKey] = &AccessLogFilter{!al.disabled, nil}
+	bag[AccessLogEnabledKey] = &AccessLogFilter{!al.disabled, nil, nil}
 }
 
 func (*accessLogDisabled) Response(filters.FilterContext) {}

filters/accesslog/disable_test.go

@@ -1,9 +1,10 @@
 package accesslog
 
 import (
-	"github.com/google/go-cmp/cmp"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+
 	"github.com/zalando/skipper/filters"
 	"github.com/zalando/skipper/filters/filtertest"
 )
@@ -18,31 +19,31 @@ func TestAccessLogDisabled(t *testing.T) {
 		{
 			msg:    "false-value-enables-access-log",
 			state:  []interface{}{"false"},
-			result: AccessLogFilter{true, nil},
+			result: AccessLogFilter{Enable: true, Prefixes: nil},
 			err:    nil,
 		},
 		{
 			msg:    "true-value-disables-access-log",
 			state:  []interface{}{"true"},
-			result: AccessLogFilter{false, nil},
+			result: AccessLogFilter{Enable: false, Prefixes: nil},
 			err:    nil,
 		},
 		{
 			msg:    "unknown-argument-leads-to-error",
 			state:  []interface{}{"unknownValue"},
-			result: AccessLogFilter{true, nil},
+			result: AccessLogFilter{Enable: true, Prefixes: nil},
 			err:    filters.ErrInvalidFilterParameters,
 		},
 		{
 			msg:    "no-arguments-lead-to-error",
 			state:  []interface{}{},
-			result: AccessLogFilter{true, nil},
+			result: AccessLogFilter{Enable: true, Prefixes: nil},
 			err:    filters.ErrInvalidFilterParameters,
 		},
 		{
 			msg:    "multiple-arguments-lead-to-error",
 			state:  []interface{}{"true", "second"},
-			result: AccessLogFilter{true, nil},
+			result: AccessLogFilter{Enable: true, Prefixes: nil},
 			err:    filters.ErrInvalidFilterParameters,
 		},
 	} {

filters/builtin/builtin.go

@@ -223,6 +223,7 @@ func Filters() []filters.Spec {
 		//lint:ignore SA1019 due to backward compatibility
 		accesslog.NewAccessLogDisabled(),
 		accesslog.NewDisableAccessLog(),
+		accesslog.NewMaskAccessLogQuery(),
 		accesslog.NewEnableAccessLog(),
 		auth.NewForwardToken(),
 		auth.NewForwardTokenField(),

filters/filters.go

@@ -332,6 +332,7 @@ const (
 	HeaderToQueryName                          = "headerToQuery"
 	QueryToHeaderName                          = "queryToHeader"
 	DisableAccessLogName                       = "disableAccessLog"
+	MaskAccessLogQueryName                     = "maskAccessLogQuery"
 	EnableAccessLogName                        = "enableAccessLog"
 	AuditLogName                               = "auditLog"
 	UnverifiedAuditLogName                     = "unverifiedAuditLog"

logging/access.go

@@ -2,13 +2,16 @@ package logging
 
 import (
 	"fmt"
+	"maps"
 	"net"
 	"net/http"
 	"strings"
 	"time"
 
+	"github.com/cespare/xxhash/v2"
 	"github.com/sirupsen/logrus"
 
+	al "github.com/zalando/skipper/filters/accesslog"
 	flowidFilter "github.com/zalando/skipper/filters/flowid"
 	logFilter "github.com/zalando/skipper/filters/log"
 )
@@ -109,6 +112,27 @@ func stripQueryString(u string) string {
 	}
 }
 
+// maskQueryParams masks (i.e., hashing) specific query parameters in the provided request's URI.
+// Returns the obfuscated URI.
+func maskQueryParams(req *http.Request, maskedQueryParams map[string]struct{}) string {
+	strippedURI := stripQueryString(req.RequestURI)
+
+	params := req.URL.Query()
+	for k := range maps.Keys(maskedQueryParams) {
+		val := params.Get(k)
+		if val == "" {
+			continue
+		}
+		params.Set(k, fmt.Sprintf("%d", hash(val)))
+	}
+
+	return fmt.Sprintf("%s?%s", strippedURI, params.Encode())
+}
+
+func hash(val string) uint64 {
+	return xxhash.Sum64String(val)
+}
+
 // Logs an access event in Apache combined log format (with a minor customization with the duration).
 // Additional allows to provide extra data that may be also logged, depending on the specific log format.
 func LogAccess(entry *AccessEntry, additional map[string]interface{}) {
@@ -144,6 +168,8 @@ func LogAccess(entry *AccessEntry, additional map[string]interface{}) {
 		uri = entry.Request.RequestURI
 		if stripQuery {
 			uri = stripQueryString(uri)
+		} else if keys, ok := additional[al.KeyMaskedQueryParams].(map[string]struct{}); ok && len(keys) > 0 {
+			uri = maskQueryParams(entry.Request, keys)
 		}
 
 		auditHeader = entry.Request.Header.Get(logFilter.UnverifiedAuditHeader)
@@ -166,9 +192,9 @@ func LogAccess(entry *AccessEntry, additional map[string]interface{}) {
 		"auth-user":      authUser,
 	}
 
-	for k, v := range additional {
-		logData[k] = v
-	}
+	delete(additional, al.KeyMaskedQueryParams)
+
+	maps.Copy(logData, additional)
 
 	logEntry := accessLog.WithFields(logData)
 	if entry.Request != nil {