Skip to content

Commit

Permalink
Merge pull request #776 from wallyqs/connect-pass-regex-fixes
Browse files Browse the repository at this point in the history 
Fixes to remove traced password
  • Loading branch information
@wallyqs
wallyqs committed Oct 13, 2018
2 parents 71eb6d8 + a0fe8fd commit 3b0b139
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 26 deletions.
28 changes: 18 additions & 10 deletions server/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
141 changes: 125 additions & 16 deletions server/log_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
})
}
}

0 comments on commit 3b0b139

Please sign in to comment.