Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Redact password #7198

Merged
merged 2 commits into from
Dec 18, 2020
Merged

Redact password #7198

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
52969a5
redact password
5antelope Dec 16, 2020
479a0ec
redact fix length
5antelope Dec 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions go/vt/mysqlctl/query.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,10 @@ func limitString(s string, limit int) string {
func (mysqld *Mysqld) executeSuperQueryListConn(ctx context.Context, conn *dbconnpool.PooledDBConnection, queryList []string) error {
const LogQueryLengthLimit = 200
for _, query := range queryList {
log.Infof("exec %s", limitString(redactMasterPassword(query), LogQueryLengthLimit))
log.Infof("exec %s", limitString(redactPassword(query), LogQueryLengthLimit))
if _, err := mysqld.executeFetchContext(ctx, conn, query, 10000, false); err != nil {
log.Errorf("ExecuteFetch(%v) failed: %v", redactMasterPassword(query), redactMasterPassword(err.Error()))
return fmt.Errorf("ExecuteFetch(%v) failed: %v", redactMasterPassword(query), redactMasterPassword(err.Error()))
log.Errorf("ExecuteFetch(%v) failed: %v", redactPassword(query), redactPassword(err.Error()))
return fmt.Errorf("ExecuteFetch(%v) failed: %v", redactPassword(query), redactPassword(err.Error()))
}
}
return nil
Expand Down Expand Up @@ -213,16 +213,28 @@ func (mysqld *Mysqld) fetchVariables(ctx context.Context, pattern string) (map[s
const (
masterPasswordStart = " MASTER_PASSWORD = '"
masterPasswordEnd = "',\n"
passwordStart = " PASSWORD = '"
passwordEnd = "'"
)

func redactMasterPassword(input string) string {
func redactPassword(input string) string {
i := strings.Index(input, masterPasswordStart)
// We have master password in the query, try to redact it
if i != -1 {
j := strings.Index(input[i+len(masterPasswordStart):], masterPasswordEnd)
if j == -1 {
return input
}
input = input[:i+len(masterPasswordStart)] + strings.Repeat("*", j) + input[i+len(masterPasswordStart)+j:]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we preserving the lenght of the password, should we not keep it a constant length? We do not want people to be able to infer the length either from the logs.
Otherwise, LGTM

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good point - updated

}
// We also check if we have any password keyword in the query
i = strings.Index(input, passwordStart)
if i == -1 {
return input
}
j := strings.Index(input[i+len(masterPasswordStart):], masterPasswordEnd)
j := strings.Index(input[i+len(passwordStart):], passwordEnd)
if j == -1 {
return input
}
return input[:i+len(masterPasswordStart)] + strings.Repeat("*", j) + input[i+len(masterPasswordStart)+j:]
return input[:i+len(passwordStart)] + strings.Repeat("*", j) + input[i+len(passwordStart)+j:]
}
28 changes: 26 additions & 2 deletions go/vt/mysqlctl/replication_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import (
)

func testRedacted(t *testing.T, source, expected string) {
if r := redactMasterPassword(source); r != expected {
t.Errorf("redactMasterPassword bad result: %v\nWas expecting:%v", r, expected)
if r := redactPassword(source); r != expected {
t.Errorf("redactPassword bad result: %v\nWas expecting:%v", r, expected)
}
}

Expand Down Expand Up @@ -56,3 +56,27 @@ func TestRedactMasterPassword(t *testing.T) {
MASTER_PASSWORD = 'AAA`, `CHANGE MASTER TO
MASTER_PASSWORD = 'AAA`)
}

func TestRedactPassword(t *testing.T) {
// regular case
testRedacted(t, `START xxx USER = 'vt_repl', PASSWORD = 'AAA'`,
`START xxx USER = 'vt_repl', PASSWORD = '***'`)

// empty password
testRedacted(t, `START xxx USER = 'vt_repl', PASSWORD = ''`,
`START xxx USER = 'vt_repl', PASSWORD = ''`)

// no end match
testRedacted(t, `START xxx USER = 'vt_repl', PASSWORD = 'AAA`,
`START xxx USER = 'vt_repl', PASSWORD = 'AAA`)

// both master password and password
testRedacted(t, `START xxx
MASTER_PASSWORD = 'AAA',
PASSWORD = 'BBB'
`,
`START xxx
MASTER_PASSWORD = '***',
PASSWORD = '***'
`)
}