Skip to content

Commit

Permalink
secrets/database: advanced TTL management for static roles (#22484)
Browse files Browse the repository at this point in the history
* add rotation_schedule field to db backend

* add cron schedule field

* use priority queue with scheduled rotation types

* allow marshalling of cron schedule type

* return warning on use of mutually exclusive fields

* handle mutual exclusion of rotation fields (#22306)

* handle mutual exclusion of rotation fields

* fix import

* adv ttl mgmt: add rotation_window field (#22303)

* adv ttl mgmt: add rotation_window field

* do some rotation_window validation and add unit tests

* adv ttl mgmt: Ensure initialization sets appropriate rotation schedule (#22341)

* general cleanup and refactor rotation type checks

* make NextRotationTime account for the rotation type

* add comments

* add unit tests to handle mutual exclusion (#22352)

* add unit tests to handle mutual exclusion

* revert rotation_test.go and add missing test case to path_roles_test.go

* adv ttl mgmt: add tests for init queue (#22376)

* Vault 18908/handle manual rotation (#22389)

* support manual rotation for schedule based roles

* update description and naming

* adv ttl mgmt: consider rotation window (#22448)

* consider rotation window

ensure rotations only occur within a rotation window for schedule-based
rotations

* use helper method to set priority in rotateCredential

* fix bug with priority check

* remove test for now

* add and remove comments

* add unit tests for manual rotation (#22453)

* adv ttl mgmt: add tests for rotation_window

* adv ttl mgmt: refactor window tests (#22472)

* Handle GET static-creds endpoint (#22476)

* update read static-creds endpoint to include correct resp data

* return rotation_window if set

* update

* add changelog

* add unit test for static-creds read endpoint (#22505)

---------

Co-authored-by: Milena Zlaticanin <60530402+Zlaticanin@users.noreply.github.com>
  • Loading branch information
fairclothjm and Zlaticanin committed Aug 24, 2023
1 parent 749b0f7 commit 83f3e39
Show file tree
Hide file tree
Showing 10 changed files with 760 additions and 122 deletions.
10 changes: 10 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 Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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) {
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

0 comments on commit 83f3e39

Please sign in to comment.