Fix MySQL Plugin password special character escape bug (#8040)

* Fix MySQL password escape bug * Add test * Add debug output * Add debug line * Added debug output * Debug * Debug * Update vendor * Remove debug comments
hashicorp · Jan 7, 2020 · f813caa · f813caa
1 parent 31bbb81
commit f813caa
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 11 deletions.
diff --git a/plugins/database/mysql/mysql_test.go b/plugins/database/mysql/mysql_test.go
@@ -20,7 +20,7 @@ import (
 
 var _ dbplugin.Database = (*MySQL)(nil)
 
-func prepareMySQLTestContainer(t *testing.T, legacy bool) (cleanup func(), retURL string) {
+func prepareMySQLTestContainer(t *testing.T, legacy bool, pw string) (cleanup func(), retURL string) {
 	if os.Getenv("MYSQL_URL") != "" {
 		return func() {}, os.Getenv("MYSQL_URL")
 	}
@@ -35,7 +35,7 @@ func prepareMySQLTestContainer(t *testing.T, legacy bool) (cleanup func(), retUR
 		imageVersion = "5.6"
 	}
 
-	resource, err := pool.Run("mysql", imageVersion, []string{"MYSQL_ROOT_PASSWORD=secret"})
+	resource, err := pool.Run("mysql", imageVersion, []string{"MYSQL_ROOT_PASSWORD=" + pw})
 	if err != nil {
 		t.Fatalf("Could not start local MySQL docker container: %s", err)
 	}
@@ -44,7 +44,7 @@ func prepareMySQLTestContainer(t *testing.T, legacy bool) (cleanup func(), retUR
 		docker.CleanupResource(t, pool, resource)
 	}
 
-	retURL = fmt.Sprintf("root:secret@(localhost:%s)/mysql?parseTime=true", resource.GetPort("3306/tcp"))
+	retURL = fmt.Sprintf("root:%s@(localhost:%s)/mysql?parseTime=true", pw, resource.GetPort("3306/tcp"))
 
 	// exponential backoff-retry
 	if err = pool.Retry(func() error {
@@ -65,7 +65,7 @@ func prepareMySQLTestContainer(t *testing.T, legacy bool) (cleanup func(), retUR
 }
 
 func TestMySQL_Initialize(t *testing.T) {
-	cleanup, connURL := prepareMySQLTestContainer(t, false)
+	cleanup, connURL := prepareMySQLTestContainer(t, false, "secret")
 	defer cleanup()
 
 	connectionDetails := map[string]interface{}{
@@ -100,7 +100,7 @@ func TestMySQL_Initialize(t *testing.T) {
 }
 
 func TestMySQL_CreateUser(t *testing.T) {
-	cleanup, connURL := prepareMySQLTestContainer(t, false)
+	cleanup, connURL := prepareMySQLTestContainer(t, false, "secret")
 	defer cleanup()
 
 	connectionDetails := map[string]interface{}{
@@ -162,7 +162,7 @@ func TestMySQL_CreateUser(t *testing.T) {
 }
 
 func TestMySQL_CreateUser_Legacy(t *testing.T) {
-	cleanup, connURL := prepareMySQLTestContainer(t, true)
+	cleanup, connURL := prepareMySQLTestContainer(t, true, "secret")
 	defer cleanup()
 
 	connectionDetails := map[string]interface{}{
@@ -211,7 +211,7 @@ func TestMySQL_CreateUser_Legacy(t *testing.T) {
 }
 
 func TestMySQL_RotateRootCredentials(t *testing.T) {
-	cleanup, connURL := prepareMySQLTestContainer(t, false)
+	cleanup, connURL := prepareMySQLTestContainer(t, false, "secret")
 	defer cleanup()
 
 	connURL = strings.Replace(connURL, "root:secret", `{{username}}:{{password}}`, -1)
@@ -247,7 +247,7 @@ func TestMySQL_RotateRootCredentials(t *testing.T) {
 }
 
 func TestMySQL_RevokeUser(t *testing.T) {
-	cleanup, connURL := prepareMySQLTestContainer(t, false)
+	cleanup, connURL := prepareMySQLTestContainer(t, false, "secret")
 	defer cleanup()
 
 	connectionDetails := map[string]interface{}{
@@ -311,7 +311,7 @@ func TestMySQL_RevokeUser(t *testing.T) {
 }
 
 func TestMySQL_SetCredentials(t *testing.T) {
-	cleanup, connURL := prepareMySQLTestContainer(t, false)
+	cleanup, connURL := prepareMySQLTestContainer(t, false, "secret")
 	defer cleanup()
 
 	// create the database user and verify we can access
@@ -368,6 +368,35 @@ func TestMySQL_SetCredentials(t *testing.T) {
 	}
 }
 
+func TestMySQL_Initialize_ReservedChars(t *testing.T) {
+	pw := "#secret!%25#{@}"
+	cleanup, connURL := prepareMySQLTestContainer(t, false, pw)
+	defer cleanup()
+
+	// Revert password set to test replacement by db.Init
+	connURL = strings.ReplaceAll(connURL, pw, "{{password}}")
+
+	connectionDetails := map[string]interface{}{
+		"connection_url": connURL,
+		"password":       pw,
+	}
+
+	db := new(MetadataLen, MetadataLen, UsernameLen)
+	_, err := db.Init(context.Background(), connectionDetails, true)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	if !db.Initialized {
+		t.Fatal("Database should be initialized")
+	}
+
+	err = db.Close()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+}
+
 func testCredsExist(t testing.TB, connURL, username, password string) error {
 	// Log in with the new creds
 	connURL = strings.Replace(connURL, "root:secret", fmt.Sprintf("%s:%s", username, password), 1)

diff --git a/sdk/database/helper/connutil/sql.go b/sdk/database/helper/connutil/sql.go
@@ -55,11 +55,17 @@ func (c *SQLConnectionProducer) Init(ctx context.Context, conf map[string]interf
 		return nil, fmt.Errorf("connection_url cannot be empty")
 	}
 
+	// Don't escape special characters for MySQL password
+	password := c.Password
+	if c.Type != "mysql" {
+		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),
-		"password": url.PathEscape(c.Password),
+		"password": password,
 	})
 
 	if c.MaxOpenConnections == 0 {

diff --git a/vendor/github.com/hashicorp/vault/sdk/database/helper/connutil/sql.go b/vendor/github.com/hashicorp/vault/sdk/database/helper/connutil/sql.go