secrets/database: Add parameter to disable escaping username and password chars for DB connections (#13414)

* Add a parameter that disables escaping characters in the username or password fields for secrets engines database connections

* Always disallow template variables inside the username or password

changelog/13414.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+secrets/database: Add database configuration parameter 'disable_escaping' for username and password when connecting to a database.
+```

sdk/database/helper/connutil/sql.go

@@ -26,6 +26,7 @@ type SQLConnectionProducer struct {
 	MaxConnectionLifetimeRaw interface{} `json:"max_connection_lifetime" mapstructure:"max_connection_lifetime" structs:"max_connection_lifetime"`
 	Username                 string      `json:"username" mapstructure:"username" structs:"username"`
 	Password                 string      `json:"password" mapstructure:"password" structs:"password"`
+	DisableEscaping          bool        `json:"disable_escaping" mapstructure:"disable_escaping" structs:"disable_escaping"`
 
 	Type                  string
 	RawConfig             map[string]interface{}
@@ -55,16 +56,32 @@ func (c *SQLConnectionProducer) Init(ctx context.Context, conf map[string]interf
 		return nil, fmt.Errorf("connection_url cannot be empty")
 	}
 
+	// Do not allow the username or password template pattern to be used as
+	// part of the user-supplied username or password
+	if strings.Contains(c.Username, "{{username}}") ||
+		strings.Contains(c.Username, "{{password}}") ||
+		strings.Contains(c.Password, "{{username}}") ||
+		strings.Contains(c.Password, "{{password}}") {
+
+		return nil, fmt.Errorf("username and/or password cannot contain the template variables")
+	}
+
 	// Don't escape special characters for MySQL password
+	// Also don't escape special characters for the username and password if
+	// the disable_escaping parameter is set to true
+	username := c.Username
 	password := c.Password
-	if c.Type != "mysql" {
+	if !c.DisableEscaping {
+		username = url.PathEscape(c.Username)
+	}
+	if (c.Type != "mysql") && !c.DisableEscaping {
 		password = url.PathEscape(c.Password)
 	}
 
 	// QueryHelper doesn't do any SQL escaping, but if it starts to do so
 	// then maybe we won't be able to use it to do URL substitution any more.
 	c.ConnectionURL = dbutil.QueryHelper(c.ConnectionURL, map[string]string{
-		"username": url.PathEscape(c.Username),
+		"username": username,
 		"password": password,
 	})
 

sdk/database/helper/connutil/sql_test.go

@@ -3,7 +3,10 @@ package connutil
 import (
 	"context"
 	"net/url"
+	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestSQLPasswordChars(t *testing.T) {
@@ -16,9 +19,6 @@ func TestSQLPasswordChars(t *testing.T) {
 		{"postgres", "pass/word"},
 		{"postgres", "p@ssword"},
 		{"postgres", "pass\"word\""},
-		// Much to my surprise, CREATE USER "{{password}}" PASSWORD 'foo' worked.
-		{"{{password}}", "foo"},
-		{"user", "{{username}}"},
 	}
 	for _, tc := range testCases {
 		t.Logf("username %q password %q", tc.Username, tc.Password)
@@ -26,9 +26,10 @@ func TestSQLPasswordChars(t *testing.T) {
 		sql := &SQLConnectionProducer{}
 		ctx := context.Background()
 		conf := map[string]interface{}{
-			"connection_url": "postgres://{{username}}:{{password}}@localhost:5432/mydb",
-			"username":       tc.Username,
-			"password":       tc.Password,
+			"connection_url":   "postgres://{{username}}:{{password}}@localhost:5432/mydb",
+			"username":         tc.Username,
+			"password":         tc.Password,
+			"disable_escaping": false,
 		}
 		_, err := sql.Init(ctx, conf, false)
 		if err != nil {
@@ -54,3 +55,84 @@ func TestSQLPasswordChars(t *testing.T) {
 		}
 	}
 }
+
+func TestSQLDisableEscaping(t *testing.T) {
+	testCases := []struct {
+		Username        string
+		Password        string
+		DisableEscaping bool
+	}{
+		{"mssql{0}", "password{0}", true},
+		{"mssql{0}", "password{0}", false},
+		{"ms\"sql\"", "pass\"word\"", true},
+		{"ms\"sql\"", "pass\"word\"", false},
+		{"ms'sq;l", "pass'wor;d", true},
+		{"ms'sq;l", "pass'wor;d", false},
+	}
+	for _, tc := range testCases {
+		t.Logf("username %q password %q disable_escaling %t", tc.Username, tc.Password, tc.DisableEscaping)
+
+		sql := &SQLConnectionProducer{}
+		ctx := context.Background()
+		conf := map[string]interface{}{
+			"connection_url":   "server=localhost;port=1433;user id={{username}};password={{password}};database=mydb;",
+			"username":         tc.Username,
+			"password":         tc.Password,
+			"disable_escaping": tc.DisableEscaping,
+		}
+		_, err := sql.Init(ctx, conf, false)
+		if err != nil {
+			t.Errorf("Init error on %q %q: %+v", tc.Username, tc.Password, err)
+		} else {
+			if tc.DisableEscaping {
+				if !strings.Contains(sql.ConnectionURL, tc.Username) || !strings.Contains(sql.ConnectionURL, tc.Password) {
+					t.Errorf("Raw username and/or password missing from ConnectionURL")
+				}
+			} else {
+				if strings.Contains(sql.ConnectionURL, tc.Username) || strings.Contains(sql.ConnectionURL, tc.Password) {
+					t.Errorf("Raw username and/or password was present in ConnectionURL")
+				}
+			}
+		}
+	}
+}
+
+func TestSQLDisallowTemplates(t *testing.T) {
+	testCases := []struct {
+		Username string
+		Password string
+	}{
+		{"{{username}}", "pass"},
+		{"{{password}}", "pass"},
+		{"user", "{{username}}"},
+		{"user", "{{password}}"},
+		{"{{username}}", "{{password}}"},
+		{"abc{username}xyz", "123{password}789"},
+		{"abc{{username}}xyz", "123{{password}}789"},
+		{"abc{{{username}}}xyz", "123{{{password}}}789"},
+	}
+	for _, disableEscaping := range []bool{true, false} {
+		for _, tc := range testCases {
+			t.Logf("username %q password %q disable_escaping %t", tc.Username, tc.Password, disableEscaping)
+
+			sql := &SQLConnectionProducer{}
+			ctx := context.Background()
+			conf := map[string]interface{}{
+				"connection_url":   "server=localhost;port=1433;user id={{username}};password={{password}};database=mydb;",
+				"username":         tc.Username,
+				"password":         tc.Password,
+				"disable_escaping": disableEscaping,
+			}
+			_, err := sql.Init(ctx, conf, false)
+			if disableEscaping {
+				if err != nil {
+					if !assert.EqualError(t, err, "username and/or password cannot contain the template variables") {
+						t.Errorf("Init error on %q %q: %+v", tc.Username, tc.Password, err)
+					}
+				} else {
+					assert.Equal(t, sql.ConnectionURL, "server=localhost;port=1433;user id=abc{username}xyz;password=123{password}789;database=mydb;")
+				}
+			}
+		}
+	}
+}