From 00ab56060a6ef56a9e0e104cf2e3afbec6705562 Mon Sep 17 00:00:00 2001 From: Sean Chittenden Date: Sun, 3 Jul 2016 16:01:39 -0700 Subject: [PATCH 1/2] Use `lib/pq`'s `QuoteIdentifier()` on all identifiers and Prepare for all literals. --- builtin/logical/postgresql/backend_test.go | 5 ++-- builtin/logical/postgresql/secret_creds.go | 31 +++++++++++----------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/builtin/logical/postgresql/backend_test.go b/builtin/logical/postgresql/backend_test.go index eee77ef0a5..f622a4d514 100644 --- a/builtin/logical/postgresql/backend_test.go +++ b/builtin/logical/postgresql/backend_test.go @@ -238,9 +238,8 @@ func testAccStepReadCreds(t *testing.T, b logical.Backend, s logical.Storage, na } returnedRows := func() int { - stmt, err := db.Prepare(fmt.Sprintf( - "SELECT DISTINCT schemaname FROM pg_tables WHERE has_table_privilege('%s', 'information_schema.role_column_grants', 'select');", - d.Username)) + stmt, err := db.Prepare("SELECT DISTINCT schemaname FROM pg_tables WHERE has_table_privilege($1, 'information_schema.role_column_grants', 'select');", + d.Username) if err != nil { return -1 } diff --git a/builtin/logical/postgresql/secret_creds.go b/builtin/logical/postgresql/secret_creds.go index 5b78faaa6a..11acb27ae6 100644 --- a/builtin/logical/postgresql/secret_creds.go +++ b/builtin/logical/postgresql/secret_creds.go @@ -99,8 +99,7 @@ func (b *backend) secretCredsRevoke( // Check if the role exists var exists bool - query := fmt.Sprintf("SELECT exists (SELECT rolname FROM pg_roles WHERE rolname='%s');", username) - err = db.QueryRow(query).Scan(&exists) + err = db.QueryRow("SELECT exists (SELECT rolname FROM pg_roles WHERE rolname=$1);", username).Scan(&exists) if err != nil && err != sql.ErrNoRows { return nil, err } @@ -113,9 +112,7 @@ func (b *backend) secretCredsRevoke( // the role // This isn't done in a transaction because even if we fail along the way, // we want to remove as much access as possible - stmt, err := db.Prepare(fmt.Sprintf( - "SELECT DISTINCT table_schema FROM information_schema.role_column_grants WHERE grantee='%s';", - username)) + stmt, err := db.Prepare("SELECT DISTINCT table_schema FROM information_schema.role_column_grants WHERE grantee=$1;", username) if err != nil { return nil, err } @@ -127,7 +124,8 @@ func (b *backend) secretCredsRevoke( } defer rows.Close() - var revocationStmts []string + const initialNumRevocations = 16 + revocationStmts := make([]string, 0, initialNumRevocations) for rows.Next() { var schema string err = rows.Scan(&schema) @@ -136,21 +134,23 @@ func (b *backend) secretCredsRevoke( continue } revocationStmts = append(revocationStmts, fmt.Sprintf( - "REVOKE ALL PRIVILEGES ON ALL TABLES IN SCHEMA %s FROM %s;", - schema, pq.QuoteIdentifier(username))) + `REVOKE ALL PRIVILEGES ON ALL TABLES IN SCHEMA %s FROM %s;`, + pq.QuoteIdentifier(schema), + pq.QuoteIdentifier(username))) revocationStmts = append(revocationStmts, fmt.Sprintf( - "REVOKE USAGE ON SCHEMA %s FROM %s;", - schema, pq.QuoteIdentifier(username))) + `REVOKE USAGE ON SCHEMA %s FROM %s;`, + pq.QuoteIdentifier(schema), + pq.QuoteIdentifier(username))) } // for good measure, revoke all privileges and usage on schema public revocationStmts = append(revocationStmts, fmt.Sprintf( - "REVOKE ALL PRIVILEGES ON ALL TABLES IN SCHEMA public FROM %s;", + `REVOKE ALL PRIVILEGES ON ALL TABLES IN SCHEMA public FROM %s;`, pq.QuoteIdentifier(username))) revocationStmts = append(revocationStmts, fmt.Sprintf( - "REVOKE USAGE ON SCHEMA public FROM %s;", + `REVOKE USAGE ON SCHEMA public FROM %s;`, pq.QuoteIdentifier(username))) // get the current database name so we can issue a REVOKE CONNECT for @@ -162,8 +162,9 @@ func (b *backend) secretCredsRevoke( if dbname.Valid { revocationStmts = append(revocationStmts, fmt.Sprintf( - "REVOKE CONNECT ON DATABASE %s FROM %s;", - dbname.String, pq.QuoteIdentifier(username))) + `REVOKE CONNECT ON DATABASE %s FROM %s;`, + pq.QuoteIdentifier(dbname.String), + pq.QuoteIdentifier(username))) } // again, here, we do not stop on error, as we want to remove as @@ -192,7 +193,7 @@ func (b *backend) secretCredsRevoke( // Drop this user stmt, err = db.Prepare(fmt.Sprintf( - "DROP ROLE IF EXISTS %s;", pq.QuoteIdentifier(username))) + `DROP ROLE IF EXISTS %s;`, pq.QuoteIdentifier(username))) if err != nil { return nil, err } From f66cd755831e940dbe7d01395e03467f43e68eec Mon Sep 17 00:00:00 2001 From: Sean Chittenden Date: Sun, 3 Jul 2016 16:20:27 -0700 Subject: [PATCH 2/2] Move the parameter down to where the statement is executed. --- builtin/logical/postgresql/backend_test.go | 5 ++--- builtin/logical/postgresql/secret_creds.go | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/builtin/logical/postgresql/backend_test.go b/builtin/logical/postgresql/backend_test.go index f622a4d514..3e90eb1d9d 100644 --- a/builtin/logical/postgresql/backend_test.go +++ b/builtin/logical/postgresql/backend_test.go @@ -238,14 +238,13 @@ func testAccStepReadCreds(t *testing.T, b logical.Backend, s logical.Storage, na } returnedRows := func() int { - stmt, err := db.Prepare("SELECT DISTINCT schemaname FROM pg_tables WHERE has_table_privilege($1, 'information_schema.role_column_grants', 'select');", - d.Username) + stmt, err := db.Prepare("SELECT DISTINCT schemaname FROM pg_tables WHERE has_table_privilege($1, 'information_schema.role_column_grants', 'select');") if err != nil { return -1 } defer stmt.Close() - rows, err := stmt.Query() + rows, err := stmt.Query(d.Username) if err != nil { return -1 } diff --git a/builtin/logical/postgresql/secret_creds.go b/builtin/logical/postgresql/secret_creds.go index 11acb27ae6..5d127aca63 100644 --- a/builtin/logical/postgresql/secret_creds.go +++ b/builtin/logical/postgresql/secret_creds.go @@ -112,13 +112,13 @@ func (b *backend) secretCredsRevoke( // the role // This isn't done in a transaction because even if we fail along the way, // we want to remove as much access as possible - stmt, err := db.Prepare("SELECT DISTINCT table_schema FROM information_schema.role_column_grants WHERE grantee=$1;", username) + stmt, err := db.Prepare("SELECT DISTINCT table_schema FROM information_schema.role_column_grants WHERE grantee=$1;") if err != nil { return nil, err } defer stmt.Close() - rows, err := stmt.Query() + rows, err := stmt.Query(username) if err != nil { return nil, err }