diff --git a/server/client.go b/server/client.go index bbbf66fd0c..436c26a49f 100644 --- a/server/client.go +++ b/server/client.go @@ -756,28 +756,36 @@ func (c *client) processErr(errStr string) { } // Password pattern matcher. -var passPat = regexp.MustCompile(`"?\s*pass\S*?"?[:=]\s*"?(([^"])*)`) +var passPat = regexp.MustCompile(`"?\s*pass\S*?"?\s*[:=]\s*"?(([^",\r\n}])*)`) -// This will remove any notion of passwords from trace messages -// for logging. +// removePassFromTrace removes any notion of passwords from trace +// messages for logging. func removePassFromTrace(arg []byte) []byte { - - if !bytes.Contains(arg, []byte("pass")) { + if !bytes.Contains(arg, []byte(`pass`)) { return arg } - m := passPat.FindAllSubmatch(arg, -1) + // Take a copy of the connect proto just for the trace message. + var _arg [4096]byte + buf := append(_arg[:0], arg...) + + m := passPat.FindAllSubmatchIndex(buf, -1) if len(m) == 0 { return arg } - for _, match := range m { - if len(match) != 3 { + redactedPass := []byte("[REDACTED]") + for _, i := range m { + if len(i) < 4 { continue } - arg = bytes.Replace(arg, match[1], []byte("[REDACTED]"), 1) + start := i[2] + end := i[3] + // Replace password substring. + buf = append(buf[:start], append(redactedPass, buf[end:]...)...) + break } - return arg + return buf } func (c *client) processConnect(arg []byte) error { diff --git a/server/log_test.go b/server/log_test.go index d9b06822d9..1433ac435b 100644 --- a/server/log_test.go +++ b/server/log_test.go @@ -218,20 +218,129 @@ func TestNoPasswordsFromConnectTrace(t *testing.T) { } func TestRemovePassFromTrace(t *testing.T) { - pass := []byte("s3cr3t") - check := func(r []byte) { - t.Helper() - if bytes.Contains(r, pass) { - t.Fatalf("Found password in %q", r) - } - } - check(removePassFromTrace([]byte("CONNECT {\"user\":\"derek\",\"pass\":\"s3cr3t\"}\r\n"))) - check(removePassFromTrace([]byte("CONNECT {\"user\":\"derek\",\"pass\": \"s3cr3t\"}\r\n"))) - check(removePassFromTrace([]byte("CONNECT {\"user\":\"derek\",\"pass\": \"s3cr3t\" }\r\n"))) - check(removePassFromTrace([]byte("CONNECT {\"pass\":\"s3cr3t\",}\r\n"))) - check(removePassFromTrace([]byte("CONNECT {pass:s3cr3t , password = s3cr3t}"))) - check(removePassFromTrace([]byte("CONNECT {\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"foo\",\"pass\":\"s3cr3t\",\"tls_required\":false,\"name\":\"APM7JU94z77YzP6WTBEiuw\"}\r\n"))) - check(removePassFromTrace([]byte("CONNECT {pass:s3cr3t\r\n"))) - check(removePassFromTrace([]byte("CONNECT {\"password\":\"s3cr3t\",}\r\n"))) - check(removePassFromTrace([]byte("CONNECT {\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"foo\",\"password\":\"s3cr3t\",\"tls_required\":false,\"name\":\"APM7JU94z77YzP6WTBEiuw\"}\r\n"))) + tests := []struct { + name string + input string + expected string + }{ + { + "user and pass", + "CONNECT {\"user\":\"derek\",\"pass\":\"s3cr3t\"}\r\n", + "CONNECT {\"user\":\"derek\",\"pass\":\"[REDACTED]\"}\r\n", + }, + { + "user and pass extra space", + "CONNECT {\"user\":\"derek\",\"pass\": \"s3cr3t\"}\r\n", + "CONNECT {\"user\":\"derek\",\"pass\": \"[REDACTED]\"}\r\n", + }, + { + "user and pass is empty", + "CONNECT {\"user\":\"derek\",\"pass\":\"\"}\r\n", + "CONNECT {\"user\":\"derek\",\"pass\":\"[REDACTED]\"}\r\n", + }, + { + "user and pass is empty whitespace", + "CONNECT {\"user\":\"derek\",\"pass\":\" \"}\r\n", + "CONNECT {\"user\":\"derek\",\"pass\":\"[REDACTED]\"}\r\n", + }, + { + "user and pass whitespace", + "CONNECT {\"user\":\"derek\",\"pass\": \"s3cr3t\" }\r\n", + "CONNECT {\"user\":\"derek\",\"pass\": \"[REDACTED]\" }\r\n", + }, + { + "only pass", + "CONNECT {\"pass\":\"s3cr3t\",}\r\n", + "CONNECT {\"pass\":\"[REDACTED]\",}\r\n", + }, + { + "invalid json", + "CONNECT {pass:s3cr3t , password = s3cr3t}", + "CONNECT {pass:[REDACTED], password = s3cr3t}", + }, + { + "invalid json no whitespace after key", + "CONNECT {pass:s3cr3t , password= s3cr3t}", + "CONNECT {pass:[REDACTED], password= s3cr3t}", + }, + { + "both pass and wrong password key", + `CONNECT {"pass":"s3cr3t4", "password": "s3cr3t4"}`, + `CONNECT {"pass":"[REDACTED]", "password": "s3cr3t4"}`, + }, + { + "invalid json", + "CONNECT {user = hello, password = s3cr3t}", + "CONNECT {user = hello, password = [REDACTED]}", + }, + { + "complete connect", + "CONNECT {\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"foo\",\"pass\":\"s3cr3t\",\"tls_required\":false,\"name\":\"APM7JU94z77YzP6WTBEiuw\"}\r\n", + "CONNECT {\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"foo\",\"pass\":\"[REDACTED]\",\"tls_required\":false,\"name\":\"APM7JU94z77YzP6WTBEiuw\"}\r\n", + }, + { + "invalid json with only pass key", + "CONNECT {pass:s3cr3t\r\n", + "CONNECT {pass:[REDACTED]\r\n", + }, + { + "invalid password key also filtered", + "CONNECT {\"password\":\"s3cr3t\",}\r\n", + "CONNECT {\"password\":\"[REDACTED]\",}\r\n", + }, + { + "single long password with whitespace", + "CONNECT {\"pass\":\"secret password which is very long\",}\r\n", + "CONNECT {\"pass\":\"[REDACTED]\",}\r\n", + }, + { + "single long pass key is filtered", + "CONNECT {\"pass\":\"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\"}\r\n", + "CONNECT {\"pass\":\"[REDACTED]\"}\r\n", + }, + { + "duplicate keys only filtered once", + "CONNECT {\"pass\":\"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\",\"pass\":\"BBBBBBBBBBBBBBBBBBBB\",\"password\":\"CCCCCCCCCCCCCCCC\"}\r\n", + "CONNECT {\"pass\":\"[REDACTED]\",\"pass\":\"BBBBBBBBBBBBBBBBBBBB\",\"password\":\"CCCCCCCCCCCCCCCC\"}\r\n", + }, + { + "invalid json with multiple keys only one is filtered", + "CONNECT {pass = \"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\",pass= \"BBBBBBBBBBBBBBBBBBBB\",password =\"CCCCCCCCCCCCCCCC\"}\r\n", + "CONNECT {pass = \"[REDACTED]\",pass= \"BBBBBBBBBBBBBBBBBBBB\",password =\"CCCCCCCCCCCCCCCC\"}\r\n", + }, + { + "complete connect protocol", + "CONNECT {\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"foo\",\"pass\":\"s3cr3t\",\"tls_required\":false,\"name\":\"APM7JU94z77YzP6WTBEiuw\"}\r\n", + "CONNECT {\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"foo\",\"pass\":\"[REDACTED]\",\"tls_required\":false,\"name\":\"APM7JU94z77YzP6WTBEiuw\"}\r\n", + }, + { + "user and pass are filterered", + "CONNECT {\"user\":\"s3cr3t\",\"pass\":\"s3cr3t\"}\r\n", + "CONNECT {\"user\":\"s3cr3t\",\"pass\":\"[REDACTED]\"}\r\n", + }, + { + "complete connect using password key with user and password being the same", + "CONNECT {\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"s3cr3t\",\"pass\":\"s3cr3t\",\"tls_required\":false,\"name\":\"...\"}\r\n", + "CONNECT {\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"s3cr3t\",\"pass\":\"[REDACTED]\",\"tls_required\":false,\"name\":\"...\"}\r\n", + }, + { + "complete connect with user password and name all the same", + "CONNECT {\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"s3cr3t\",\"pass\":\"s3cr3t\",\"tls_required\":false,\"name\":\"s3cr3t\"}\r\n", + "CONNECT {\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"s3cr3t\",\"pass\":\"[REDACTED]\",\"tls_required\":false,\"name\":\"s3cr3t\"}\r\n", + }, + { + "complete connect extra white space at the beginning", + "CONNECT {\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"s3cr3t\",\"pass\":\"s3cr3t\",\"tls_required\":false,\"name\":\"foo\"}\r\n", + "CONNECT {\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"s3cr3t\",\"pass\":\"[REDACTED]\",\"tls_required\":false,\"name\":\"foo\"}\r\n", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + output := removePassFromTrace([]byte(test.input)) + if !bytes.Equal(output, []byte(test.expected)) { + t.Errorf("\nExpected %q\n got: %q", test.expected, string(output)) + } + }) + } }