Skip to content

Commit

Permalink
Filter traced passwords from parsed JSON
Browse files Browse the repository at this point in the history
Signed-off-by: Waldemar Quevedo <wally@synadia.com>
  • Loading branch information
wallyqs committed Oct 4, 2018
1 parent a63fb8b commit e2b4b1c
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 71 deletions.
49 changes: 19 additions & 30 deletions server/client.go
Expand Up @@ -21,7 +21,6 @@ import (
"io"
"math/rand"
"net"
"regexp"
"strings"
"sync"
"sync/atomic"
Expand Down Expand Up @@ -754,42 +753,32 @@ func (c *client) processErr(errStr string) {
c.closeConnection(ParseError)
}

// Password pattern matcher.
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")) {
return arg
}
// Take a copy of the connect proto just for the trace message.
a := make([]byte, len(arg))
copy(a, arg)

m := passPat.FindAllSubmatchIndex(a, -1)
if len(m) == 0 {
return a
m := map[string]interface{}{}
err := json.Unmarshal(arg, &m)
if err != nil {
// Leave as is to be able to trace malformed connect
// messages.
return arg
}

j := 0
redactedPass := []byte("[REDACTED]")
redactedPassLen := len(redactedPass)
for _, i := range m {
if len(i) < 6 {
continue
}
// Need to shift the index in case password appears more than once,
// since the substrings are being removed in place.
start := i[2] + j
end := i[5] + j
passLen := end - start
j += redactedPassLen - passLen

// Replace all possible password substrings.
a = append(a[:start], append(redactedPass, a[end:]...)...)
redactedPass := "[REDACTED]"
if _, ok := m["password"]; ok {
m["password"] = redactedPass
}
if _, ok := m["pass"]; ok {
m["pass"] = redactedPass
}
result, err := json.Marshal(m)
if err != nil {
return arg
}
return a
return result
}

func (c *client) processConnect(arg []byte) error {
Expand Down
100 changes: 59 additions & 41 deletions server/log_test.go
Expand Up @@ -219,89 +219,107 @@ func TestNoPasswordsFromConnectTrace(t *testing.T) {

func TestRemovePassFromTrace(t *testing.T) {
tests := []struct {
name string
input string
expected string
}{
{
"CONNECT {\"user\":\"derek\",\"pass\":\"s3cr3t\"}\r\n",
"CONNECT {\"user\":\"derek\",\"pass\":\"[REDACTED]\"}\r\n",
"user and pass",
`{"user":"derek","pass":"s3cr3t"}`,
`{"pass":"[REDACTED]","user":"derek"}`,
},
{
"CONNECT {\"user\":\"derek\",\"pass\": \"s3cr3t\"}\r\n",
"CONNECT {\"user\":\"derek\",\"pass\": \"[REDACTED]\"}\r\n",
"user and pass",
"{\"user\":\"derek\",\"pass\": \"s3cr3t\" }",
"{\"pass\":\"[REDACTED]\",\"user\":\"derek\"}",
},
{
"CONNECT {\"user\":\"derek\",\"pass\": \"s3cr3t\" }\r\n",
"CONNECT {\"user\":\"derek\",\"pass\": \"[REDACTED]\" }\r\n",
"user and password",
`{"user":"derek","password": "s3cr3t"}`,
`{"password":"[REDACTED]","user":"derek"}`,
},
{
"CONNECT {\"pass\":\"s3cr3t\",}\r\n",
"CONNECT {\"pass\":\"[REDACTED]\",}\r\n",
"only pass",
"{\"pass\":\"s3cr3t\"}",
"{\"pass\":\"[REDACTED]\"}",
},
{
"CONNECT {pass:s3cr3t , password = s3cr3t}",
"CONNECT {pass:[REDACTED], password = [REDACTED]}",
"only password",
"{\"password\":\"s3cr3t\"}",
"{\"password\":\"[REDACTED]\"}",
},
{
"CONNECT {pass:s3cr3t , password= s3cr3t}",
"CONNECT {pass:[REDACTED], password= [REDACTED]}",
"pass and password",
`{"pass":"s3cr3t","password":"s3cr3t"}`,
`{"pass":"[REDACTED]","password":"[REDACTED]"}`,
},
{
`CONNECT {"pass":"s3cr3t4", "password": "s3cr3t4"}`,
`CONNECT {"pass":"[REDACTED]", "password": "[REDACTED]"}`,
"long password",
"{\"pass\":\"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\"}",
"{\"pass\":\"[REDACTED]\"}",
},
{
"CONNECT {user = hello, password = s3cr3t}",
"CONNECT {user = hello, password = [REDACTED]}",
"multiple passwords",
"{\"pass\":\"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\",\"pass\":\"BBBBBBBBBBBBBBBBBBBB\",\"password\":\"CCCCCCCCCCCCCCCC\"}",
"{\"pass\":\"[REDACTED]\",\"password\":\"[REDACTED]\"}",
},
// NOTE: In the result the keys are changed to be in alphabetical order
{
"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",
"full connect example",
"{\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"foo\",\"pass\":\"s3cr3t\",\"tls_required\":false,\"name\":\"APM7JU94z77YzP6WTBEiuw\"}",
"{\"echo\":true,\"name\":\"APM7JU94z77YzP6WTBEiuw\",\"pass\":\"[REDACTED]\",\"pedantic\":false,\"tls_required\":false,\"user\":\"foo\",\"verbose\":false}",
},
{
"CONNECT {pass:s3cr3t\r\n",
"CONNECT {pass:[REDACTED]\r\n",
"full connect sample",
"{\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"foo\",\"password\":\"s3cr3t\",\"tls_required\":false,\"name\":\"APM7JU94z77YzP6WTBEiuw\"}",
"{\"echo\":true,\"name\":\"APM7JU94z77YzP6WTBEiuw\",\"password\":\"[REDACTED]\",\"pedantic\":false,\"tls_required\":false,\"user\":\"foo\",\"verbose\":false}",
},
{
"CONNECT {\"password\":\"s3cr3t\",}\r\n",
"CONNECT {\"password\":\"[REDACTED]\",}\r\n",
"user and password are the same",
"{\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"s3cr3t\",\"password\":\"s3cr3t\",\"tls_required\":false,\"name\":\"...\"}",
"{\"echo\":true,\"name\":\"...\",\"password\":\"[REDACTED]\",\"pedantic\":false,\"tls_required\":false,\"user\":\"s3cr3t\",\"verbose\":false}",
},
{
"CONNECT {\"password\":\"very secret password which is very long\",}\r\n",
"CONNECT {\"password\":\"[REDACTED]\",}\r\n",
"user, name and password are the same",
"{\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"s3cr3t\",\"pass\":\"s3cr3t\",\"tls_required\":false,\"name\":\"s3cr3t\"}\r\n",
"{\"echo\":true,\"name\":\"s3cr3t\",\"pass\":\"[REDACTED]\",\"pedantic\":false,\"tls_required\":false,\"user\":\"s3cr3t\",\"verbose\":false}",
},
// No password leaves the connect message as is so keys are not sorted
{
"CONNECT {\"pass\":\"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\"}\r\n",
"CONNECT {\"pass\":\"[REDACTED]\"}\r\n",
"full connect example",
"{\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"foo\",\"tls_required\":false,\"name\":\"APM7JU94z77YzP6WTBEiuw\"}",
"{\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"foo\",\"tls_required\":false,\"name\":\"APM7JU94z77YzP6WTBEiuw\"}",
},
// If JSON payload is invalid then traced the message as is
{
"CONNECT {\"pass\":\"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\",\"pass\":\"BBBBBBBBBBBBBBBBBBBB\",\"password\":\"CCCCCCCCCCCCCCCC\"}\r\n",
"CONNECT {\"pass\":\"[REDACTED]\",\"pass\":\"[REDACTED]\",\"password\":\"[REDACTED]\"}\r\n",
"invalid JSON",
"{pass:s3cr3t , password= s3cr3t}\r\n",
"{pass:s3cr3t , password= s3cr3t}\r\n",
},
{
"CONNECT {pass = \"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\",pass= \"BBBBBBBBBBBBBBBBBBBB\",password =\"CCCCCCCCCCCCCCCC\"}\r\n",
"CONNECT {pass = \"[REDACTED]\",pass= \"[REDACTED]\",password =\"[REDACTED]\"}\r\n",
"invalid JSON",
"{user = hello, password = s3cr3t}",
"{user = hello, password = s3cr3t}",
},
{
"CONNECT {\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"foo\",\"password\":\"s3cr3t\",\"tls_required\":false,\"name\":\"APM7JU94z77YzP6WTBEiuw\"}\r\n",
"CONNECT {\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"foo\",\"password\":\"[REDACTED]\",\"tls_required\":false,\"name\":\"APM7JU94z77YzP6WTBEiuw\"}\r\n",
"invalid JSON",
"{pass:s3cr3t\r\n",
"{pass:s3cr3t\r\n",
},
{
"CONNECT {\"user\":\"s3cr3t\",\"pass\":\"s3cr3t\"}\r\n",
"CONNECT {\"user\":\"s3cr3t\",\"pass\":\"[REDACTED]\"}\r\n",
"invalid JSON",
"{\"password\":\"s3cr3t\",}\r\n",
"{\"password\":\"s3cr3t\",}\r\n",
},
{
"CONNECT {\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"s3cr3t\",\"password\":\"s3cr3t\",\"tls_required\":false,\"name\":\"...\"}\r\n",
"CONNECT {\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"s3cr3t\",\"password\":\"[REDACTED]\",\"tls_required\":false,\"name\":\"...\"}\r\n",
},
{
"CONNECT {\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"s3cr3t\",\"password\":\"s3cr3t\",\"tls_required\":false,\"name\":\"s3cr3t\"}\r\n",
"CONNECT {\"echo\":true,\"verbose\":false,\"pedantic\":false,\"user\":\"s3cr3t\",\"password\":\"[REDACTED]\",\"tls_required\":false,\"name\":\"s3cr3t\"}\r\n",
"invalid JSON",
"{pass: \"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\",pass= \"BBBBBBBBBBBBBBBBBBBB\",password =\"CCCCCCCCCCCCCCCC\"}\r\n",
"{pass: \"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\",pass= \"BBBBBBBBBBBBBBBBBBBB\",password =\"CCCCCCCCCCCCCCCC\"}\r\n",
},
}

for _, test := range tests {
t.Run("", func(t *testing.T) {
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))
Expand Down

0 comments on commit e2b4b1c

Please sign in to comment.