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

Add the ability to set seconds in cron schedule for testing purposes #22531

Merged
merged 24 commits into from
Aug 25, 2023
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
2d934c9
add rotation_schedule field to db backend
fairclothjm Aug 3, 2023
5511ed4
add cron schedule field
fairclothjm Aug 4, 2023
6b95acf
use priority queue with scheduled rotation types
fairclothjm Aug 4, 2023
417466c
allow marshalling of cron schedule type
fairclothjm Aug 4, 2023
4779495
return warning on use of mutually exclusive fields
fairclothjm Aug 4, 2023
acb5ebb
handle mutual exclusion of rotation fields (#22306)
Zlaticanin Aug 11, 2023
7ceac55
adv ttl mgmt: add rotation_window field (#22303)
fairclothjm Aug 12, 2023
6aedaaf
adv ttl mgmt: Ensure initialization sets appropriate rotation schedul…
fairclothjm Aug 15, 2023
7a542fc
add unit tests to handle mutual exclusion (#22352)
Zlaticanin Aug 16, 2023
c270552
adv ttl mgmt: add tests for init queue (#22376)
fairclothjm Aug 16, 2023
f23ff91
Vault 18908/handle manual rotation (#22389)
Zlaticanin Aug 17, 2023
1794670
adv ttl mgmt: consider rotation window (#22448)
fairclothjm Aug 18, 2023
519eacf
add unit tests for manual rotation (#22453)
Zlaticanin Aug 18, 2023
38c3d76
adv ttl mgmt: add tests for rotation_window
fairclothjm Aug 21, 2023
d725e4a
adv ttl mgmt: refactor window tests (#22472)
fairclothjm Aug 21, 2023
fdeb705
Handle GET static-creds endpoint (#22476)
Zlaticanin Aug 21, 2023
552e804
add changelog
fairclothjm Aug 21, 2023
4fa0f54
add unit test for static-creds read endpoint (#22505)
Zlaticanin Aug 22, 2023
aee50be
Add the ability to set seconds in cron schedule for testing purposes
Zlaticanin Aug 23, 2023
ba3a668
Merge branch 'main' into VAULT-19131/toggle-seconds
Zlaticanin Aug 25, 2023
324d13e
update test so we don't use global var
Zlaticanin Aug 25, 2023
7a9f888
merge mainMerge branch 'VAULT-19131/toggle-seconds' of https://github…
Zlaticanin Aug 25, 2023
6318db4
update with suggestions
Zlaticanin Aug 25, 2023
b42bd75
Merge branch 'main' into VAULT-19131/toggle-seconds
Zlaticanin Aug 25, 2023
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: 8 additions & 0 deletions builtin/logical/database/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -35,6 +36,8 @@ const (
minRootCredRollbackAge = 1 * time.Minute
)

var scheduleOptions = cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow
Zlaticanin marked this conversation as resolved.
Show resolved Hide resolved

type dbPluginInstance struct {
sync.RWMutex
database databaseVersionWrapper
Expand Down Expand Up @@ -127,6 +130,9 @@ func Backend(conf *logical.BackendConfig) *databaseBackend {
b.connections = syncmap.NewSyncMap[string, *dbPluginInstance]()
b.queueCtx, b.cancelQueueCtx = context.WithCancel(context.Background())
b.roleLocks = locksutil.CreateLocks()

parser := cron.NewParser(scheduleOptions)
b.scheduleParser = parser
return &b
}

Expand Down Expand Up @@ -176,6 +182,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) {
Expand Down
10 changes: 9 additions & 1 deletion builtin/logical/database/path_creds_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
177 changes: 164 additions & 13 deletions builtin/logical/database/path_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
Loading