Improve query sanitization to prevent a password leak in the logs

CHANGELOG.md

@@ -37,6 +37,7 @@
 - [#6419](https://github.com/influxdata/influxdb/issues/6419): Fix panic in transform iterator on division. @thbourlove
 - [#6109](https://github.com/influxdata/influxdb/issues/6109): Cache maximum memory size exceeded on startup
 - [#6427](https://github.com/influxdata/influxdb/pull/6427): Fix setting uint config options via env vars
+- [#3883](https://github.com/influxdata/influxdb/issues/3883): Improve query sanitization to prevent a password leak in the logs.
 
 ## v0.12.1 [2016-04-08]
 

influxql/sanitize.go

@@ -0,0 +1,47 @@
+package influxql
+
+import (
+	"bytes"
+	"regexp"
+)
+
+var (
+	sanitizeSetPassword = regexp.MustCompile(`(?i)password\s+for[^=]*=\s+(["']?[^\s"]+["']?)`)
+
+	sanitizeCreatePassword = regexp.MustCompile(`(?i)with\s+password\s+(["']?[^\s"]+["']?)`)
+)
+
+// Sanitize attempts to sanitize passwords out of a raw query.
+// It looks for patterns that may be related to the SET PASSWORD and CREATE USER
+// statements and will redact the password that should be there. It will attempt
+// to redact information from common invalid queries too, but it's not guaranteed
+// to succeed on improper queries.
+//
+// This function works on the raw query and attempts to retain the original input
+// as much as possible.
+func Sanitize(query string) string {
+	if matches := sanitizeSetPassword.FindAllStringSubmatchIndex(query, -1); matches != nil {
+		var buf bytes.Buffer
+		i := 0
+		for _, match := range matches {
+			buf.WriteString(query[i:match[2]])
+			buf.WriteString("[REDACTED]")
+			i = match[3]
+		}
+		buf.WriteString(query[i:])
+		query = buf.String()
+	}
+
+	if matches := sanitizeCreatePassword.FindAllStringSubmatchIndex(query, -1); matches != nil {
+		var buf bytes.Buffer
+		i := 0
+		for _, match := range matches {
+			buf.WriteString(query[i:match[2]])
+			buf.WriteString("[REDACTED]")
+			i = match[3]
+		}
+		buf.WriteString(query[i:])
+		query = buf.String()
+	}
+	return query
+}

influxql/sanitize_test.go

@@ -0,0 +1,49 @@
+package influxql_test
+
+import (
+	"testing"
+
+	"github.com/influxdata/influxdb/influxql"
+)
+
+func TestSanitize(t *testing.T) {
+	var tests = []struct {
+		s    string
+		stmt string
+	}{
+		// Proper statements that should be redacted.
+		{
+			s:    `create user "admin" with password 'admin'`,
+			stmt: `create user "admin" with password [REDACTED]`,
+		},
+		{
+			s:    `set password for "admin" = 'admin'`,
+			stmt: `set password for "admin" = [REDACTED]`,
+		},
+
+		// Common invalid statements that should still be redacted.
+		{
+			s:    `create user "admin" with password "admin"`,
+			stmt: `create user "admin" with password [REDACTED]`,
+		},
+		{
+			s:    `set password for "admin" = "admin"`,
+			stmt: `set password for "admin" = [REDACTED]`,
+		},
+	}
+
+	for i, tt := range tests {
+		stmt := influxql.Sanitize(tt.s)
+		if tt.stmt != stmt {
+			t.Errorf("%d. %q\n\nsanitize mismatch:\n\nexp=%#v\n\ngot=%#v\n\n", i, tt.s, tt.stmt, stmt)
+		}
+	}
+}
+
+func BenchmarkSanitize(b *testing.B) {
+	b.ReportAllocs()
+	q := `create user "admin" with password 'admin'; set password for "admin" = 'admin'`
+	for i := 0; i < b.N; i++ {
+		influxql.Sanitize(q)
+	}
+}

services/httpd/handler.go

@@ -257,23 +257,17 @@ func (h *Handler) serveQuery(w http.ResponseWriter, r *http.Request, user *meta.
 	p := influxql.NewParser(strings.NewReader(qp))
 	db := q.Get("db")
 
+	// Sanitize the request query params so it doesn't show up in the response logger.
+	// Do this before anything else so a parsing error doesn't leak passwords.
+	sanitize(r)
+
 	// Parse query from query string.
 	query, err := p.ParseQuery()
 	if err != nil {
 		httpError(w, "error parsing query: "+err.Error(), pretty, http.StatusBadRequest)
 		return
 	}
 
-	// Sanitize statements with passwords.
-	for _, s := range query.Statements {
-		switch stmt := s.(type) {
-		case *influxql.CreateUserStatement:
-			sanitize(r, stmt.Password)
-		case *influxql.SetPasswordUserStatement:
-			sanitize(r, stmt.Password)
-		}
-	}
-
 	// Check authorization.
 	if h.requireAuthentication {
 		if err := h.QueryAuthorizer.AuthorizeQuery(user, query, db); err != nil {

services/httpd/response_logger.go

@@ -7,6 +7,8 @@ import (
 	"strconv"
 	"strings"
 	"time"
+
+	"github.com/influxdata/influxdb/influxql"
 )
 
 type loggingResponseWriter interface {
@@ -156,6 +158,10 @@ func parseUsername(r *http.Request) string {
 }
 
 // Sanitize passwords from query string for logging.
-func sanitize(r *http.Request, s string) {
-	r.URL.RawQuery = strings.Replace(r.URL.RawQuery, s, "[REDACTED]", -1)
+func sanitize(r *http.Request) {
+	values := r.URL.Query()
+	for i, q := range values["q"] {
+		values["q"][i] = influxql.Sanitize(q)
+	}
+	r.URL.RawQuery = values.Encode()
 }