Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport of db: honor static role TTL across restarts when skip import rotation i… into release/1.18.x #29540

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions builtin/logical/database/path_creds_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,9 +256,11 @@ func (b *databaseBackend) pathStaticCredsRead() framework.OperationFunc {
}

respData := map[string]interface{}{
"username": role.StaticAccount.Username,
"ttl": role.StaticAccount.CredentialTTL().Seconds(),
"last_vault_rotation": role.StaticAccount.LastVaultRotation,
"username": role.StaticAccount.Username,
"ttl": role.StaticAccount.CredentialTTL().Seconds(),
}
if !role.StaticAccount.LastVaultRotation.IsZero() {
respData["last_vault_rotation"] = role.StaticAccount.LastVaultRotation
}

if role.StaticAccount.UsesRotationPeriod() {
Expand Down
23 changes: 16 additions & 7 deletions builtin/logical/database/path_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -713,14 +713,22 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l
switch req.Operation {
case logical.CreateOperation:
if role.SkipImportRotation {
b.Logger().Trace("skipping static role import rotation", "role", name)
// synthetically set lastVaultRotation to now, so that it gets
// queued correctly
lastVaultRotation = time.Now()
// we intentionally do not set role.StaticAccount.LastVaultRotation
b.Logger().Debug("skipping static role import rotation", "role", name)

// Synthetically set lastVaultRotation to now, so that it gets
// queued correctly.
// NOTE: We intentionally do not set role.StaticAccount.LastVaultRotation
// because the zero value indicates Vault has not rotated the
// password yet
lastVaultRotation = time.Now()

// NextVaultRotation allows calculating the TTL on GET /static-creds
// requests and to calculate the queue priority in populateQueue()
// across restarts. We can't rely on LastVaultRotation in these
// cases bacause, when import rotation is skipped, LastVaultRotation
// is set to a zero value in storage.
role.StaticAccount.SetNextVaultRotation(lastVaultRotation)

// we were told to not rotate, just add the entry
err := b.StoreStaticRole(ctx, req.Storage, role)
if err != nil {
Expand Down Expand Up @@ -932,7 +940,7 @@ type staticAccount struct {
// querying for the next schedule expiry since the last known vault rotation.
func (s *staticAccount) NextRotationTime() time.Time {
if s.UsesRotationPeriod() {
return s.LastVaultRotation.Add(s.RotationPeriod)
return s.NextVaultRotation
}
return s.Schedule.Next(time.Now())
}
Expand Down Expand Up @@ -981,7 +989,8 @@ func (s *staticAccount) ShouldRotate(priority int64, t time.Time) bool {
return priority <= t.Unix() && s.IsInsideRotationWindow(t)
}

// SetNextVaultRotation
// SetNextVaultRotation sets the next vault rotation to time t plus the role's
// rotation period or to the next schedule.
func (s *staticAccount) SetNextVaultRotation(t time.Time) {
if s.UsesRotationPeriod() {
s.NextVaultRotation = t.Add(s.RotationPeriod)
Expand Down
16 changes: 16 additions & 0 deletions builtin/logical/database/path_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1388,6 +1388,22 @@ func createRoleWithData(t *testing.T, b *databaseBackend, s logical.Storage, moc
}
}

func readStaticCred(t *testing.T, b *databaseBackend, s logical.Storage, mockDB *mockNewDatabase, roleName string) *logical.Response {
t.Helper()
mockDB.On("UpdateUser", mock.Anything, mock.Anything).
Return(v5.UpdateUserResponse{}, nil).
Once()
resp, err := b.HandleRequest(context.Background(), &logical.Request{
Operation: logical.ReadOperation,
Path: "static-creds/" + roleName,
Storage: s,
})
if err != nil || (resp != nil && resp.IsError()) {
t.Fatal(resp, err)
}
return resp
}

const testRoleStaticCreate = `
CREATE ROLE "{{name}}" WITH
LOGIN
Expand Down
26 changes: 25 additions & 1 deletion builtin/logical/database/rotation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1669,14 +1669,38 @@ func requireWALs(t *testing.T, storage logical.Storage, expectedCount int) []str
return wals
}

// getBackendWithConfig returns an initialized test backend for the given
// BackendConfig
func getBackendInitQueue(t *testing.T, c *logical.BackendConfig, tickInterval string) (*databaseBackend, *logical.BackendConfig, *mockNewDatabase) {
t.Helper()
// make queue ticks more frequent for tests
c.Config[queueTickIntervalKey] = tickInterval
c.StorageView = &logical.InmemStorage{}
// Create and init the backend ourselves instead of using a Factory because
// the factory function kicks off threads that cause racy tests.
b := Backend(c)
ctx := context.Background()
if err := b.Setup(ctx, c); err != nil {
t.Fatal(err)
}
b.schedule = &TestSchedule{}
b.credRotationQueue = queue.New()
b.initQueue(ctx, c)

mockDB := setupMockDB(b)

return b, c, mockDB
}

func getBackend(t *testing.T) (*databaseBackend, logical.Storage, *mockNewDatabase) {
t.Helper()
config := logical.TestBackendConfig()
config.StorageView = &logical.InmemStorage{}
// Create and init the backend ourselves instead of using a Factory because
// the factory function kicks off threads that cause racy tests.
b := Backend(config)
if err := b.Setup(context.Background(), config); err != nil {
ctx := context.Background()
if err := b.Setup(ctx, config); err != nil {
t.Fatal(err)
}
b.schedule = &TestSchedule{}
Expand Down
3 changes: 3 additions & 0 deletions changelog/29537.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
database: Fix a bug where static role passwords are erroneously rotated across backend restarts when using skip import rotation.
```
Loading