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

adv ttl mgmt: add rotation_window field #22303

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
4 changes: 3 additions & 1 deletion builtin/logical/database/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,10 @@ func Backend(conf *logical.BackendConfig) *databaseBackend {
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 | cron.Descriptor,
cron.Second | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.DowOptional,
)
b.scheduleParser = parser
return &b
Expand Down
48 changes: 42 additions & 6 deletions builtin/logical/database/path_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +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.`,
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,
Expand Down Expand Up @@ -299,11 +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
}
data["rotation_schedule"] = role.StaticAccount.RotationSchedule

// only return one of the mutually exclusive fields in the response
if role.StaticAccount.RotationPeriod != 0 {
data["rotation_period"] = role.StaticAccount.RotationPeriod.Seconds()
} else if role.StaticAccount.RotationSchedule != "" {
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 {
Expand Down Expand Up @@ -547,6 +562,7 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l
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
Expand All @@ -565,6 +581,10 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l
role.StaticAccount.RotationPeriod = time.Duration(rotationPeriodSeconds) * time.Second
// Unset rotation schedule if rotation period is set
role.StaticAccount.RotationSchedule = ""

if rotationWindowOk {
return logical.ErrorResponse("rotation_window is invalid with use of rotation_period"), nil
}
}

if rotationScheduleOk {
Expand All @@ -581,6 +601,14 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l
role.StaticAccount.Schedule = *sched
// Unset rotation period if rotation schedule is set
role.StaticAccount.RotationPeriod = 0

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
}
}

if rotationStmtsRaw, ok := data.GetOk("rotation_statements"); ok {
Expand Down Expand Up @@ -654,10 +682,12 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l
}
}

if rotationPeriodOk {
_, 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 {
} else if rotationScheduleChanged {
next := role.StaticAccount.Schedule.Next(lvr)
b.logger.Debug("init priority for Schedule", "lvr", lvr)
b.logger.Debug("init priority for Schedule", "next", next)
Expand Down Expand Up @@ -783,6 +813,12 @@ type staticAccount struct {
// 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
Expand Down
73 changes: 69 additions & 4 deletions builtin/logical/database/path_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"encoding/json"
"errors"
"fmt"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -254,6 +255,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{}{
Expand All @@ -266,12 +269,62 @@ 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 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{}{
Expand Down Expand Up @@ -305,7 +358,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)
}
Expand Down Expand Up @@ -341,7 +399,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
Expand Down
3 changes: 3 additions & 0 deletions builtin/logical/database/rotation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
Loading