From 746d4b2855fd659a8301a53fa1567ee24ce93dad Mon Sep 17 00:00:00 2001 From: Michel Vocks Date: Tue, 17 Dec 2019 17:56:13 +0100 Subject: [PATCH 1/9] Fix MySQL password escape bug --- sdk/database/helper/connutil/sql.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/sdk/database/helper/connutil/sql.go b/sdk/database/helper/connutil/sql.go index fba5c5fce8ef..796df8b16f8d 100644 --- 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 { From 4520c6aed43173a2d46eec9b35216940a8739013 Mon Sep 17 00:00:00 2001 From: Michel Vocks Date: Wed, 18 Dec 2019 11:40:56 +0100 Subject: [PATCH 2/9] Add test --- plugins/database/mysql/mysql_test.go | 47 ++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/plugins/database/mysql/mysql_test.go b/plugins/database/mysql/mysql_test.go index 9825408d8212..8f7ea22ade66 100644 --- 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) From 9996d4f44d6bc2d1ccf1270f097593e9ebaa1abc Mon Sep 17 00:00:00 2001 From: Michel Vocks Date: Thu, 19 Dec 2019 15:54:09 +0100 Subject: [PATCH 3/9] Add debug output --- plugins/database/mysql/mysql_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/database/mysql/mysql_test.go b/plugins/database/mysql/mysql_test.go index 8f7ea22ade66..1ce38f2ef47e 100644 --- a/plugins/database/mysql/mysql_test.go +++ b/plugins/database/mysql/mysql_test.go @@ -384,7 +384,7 @@ func TestMySQL_Initialize_ReservedChars(t *testing.T) { db := new(MetadataLen, MetadataLen, UsernameLen) _, err := db.Init(context.Background(), connectionDetails, true) if err != nil { - t.Fatalf("err: %s", err) + t.Fatalf("connURL: %s; err: %s", connURL, err) } if !db.Initialized { From 870e0f8ca3a460baf3c07b03f1d128d2c2069367 Mon Sep 17 00:00:00 2001 From: Michel Vocks Date: Thu, 19 Dec 2019 16:41:20 +0100 Subject: [PATCH 4/9] Add debug line --- plugins/database/mysql/mysql_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/database/mysql/mysql_test.go b/plugins/database/mysql/mysql_test.go index 1ce38f2ef47e..9823bcbf0da7 100644 --- a/plugins/database/mysql/mysql_test.go +++ b/plugins/database/mysql/mysql_test.go @@ -384,7 +384,7 @@ func TestMySQL_Initialize_ReservedChars(t *testing.T) { db := new(MetadataLen, MetadataLen, UsernameLen) _, err := db.Init(context.Background(), connectionDetails, true) if err != nil { - t.Fatalf("connURL: %s; err: %s", connURL, err) + t.Fatalf("connURL: %s; err: %s", db.ConnectionURL, err) } if !db.Initialized { From a23b159e0ac42c187ec83d5d298394c1edc61640 Mon Sep 17 00:00:00 2001 From: Michel Vocks Date: Thu, 19 Dec 2019 17:35:02 +0100 Subject: [PATCH 5/9] Added debug output --- sdk/database/helper/connutil/sql.go | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/database/helper/connutil/sql.go b/sdk/database/helper/connutil/sql.go index 796df8b16f8d..1aa6b5df26f5 100644 --- a/sdk/database/helper/connutil/sql.go +++ b/sdk/database/helper/connutil/sql.go @@ -57,6 +57,7 @@ func (c *SQLConnectionProducer) Init(ctx context.Context, conf map[string]interf // Don't escape special characters for MySQL password password := c.Password + fmt.Printf("Type: %s\n", c.Type) if c.Type != "mysql" { password = url.PathEscape(c.Password) } From 31b27add66fdb061c96a21929faf9d87d285cb6e Mon Sep 17 00:00:00 2001 From: Michel Vocks Date: Fri, 20 Dec 2019 11:44:40 +0100 Subject: [PATCH 6/9] Debug --- plugins/database/mysql/mysql_test.go | 2 +- sdk/database/helper/connutil/sql.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/database/mysql/mysql_test.go b/plugins/database/mysql/mysql_test.go index 9823bcbf0da7..227866f7fc05 100644 --- a/plugins/database/mysql/mysql_test.go +++ b/plugins/database/mysql/mysql_test.go @@ -384,7 +384,7 @@ func TestMySQL_Initialize_ReservedChars(t *testing.T) { db := new(MetadataLen, MetadataLen, UsernameLen) _, err := db.Init(context.Background(), connectionDetails, true) if err != nil { - t.Fatalf("connURL: %s; err: %s", db.ConnectionURL, err) + t.Fatalf("Type: %s; connURL: %s; err: %s", db.SQLConnectionProducer.Type, db.ConnectionURL, err) } if !db.Initialized { diff --git a/sdk/database/helper/connutil/sql.go b/sdk/database/helper/connutil/sql.go index 1aa6b5df26f5..796df8b16f8d 100644 --- a/sdk/database/helper/connutil/sql.go +++ b/sdk/database/helper/connutil/sql.go @@ -57,7 +57,6 @@ func (c *SQLConnectionProducer) Init(ctx context.Context, conf map[string]interf // Don't escape special characters for MySQL password password := c.Password - fmt.Printf("Type: %s\n", c.Type) if c.Type != "mysql" { password = url.PathEscape(c.Password) } From 3252379e7df7562ed43162a46c8bc5f116dfe0ef Mon Sep 17 00:00:00 2001 From: Michel Vocks Date: Thu, 2 Jan 2020 08:35:39 +0100 Subject: [PATCH 7/9] Debug --- sdk/database/helper/connutil/sql.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/database/helper/connutil/sql.go b/sdk/database/helper/connutil/sql.go index 796df8b16f8d..e5e078a71346 100644 --- a/sdk/database/helper/connutil/sql.go +++ b/sdk/database/helper/connutil/sql.go @@ -58,7 +58,7 @@ func (c *SQLConnectionProducer) Init(ctx context.Context, conf map[string]interf // Don't escape special characters for MySQL password password := c.Password if c.Type != "mysql" { - password = url.PathEscape(c.Password) + //password = url.PathEscape(c.Password) } // QueryHelper doesn't do any SQL escaping, but if it starts to do so From efdda6e0825ce0d0e0e31d22ee907b90011968a9 Mon Sep 17 00:00:00 2001 From: Michel Vocks Date: Thu, 2 Jan 2020 13:06:19 +0100 Subject: [PATCH 8/9] Update vendor --- plugins/database/mysql/mysql_test.go | 2 +- .../github.com/hashicorp/vault/api/client.go | 20 +++++++++++++++---- .../vault/sdk/database/helper/connutil/sql.go | 8 +++++++- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/plugins/database/mysql/mysql_test.go b/plugins/database/mysql/mysql_test.go index 227866f7fc05..8f7ea22ade66 100644 --- a/plugins/database/mysql/mysql_test.go +++ b/plugins/database/mysql/mysql_test.go @@ -384,7 +384,7 @@ func TestMySQL_Initialize_ReservedChars(t *testing.T) { db := new(MetadataLen, MetadataLen, UsernameLen) _, err := db.Init(context.Background(), connectionDetails, true) if err != nil { - t.Fatalf("Type: %s; connURL: %s; err: %s", db.SQLConnectionProducer.Type, db.ConnectionURL, err) + t.Fatalf("err: %s", err) } if !db.Initialized { diff --git a/vendor/github.com/hashicorp/vault/api/client.go b/vendor/github.com/hashicorp/vault/api/client.go index 38cfbd6d62bc..a6a8bb6091c1 100644 --- a/vendor/github.com/hashicorp/vault/api/client.go +++ b/vendor/github.com/hashicorp/vault/api/client.go @@ -678,6 +678,12 @@ func (c *Client) SetPolicyOverride(override bool) { c.policyOverride = override } +// portMap defines the standard port map +var portMap = map[string]string{ + "http": "80", + "https": "443", +} + // NewRequest creates a new raw request object to query the Vault server // configured for this client. This is an advanced method and generally // doesn't need to be called externally. @@ -694,10 +700,16 @@ func (c *Client) NewRequest(method, requestPath string) *Request { // record and take the highest match; this is not designed for high-availability, just discovery var host string = addr.Host if addr.Port() == "" { - // Internet Draft specifies that the SRV record is ignored if a port is given - _, addrs, err := net.LookupSRV("http", "tcp", addr.Hostname()) - if err == nil && len(addrs) > 0 { - host = fmt.Sprintf("%s:%d", addrs[0].Target, addrs[0].Port) + // Avoid lookup of SRV record if scheme is known + port, ok := portMap[addr.Scheme] + if ok { + host = net.JoinHostPort(host, port) + } else { + // Internet Draft specifies that the SRV record is ignored if a port is given + _, addrs, err := net.LookupSRV("http", "tcp", addr.Hostname()) + if err == nil && len(addrs) > 0 { + host = fmt.Sprintf("%s:%d", addrs[0].Target, addrs[0].Port) + } } } 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 index fba5c5fce8ef..e5e078a71346 100644 --- a/vendor/github.com/hashicorp/vault/sdk/database/helper/connutil/sql.go +++ b/vendor/github.com/hashicorp/vault/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 { From b325c88959d13c28c655d895bd9c361a3b6dcd75 Mon Sep 17 00:00:00 2001 From: Michel Vocks Date: Thu, 2 Jan 2020 13:07:23 +0100 Subject: [PATCH 9/9] Remove debug comments --- sdk/database/helper/connutil/sql.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/database/helper/connutil/sql.go b/sdk/database/helper/connutil/sql.go index e5e078a71346..796df8b16f8d 100644 --- a/sdk/database/helper/connutil/sql.go +++ b/sdk/database/helper/connutil/sql.go @@ -58,7 +58,7 @@ func (c *SQLConnectionProducer) Init(ctx context.Context, conf map[string]interf // Don't escape special characters for MySQL password password := c.Password if c.Type != "mysql" { - //password = url.PathEscape(c.Password) + password = url.PathEscape(c.Password) } // QueryHelper doesn't do any SQL escaping, but if it starts to do so