From 506a04b56c4d7bfd330b0fdb637eb82f12702c57 Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Thu, 20 Jun 2019 16:20:44 -0500 Subject: [PATCH 1/4] remove create/update database user for static accounts --- builtin/logical/database/path_roles.go | 227 ++++++++---------- builtin/logical/database/path_roles_test.go | 35 +-- builtin/logical/database/rotation_test.go | 133 ++++++---- plugins/database/postgresql/postgresql.go | 17 +- .../source/api/secret/databases/index.html.md | 30 --- 5 files changed, 195 insertions(+), 247 deletions(-) diff --git a/builtin/logical/database/path_roles.go b/builtin/logical/database/path_roles.go index b6ce344c7ef9..aa2a4924054b 100644 --- a/builtin/logical/database/path_roles.go +++ b/builtin/logical/database/path_roles.go @@ -2,7 +2,6 @@ package database import ( "context" - "errors" "fmt" "strings" "time" @@ -88,32 +87,6 @@ func fieldsForType(roleType string) map[string]*framework.FieldSchema { Type: framework.TypeString, Description: "Name of the database this role acts on.", }, - "creation_statements": { - Type: framework.TypeStringSlice, - Description: `Specifies the database statements executed to - create and configure a user. See the plugin's API page for more - information on support and formatting for this parameter.`, - }, - "revocation_statements": { - Type: framework.TypeStringSlice, - Description: `Specifies the database statements to be executed - to revoke a user. See the plugin's API page for more information - on support and formatting for this parameter.`, - }, - "renew_statements": { - Type: framework.TypeStringSlice, - Description: `Specifies the database statements to be executed - to renew a user. Not every plugin type will support this - functionality. See the plugin's API page for more information on - support and formatting for this parameter. `, - }, - "rollback_statements": { - Type: framework.TypeStringSlice, - Description: `Specifies the database statements to be executed - rollback a create operation in the event of an error. Not every plugin - type will support this functionality. See the plugin's API page for - more information on support and formatting for this parameter.`, - }, } // Get the fields that are specific to the type of role, and add them to the @@ -141,11 +114,36 @@ func dynamicFields() map[string]*framework.FieldSchema { Type: framework.TypeDurationSecond, Description: "Default ttl for role.", }, - "max_ttl": { Type: framework.TypeDurationSecond, Description: "Maximum time a credential is valid for", }, + "creation_statements": { + Type: framework.TypeStringSlice, + Description: `Specifies the database statements executed to + create and configure a user. See the plugin's API page for more + information on support and formatting for this parameter.`, + }, + "revocation_statements": { + Type: framework.TypeStringSlice, + Description: `Specifies the database statements to be executed + to revoke a user. See the plugin's API page for more information + on support and formatting for this parameter.`, + }, + "renew_statements": { + Type: framework.TypeStringSlice, + Description: `Specifies the database statements to be executed + to renew a user. Not every plugin type will support this + functionality. See the plugin's API page for more information on + support and formatting for this parameter. `, + }, + "rollback_statements": { + Type: framework.TypeStringSlice, + Description: `Specifies the database statements to be executed + rollback a create operation in the event of an error. Not every plugin + type will support this functionality. See the plugin's API page for + more information on support and formatting for this parameter.`, + }, } return fields } @@ -172,13 +170,6 @@ func staticFields() map[string]*framework.FieldSchema { this functionality. See the plugin's API page for more information on support and formatting for this parameter.`, }, - "revoke_user_on_delete": { - Type: framework.TypeBool, - Default: false, - Description: `Revoke the database user identified by the username when - this Role is deleted. Revocation will use the configured - revocation_statements if provided. Default false.`, - }, } return fields } @@ -219,34 +210,7 @@ func (b *databaseBackend) pathStaticRoleDelete(ctx context.Context, req *logical // Remove the item from the queue _, _ = b.popFromRotationQueueByKey(name) - // If this role is a static account, we need to revoke the user from the - // database - role, err := b.StaticRole(ctx, req.Storage, name) - if err != nil { - return nil, err - } - if role == nil { - return nil, nil - } - - // Clean up the static useraccount, if it exists - revoke := role.StaticAccount.RevokeUserOnDelete - if revoke { - db, err := b.GetConnection(ctx, req.Storage, role.DBName) - if err != nil { - return nil, err - } - - db.RLock() - defer db.RUnlock() - - if err := db.RevokeUser(ctx, role.Statements, role.StaticAccount.Username); err != nil { - b.CloseIfShutdown(db, err) - return nil, err - } - } - - err = req.Storage.Delete(ctx, databaseStaticRolePath+name) + err := req.Storage.Delete(ctx, databaseStaticRolePath+name) if err != nil { return nil, err } @@ -262,14 +226,19 @@ func (b *databaseBackend) pathStaticRoleRead(ctx context.Context, req *logical.R if role == nil { return nil, nil } - data := pathRoleReadCommon(role) + + data := map[string]interface{}{ + "db_name": role.DBName, + "rotation_statements": role.Statements.Rotation, + } + + // guard against nil StaticAccount; shouldn't happen but we'll be safe if role.StaticAccount != nil { data["username"] = role.StaticAccount.Username data["rotation_period"] = role.StaticAccount.RotationPeriod.Seconds() if !role.StaticAccount.LastVaultRotation.IsZero() { data["last_vault_rotation"] = role.StaticAccount.LastVaultRotation } - data["revoke_user_on_delete"] = role.StaticAccount.RevokeUserOnDelete } return &logical.Response{ @@ -286,12 +255,6 @@ func (b *databaseBackend) pathRoleRead(ctx context.Context, req *logical.Request return nil, nil } - return &logical.Response{ - Data: pathRoleReadCommon(role), - }, nil -} - -func pathRoleReadCommon(role *roleEntry) map[string]interface{} { data := map[string]interface{}{ "db_name": role.DBName, "creation_statements": role.Statements.Creation, @@ -317,7 +280,10 @@ func pathRoleReadCommon(role *roleEntry) map[string]interface{} { if len(role.Statements.Rotation) == 0 { data["rotation_statements"] = []string{} } - return data + + return &logical.Response{ + Data: data, + }, nil } func (b *databaseBackend) pathRoleList(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -355,20 +321,65 @@ func (b *databaseBackend) pathRoleCreateUpdate(ctx context.Context, req *logical role = &roleEntry{} } - if err := pathRoleCreateUpdateCommon(ctx, role, req.Operation, data); err != nil { - return logical.ErrorResponse(err.Error()), nil + createOperation := (req.Operation == logical.CreateOperation) + + // DB Attributes + { + if dbNameRaw, ok := data.GetOk("db_name"); ok { + role.DBName = dbNameRaw.(string) + } else if createOperation { + role.DBName = data.Get("db_name").(string) + } + if role.DBName == "" { + return logical.ErrorResponse("database name is required"), nil + } + } + + // Statements + { + if creationStmtsRaw, ok := data.GetOk("creation_statements"); ok { + role.Statements.Creation = creationStmtsRaw.([]string) + } else if createOperation { + role.Statements.Creation = data.Get("creation_statements").([]string) + } + + if revocationStmtsRaw, ok := data.GetOk("revocation_statements"); ok { + role.Statements.Revocation = revocationStmtsRaw.([]string) + } else if createOperation { + role.Statements.Revocation = data.Get("revocation_statements").([]string) + } + + if rollbackStmtsRaw, ok := data.GetOk("rollback_statements"); ok { + role.Statements.Rollback = rollbackStmtsRaw.([]string) + } else if createOperation { + role.Statements.Rollback = data.Get("rollback_statements").([]string) + } + + if renewStmtsRaw, ok := data.GetOk("renew_statements"); ok { + role.Statements.Renewal = renewStmtsRaw.([]string) + } else if createOperation { + role.Statements.Renewal = data.Get("renew_statements").([]string) + } + + // Do not persist deprecated statements that are populated on role read + role.Statements.CreationStatements = "" + role.Statements.RevocationStatements = "" + role.Statements.RenewStatements = "" + role.Statements.RollbackStatements = "" } + role.Statements.Revocation = strutil.RemoveEmpty(role.Statements.Revocation) + // TTLs { if defaultTTLRaw, ok := data.GetOk("default_ttl"); ok { role.DefaultTTL = time.Duration(defaultTTLRaw.(int)) * time.Second - } else if req.Operation == logical.CreateOperation { + } else if createOperation { role.DefaultTTL = time.Duration(data.Get("default_ttl").(int)) * time.Second } if maxTTLRaw, ok := data.GetOk("max_ttl"); ok { role.MaxTTL = time.Duration(maxTTLRaw.(int)) * time.Second - } else if req.Operation == logical.CreateOperation { + } else if createOperation { role.MaxTTL = time.Duration(data.Get("max_ttl").(int)) * time.Second } } @@ -422,8 +433,15 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l createRole = true } - if err := pathRoleCreateUpdateCommon(ctx, role, req.Operation, data); err != nil { - return logical.ErrorResponse(err.Error()), nil + // DB Attributes + if dbNameRaw, ok := data.GetOk("db_name"); ok { + role.DBName = dbNameRaw.(string) + } else if createRole { + role.DBName = data.Get("db_name").(string) + } + + if role.DBName == "" { + return logical.ErrorResponse("database name is a required field"), nil } username := data.Get("username").(string) @@ -458,8 +476,6 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l role.Statements.Rotation = data.Get("rotation_statements").([]string) } - role.StaticAccount.RevokeUserOnDelete = data.Get("revoke_user_on_delete").(bool) - // lvr represents the roles' LastVaultRotation lvr := role.StaticAccount.LastVaultRotation @@ -504,57 +520,6 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l return nil, nil } -func pathRoleCreateUpdateCommon(ctx context.Context, role *roleEntry, operation logical.Operation, data *framework.FieldData) error { - // DB Attributes - { - if dbNameRaw, ok := data.GetOk("db_name"); ok { - role.DBName = dbNameRaw.(string) - } else if operation == logical.CreateOperation { - role.DBName = data.Get("db_name").(string) - } - if role.DBName == "" { - return errors.New("empty database name attribute") - } - } - - // Statements - { - if creationStmtsRaw, ok := data.GetOk("creation_statements"); ok { - role.Statements.Creation = creationStmtsRaw.([]string) - } else if operation == logical.CreateOperation { - role.Statements.Creation = data.Get("creation_statements").([]string) - } - - if revocationStmtsRaw, ok := data.GetOk("revocation_statements"); ok { - role.Statements.Revocation = revocationStmtsRaw.([]string) - } else if operation == logical.CreateOperation { - role.Statements.Revocation = data.Get("revocation_statements").([]string) - } - - if rollbackStmtsRaw, ok := data.GetOk("rollback_statements"); ok { - role.Statements.Rollback = rollbackStmtsRaw.([]string) - } else if operation == logical.CreateOperation { - role.Statements.Rollback = data.Get("rollback_statements").([]string) - } - - if renewStmtsRaw, ok := data.GetOk("renew_statements"); ok { - role.Statements.Renewal = renewStmtsRaw.([]string) - } else if operation == logical.CreateOperation { - role.Statements.Renewal = data.Get("renew_statements").([]string) - } - - // Do not persist deprecated statements that are populated on role read - role.Statements.CreationStatements = "" - role.Statements.RevocationStatements = "" - role.Statements.RenewStatements = "" - role.Statements.RollbackStatements = "" - } - - role.Statements.Revocation = strutil.RemoveEmpty(role.Statements.Revocation) - - return nil -} - type roleEntry struct { DBName string `json:"db_name"` Statements dbplugin.Statements `json:"statements"` diff --git a/builtin/logical/database/path_roles_test.go b/builtin/logical/database/path_roles_test.go index 085c5cf34d7e..700eb55c5f5b 100644 --- a/builtin/logical/database/path_roles_test.go +++ b/builtin/logical/database/path_roles_test.go @@ -62,17 +62,17 @@ func TestBackend_StaticRole_Config(t *testing.T) { }{ "basic": { account: map[string]interface{}{ - "username": "statictest", + "username": dbUser, "rotation_period": "5400s", }, expected: map[string]interface{}{ - "username": "statictest", + "username": dbUser, "rotation_period": float64(5400), }, }, "missing rotation period": { account: map[string]interface{}{ - "username": "statictest", + "username": dbUser, }, err: errors.New("rotation_period is required to create static accounts"), }, @@ -81,13 +81,9 @@ func TestBackend_StaticRole_Config(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { data := map[string]interface{}{ - "name": "plugin-role-test", - "db_name": "plugin-test", - "creation_statements": testRoleStaticCreate, - "rotation_statements": testRoleStaticUpdate, - "revocation_statements": defaultRevocationSQL, - "default_ttl": "5m", - "max_ttl": "10m", + "name": "plugin-role-test", + "db_name": "plugin-test", + "rotation_statements": testRoleStaticUpdate, } for k, v := range tc.account { @@ -226,15 +222,11 @@ func TestBackend_StaticRole_Updates(t *testing.T) { } data = map[string]interface{}{ - "name": "plugin-role-test-updates", - "db_name": "plugin-test", - "creation_statements": testRoleStaticCreate, - "rotation_statements": testRoleStaticUpdate, - "revocation_statements": defaultRevocationSQL, - "default_ttl": "5m", - "max_ttl": "10m", - "username": "statictest", - "rotation_period": "5400s", + "name": "plugin-role-test-updates", + "db_name": "plugin-test", + "rotation_statements": testRoleStaticUpdate, + "username": dbUser, + "rotation_period": "5400s", } req = &logical.Request{ @@ -285,7 +277,7 @@ func TestBackend_StaticRole_Updates(t *testing.T) { updateData := map[string]interface{}{ "name": "plugin-role-test-updates", "db_name": "plugin-test", - "username": "statictest", + "username": dbUser, "rotation_period": "6400s", } req = &logical.Request{ @@ -340,7 +332,7 @@ func TestBackend_StaticRole_Updates(t *testing.T) { updateData = map[string]interface{}{ "name": "plugin-role-test-updates", "db_name": "plugin-test", - "username": "statictest", + "username": dbUser, "rotation_statements": testRoleStaticUpdateRotation, } req = &logical.Request{ @@ -514,7 +506,6 @@ const testRoleStaticCreate = ` CREATE ROLE "{{name}}" WITH LOGIN PASSWORD '{{password}}'; -GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public TO "{{name}}"; ` const testRoleStaticUpdate = ` diff --git a/builtin/logical/database/rotation_test.go b/builtin/logical/database/rotation_test.go index bec4c5a613e9..4455bb383f7a 100644 --- a/builtin/logical/database/rotation_test.go +++ b/builtin/logical/database/rotation_test.go @@ -2,6 +2,7 @@ package database import ( "context" + "log" "strings" "testing" "time" @@ -10,11 +11,14 @@ import ( "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/sdk/framework" + "github.com/hashicorp/vault/sdk/helper/dbtxn" "github.com/hashicorp/vault/sdk/logical" - _ "github.com/lib/pq" + "github.com/lib/pq" ) +const dbUser = "vaultstatictest" + func TestBackend_StaticRole_Rotate_basic(t *testing.T) { cluster, sys := getCluster(t) defer cluster.Cleanup() @@ -36,6 +40,11 @@ func TestBackend_StaticRole_Rotate_basic(t *testing.T) { cleanup, connURL := preparePostgresTestContainer(t, config.StorageView, b) defer cleanup() + // create the database user + createTestPGUser(t, connURL, dbUser, "password", testRoleStaticCreate) + + verifyPgConn(t, dbUser, "password", connURL) + // Configure a connection data := map[string]interface{}{ "connection_url": connURL, @@ -57,13 +66,11 @@ func TestBackend_StaticRole_Rotate_basic(t *testing.T) { } data = map[string]interface{}{ - "name": "plugin-role-test", - "db_name": "plugin-test", - "creation_statements": testRoleStaticCreate, - "rotation_statements": testRoleStaticUpdate, - "revocation_statements": defaultRevocationSQL, - "username": "statictest", - "rotation_period": "5400s", + "name": "plugin-role-test", + "db_name": "plugin-test", + "rotation_statements": testRoleStaticUpdate, + "username": dbUser, + "rotation_period": "5400s", } req = &logical.Request{ @@ -99,9 +106,7 @@ func TestBackend_StaticRole_Rotate_basic(t *testing.T) { } // Verify username/password - if err := verifyPgConn(t, username, password, connURL); err != nil { - t.Fatal(err) - } + verifyPgConn(t, dbUser, "password", connURL) // Re-read the creds, verifying they aren't changing on read data = map[string]interface{}{} @@ -156,9 +161,7 @@ func TestBackend_StaticRole_Rotate_basic(t *testing.T) { } // Verify new username/password - if err := verifyPgConn(t, username, newPassword, connURL); err != nil { - t.Fatal(err) - } + verifyPgConn(t, username, newPassword, connURL) } // Sanity check to make sure we don't allow an attempt of rotating credentials @@ -245,10 +248,7 @@ func TestBackend_StaticRole_Rotate_NonStaticError(t *testing.T) { } // Verify username/password - if err := verifyPgConn(t, username, password, connURL); err != nil { - t.Fatal(err) - } - + verifyPgConn(t, dbUser, "password", connURL) // Trigger rotation data = map[string]interface{}{"name": "plugin-role-test"} req = &logical.Request{ @@ -331,13 +331,11 @@ func TestBackend_StaticRole_Revoke_user(t *testing.T) { for k, tc := range testCases { t.Run(k, func(t *testing.T) { data = map[string]interface{}{ - "name": "plugin-role-test", - "db_name": "plugin-test", - "creation_statements": testRoleStaticCreate, - "rotation_statements": testRoleStaticUpdate, - "revocation_statements": defaultRevocationSQL, - "username": "statictest", - "rotation_period": "5400s", + "name": "plugin-role-test", + "db_name": "plugin-test", + "rotation_statements": testRoleStaticUpdate, + "username": dbUser, + "rotation_period": "5400s", } if tc.revoke != nil { data["revoke_user_on_delete"] = *tc.revoke @@ -376,9 +374,7 @@ func TestBackend_StaticRole_Revoke_user(t *testing.T) { } // Verify username/password - if err := verifyPgConn(t, username, password, connURL); err != nil { - t.Fatal(err) - } + verifyPgConn(t, username, password, connURL) // delete the role, expect the default where the user is not destroyed // Read the creds @@ -394,25 +390,58 @@ func TestBackend_StaticRole_Revoke_user(t *testing.T) { } // Verify new username/password still work - if err := verifyPgConn(t, username, password, connURL); err != nil { - if !tc.expectVerifyErr { - t.Fatal(err) - } - } + verifyPgConn(t, username, password, connURL) }) } } -func verifyPgConn(t *testing.T, username, password, connURL string) error { +func createTestPGUser(t *testing.T, connURL string, username, password, query string) { + t.Helper() + log.Printf("[TRACE] Creating test user") + conn, err := pq.ParseURL(connURL) + if err != nil { + t.Fatal(err) + } + + db, err := sql.Open("postgres", conn) + defer db.Close() + if err != nil { + t.Fatal(err) + } + + // Start a transaction + ctx := context.Background() + tx, err := db.BeginTx(ctx, nil) + if err != nil { + t.Fatal(err) + } + defer func() { + _ = tx.Rollback() + }() + + m := map[string]string{ + "name": username, + "password": password, + } + if err := dbtxn.ExecuteTxQuery(ctx, tx, m, query); err != nil { + t.Fatal(err) + } + // Commit the transaction + if err := tx.Commit(); err != nil { + t.Fatal(err) + } +} + +func verifyPgConn(t *testing.T, username, password, connURL string) { + t.Helper() cURL := strings.Replace(connURL, "postgres:secret", username+":"+password, 1) db, err := sql.Open("postgres", cURL) if err != nil { - return err + t.Fatal(err) } if err := db.Ping(); err != nil { - return err + t.Fatal(err) } - return db.Close() } // WAL testing @@ -508,12 +537,10 @@ func TestBackend_Static_QueueWAL_discard_role_newer_rotation_date(t *testing.T) // Create role data = map[string]interface{}{ - "name": roleName, - "db_name": "plugin-test", - "creation_statements": testRoleStaticCreate, - "rotation_statements": testRoleStaticUpdate, - "revocation_statements": defaultRevocationSQL, - "username": "statictest", + "name": roleName, + "db_name": "plugin-test", + "rotation_statements": testRoleStaticUpdate, + "username": dbUser, // Low value here, to make sure the backend rotates this password at least // once before we compare it to the WAL "rotation_period": "10s", @@ -548,7 +575,7 @@ func TestBackend_Static_QueueWAL_discard_role_newer_rotation_date(t *testing.T) RoleName: roleName, NewPassword: walPassword, LastVaultRotation: oldRotationTime, - Username: "statictest", + Username: dbUser, }) if err != nil { t.Fatalf("error with PutWAL: %s", err) @@ -664,6 +691,11 @@ func TestBackend_StaticRole_Rotations_PostgreSQL(t *testing.T) { // Configure backend, add item and confirm length cleanup, connURL := preparePostgresTestContainer(t, config.StorageView, b) defer cleanup() + testCases := []string{"65", "130", "5400"} + // Create database users ahead + for _, tc := range testCases { + createTestPGUser(t, connURL, dbUser+tc, "password", testRoleStaticCreate) + } // Configure a connection data := map[string]interface{}{ @@ -685,17 +717,14 @@ func TestBackend_StaticRole_Rotations_PostgreSQL(t *testing.T) { } // Create three static roles with different rotation periods - testCases := []string{"65", "130", "5400"} for _, tc := range testCases { roleName := "plugin-static-role-" + tc data = map[string]interface{}{ - "name": roleName, - "db_name": "plugin-test", - "creation_statements": testRoleStaticCreate, - "rotation_statements": testRoleStaticUpdate, - "revocation_statements": defaultRevocationSQL, - "username": "statictest" + tc, - "rotation_period": tc, + "name": roleName, + "db_name": "plugin-test", + "rotation_statements": testRoleStaticUpdate, + "username": dbUser + tc, + "rotation_period": tc, } req = &logical.Request{ diff --git a/plugins/database/postgresql/postgresql.go b/plugins/database/postgresql/postgresql.go index 94d3650ac55f..1e9c168165f6 100644 --- a/plugins/database/postgresql/postgresql.go +++ b/plugins/database/postgresql/postgresql.go @@ -99,8 +99,8 @@ func (p *PostgreSQL) getConnection(ctx context.Context) (*sql.DB, error) { // passwords in the database in the event an updated database fails to save in // Vault's storage. func (p *PostgreSQL) SetCredentials(ctx context.Context, statements dbplugin.Statements, staticUser dbplugin.StaticUserConfig) (username, password string, err error) { - if len(statements.Creation) == 0 { - return "", "", errors.New("empty creation statements") + if len(statements.Rotation) == 0 { + return "", "", errors.New("empty rotation statements") } username = staticUser.Username @@ -126,16 +126,9 @@ func (p *PostgreSQL) SetCredentials(ctx context.Context, statements dbplugin.Sta return "", "", err } - // Default to using Creation statements, which are required by the Vault - // backend. If the user exists, use the rotation statements, using the default - // ones if there are none provided - stmts := statements.Creation - if exists { - stmts = statements.Rotation - if len(stmts) == 0 { - stmts = []string{defaultPostgresRotateCredentialsSQL} - } - } + // Vault requires the database user already exist, and that the credentials + // used to execute the rotation statements has sufficient privileges. + stmts := statements.Rotation // Start a transaction tx, err := db.BeginTx(ctx, nil) diff --git a/website/source/api/secret/databases/index.html.md b/website/source/api/secret/databases/index.html.md index 55d67a160cc6..e40e9aa0b53e 100644 --- a/website/source/api/secret/databases/index.html.md +++ b/website/source/api/secret/databases/index.html.md @@ -425,35 +425,11 @@ Static Roles, please see the database-specific documentation. - `db_name` `(string: )` - The name of the database connection to use for this role. -- `creation_statements` `(list: )` – Specifies the database - statements executed to create and configure a user. See the plugin's API page - for more information on support and formatting for this parameter. - -- `revocation_statements` `(list: [])` – Specifies the database statements to - be executed to revoke a user. See the plugin's API page for more information - on support and formatting for this parameter. - -- `rollback_statements` `(list: [])` – Specifies the database statements to be - executed rollback a create operation in the event of an error. Not every - plugin type will support this functionality. See the plugin's API page for - more information on support and formatting for this parameter. - -- `renew_statements` `(list: [])` – Specifies the database statements to be - executed to renew a user. Not every plugin type will support this - functionality. See the plugin's API page for more information on support and - formatting for this parameter. - - `rotation_statements` `(list: [])` – Specifies the database statements to be executed to rotate the password for the configured database user. Not every plugin type will support this functionality. See the plugin's API page for more information on support and formatting for this parameter. -- `revoke_user_on_delete` `(boolean: false)` – Specifies if Vault should attempt - to revoke the database user associated with this static role, indicated by the - `username`. If `true`, when Vault deletes this Role it will attempt to revoke - the database user using the configured `revocation_statements` if they exist. - Default `false` - ### Sample Payload @@ -462,7 +438,6 @@ Static Roles, please see the database-specific documentation. { "db_name": "mysql", "username": "static-database-user", - "creation_statements": ["CREATE USER '{{name}}'@'%' IDENTIFIED BY '{{password}}'", "GRANT SELECT ON *.* TO '{{name}}'@'%'"], "rotation_statements": ["ALTER USER "{{name}}" WITH PASSWORD '{{password}}';"], "rotation_period": "1h" } @@ -506,13 +481,8 @@ $ curl \ "data": { "db_name": "mysql", "username":"static-user", - "creation_statements": ["CREATE ROLE \"{{name}}\" WITH LOGIN PASSWORD '{{password}}' VALID UNTIL '{{expiration}}';"], "GRANT SELECT ON ALL TABLES IN SCHEMA public TO \"{{name}}\";"], "rotation_statements": ["ALTER USER "{{name}}" WITH PASSWORD '{{password}}';"], "rotation_period":"1h", - "renew_statements": [], - "revocation_statements": [], - "rollback_statements": [] - "revoke_user_on_delete": false, }, } ``` From 456a938e7b3a7132976839ca0f0a71d243e2c051 Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Thu, 20 Jun 2019 17:51:54 -0500 Subject: [PATCH 2/4] update tests after create/delete removed --- builtin/logical/database/path_roles_test.go | 21 ++++++++++++++------- builtin/logical/database/rotation_test.go | 11 ++++++++++- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/builtin/logical/database/path_roles_test.go b/builtin/logical/database/path_roles_test.go index 700eb55c5f5b..0e9bf4f003f7 100644 --- a/builtin/logical/database/path_roles_test.go +++ b/builtin/logical/database/path_roles_test.go @@ -34,6 +34,9 @@ func TestBackend_StaticRole_Config(t *testing.T) { cleanup, connURL := preparePostgresTestContainer(t, config.StorageView, b) defer cleanup() + // create the database user + createTestPGUser(t, connURL, dbUser, "password", testRoleStaticCreate) + // Configure a connection data := map[string]interface{}{ "connection_url": connURL, @@ -201,6 +204,9 @@ func TestBackend_StaticRole_Updates(t *testing.T) { cleanup, connURL := preparePostgresTestContainer(t, config.StorageView, b) defer cleanup() + // create the database user + createTestPGUser(t, connURL, dbUser, "password", testRoleStaticCreate) + // Configure a connection data := map[string]interface{}{ "connection_url": connURL, @@ -391,6 +397,9 @@ func TestBackend_StaticRole_Role_name_check(t *testing.T) { cleanup, connURL := preparePostgresTestContainer(t, config.StorageView, b) defer cleanup() + // create the database user + createTestPGUser(t, connURL, dbUser, "password", testRoleStaticCreate) + // Configure a connection data := map[string]interface{}{ "connection_url": connURL, @@ -458,13 +467,11 @@ func TestBackend_StaticRole_Role_name_check(t *testing.T) { // repeat, with a static role first data = map[string]interface{}{ - "name": "plugin-role-test-2", - "db_name": "plugin-test", - "creation_statements": testRoleStaticCreate, - "rotation_statements": testRoleStaticUpdate, - "revocation_statements": defaultRevocationSQL, - "username": "testusername", - "rotation_period": "1h", + "name": "plugin-role-test-2", + "db_name": "plugin-test", + "rotation_statements": testRoleStaticUpdate, + "username": dbUser, + "rotation_period": "1h", } req = &logical.Request{ diff --git a/builtin/logical/database/rotation_test.go b/builtin/logical/database/rotation_test.go index 4455bb383f7a..29530fcbdd7b 100644 --- a/builtin/logical/database/rotation_test.go +++ b/builtin/logical/database/rotation_test.go @@ -106,7 +106,7 @@ func TestBackend_StaticRole_Rotate_basic(t *testing.T) { } // Verify username/password - verifyPgConn(t, dbUser, "password", connURL) + verifyPgConn(t, dbUser, password, connURL) // Re-read the creds, verifying they aren't changing on read data = map[string]interface{}{} @@ -188,6 +188,9 @@ func TestBackend_StaticRole_Rotate_NonStaticError(t *testing.T) { cleanup, connURL := preparePostgresTestContainer(t, config.StorageView, b) defer cleanup() + // create the database user + createTestPGUser(t, connURL, dbUser, "password", testRoleStaticCreate) + // Configure a connection data := map[string]interface{}{ "connection_url": connURL, @@ -289,6 +292,9 @@ func TestBackend_StaticRole_Revoke_user(t *testing.T) { cleanup, connURL := preparePostgresTestContainer(t, config.StorageView, b) defer cleanup() + // create the database user + createTestPGUser(t, connURL, dbUser, "password", testRoleStaticCreate) + // Configure a connection data := map[string]interface{}{ "connection_url": connURL, @@ -511,6 +517,9 @@ func TestBackend_Static_QueueWAL_discard_role_newer_rotation_date(t *testing.T) cleanup, connURL := preparePostgresTestContainer(t, config.StorageView, b) defer cleanup() + // create the database user + createTestPGUser(t, connURL, dbUser, "password", testRoleStaticCreate) + // Configure a connection data := map[string]interface{}{ "connection_url": connURL, From 18265ec0a67cceaf65d06c1d5d4b080954bb5b1f Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Fri, 21 Jun 2019 13:30:52 -0500 Subject: [PATCH 3/4] small cleanups --- builtin/logical/database/path_roles.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/builtin/logical/database/path_roles.go b/builtin/logical/database/path_roles.go index aa2a4924054b..c63eb980adaa 100644 --- a/builtin/logical/database/path_roles.go +++ b/builtin/logical/database/path_roles.go @@ -235,12 +235,17 @@ func (b *databaseBackend) pathStaticRoleRead(ctx context.Context, req *logical.R // guard against nil StaticAccount; shouldn't happen but we'll be safe if role.StaticAccount != nil { data["username"] = role.StaticAccount.Username + data["rotation_statements"] = role.Statements.Rotation data["rotation_period"] = role.StaticAccount.RotationPeriod.Seconds() if !role.StaticAccount.LastVaultRotation.IsZero() { data["last_vault_rotation"] = role.StaticAccount.LastVaultRotation } } + if len(role.Statements.Rotation) == 0 { + data["rotation_statements"] = []string{} + } + return &logical.Response{ Data: data, }, nil @@ -261,7 +266,6 @@ func (b *databaseBackend) pathRoleRead(ctx context.Context, req *logical.Request "revocation_statements": role.Statements.Revocation, "rollback_statements": role.Statements.Rollback, "renew_statements": role.Statements.Renewal, - "rotation_statements": role.Statements.Rotation, "default_ttl": role.DefaultTTL.Seconds(), "max_ttl": role.MaxTTL.Seconds(), } @@ -277,9 +281,6 @@ func (b *databaseBackend) pathRoleRead(ctx context.Context, req *logical.Request if len(role.Statements.Renewal) == 0 { data["renew_statements"] = []string{} } - if len(role.Statements.Rotation) == 0 { - data["rotation_statements"] = []string{} - } return &logical.Response{ Data: data, @@ -425,7 +426,7 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l // createRole is a boolean to indicate if this is a new role creation. This is // can be used later by database plugins that distinguish between creating and // updating roles, and may use seperate statements depending on the context. - createRole := req.Operation == logical.CreateOperation + createRole := (req.Operation == logical.CreateOperation) if role == nil { role = &roleEntry{ StaticAccount: &staticAccount{}, From 1367ae43255dbaeceb4167da00bef76d4b7365a3 Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Fri, 21 Jun 2019 14:34:20 -0500 Subject: [PATCH 4/4] update postgresql setcredentials test --- .../database/postgresql/postgresql_test.go | 77 +++++++++++++------ 1 file changed, 55 insertions(+), 22 deletions(-) diff --git a/plugins/database/postgresql/postgresql_test.go b/plugins/database/postgresql/postgresql_test.go index cd803b84567a..3fbbb439ebbd 100644 --- a/plugins/database/postgresql/postgresql_test.go +++ b/plugins/database/postgresql/postgresql_test.go @@ -12,6 +12,8 @@ import ( "github.com/hashicorp/vault/helper/testhelpers/docker" "github.com/hashicorp/vault/sdk/database/dbplugin" + "github.com/hashicorp/vault/sdk/helper/dbtxn" + "github.com/lib/pq" "github.com/ory/dockertest" ) @@ -321,6 +323,10 @@ func TestPostgresSQL_SetCredentials(t *testing.T) { cleanup, connURL := preparePostgresTestContainer(t) defer cleanup() + // create the database user + dbUser := "vaultstatictest" + createTestPGUser(t, connURL, dbUser, "password", testRoleStaticCreate) + connectionDetails := map[string]interface{}{ "connection_url": connURL, } @@ -337,18 +343,18 @@ func TestPostgresSQL_SetCredentials(t *testing.T) { } usernameConfig := dbplugin.StaticUserConfig{ - Username: "test", + Username: dbUser, Password: password, } - // Test with no configured Creation Statement + // Test with no configured Rotation Statement username, password, err := db.SetCredentials(context.Background(), dbplugin.Statements{}, usernameConfig) if err == nil { t.Fatalf("err: %s", err) } statements := dbplugin.Statements{ - Creation: []string{testPostgresStaticRole}, + Rotation: []string{testPostgresStaticRoleRotate}, } // User should not exist, make sure we can create username, password, err = db.SetCredentials(context.Background(), statements, usernameConfig) @@ -360,8 +366,7 @@ func TestPostgresSQL_SetCredentials(t *testing.T) { t.Fatalf("Could not connect with new credentials: %s", err) } - // call SetCredentials again, the user will already exist, password will - // change. Without rotation statements, this should use the defaults + // call SetCredentials again, password will change newPassword, _ := db.GenerateCredentials(context.Background()) usernameConfig.Password = newPassword username, password, err = db.SetCredentials(context.Background(), statements, usernameConfig) @@ -376,23 +381,6 @@ func TestPostgresSQL_SetCredentials(t *testing.T) { if err := testCredsExist(t, connURL, username, password); err != nil { t.Fatalf("Could not connect with new credentials: %s", err) } - - // generate a new password and supply owr own rotation statements - newPassword2, _ := db.GenerateCredentials(context.Background()) - usernameConfig.Password = newPassword2 - statements.Rotation = []string{testPostgresStaticRoleRotate, testPostgresStaticRoleGrant} - username, password, err = db.SetCredentials(context.Background(), statements, usernameConfig) - if err != nil { - t.Fatalf("err: %s", err) - } - - if password != newPassword2 { - t.Fatal("passwords should have changed") - } - - if err := testCredsExist(t, connURL, username, password); err != nil { - t.Fatalf("Could not connect with new credentials: %s", err) - } } func testCredsExist(t testing.TB, connURL, username, password string) error { @@ -484,6 +472,12 @@ CREATE ROLE "{{name}}" WITH GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public TO "{{name}}"; ` +const testRoleStaticCreate = ` +CREATE ROLE "{{name}}" WITH + LOGIN + PASSWORD '{{password}}'; +` + const testPostgresStaticRoleRotate = ` ALTER ROLE "{{name}}" WITH PASSWORD '{{password}}'; ` @@ -491,3 +485,42 @@ ALTER ROLE "{{name}}" WITH PASSWORD '{{password}}'; const testPostgresStaticRoleGrant = ` GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public TO "{{name}}"; ` + +// This is a copy of a test helper method also found in +// builtin/logical/database/rotation_test.go , and should be moved into a shared +// helper file in the future. +func createTestPGUser(t *testing.T, connURL string, username, password, query string) { + t.Helper() + conn, err := pq.ParseURL(connURL) + if err != nil { + t.Fatal(err) + } + + db, err := sql.Open("postgres", conn) + defer db.Close() + if err != nil { + t.Fatal(err) + } + + // Start a transaction + ctx := context.Background() + tx, err := db.BeginTx(ctx, nil) + if err != nil { + t.Fatal(err) + } + defer func() { + _ = tx.Rollback() + }() + + m := map[string]string{ + "name": username, + "password": password, + } + if err := dbtxn.ExecuteTxQuery(ctx, tx, m, query); err != nil { + t.Fatal(err) + } + // Commit the transaction + if err := tx.Commit(); err != nil { + t.Fatal(err) + } +}