Mask URL params in server logs for webhook POSTs (#12761)

changes/12570-mask-webhook-url-logs

@@ -0,0 +1 @@
+- Updated server logging for webhook requests to mask URL query values if the query param name includes "secret", "token", "key", "password".

server/utils.go

@@ -13,6 +13,8 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"net/url"
+	"strings"
 	"time"
 
 	"github.com/fleetdm/fleet/v4/pkg/fleethttp"
@@ -49,18 +51,49 @@ func PostJSONWithTimeout(ctx context.Context, url string, v interface{}) error {
 
 	resp, err := client.Do(req)
 	if err != nil {
-		return fmt.Errorf("failed to POST to %s: %s, request-size=%d", url, err, len(jsonBytes))
+		return fmt.Errorf("failed to POST to %s: %s, request-size=%d", maskSecretURLParams(url), err, len(jsonBytes))
 	}
 	defer resp.Body.Close()
 
 	if !httpSuccessStatus(resp.StatusCode) {
 		body, _ := ioutil.ReadAll(resp.Body)
-		return fmt.Errorf("error posting to %s: %d. %s", url, resp.StatusCode, string(body))
+		return fmt.Errorf("error posting to %s: %d. %s", maskSecretURLParams(url), resp.StatusCode, string(body))
 	}
 
 	return nil
 }
 
+// maskSecretURLParams masks URL query values if the query param name includes "secret", "token",
+// "key", "password". It accepts a raw string and returns a redacted string if the raw string is
+// URL-parseable. If it is not URL-parseable, the raw string is returned unchanged.
+func maskSecretURLParams(rawURL string) string {
+	u, err := url.Parse(rawURL)
+	if err != nil {
+		return rawURL
+	}
+
+	keywords := []string{"secret", "token", "key", "password"}
+	containsKeyword := func(s string) bool {
+		s = strings.ToLower(s)
+		for _, kw := range keywords {
+			if strings.Contains(s, kw) {
+				return true
+			}
+		}
+		return false
+	}
+
+	q := u.Query()
+	for k := range q {
+		if containsKeyword(k) {
+			q[k] = []string{"MASKED"}
+		}
+	}
+	u.RawQuery = q.Encode()
+
+	return u.Redacted()
+}
+
 // TODO: Consider moving other crypto functions from server/mdm/apple/util to here
 
 // DecodePrivateKeyPEM decodes PEM-encoded private key data.

server/utils_test.go

@@ -0,0 +1,75 @@
+package server
+
+import (
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestMaskSecretURLParams(t *testing.T) {
+	secretKeywords := []string{"secret", "token", "key", "password"}
+	mask := "MASKED"
+
+	type testCase struct {
+		name     string
+		rawURL   string
+		expected string
+	}
+
+	cases := []testCase{
+		{
+			name:     "no params",
+			rawURL:   "https://example.com",
+			expected: "https://example.com",
+		},
+		{
+			name:     "user info redacted",
+			rawURL:   "https://user:P@$$w0rD@example.com/foo/bar?baz=qux&secret_key=baz",
+			expected: "https://user:xxxxx@example.com/foo/bar?baz=qux&secret_key=" + mask,
+		},
+	}
+	for i, kw := range secretKeywords {
+		cases = append(cases, testCase{
+			name:     "single " + kw,
+			rawURL:   "https://example.com?" + kw + "=foo",
+			expected: "https://example.com?" + kw + "=" + mask,
+		})
+		cases = append(cases, testCase{
+			name:     "multiple " + kw,
+			rawURL:   "https://example.com?" + kw + "=foo" + "&bar_" + kw + "=bar",
+			expected: "https://example.com?" + kw + "=" + mask + "&bar_" + kw + "=" + mask,
+		})
+		cases = append(cases, testCase{
+			name:     "multiple " + kw + " with other params",
+			rawURL:   "https://example.com?foo=bar&" + kw + "=foo" + "&bar_" + kw + "=bar",
+			expected: "https://example.com?foo=bar&" + kw + "=" + mask + "&bar_" + kw + "=" + mask,
+		})
+		cases = append(cases, testCase{
+			name:     "multiple " + kw + " with other params and fragment",
+			rawURL:   "https://example.com?foo=bar&" + kw + "=foo" + "&bar_" + kw + "=bar#fragment",
+			expected: "https://example.com?foo=bar&" + kw + "=" + mask + "&bar_" + kw + "=" + mask + "#fragment",
+		})
+		kw2 := secretKeywords[(i+1)%len(secretKeywords)]
+		cases = append(cases, testCase{
+			name:     "combined " + kw + " and " + kw2,
+			rawURL:   "https://example.com?foo=bar&" + kw + "=foo" + "&bar_" + kw2 + "=bar",
+			expected: "https://example.com?foo=bar&" + kw + "=" + mask + "&bar_" + kw2 + "=" + mask,
+		})
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			masked := maskSecretURLParams(c.rawURL)
+			got, err := url.Parse(masked)
+			require.NoError(t, err)
+			want, err := url.Parse(c.expected)
+			require.NoError(t, err)
+			require.EqualValues(t, got.Query(), want.Query())
+			require.Equal(t, got.Fragment, want.Fragment)
+			require.Equal(t, got.Host, want.Host)
+			require.Equal(t, got.Path, want.Path)
+			require.Equal(t, got.Scheme, want.Scheme)
+		})
+	}
+}