diff --git a/changes/12570-mask-webhook-url-logs b/changes/12570-mask-webhook-url-logs new file mode 100644 index 0000000000..b86d5d399a --- /dev/null +++ b/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". diff --git a/server/utils.go b/server/utils.go index af207709ca..0c13dab006 100644 --- a/server/utils.go +++ b/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. diff --git a/server/utils_test.go b/server/utils_test.go new file mode 100644 index 0000000000..849351e434 --- /dev/null +++ b/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) + }) + } +}