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

builtin/logical/database/path_roles.go

@@ -310,9 +310,9 @@ func (b *databaseBackend) pathStaticRoleRead(ctx context.Context, req *logical.R
 		}
 
 		// only return one of the mutually exclusive fields in the response
-		if role.StaticAccount.RotationPeriod != 0 {
+		if role.StaticAccount.UsesRotationPeriod() {
 			data["rotation_period"] = role.StaticAccount.RotationPeriod.Seconds()
-		} else if role.StaticAccount.RotationSchedule != "" {
+		} else if role.StaticAccount.UsesRotationSchedule() {
 			data["rotation_schedule"] = role.StaticAccount.RotationSchedule
 			// rotation_window is only valid with rotation_schedule
 			if role.StaticAccount.RotationWindow != 0 {
@@ -579,16 +579,18 @@ 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
-		// 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
 		}
+
+		// Unset rotation schedule and window if rotation period is set since
+		// these are mutually exclusive
+		role.StaticAccount.RotationSchedule = ""
+		role.StaticAccount.RotationWindow = 0
 	}
 
 	if rotationScheduleOk {
-		// TODO(JM): validate this isn't less than defaultQueueTickSeconds?
 		schedule, err := b.scheduleParser.Parse(rotationSchedule)
 		if err != nil {
 			return logical.ErrorResponse("could not parse rotation_schedule", "error", err), nil
@@ -599,8 +601,6 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l
 			return logical.ErrorResponse("could not parse rotation_schedule"), nil
 		}
 		role.StaticAccount.Schedule = *sched
-		// Unset rotation period if rotation schedule is set
-		role.StaticAccount.RotationPeriod = 0
 
 		if rotationWindowOk {
 			rotationWindowSeconds := rotationWindowSecondsRaw.(int)
@@ -609,6 +609,10 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l
 			}
 			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 {
@@ -689,8 +693,7 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l
 		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)
-		b.logger.Debug("init priority for Schedule", "next", next)
+		b.logger.Debug("init priority for Schedule", "lvr", lvr, "next", next)
 		item.Priority = role.StaticAccount.Schedule.Next(lvr).Unix()
 	}
 
@@ -804,9 +807,6 @@ type staticAccount struct {
 	// determine if a password needs to be rotated
 	RotationPeriod time.Duration `json:"rotation_period"`
 
-	// NextVaultRotation represents the next time Vault should rotate the password
-	NextVaultRotation time.Time `json:"next_vault_rotation"`
-
 	// 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
@@ -826,14 +826,33 @@ type staticAccount struct {
 	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)
+}
+
+// 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 == ""
 }
 
 // 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

builtin/logical/database/rotation.go

@@ -94,6 +94,11 @@ 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
+
+				// TODO(JM): ensure we don't process schedule-based rotations
+				// outside the rotation_window
 				log.Info("found WAL for role", "role", item.Key, "WAL ID", walEntry.walID)
 				item.Value = walEntry.walID
 				item.Priority = time.Now().Unix()
@@ -220,6 +225,8 @@ func (b *databaseBackend) rotateCredential(ctx context.Context, s logical.Storag
 
 	// If "now" is less than the Item priority, then this item does not need to
 	// be rotated
+	// TODO(JM): ensure we don't process schedule-based rotations
+	// outside the rotation_window
 	if time.Now().Unix() < item.Priority {
 		if err := b.pushItem(item); err != nil {
 			logger.Error("unable to push item on to queue", "error", err)
@@ -267,19 +274,15 @@ func (b *databaseBackend) rotateCredential(ctx context.Context, s logical.Storag
 	}
 
 	// Update priority and push updated Item to the queue
-	if role.StaticAccount.RotationSchedule != "" {
-		schedule, err := b.scheduleParser.Parse(role.StaticAccount.RotationSchedule)
-		if err != nil {
-			b.logger.Error("could not parse rotation_schedule", "error", err)
-			return true
-		}
-		next := schedule.Next(lvr)
-		b.logger.Debug("update priority for Schedule", "lvr", lvr)
-		b.logger.Debug("update priority for Schedule", "next", next)
-		item.Priority = schedule.Next(lvr).Unix()
-	} else {
-		b.logger.Debug("update priority for RotationPeriod", "lvr", lvr, "next", lvr.Add(role.StaticAccount.RotationPeriod))
+	if role.StaticAccount.UsesRotationSchedule() {
+		next := role.StaticAccount.Schedule.Next(lvr)
+		logger.Debug("update priority for Schedule", "next", next)
+		item.Priority = role.StaticAccount.Schedule.Next(lvr).Unix()
+	} else if role.StaticAccount.UsesRotationPeriod() {
+		logger.Debug("update priority for RotationPeriod", "lvr", lvr, "next", lvr.Add(role.StaticAccount.RotationPeriod))
 		item.Priority = lvr.Add(role.StaticAccount.RotationPeriod).Unix()
+	} else {
+		logger.Error("rotation has not been properly configured")
 	}
 
 	if err := b.pushItem(item); err != nil {