Update approach to passing masked keys

Signed-off-by: Remy Chantenay <[email protected]>

Author: remychantenay

Diff for: logging/access.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"net"
 	"net/http"
-	"strconv"
 	"strings"
 	"time"
 
@@ -23,6 +22,10 @@ const (
 	combinedLogFormat = commonLogFormat + ` "%s" "%s"`
 	// We add the duration in ms, a requested host and a flow id and audit log
 	accessLogFormat = combinedLogFormat + " %d %s %s %s\n"
+
+	// KeyMaskedQueryParams represents the key used to store and retrieve masked query parameters
+	// from the additional data.
+	KeyMaskedQueryParams = "maskedQueryParams"
 )
 
 type accessLogFormatter struct {
@@ -111,13 +114,32 @@ 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 []string) string {
+	strippedURI := stripQueryString(req.RequestURI)
+
+	params := req.URL.Query()
+	for i := range maskedQueryParams {
+		val := params.Get(maskedQueryParams[i])
+		if val == "" {
+			continue
+		}
+
+		hashed := hash(val)
+		params.Set(maskedQueryParams[i], fmt.Sprintf("%d", hashed))
+	}
+
+	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{}, maskedQueryParams []string) {
+func LogAccess(entry *AccessEntry, additional map[string]interface{}) {
 	if accessLog == nil || entry == nil {
 		return
 	}
@@ -150,21 +172,10 @@ func LogAccess(entry *AccessEntry, additional map[string]interface{}, maskedQuer
 		uri = entry.Request.RequestURI
 		if stripQuery {
 			uri = stripQueryString(uri)
-		} else if len(maskedQueryParams) != 0 {
-			strippedURI := stripQueryString(uri)
-
-			params := entry.Request.URL.Query()
-			for i := range maskedQueryParams {
-				val := params.Get(maskedQueryParams[i])
-				if val == "" {
-					continue
-				}
-
-				hashed := hash(val)
-				params.Set(maskedQueryParams[i], strconv.Itoa(int(hashed)))
+		} else if keys, ok := additional[KeyMaskedQueryParams].([]string); ok {
+			if len(keys) > 0 {
+				uri = maskQueryParams(entry.Request, keys)
 			}
-
-			uri = fmt.Sprintf("%s?%s", strippedURI, params.Encode())
 		}
 
 		auditHeader = entry.Request.Header.Get(logFilter.UnverifiedAuditHeader)
@@ -187,6 +198,7 @@ func LogAccess(entry *AccessEntry, additional map[string]interface{}, maskedQuer
 		"auth-user":      authUser,
 	}
 
+	delete(additional, KeyMaskedQueryParams)
 	for k, v := range additional {
 		logData[k] = v
 	}

Diff for: logging/access_test.go

@@ -80,19 +80,18 @@ func testAccessEntryWithQueryParameters(params url.Values) *AccessEntry {
 }
 
 func testAccessLog(t *testing.T, entry *AccessEntry, expectedOutput string, o Options) {
-	testAccessLogExtended(t, entry, nil, nil, expectedOutput, o)
+	testAccessLogExtended(t, entry, nil, expectedOutput, o)
 }
 
 func testAccessLogExtended(t *testing.T, entry *AccessEntry,
 	additional map[string]interface{},
-	maskedQueryParams []string,
 	expectedOutput string,
 	o Options,
 ) {
 	var buf bytes.Buffer
 	o.AccessLogOutput = &buf
 	Init(o)
-	LogAccess(entry, additional, maskedQueryParams)
+	LogAccess(entry, additional)
 	got := buf.String()
 	if got != "" {
 		got = got[:len(got)-1]
@@ -133,18 +132,18 @@ func TestAccessLogFormatJSON(t *testing.T) {
 }
 
 func TestAccessLogFormatJSONWithAdditionalData(t *testing.T) {
-	testAccessLogExtended(t, testAccessEntry(), map[string]interface{}{"extra": "extra"}, nil, logExtendedJSONOutput, Options{AccessLogJSONEnabled: true})
+	testAccessLogExtended(t, testAccessEntry(), map[string]interface{}{"extra": "extra"}, logExtendedJSONOutput, Options{AccessLogJSONEnabled: true})
 }
 
 func TestAccessLogFormatJSONWithMaskedQueryParameters(t *testing.T) {
-	maskedQueryParams := []string{"key_1"}
+	additional := map[string]interface{}{KeyMaskedQueryParams: []string{"foo"}}
 
 	params := url.Values{}
-	params.Add("key_1", "value_1")
+	params.Add("foo", "bar")
 	testAccessLogExtended(t,
 		testAccessEntryWithQueryParameters(params),
-		nil, maskedQueryParams,
-		`{"audit":"","auth-user":"","duration":42,"flow-id":"","host":"127.0.0.1","level":"info","method":"GET","msg":"","proto":"HTTP/1.1","referer":"","requested-host":"example.com","response-size":2326,"status":418,"timestamp":"10/Oct/2000:13:55:36 -0700","uri":"/apache_pb.gif?key_1=3272669212","user-agent":""}`,
+		additional,
+		`{"audit":"","auth-user":"","duration":42,"flow-id":"","host":"127.0.0.1","level":"info","method":"GET","msg":"","proto":"HTTP/1.1","referer":"","requested-host":"example.com","response-size":2326,"status":418,"timestamp":"10/Oct/2000:13:55:36 -0700","uri":"/apache_pb.gif?foo=5234164152756840025","user-agent":""}`,
 		Options{AccessLogJSONEnabled: true},
 	)
 }
@@ -340,10 +339,10 @@ func TestAccessLogStripQuery(t *testing.T) {
 }
 
 func TestHashQueryParamValue(t *testing.T) {
-	want := "2972666014"
-	got := hashQueryParamValue("foo")
+	want := uint64(3728699739546630719)
+	got := hash("foo")
 
 	if got != want {
-		t.Errorf("\ngot %q\nwant %q", got, want)
+		t.Errorf("\ngot %v\nwant %v", got, want)
 	}
 }

Diff for: logging/log_test.go

@@ -38,7 +38,7 @@ func TestCustomPrefixForApplicationLog(t *testing.T) {
 func TestCustomOutputForAccessLog(t *testing.T) {
 	var buf bytes.Buffer
 	Init(Options{AccessLogOutput: &buf})
-	LogAccess(&AccessEntry{StatusCode: http.StatusTeapot}, nil, nil)
+	LogAccess(&AccessEntry{StatusCode: http.StatusTeapot}, nil)
 	if !strings.Contains(buf.String(), strconv.Itoa(http.StatusTeapot)) {
 		t.Error("failed to use custom access log output")
 	}

Diff for: proxy/proxy.go

@@ -1579,8 +1579,15 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 			}
 
 			additionalData, _ := ctx.stateBag[al.AccessLogAdditionalDataKey].(map[string]interface{})
+			if len(accessLogEnabled.MaskedQueryParams) > 0 {
+				if additionalData == nil {
+					additionalData = make(map[string]interface{})
+				}
+
+				additionalData[logging.KeyMaskedQueryParams] = accessLogEnabled.MaskedQueryParams
+			}
 
-			logging.LogAccess(entry, additionalData, accessLogEnabled.MaskedQueryParams)
+			logging.LogAccess(entry, additionalData)
 		}
 
 		// This flush is required in I/O error