diff --git a/builtin/logical/database/backend.go b/builtin/logical/database/backend.go index 6836370933fc..2a4f3fcf5a9c 100644 --- a/builtin/logical/database/backend.go +++ b/builtin/logical/database/backend.go @@ -25,6 +25,7 @@ import ( "github.com/hashicorp/vault/sdk/helper/locksutil" "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/sdk/queue" + "github.com/robfig/cron/v3" ) const ( @@ -127,6 +128,13 @@ func Backend(conf *logical.BackendConfig) *databaseBackend { b.connections = syncmap.NewSyncMap[string, *dbPluginInstance]() b.queueCtx, b.cancelQueueCtx = context.WithCancel(context.Background()) b.roleLocks = locksutil.CreateLocks() + + // TODO(JM): don't allow seconds in production, this is helpful for + // development/testing though + parser := cron.NewParser( + cron.Second | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.DowOptional, + ) + b.scheduleParser = parser return &b } @@ -176,6 +184,8 @@ type databaseBackend struct { // the running gauge collection process gaugeCollectionProcess *metricsutil.GaugeCollectionProcess gaugeCollectionProcessStop sync.Once + + scheduleParser cron.Parser } func (b *databaseBackend) DatabaseConfig(ctx context.Context, s logical.Storage, name string) (*DatabaseConfig, error) { diff --git a/builtin/logical/database/path_creds_create.go b/builtin/logical/database/path_creds_create.go index d3e95876b501..9e1b6f2fb308 100644 --- a/builtin/logical/database/path_creds_create.go +++ b/builtin/logical/database/path_creds_create.go @@ -249,10 +249,18 @@ func (b *databaseBackend) pathStaticCredsRead() framework.OperationFunc { respData := map[string]interface{}{ "username": role.StaticAccount.Username, "ttl": role.StaticAccount.CredentialTTL().Seconds(), - "rotation_period": role.StaticAccount.RotationPeriod.Seconds(), "last_vault_rotation": role.StaticAccount.LastVaultRotation, } + if role.StaticAccount.UsesRotationPeriod() { + respData["rotation_period"] = role.StaticAccount.RotationPeriod.Seconds() + } else if role.StaticAccount.UsesRotationSchedule() { + respData["rotation_schedule"] = role.StaticAccount.RotationSchedule + if role.StaticAccount.RotationWindow.Seconds() != 0 { + respData["rotation_window"] = role.StaticAccount.RotationWindow.Seconds() + } + } + switch role.CredentialType { case v5.CredentialTypePassword: respData["password"] = role.StaticAccount.Password diff --git a/builtin/logical/database/path_roles.go b/builtin/logical/database/path_roles.go index 31708415af6b..5366ac711c0c 100644 --- a/builtin/logical/database/path_roles.go +++ b/builtin/logical/database/path_roles.go @@ -17,6 +17,7 @@ import ( "github.com/hashicorp/vault/sdk/helper/locksutil" "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/sdk/queue" + "github.com/robfig/cron/v3" ) func pathListRoles(b *databaseBackend) []*framework.Path { @@ -196,7 +197,18 @@ func staticFields() map[string]*framework.FieldSchema { Type: framework.TypeDurationSecond, Description: `Period for automatic credential rotation of the given username. Not valid unless used with - "username".`, + "username". Mutually exclusive with "rotation_schedule."`, + }, + "rotation_schedule": { + Type: framework.TypeString, + Description: `Schedule for automatic credential rotation of the + given username. Mutually exclusive with "rotation_period."`, + }, + "rotation_window": { + Type: framework.TypeDurationSecond, + Description: `The window of time in which rotations are allowed to + occur starting from a given "rotation_schedule". Requires "rotation_schedule" + to be specified`, }, "rotation_statements": { Type: framework.TypeStringSlice, @@ -293,10 +305,20 @@ func (b *databaseBackend) pathStaticRoleRead(ctx context.Context, req *logical.R 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 } + + // only return one of the mutually exclusive fields in the response + if role.StaticAccount.UsesRotationPeriod() { + data["rotation_period"] = role.StaticAccount.RotationPeriod.Seconds() + } else if role.StaticAccount.UsesRotationSchedule() { + data["rotation_schedule"] = role.StaticAccount.RotationSchedule + // rotation_window is only valid with rotation_schedule + if role.StaticAccount.RotationWindow != 0 { + data["rotation_window"] = role.StaticAccount.RotationWindow.Seconds() + } + } } if len(role.CredentialConfig) > 0 { @@ -480,6 +502,7 @@ func (b *databaseBackend) pathRoleCreateUpdate(ctx context.Context, req *logical } func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { + response := &logical.Response{} name := data.Get("name").(string) if name == "" { return logical.ErrorResponse("empty role name attribute given"), nil @@ -536,12 +559,18 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l } role.StaticAccount.Username = username - // If it's a Create operation, both username and rotation_period must be included - rotationPeriodSecondsRaw, ok := data.GetOk("rotation_period") - if !ok && createRole { - return logical.ErrorResponse("rotation_period is required to create static accounts"), nil + rotationPeriodSecondsRaw, rotationPeriodOk := data.GetOk("rotation_period") + rotationSchedule := data.Get("rotation_schedule").(string) + rotationScheduleOk := rotationSchedule != "" + rotationWindowSecondsRaw, rotationWindowOk := data.GetOk("rotation_window") + + if rotationScheduleOk && rotationPeriodOk { + return logical.ErrorResponse("mutually exclusive fields rotation_period and rotation_schedule were both specified; only one of them can be provided"), nil + } else if createRole && (!rotationScheduleOk && !rotationPeriodOk) { + return logical.ErrorResponse("one of rotation_schedule or rotation_period must be provided to create a static account"), nil } - if ok { + + if rotationPeriodOk { rotationPeriodSeconds := rotationPeriodSecondsRaw.(int) if rotationPeriodSeconds < defaultQueueTickSeconds { // If rotation frequency is specified, and this is an update, the value @@ -550,6 +579,40 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l return logical.ErrorResponse(fmt.Sprintf("rotation_period must be %d seconds or more", defaultQueueTickSeconds)), nil } role.StaticAccount.RotationPeriod = time.Duration(rotationPeriodSeconds) * time.Second + + if rotationWindowOk { + return logical.ErrorResponse("rotation_window is invalid with use of rotation_period"), nil + } + + // Unset rotation schedule and window if rotation period is set since + // these are mutually exclusive + role.StaticAccount.RotationSchedule = "" + role.StaticAccount.RotationWindow = 0 + } + + if rotationScheduleOk { + schedule, err := b.scheduleParser.Parse(rotationSchedule) + if err != nil { + return logical.ErrorResponse("could not parse rotation_schedule", "error", err), nil + } + role.StaticAccount.RotationSchedule = rotationSchedule + sched, ok := schedule.(*cron.SpecSchedule) + if !ok { + return logical.ErrorResponse("could not parse rotation_schedule"), nil + } + role.StaticAccount.Schedule = *sched + + if rotationWindowOk { + rotationWindowSeconds := rotationWindowSecondsRaw.(int) + if rotationWindowSeconds < minRotationWindowSeconds { + return logical.ErrorResponse(fmt.Sprintf("rotation_window must be %d seconds or more", minRotationWindowSeconds)), nil + } + role.StaticAccount.RotationWindow = time.Duration(rotationWindowSeconds) * time.Second + } + + // Unset rotation period if rotation schedule is set since these are + // mutually exclusive + role.StaticAccount.RotationPeriod = 0 } if rotationStmtsRaw, ok := data.GetOk("rotation_statements"); ok { @@ -623,14 +686,23 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l } } - item.Priority = lvr.Add(role.StaticAccount.RotationPeriod).Unix() + _, rotationPeriodChanged := data.Raw["rotation_period"] + _, rotationScheduleChanged := data.Raw["rotation_schedule"] + if rotationPeriodChanged { + b.logger.Debug("init priority for RotationPeriod", "lvr", lvr, "next", lvr.Add(role.StaticAccount.RotationPeriod)) + item.Priority = lvr.Add(role.StaticAccount.RotationPeriod).Unix() + } else if rotationScheduleChanged { + next := role.StaticAccount.Schedule.Next(lvr) + b.logger.Debug("init priority for Schedule", "lvr", lvr, "next", next) + item.Priority = role.StaticAccount.Schedule.Next(lvr).Unix() + } // Add their rotation to the queue if err := b.pushItem(item); err != nil { return nil, err } - return nil, nil + return response, nil } type roleEntry struct { @@ -730,24 +802,103 @@ type staticAccount struct { // LastVaultRotation represents the last time Vault rotated the password LastVaultRotation time.Time `json:"last_vault_rotation"` + // NextVaultRotation represents the next time Vault is expected to rotate + // the password + NextVaultRotation time.Time `json:"next_vault_rotation"` + // RotationPeriod is number in seconds between each rotation, effectively a // "time to live". This value is compared to the LastVaultRotation to // determine if a password needs to be rotated RotationPeriod time.Duration `json:"rotation_period"` + // RotationSchedule is a "chron style" string representing the allowed + // schedule for each rotation. + // e.g. "1 0 * * *" would rotate at one minute past midnight (00:01) every + // day. + RotationSchedule string `json:"rotation_schedule"` + + // RotationWindow is number in seconds in which rotations are allowed to + // occur starting from a given rotation_schedule. + RotationWindow time.Duration `json:"rotation_window"` + + // Schedule holds the parsed "chron style" string representing the allowed + // schedule for each rotation. + Schedule cron.SpecSchedule `json:"schedule"` + // RevokeUser is a boolean flag to indicate if Vault should revoke the // database user when the role is deleted RevokeUserOnDelete bool `json:"revoke_user_on_delete"` } -// NextRotationTime calculates the next rotation by adding the Rotation Period -// to the last known vault rotation +// NextRotationTime calculates the next rotation for period and schedule-based +// rotations. +// +// Period-based expiries are calculated by adding the Rotation Period to the +// last known vault rotation. Schedule-based expiries are calculated by +// querying for the next schedule expiry since the last known vault rotation. func (s *staticAccount) NextRotationTime() time.Time { - return s.LastVaultRotation.Add(s.RotationPeriod) + if s.UsesRotationPeriod() { + return s.LastVaultRotation.Add(s.RotationPeriod) + } + return s.Schedule.Next(s.LastVaultRotation) +} + +// NextRotationTimeFromInput calculates the next rotation time for period and +// schedule-based roles based on the input. +func (s *staticAccount) NextRotationTimeFromInput(input time.Time) time.Time { + if s.UsesRotationPeriod() { + return input.Add(s.RotationPeriod) + } + return s.Schedule.Next(input) +} + +// UsesRotationSchedule returns true if the given static account has been +// configured to rotate credentials on a schedule (i.e. NOT on a rotation period). +func (s *staticAccount) UsesRotationSchedule() bool { + return s.RotationSchedule != "" && s.RotationPeriod == 0 +} + +// UsesRotationPeriod returns true if the given static account has been +// configured to rotate credentials on a period (i.e. NOT on a rotation schedule). +func (s *staticAccount) UsesRotationPeriod() bool { + return s.RotationPeriod != 0 && s.RotationSchedule == "" +} + +// IsInsideRotationWindow returns true if the current time t is within a given +// static account's rotation window. +// +// Returns true if the rotation window is not set. In this case, the rotation +// window is effectively the span of time between two consecutive rotation +// schedules and we should not prevent rotation. +func (s *staticAccount) IsInsideRotationWindow(t time.Time) bool { + if s.UsesRotationSchedule() && s.RotationWindow != 0 { + return t.Before(s.NextVaultRotation.Add(s.RotationWindow)) + } + return true +} + +// ShouldRotate returns true if a given static account should have its +// credentials rotated. +// +// This will return true when the priority <= the current Unix time. If this +// static account is schedule-based with a rotation window, this method will +// return false if now is outside the rotation window. +func (s *staticAccount) ShouldRotate(priority int64) bool { + now := time.Now() + return priority <= now.Unix() && s.IsInsideRotationWindow(now) +} + +// SetNextVaultRotation +func (s *staticAccount) SetNextVaultRotation(t time.Time) { + if s.UsesRotationPeriod() { + s.NextVaultRotation = t.Add(s.RotationPeriod) + } else { + s.NextVaultRotation = s.Schedule.Next(t) + } } // CredentialTTL calculates the approximate time remaining until the credential is -// no longer valid. This is approximate because the periodic rotation is only +// no longer valid. This is approximate because the rotation expiry is only // checked approximately every 5 seconds, and each rotation can take a small // amount of time to process. This can result in a negative TTL time while the // rotation function processes the Static Role and performs the rotation. If the diff --git a/builtin/logical/database/path_roles_test.go b/builtin/logical/database/path_roles_test.go index f012df753546..4cc142d783c9 100644 --- a/builtin/logical/database/path_roles_test.go +++ b/builtin/logical/database/path_roles_test.go @@ -7,6 +7,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "strings" "testing" "time" @@ -16,6 +17,7 @@ import ( postgreshelper "github.com/hashicorp/vault/helper/testhelpers/postgresql" v5 "github.com/hashicorp/vault/sdk/database/dbplugin/v5" "github.com/hashicorp/vault/sdk/logical" + "github.com/robfig/cron/v3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" ) @@ -254,6 +256,8 @@ func TestBackend_StaticRole_Config(t *testing.T) { path string expected map[string]interface{} err error + // use this field to check partial error strings, otherwise use err + errContains string }{ "basic": { account: map[string]interface{}{ @@ -266,12 +270,71 @@ func TestBackend_StaticRole_Config(t *testing.T) { "rotation_period": float64(5400), }, }, - "missing rotation period": { + "missing required fields": { account: map[string]interface{}{ "username": dbUser, }, path: "plugin-role-test", - err: errors.New("rotation_period is required to create static accounts"), + err: errors.New("one of rotation_schedule or rotation_period must be provided to create a static account"), + }, + "rotation_period with rotation_schedule": { + account: map[string]interface{}{ + "username": dbUser, + "rotation_period": "5400s", + "rotation_schedule": "* * * * *", + }, + path: "plugin-role-test", + err: errors.New("mutually exclusive fields rotation_period and rotation_schedule were both specified; only one of them can be provided"), + }, + "rotation window invalid with rotation_period": { + account: map[string]interface{}{ + "username": dbUser, + "rotation_period": "5400s", + "rotation_window": "3600s", + }, + path: "disallowed-role", + err: errors.New("rotation_window is invalid with use of rotation_period"), + }, + "happy path for rotation_schedule": { + account: map[string]interface{}{ + "username": dbUser, + "rotation_schedule": "* * * * *", + }, + path: "plugin-role-test", + expected: map[string]interface{}{ + "username": dbUser, + "rotation_schedule": "* * * * *", + }, + }, + "happy path for rotation_schedule and rotation_window": { + account: map[string]interface{}{ + "username": dbUser, + "rotation_schedule": "* * * * *", + "rotation_window": "3600s", + }, + path: "plugin-role-test", + expected: map[string]interface{}{ + "username": dbUser, + "rotation_schedule": "* * * * *", + "rotation_window": float64(3600), + }, + }, + "error parsing rotation_schedule": { + account: map[string]interface{}{ + "username": dbUser, + "rotation_schedule": "foo", + }, + path: "plugin-role-test", + errContains: "could not parse rotation_schedule", + }, + "rotation_window invalid": { + account: map[string]interface{}{ + "username": dbUser, + "rotation_schedule": "* * * * *", + "rotation_window": "59s", + }, + path: "plugin-role-test", + err: errors.New(fmt.Sprintf("rotation_window must be %d seconds or more", minRotationWindowSeconds)), }, "disallowed role config": { account: map[string]interface{}{ @@ -305,7 +368,12 @@ func TestBackend_StaticRole_Config(t *testing.T) { } resp, err = b.HandleRequest(namespace.RootContext(nil), req) - if err != nil || (resp != nil && resp.IsError()) { + if tc.errContains != "" { + if !strings.Contains(resp.Error().Error(), tc.errContains) { + t.Fatalf("expected err message: (%s), got (%s), response error: (%s)", tc.err, err, resp.Error()) + } + return + } else if err != nil || (resp != nil && resp.IsError()) { if tc.err == nil { t.Fatalf("err:%s resp:%#v\n", err, resp) } @@ -341,7 +409,14 @@ func TestBackend_StaticRole_Config(t *testing.T) { expected := tc.expected actual := make(map[string]interface{}) - dataKeys := []string{"username", "password", "last_vault_rotation", "rotation_period"} + dataKeys := []string{ + "username", + "password", + "last_vault_rotation", + "rotation_period", + "rotation_schedule", + "rotation_window", + } for _, key := range dataKeys { if v, ok := resp.Data[key]; ok { actual[key] = v @@ -388,6 +463,186 @@ func TestBackend_StaticRole_Config(t *testing.T) { } } +func TestBackend_StaticRole_ReadCreds(t *testing.T) { + cluster, sys := getCluster(t) + defer cluster.Cleanup() + + config := logical.TestBackendConfig() + config.StorageView = &logical.InmemStorage{} + config.System = sys + + lb, err := Factory(context.Background(), config) + if err != nil { + t.Fatal(err) + } + b, ok := lb.(*databaseBackend) + if !ok { + t.Fatal("could not convert to db backend") + } + defer b.Cleanup(context.Background()) + + cleanup, connURL := postgreshelper.PrepareTestContainer(t, "") + defer cleanup() + + // create the database user + createTestPGUser(t, connURL, dbUser, dbUserDefaultPassword, testRoleStaticCreate) + + verifyPgConn(t, dbUser, dbUserDefaultPassword, connURL) + + // Configure a connection + data := map[string]interface{}{ + "connection_url": connURL, + "plugin_name": "postgresql-database-plugin", + "verify_connection": false, + "allowed_roles": []string{"*"}, + "name": "plugin-test", + } + + req := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "config/plugin-test", + Storage: config.StorageView, + Data: data, + } + resp, err := b.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } + + testCases := map[string]struct { + account map[string]interface{} + path string + expected map[string]interface{} + }{ + "happy path for rotation_period": { + account: map[string]interface{}{ + "username": dbUser, + "rotation_period": "5400s", + }, + path: "plugin-role-test", + expected: map[string]interface{}{ + "username": dbUser, + "rotation_period": float64(5400), + }, + }, + "happy path for rotation_schedule": { + account: map[string]interface{}{ + "username": dbUser, + "rotation_schedule": "* * * * *", + }, + path: "plugin-role-test", + expected: map[string]interface{}{ + "username": dbUser, + "rotation_schedule": "* * * * *", + }, + }, + "happy path for rotation_schedule and rotation_window": { + account: map[string]interface{}{ + "username": dbUser, + "rotation_schedule": "* * * * *", + "rotation_window": "3600s", + }, + path: "plugin-role-test", + expected: map[string]interface{}{ + "username": dbUser, + "rotation_schedule": "* * * * *", + "rotation_window": float64(3600), + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + data = map[string]interface{}{ + "name": "plugin-role-test", + "db_name": "plugin-test", + "rotation_statements": testRoleStaticUpdate, + "username": dbUser, + } + + for k, v := range tc.account { + data[k] = v + } + + req = &logical.Request{ + Operation: logical.CreateOperation, + Path: "static-roles/plugin-role-test", + Storage: config.StorageView, + Data: data, + } + + resp, err = b.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } + + // Read the creds + data = map[string]interface{}{} + req = &logical.Request{ + Operation: logical.ReadOperation, + Path: "static-creds/plugin-role-test", + Storage: config.StorageView, + Data: data, + } + + resp, err = b.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } + + expected := tc.expected + actual := make(map[string]interface{}) + dataKeys := []string{ + "username", + "password", + "last_vault_rotation", + "rotation_period", + "rotation_schedule", + "rotation_window", + "ttl", + } + for _, key := range dataKeys { + if v, ok := resp.Data[key]; ok { + actual[key] = v + } + } + + if len(tc.expected) > 0 { + // verify a password is returned, but we don't care what it's value is + if actual["password"] == "" { + t.Fatalf("expected result to contain password, but none found") + } + if actual["ttl"] == "" { + t.Fatalf("expected result to contain ttl, but none found") + } + if v, ok := actual["last_vault_rotation"].(time.Time); !ok { + t.Fatalf("expected last_vault_rotation to be set to time.Time type, got: %#v", v) + } + + // delete these values before the comparison, since we can't know them in + // advance + delete(actual, "password") + delete(actual, "ttl") + delete(actual, "last_vault_rotation") + if diff := deep.Equal(expected, actual); diff != nil { + t.Fatal(diff) + } + } + + // Delete role for next run + req = &logical.Request{ + Operation: logical.DeleteOperation, + Path: "static-roles/plugin-role-test", + Storage: config.StorageView, + } + resp, err = b.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } + }) + } +} + func TestBackend_StaticRole_Updates(t *testing.T) { cluster, sys := getCluster(t) defer cluster.Cleanup() @@ -828,6 +1083,97 @@ func TestWALsDeletedOnRoleDeletion(t *testing.T) { requireWALs(t, storage, 1) } +func TestIsInsideRotationWindow(t *testing.T) { + for _, tc := range []struct { + name string + expected bool + data map[string]interface{} + now time.Time + timeModifier func(t time.Time) time.Time + }{ + { + "always returns true for rotation_period type", + true, + map[string]interface{}{ + "rotation_period": "86400s", + }, + time.Now(), + nil, + }, + { + "always returns true for rotation_schedule when no rotation_window set", + true, + map[string]interface{}{ + "rotation_schedule": "0 0 */2 * * *", + }, + time.Now(), + nil, + }, + { + "returns true for rotation_schedule when inside rotation_window", + true, + map[string]interface{}{ + "rotation_schedule": "0 0 */2 * * *", + "rotation_window": "3600s", + }, + time.Now(), + func(t time.Time) time.Time { + // set current time just inside window + return t.Add(-3640 * time.Second) + }, + }, + { + "returns false for rotation_schedule when outside rotation_window", + false, + map[string]interface{}{ + "rotation_schedule": "0 0 */2 * * *", + "rotation_window": "3600s", + }, + time.Now(), + func(t time.Time) time.Time { + // set current time just outside window + return t.Add(-3560 * time.Second) + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + b, s, mockDB := getBackend(t) + defer b.Cleanup(ctx) + configureDBMount(t, s) + + testTime := tc.now + if tc.data["rotation_schedule"] != nil && tc.timeModifier != nil { + rotationSchedule := tc.data["rotation_schedule"].(string) + schedule, err := b.scheduleParser.Parse(rotationSchedule) + if err != nil { + t.Fatalf("could not parse rotation_schedule: %s", err) + } + sched, ok := schedule.(*cron.SpecSchedule) + if !ok { + t.Fatalf("could not parse rotation_schedule") + } + next1 := sched.Next(tc.now) // the next rotation time we expect + next2 := sched.Next(next1) // the next rotation time after that + testTime = tc.timeModifier(next2) + } + + tc.data["username"] = "hashicorp" + tc.data["db_name"] = "mockv5" + createRoleWithData(t, b, s, mockDB, "test-role", tc.data) + role, err := b.StaticRole(ctx, s, "test-role") + if err != nil { + t.Fatal(err) + } + + isInsideWindow := role.StaticAccount.IsInsideRotationWindow(testTime) + if tc.expected != isInsideWindow { + t.Fatalf("expected %t, got %t", tc.expected, isInsideWindow) + } + }) + } +} + func createRole(t *testing.T, b *databaseBackend, storage logical.Storage, mockDB *mockNewDatabase, roleName string) { t.Helper() mockDB.On("UpdateUser", mock.Anything, mock.Anything). @@ -848,6 +1194,22 @@ func createRole(t *testing.T, b *databaseBackend, storage logical.Storage, mockD } } +func createRoleWithData(t *testing.T, b *databaseBackend, s logical.Storage, mockDB *mockNewDatabase, roleName string, data map[string]interface{}) { + t.Helper() + mockDB.On("UpdateUser", mock.Anything, mock.Anything). + Return(v5.UpdateUserResponse{}, nil). + Once() + resp, err := b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.CreateOperation, + Path: "static-roles/" + roleName, + Storage: s, + Data: data, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatal(resp, err) + } +} + const testRoleStaticCreate = ` CREATE ROLE "{{name}}" WITH LOGIN diff --git a/builtin/logical/database/path_rotate_credentials.go b/builtin/logical/database/path_rotate_credentials.go index 72ce644b8926..f8ae66485e4d 100644 --- a/builtin/logical/database/path_rotate_credentials.go +++ b/builtin/logical/database/path_rotate_credentials.go @@ -224,7 +224,7 @@ func (b *databaseBackend) pathRotateRoleCredentialsUpdate() framework.OperationF item.Value = resp.WALID } } else { - item.Priority = resp.RotationTime.Add(role.StaticAccount.RotationPeriod).Unix() + item.Priority = role.StaticAccount.NextRotationTimeFromInput(resp.RotationTime).Unix() // Clear any stored WAL ID as we must have successfully deleted our WAL to get here. item.Value = "" } diff --git a/builtin/logical/database/rotation.go b/builtin/logical/database/rotation.go index ec8209b80e6b..090678bcba22 100644 --- a/builtin/logical/database/rotation.go +++ b/builtin/logical/database/rotation.go @@ -22,6 +22,9 @@ const ( // Default interval to check the queue for items needing rotation defaultQueueTickSeconds = 5 + // Minimum allowed value for rotation_window + minRotationWindowSeconds = 3600 + // Config key to set an alternate interval queueTickIntervalKey = "rotation_queue_tick_interval" @@ -91,6 +94,8 @@ func (b *databaseBackend) populateQueue(ctx context.Context, s logical.Storage) log.Warn("unable to delete WAL", "error", err, "WAL ID", walEntry.walID) } } else { + // previous rotation attempt was interrupted, so we set the + // Priority as highest to be processed immediately log.Info("found WAL for role", "role", item.Key, "WAL ID", walEntry.walID) item.Value = walEntry.walID item.Priority = time.Now().Unix() @@ -215,9 +220,8 @@ func (b *databaseBackend) rotateCredential(ctx context.Context, s logical.Storag logger = logger.With("database", role.DBName) - // If "now" is less than the Item priority, then this item does not need to - // be rotated - if time.Now().Unix() < item.Priority { + if !role.StaticAccount.ShouldRotate(item.Priority) { + // do not rotate now, push item back onto queue to be rotated later if err := b.pushItem(item); err != nil { logger.Error("unable to push item on to queue", "error", err) } @@ -264,8 +268,8 @@ func (b *databaseBackend) rotateCredential(ctx context.Context, s logical.Storag } // Update priority and push updated Item to the queue - nextRotation := lvr.Add(role.StaticAccount.RotationPeriod) - item.Priority = nextRotation.Unix() + item.Priority = role.StaticAccount.NextRotationTimeFromInput(lvr).Unix() + if err := b.pushItem(item); err != nil { logger.Warn("unable to push item on to queue", "error", err) } @@ -490,6 +494,7 @@ func (b *databaseBackend) setStaticAccount(ctx context.Context, s logical.Storag // lvr is the known LastVaultRotation lvr := time.Now() input.Role.StaticAccount.LastVaultRotation = lvr + input.Role.StaticAccount.SetNextVaultRotation(lvr) output.RotationTime = lvr entry, err := logical.StorageEntryJSON(databaseStaticRolePath+input.RoleName, input.Role) diff --git a/builtin/logical/database/rotation_test.go b/builtin/logical/database/rotation_test.go index db4df16d2a27..4e7b1dbfba77 100644 --- a/builtin/logical/database/rotation_test.go +++ b/builtin/logical/database/rotation_test.go @@ -36,7 +36,7 @@ const ( dbUserDefaultPassword = "password" ) -func TestBackend_StaticRole_Rotate_basic(t *testing.T) { +func TestBackend_StaticRole_Rotation_basic(t *testing.T) { cluster, sys := getCluster(t) defer cluster.Cleanup() @@ -82,109 +82,168 @@ func TestBackend_StaticRole_Rotate_basic(t *testing.T) { t.Fatalf("err:%s resp:%#v\n", err, resp) } - data = map[string]interface{}{ - "name": "plugin-role-test", - "db_name": "plugin-test", - "rotation_statements": testRoleStaticUpdate, - "username": dbUser, - "rotation_period": "5400s", + testCases := map[string]struct { + account map[string]interface{} + path string + expected map[string]interface{} + waitTime time.Duration + }{ + "basic with rotation_period": { + account: map[string]interface{}{ + "username": dbUser, + "rotation_period": "5400s", + }, + path: "plugin-role-test-1", + expected: map[string]interface{}{ + "username": dbUser, + "rotation_period": float64(5400), + }, + }, + "rotation_schedule is set and expires": { + account: map[string]interface{}{ + "username": dbUser, + "rotation_schedule": "*/10 * * * * *", + }, + path: "plugin-role-test-2", + expected: map[string]interface{}{ + "username": dbUser, + "rotation_schedule": "*/10 * * * * *", + }, + waitTime: 20 * time.Second, + }, } - req = &logical.Request{ - Operation: logical.CreateOperation, - Path: "static-roles/plugin-role-test", - Storage: config.StorageView, - Data: data, - } + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + data = map[string]interface{}{ + "name": "plugin-role-test", + "db_name": "plugin-test", + "rotation_statements": testRoleStaticUpdate, + "username": dbUser, + } - resp, err = b.HandleRequest(namespace.RootContext(nil), req) - if err != nil || (resp != nil && resp.IsError()) { - t.Fatalf("err:%s resp:%#v\n", err, resp) - } + for k, v := range tc.account { + data[k] = v + } - // Read the creds - data = map[string]interface{}{} - req = &logical.Request{ - Operation: logical.ReadOperation, - Path: "static-creds/plugin-role-test", - Storage: config.StorageView, - Data: data, - } + req = &logical.Request{ + Operation: logical.CreateOperation, + Path: "static-roles/" + tc.path, + Storage: config.StorageView, + Data: data, + } - resp, err = b.HandleRequest(namespace.RootContext(nil), req) - if err != nil || (resp != nil && resp.IsError()) { - t.Fatalf("err:%s resp:%#v\n", err, resp) - } + resp, err = b.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } - username := resp.Data["username"].(string) - password := resp.Data["password"].(string) - if username == "" || password == "" { - t.Fatalf("empty username (%s) or password (%s)", username, password) - } + // Read the creds + data = map[string]interface{}{} + req = &logical.Request{ + Operation: logical.ReadOperation, + Path: "static-creds/" + tc.path, + Storage: config.StorageView, + Data: data, + } - // Verify username/password - verifyPgConn(t, dbUser, password, connURL) + resp, err = b.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } - // Re-read the creds, verifying they aren't changing on read - data = map[string]interface{}{} - req = &logical.Request{ - Operation: logical.ReadOperation, - Path: "static-creds/plugin-role-test", - Storage: config.StorageView, - Data: data, - } - resp, err = b.HandleRequest(namespace.RootContext(nil), req) - if err != nil || (resp != nil && resp.IsError()) { - t.Fatalf("err:%s resp:%#v\n", err, resp) - } + username := resp.Data["username"].(string) + password := resp.Data["password"].(string) + if username == "" || password == "" { + t.Fatalf("empty username (%s) or password (%s)", username, password) + } - if username != resp.Data["username"].(string) || password != resp.Data["password"].(string) { - t.Fatal("expected re-read username/password to match, but didn't") - } + // Verify username/password + verifyPgConn(t, dbUser, password, connURL) - // Trigger rotation - data = map[string]interface{}{"name": "plugin-role-test"} - req = &logical.Request{ - Operation: logical.UpdateOperation, - Path: "rotate-role/plugin-role-test", - Storage: config.StorageView, - Data: data, - } - resp, err = b.HandleRequest(namespace.RootContext(nil), req) - if err != nil || (resp != nil && resp.IsError()) { - t.Fatalf("err:%s resp:%#v\n", err, resp) - } + // Re-read the creds, verifying they aren't changing on read + data = map[string]interface{}{} + req = &logical.Request{ + Operation: logical.ReadOperation, + Path: "static-creds/" + tc.path, + Storage: config.StorageView, + Data: data, + } + resp, err = b.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } - if resp != nil { - t.Fatalf("Expected empty response from rotate-role: (%#v)", resp) - } + if username != resp.Data["username"].(string) || password != resp.Data["password"].(string) { + t.Fatal("expected re-read username/password to match, but didn't") + } - // Re-Read the creds - data = map[string]interface{}{} - req = &logical.Request{ - Operation: logical.ReadOperation, - Path: "static-creds/plugin-role-test", - Storage: config.StorageView, - Data: data, - } - resp, err = b.HandleRequest(namespace.RootContext(nil), req) - if err != nil || (resp != nil && resp.IsError()) { - t.Fatalf("err:%s resp:%#v\n", err, resp) - } + // Trigger rotation + data = map[string]interface{}{"name": "plugin-role-test"} + req = &logical.Request{ + Operation: logical.UpdateOperation, + Path: "rotate-role/" + tc.path, + Storage: config.StorageView, + Data: data, + } + resp, err = b.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } - newPassword := resp.Data["password"].(string) - if password == newPassword { - t.Fatalf("expected passwords to differ, got (%s)", newPassword) - } + if resp != nil { + t.Fatalf("Expected empty response from rotate-role: (%#v)", resp) + } - // Verify new username/password - verifyPgConn(t, username, newPassword, connURL) + // Re-Read the creds + data = map[string]interface{}{} + req = &logical.Request{ + Operation: logical.ReadOperation, + Path: "static-creds/" + tc.path, + Storage: config.StorageView, + Data: data, + } + resp, err = b.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } + + newPassword := resp.Data["password"].(string) + if password == newPassword { + t.Fatalf("expected passwords to differ, got (%s)", newPassword) + } + + // Verify new username/password + verifyPgConn(t, username, newPassword, connURL) + + if tc.waitTime > 0 { + time.Sleep(tc.waitTime) + // Re-Read the creds after schedule expiration + data = map[string]interface{}{} + req = &logical.Request{ + Operation: logical.ReadOperation, + Path: "static-creds/" + tc.path, + Storage: config.StorageView, + Data: data, + } + resp, err = b.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } + + checkPassword := resp.Data["password"].(string) + if newPassword == checkPassword { + t.Fatalf("expected passwords to differ, got (%s)", checkPassword) + } + } + }) + } } // Sanity check to make sure we don't allow an attempt of rotating credentials // for non-static accounts, which doesn't make sense anyway, but doesn't hurt to // verify we return an error -func TestBackend_StaticRole_Rotate_NonStaticError(t *testing.T) { +func TestBackend_StaticRole_Rotation_NonStaticError(t *testing.T) { cluster, sys := getCluster(t) defer cluster.Cleanup() @@ -288,7 +347,7 @@ func TestBackend_StaticRole_Rotate_NonStaticError(t *testing.T) { } } -func TestBackend_StaticRole_Revoke_user(t *testing.T) { +func TestBackend_StaticRole_Rotation_Revoke_user(t *testing.T) { cluster, sys := getCluster(t) defer cluster.Cleanup() @@ -466,7 +525,7 @@ func verifyPgConn(t *testing.T, username, password, connURL string) { // WAL testing // // First scenario, WAL contains a role name that does not exist. -func TestBackend_Static_QueueWAL_discard_role_not_found(t *testing.T) { +func TestBackend_StaticRole_Rotation_QueueWAL_discard_role_not_found(t *testing.T) { cluster, sys := getCluster(t) defer cluster.Cleanup() @@ -507,7 +566,7 @@ func TestBackend_Static_QueueWAL_discard_role_not_found(t *testing.T) { // Second scenario, WAL contains a role name that does exist, but the role's // LastVaultRotation is greater than the WAL has -func TestBackend_Static_QueueWAL_discard_role_newer_rotation_date(t *testing.T) { +func TestBackend_StaticRole_Rotation_QueueWAL_discard_role_newer_rotation_date(t *testing.T) { cluster, sys := getCluster(t) defer cluster.Cleanup() @@ -695,7 +754,7 @@ func assertWALCount(t *testing.T, s logical.Storage, expected int, key string) { type userCreator func(t *testing.T, username, password string) -func TestBackend_StaticRole_Rotations_PostgreSQL(t *testing.T) { +func TestBackend_StaticRole_Rotation_PostgreSQL(t *testing.T) { cleanup, connURL := postgreshelper.PrepareTestContainer(t, "13.4-buster") defer cleanup() uc := userCreator(func(t *testing.T, username, password string) { @@ -707,7 +766,7 @@ func TestBackend_StaticRole_Rotations_PostgreSQL(t *testing.T) { }) } -func TestBackend_StaticRole_Rotations_MongoDB(t *testing.T) { +func TestBackend_StaticRole_Rotation_MongoDB(t *testing.T) { cleanup, connURL := mongodb.PrepareTestContainerWithDatabase(t, "5.0.10", "vaulttestdb") defer cleanup() @@ -720,7 +779,7 @@ func TestBackend_StaticRole_Rotations_MongoDB(t *testing.T) { }) } -func TestBackend_StaticRole_Rotations_MongoDBAtlas(t *testing.T) { +func TestBackend_StaticRole_Rotation_MongoDBAtlas(t *testing.T) { // To get the project ID, connect to cloud.mongodb.com, go to the vault-test project and // look at Project Settings. projID := os.Getenv("VAULT_MONGODBATLAS_PROJECT_ID") @@ -944,7 +1003,7 @@ type createUserCommand struct { } // Demonstrates a bug fix for the credential rotation not releasing locks -func TestBackend_StaticRole_LockRegression(t *testing.T) { +func TestBackend_StaticRole_Rotation_LockRegression(t *testing.T) { cluster, sys := getCluster(t) defer cluster.Cleanup() @@ -1023,7 +1082,7 @@ func TestBackend_StaticRole_LockRegression(t *testing.T) { } } -func TestBackend_StaticRole_Rotate_Invalid_Role(t *testing.T) { +func TestBackend_StaticRole_Rotation_Invalid_Role(t *testing.T) { cluster, sys := getCluster(t) defer cluster.Cleanup() @@ -1160,10 +1219,18 @@ func TestRollsPasswordForwardsUsingWAL(t *testing.T) { func TestStoredWALsCorrectlyProcessed(t *testing.T) { const walNewPassword = "new-password-from-wal" + + rotationPeriodData := map[string]interface{}{ + "username": "hashicorp", + "db_name": "mockv5", + "rotation_period": "86400s", + } + for _, tc := range []struct { name string shouldRotate bool wal *setCredentialsWAL + data map[string]interface{} }{ { "WAL is kept and used for roll forward", @@ -1174,6 +1241,7 @@ func TestStoredWALsCorrectlyProcessed(t *testing.T) { NewPassword: walNewPassword, LastVaultRotation: time.Now().Add(time.Hour), }, + rotationPeriodData, }, { "zero-time WAL is discarded on load", @@ -1184,9 +1252,10 @@ func TestStoredWALsCorrectlyProcessed(t *testing.T) { NewPassword: walNewPassword, LastVaultRotation: time.Time{}, }, + rotationPeriodData, }, { - "empty-password WAL is kept but a new password is generated", + "rotation_period empty-password WAL is kept but a new password is generated", true, &setCredentialsWAL{ RoleName: "hashicorp", @@ -1194,6 +1263,22 @@ func TestStoredWALsCorrectlyProcessed(t *testing.T) { NewPassword: "", LastVaultRotation: time.Now().Add(time.Hour), }, + rotationPeriodData, + }, + { + "rotation_schedule empty-password WAL is kept but a new password is generated", + true, + &setCredentialsWAL{ + RoleName: "hashicorp", + Username: "hashicorp", + NewPassword: "", + LastVaultRotation: time.Now().Add(time.Hour), + }, + map[string]interface{}{ + "username": "hashicorp", + "db_name": "mockv5", + "rotation_schedule": "*/10 * * * * *", + }, }, } { t.Run(tc.name, func(t *testing.T) { @@ -1209,7 +1294,7 @@ func TestStoredWALsCorrectlyProcessed(t *testing.T) { } b.credRotationQueue = queue.New() configureDBMount(t, config.StorageView) - createRole(t, b, config.StorageView, mockDB, "hashicorp") + createRoleWithData(t, b, config.StorageView, mockDB, tc.wal.RoleName, tc.data) role, err := b.StaticRole(ctx, config.StorageView, "hashicorp") if err != nil { t.Fatal(err) @@ -1247,6 +1332,7 @@ func TestStoredWALsCorrectlyProcessed(t *testing.T) { t.Fatal(err) } + nextRotationTime := role.StaticAccount.NextRotationTime() if tc.shouldRotate { if tc.wal.NewPassword != "" { // Should use WAL's new_password field @@ -1262,11 +1348,11 @@ func TestStoredWALsCorrectlyProcessed(t *testing.T) { t.Fatal() } } + // Ensure the role was not promoted for early rotation + assertPriorityUnchanged(t, item.Priority, nextRotationTime) } else { // Ensure the role was not promoted for early rotation - if item.Priority < time.Now().Add(time.Hour).Unix() { - t.Fatal("priority should be for about a week away, but was", item.Priority) - } + assertPriorityUnchanged(t, item.Priority, nextRotationTime) if role.StaticAccount.Password != initialPassword { t.Fatal("password should not have been rotated yet") } @@ -1447,3 +1533,12 @@ func newBoolPtr(b bool) *bool { v := b return &v } + +// assertPriorityUnchanged is a helper to verify that the priority is the +// expected value for a given rotation time +func assertPriorityUnchanged(t *testing.T, priority int64, nextRotationTime time.Time) { + t.Helper() + if priority != nextRotationTime.Unix() { + t.Fatalf("expected next rotation at %s, but got %s", nextRotationTime, time.Unix(priority, 0).String()) + } +} diff --git a/changelog/22484.txt b/changelog/22484.txt new file mode 100644 index 000000000000..6992e7c2fa56 --- /dev/null +++ b/changelog/22484.txt @@ -0,0 +1,4 @@ +```release-note:feature +**Database Static Role Advanced TTL Management**: Adds the ability to rotate +static roles on a defined schedule. +``` diff --git a/go.mod b/go.mod index 28632cafbc7b..a93bfed0374e 100644 --- a/go.mod +++ b/go.mod @@ -191,6 +191,7 @@ require ( github.com/prometheus/client_golang v1.14.0 github.com/prometheus/common v0.37.0 github.com/rboyer/safeio v0.2.1 + github.com/robfig/cron/v3 v3.0.1 github.com/ryanuber/columnize v2.1.0+incompatible github.com/ryanuber/go-glob v1.0.0 github.com/sasha-s/go-deadlock v0.2.0 diff --git a/go.sum b/go.sum index 5103aa8ae5d4..ec78bdec71a4 100644 --- a/go.sum +++ b/go.sum @@ -2575,6 +2575,8 @@ github.com/remyoudompheng/bigfft v0.0.0-20200410134404-eec4a21b6bb0/go.mod h1:qq github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec/go.mod h1:qqbHyh8v60DhA7CoWK5oRCqLrMHRGoxYCSS9EjAz6Eo= github.com/renier/xmlrpc v0.0.0-20170708154548-ce4a1a486c03 h1:Wdi9nwnhFNAlseAOekn6B5G/+GMtks9UKbvRU/CMM/o= github.com/renier/xmlrpc v0.0.0-20170708154548-ce4a1a486c03/go.mod h1:gRAiPF5C5Nd0eyyRdqIu9qTiFSoZzpTq727b5B8fkkU= +github.com/robfig/cron/v3 v3.0.1 h1:WdRxkvbJztn8LMz/QEvLN5sBU+xKpSqwwUO1Pjr4qDs= +github.com/robfig/cron/v3 v3.0.1/go.mod h1:eQICP3HwyT7UooqI/z+Ov+PtYAWygg1TEWWzGIFLtro= github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg= github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ= github.com/rogpeppe/go-internal v1.1.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=