New filter: maskAccessLogQuery

[remychantenay]

Summary

Adding a new maskAccessLogQuery filter to mask/obfuscate values of specific – sensitive – query parameters.

Towards #2156

Testing

# skip
hello: Path("/hello") -> maskAccessLogQuery("key_2") -> "https://www.example.org"

curl "localhost:9090/hello?key_1=foo&key_2=bar"
127.0.0.1 - - [18/Oct/2023:08:10:33 +0200] "GET /hello?key_1=foo&key_2=5234164152756840025 HTTP/1.1" 404 1256 "-" "curl/8.1.2" 555 localhost:9090 - -

[szuecs]

That's an API change.
I would suggest to use additional map[string]interface{} and have a map entry like "maskedQueryParams": []string.

[AlexanderYastrebov]

There is an exported statebag key AccessLogAdditionalDataKey to provide additional data
and additional data can override predefined fields

skipper/logging/access.go

Lines 152 to 171 in 9b57097

	logData := logrus.Fields{
	"timestamp": ts,
	"host": host,
	"method": method,
	"uri": uri,
	"proto": proto,
	"referer": referer,
	"user-agent": userAgent,
	"status": status,
	"response-size": responseSize,
	"requested-host": requestedHost,
	"duration": duration,
	"flow-id": flowId,
	"audit": auditHeader,
	"auth-user": authUser,
	}
	for k, v := range additional {
	logData[k] = v
	}

so another option is to create a filter that would mask request uri and put it into ctx.StateBag()[al.AccessLogAdditionalDataKey]["uri"].

[remychantenay]

Thank you for the feedback 👍

I updated the code and went with @szuecs's suggestion. Hope that's fine.

[AlexanderYastrebov]

Since we rely on additional data anyways lets use "uri" key - this way masking logic would belong to the filter and not to the access log package (e.g. there could be several ways to mask uri). The filter would need only to populate additional data in the statebag (i.e. does not have to return &AccessLogFilter).

You can export const UriAdditionalDataKey = "uri" if you want and use it everywhere instead of "uri" .

[AlexanderYastrebov]

This will override any preceding filter configuration.

Maybe we should think about supporting a usecase where we could specify default masked values

disableAccessLog(2,3,404,429) -> accessLogMaskQueryParams("access_token") -> fifo(2000,20,"1s")

and then user may specify additional values.
OTOH proposed solution is simple and in line with what already exists.

[remychantenay]

Hey @AlexanderYastrebov

Yeah, I wanted to keep it simple. In hindsight, I am not sure my thinking is right.

If the current implementation is a no-go, I will try and spend a bit more time and see if I can come up with something when I get back from vacay (from tomorrow till the beginning of next month.)