From 45b19a42fea2cb017fb533a7ee4eee879870beb0 Mon Sep 17 00:00:00 2001 From: Vault Automation Date: Fri, 8 May 2026 09:01:17 -0600 Subject: [PATCH] Backport On-Time Autorotation Maintained in a Queue. into release/2.x.x+ent (#14506) (#14611) * On-Time Autorotation Maintained in a Queue. (#14463) * On-Time Autorotation Maintained in a Queue. * Add changelog. * Renamed changelog correctly. * Add a check to exit-out early if initialization of the backend has not run (probably only relevant in tests, but not a bad check). * GoTestDoc added for test. * Initialize backend in tests. * Add error checks, compilation check, and move initialize function. Switch to object with a zero-value. * Remove non-existant field from backport. * Don't change prior-version locking. --------- Co-authored-by: Kit Haines --- builtin/logical/transit/backend.go | 178 ++++++++++++++++++----- builtin/logical/transit/backend_test.go | 183 ++++++++++++++++++++++++ changelog/_14463.txt | 3 + 3 files changed, 328 insertions(+), 36 deletions(-) create mode 100644 changelog/_14463.txt diff --git a/builtin/logical/transit/backend.go b/builtin/logical/transit/backend.go index 662d0bf2b6..2fe90dbfb1 100644 --- a/builtin/logical/transit/backend.go +++ b/builtin/logical/transit/backend.go @@ -4,7 +4,9 @@ package transit import ( + "container/heap" "context" + "errors" "fmt" "io" "strconv" @@ -90,6 +92,7 @@ func Backend(ctx context.Context, conf *logical.BackendConfig) (*backend, error) } b.backendUUID = conf.BackendUUID + b.initializeRotationQueue() // determine cacheSize to use. Defaults to 0 which means unlimited cacheSize := 0 @@ -131,9 +134,66 @@ type backend struct { cacheSizeChanged bool checkAutoRotateAfter time.Time autoRotateOnce sync.Once + rotationQueue *rotationQueue backendUUID string } +type keyRotationEntry struct { + rotateAt time.Time + keyPath string +} + +func (kre keyRotationEntry) isZero() bool { + return kre.rotateAt.IsZero() && kre.keyPath == "" +} + +// rotationQueue stores information about which keys need to be rotated at what time. It's a priority queue, which +// implements hash.Interface. +type rotationQueue []keyRotationEntry + +var _ heap.Interface = &rotationQueue{} + +func (rq rotationQueue) Len() int { return len(rq) } +func (rq rotationQueue) Less(i, j int) bool { + if i < 0 || j < 0 || i >= rq.Len() || j >= rq.Len() { // If out of bounds, don't switch + return false + } + return rq[i].rotateAt.Before(rq[j].rotateAt) +} + +func (rq rotationQueue) Swap(i, j int) { + if i < 0 || j < 0 || i >= len(rq) || j >= len(rq) { + return + } + rq[i], rq[j] = rq[j], rq[i] +} + +func (rq *rotationQueue) Push(kre any) { + if kre.(keyRotationEntry).isZero() { + return + } + *rq = append(*rq, kre.(keyRotationEntry)) +} + +func (rq *rotationQueue) Pop() any { + if len(*rq) == 0 { + return keyRotationEntry{} + } + old := *rq + n := len(old) + item := (*rq)[n-1] + *rq = old[0 : n-1] + return item +} + +func (rq rotationQueue) Peek() keyRotationEntry { + if len(rq) == 0 { + return keyRotationEntry{} + } else { + return rq[0] + } +} + func GetCacheSizeFromStorage(ctx context.Context, s logical.Storage) (int, error) { size := 0 entry, err := s.Get(ctx, "config/cache") @@ -223,8 +283,7 @@ func (b *backend) invalidate(ctx context.Context, key string) { // periodicFunc is a central collection of functions that run on an interval. // Anything that should be called regularly can be placed within this method. func (b *backend) periodicFunc(ctx context.Context, req *logical.Request) error { - // These operations ensure the auto-rotate only happens once simultaneously. It's an unlikely edge - // given the time scale, but a safeguard nonetheless. + // These operations ensure the auto-rotate only happens once simultaneously. var err error didAutoRotate := false autoRotateOnceFn := func() { @@ -247,6 +306,35 @@ func (b *backend) periodicFunc(ctx context.Context, req *logical.Request) error // auto rotate period defined which has passed. This operation only happens // on primary nodes and performance secondary nodes which have a local mount. func (b *backend) autoRotateKeys(ctx context.Context, req *logical.Request) error { + // Early exit if not a primary or performance secondary with a local mount. + if b.System().ReplicationState().HasState(consts.ReplicationDRSecondary|consts.ReplicationPerformanceStandby) || + (!b.System().LocalMount() && b.System().ReplicationState().HasState(consts.ReplicationPerformanceSecondary)) { + return nil + } + + // Collect errors in a multierror to ensure a single failure doesn't prevent + // all keys from being rotated. + var errs *multierror.Error + + // If we haven't initialized yet, exit out + if b.rotationQueue == nil { + errs = multierror.Append(errs, errors.New("auto-rotation can not run because backend is not initialized")) + return errs.ErrorOrNil() + } + + // Between once-per-hour rotations, check to see if any keys that need rotating are in the heap + if b.rotationQueue.Len() == 0 { + // No keys are scheduled for rotation this hour + } else { + for !b.rotationQueue.Peek().isZero() && b.rotationQueue.Peek().rotateAt.Before(time.Now()) { + kre := heap.Pop(b.rotationQueue).(keyRotationEntry) + _, err := b.rotateByPath(ctx, req, kre.keyPath) + if err != nil { + errs = multierror.Append(errs, err) + } + } + } + // Only check for autorotation once an hour to avoid unnecessarily iterating // over all keys too frequently. if time.Now().Before(b.checkAutoRotateAfter) { @@ -254,49 +342,51 @@ func (b *backend) autoRotateKeys(ctx context.Context, req *logical.Request) erro } b.checkAutoRotateAfter = time.Now().Add(1 * time.Hour) - // Early exit if not a primary or performance secondary with a local mount. - if b.System().ReplicationState().HasState(consts.ReplicationDRSecondary|consts.ReplicationPerformanceStandby) || - (!b.System().LocalMount() && b.System().ReplicationState().HasState(consts.ReplicationPerformanceSecondary)) { - return nil - } - // Retrieve all keys and loop over them to check if they need to be rotated. keys, err := req.Storage.List(ctx, "policy/") if err != nil { return err } - // Collect errors in a multierror to ensure a single failure doesn't prevent - // all keys from being rotated. - var errs *multierror.Error - for _, key := range keys { - p, _, err := b.GetPolicy(ctx, keysutil.PolicyRequest{ - Storage: req.Storage, - Name: key, - }, b.GetRandomReader()) + kre, err := b.rotateByPath(ctx, req, key) if err != nil { errs = multierror.Append(errs, err) - continue } - - // If the policy is nil, move onto the next one. - if p == nil { - continue - } - - // rotateIfRequired properly acquires/releases the lock on p - err = b.rotateIfRequired(ctx, req, key, p) - if err != nil { - errs = multierror.Append(errs, err) + if !kre.isZero() { + heap.Push(b.rotationQueue, kre) } } return errs.ErrorOrNil() } -// rotateIfRequired rotates a key if it is due for autorotation. -func (b *backend) rotateIfRequired(ctx context.Context, req *logical.Request, key string, p *keysutil.Policy) error { +func (b *backend) rotateByPath(ctx context.Context, req *logical.Request, keyPath string) (keyRotationEntry, error) { + p, _, err := b.GetPolicy(ctx, keysutil.PolicyRequest{ + Storage: req.Storage, + Name: keyPath, + }, b.GetRandomReader()) + if err != nil { + return keyRotationEntry{}, err + } + + // If the policy is nil, move onto the next one. + if p == nil { + return keyRotationEntry{}, nil + } + + // rotateIfRequired properly acquires/releases the lock on p + kre, err := b.rotateIfRequired(ctx, req, keyPath, p) + if err != nil { + return kre, err + } + + return kre, err +} + +// rotateIfRequired rotates a key if it is due for autorotation. If it isn't due for autorotation, but will be due +// soon (within an hour), it returns a keyRotationEntry. +func (b *backend) rotateIfRequired(ctx context.Context, req *logical.Request, key string, p *keysutil.Policy) (kre keyRotationEntry, err error) { if !b.System().CachingDisabled() { p.Lock(true) } @@ -304,36 +394,52 @@ func (b *backend) rotateIfRequired(ctx context.Context, req *logical.Request, ke // If the key is imported, it can only be rotated from within Vault if allowed. if p.Imported && !p.AllowImportedKeyRotation { - return nil + return keyRotationEntry{}, nil } // If the policy's automatic rotation period is 0, it should not // automatically rotate. if p.AutoRotatePeriod == 0 { - return nil + return keyRotationEntry{}, nil } // We can't auto-rotate managed keys if p.Type == keysutil.KeyType_MANAGED_KEY { - return nil + return keyRotationEntry{}, nil } // Retrieve the latest version of the policy and determine if it is time to rotate. latestKey := p.Keys[strconv.Itoa(p.LatestVersion)] - if time.Now().After(latestKey.CreationTime.Add(p.AutoRotatePeriod)) { + autoRotateAt := latestKey.CreationTime.Add(p.AutoRotatePeriod) + if time.Now().After(autoRotateAt) { if b.Logger().IsDebug() { b.Logger().Debug("automatically rotating key", "key", key) } - return p.Rotate(ctx, req.Storage, b.GetRandomReader()) - + return keyRotationEntry{}, p.Rotate(ctx, req.Storage, b.GetRandomReader()) + } else { + // Check if it will be time to rotate the key within the next hour + if time.Now().Add(1 * time.Hour).After(autoRotateAt) { + kre = keyRotationEntry{ + rotateAt: autoRotateAt, + keyPath: key, + } + return kre, nil + } } - return nil + + return keyRotationEntry{}, nil } func (b *backend) initialize(ctx context.Context, request *logical.InitializationRequest) error { return b.initializeEnt(ctx, request) } +func (b *backend) initializeRotationQueue() { + rotationQueue := &rotationQueue{} + heap.Init(rotationQueue) + b.rotationQueue = rotationQueue +} + func (b *backend) cleanup(ctx context.Context) { b.cleanupEnt(ctx) } diff --git a/builtin/logical/transit/backend_test.go b/builtin/logical/transit/backend_test.go index 955344a21e..02838d0afd 100644 --- a/builtin/logical/transit/backend_test.go +++ b/builtin/logical/transit/backend_test.go @@ -22,6 +22,7 @@ import ( "sync" "sync/atomic" "testing" + "testing/synctest" "time" uuid "github.com/hashicorp/go-uuid" @@ -1741,6 +1742,10 @@ func TestTransit_AutoRotateKeys(t *testing.T) { if err != nil { t.Fatal(err) } + err = b.Initialize(context.Background(), &logical.InitializationRequest{Storage: storage}) + if err != nil { + t.Fatal(err) + } // Write a key with the default auto rotate value (0/disabled) req := &logical.Request{ @@ -1859,6 +1864,184 @@ func TestTransit_AutoRotateKeys(t *testing.T) { } } +// TestTransit_AutoRotateKeysNotAffectedByManualRotation sets up a backend, then tests: that a (1h auto-rotate) key +// which was manually rotated is not auto-rotated 1hour after initial creation, rather 1hour after it's manual rotation. +// Because auto-rotation sets up a priority queue to do this, the ordering of that queue needs to be tested: this tests +// that by setting up two auto-rotating keys. +func TestTransit_AutoRotateKeysNotAffectedByManualRotation(t *testing.T) { + sysView := logical.TestSystemView() + storage := &logical.InmemStorage{} + + conf := &logical.BackendConfig{ + StorageView: storage, + System: sysView, + } + + synctest.Test(t, func(t *testing.T) { + b, _ := Backend(context.Background(), conf) + if b == nil { + t.Fatal("failed to create backend") + } + + err := b.Backend.Setup(context.Background(), conf) + if err != nil { + t.Fatal(err) + } + + err = b.Backend.Initialize(context.Background(), &logical.InitializationRequest{Storage: storage}) + if err != nil { + t.Fatal(err) + } + + // Write a key with 1h rotate value + req := &logical.Request{ + Storage: storage, + Operation: logical.UpdateOperation, + Path: "keys/test1", + Data: map[string]interface{}{ + "auto_rotate_period": 1 * time.Hour, + }, + } + resp, err := b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + require.NotNil(t, resp, "expected populated request") + + // Write a second key with an auto rotate value + req = &logical.Request{ + Storage: storage, + Operation: logical.UpdateOperation, + Path: "keys/test2", + Data: map[string]interface{}{ + "auto_rotate_period": 1 * time.Hour, + }, + } + resp, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + require.NotNil(t, resp, "expected populated request") + + time.Sleep(time.Minute * 10) + + // Manually Rotate the First Key + req = &logical.Request{ + Storage: storage, + Operation: logical.UpdateOperation, + Path: "keys/test1/rotate", + } + resp, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + require.NotNil(t, resp, "expected populated request") + + time.Sleep(time.Minute * 10) + + // Manually Rotate the Second Key + req = &logical.Request{ + Storage: storage, + Operation: logical.UpdateOperation, + Path: "keys/test2/rotate", + } + resp, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + require.NotNil(t, resp, "expected populated request") + + time.Sleep(time.Minute * 41) + err = b.periodicFunc(context.Background(), &logical.Request{Storage: storage}) + if err != nil { + t.Fatal(err) + } + + // Check that no auto-rotation happened when keys were only 50, 40 minutes old + // If this is the case version should be "2" - the keys were rotated once manually + req = &logical.Request{ + Storage: storage, + Operation: logical.ReadOperation, + Path: "keys/test1", + } + resp, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp == nil { + t.Fatal("expected non-nil response") + } + if resp.Data["latest_version"] != 2 { + t.Fatalf("incorrect latest_version found, got: %d, want: %d", resp.Data["latest_version"], 2) + } + req.Path = "keys/test2" + resp, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp == nil { + t.Fatal("expected non-nil response") + } + if resp.Data["latest_version"] != 2 { + t.Fatalf("incorrect latest_version found, got: %d, want: %d", resp.Data["latest_version"], 2) + } + + // This is when the auto-rotation of key test1 should happen + time.Sleep(time.Minute * 11) + err = b.periodicFunc(context.Background(), &logical.Request{Storage: storage}) + if err != nil { + t.Fatal(err) + } + + // Check that the first key auto-rotated when it reached an hour old (but the second didn't at 50 minutes) + req = &logical.Request{ + Storage: storage, + Operation: logical.ReadOperation, + Path: "keys/test1", + } + resp, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp == nil { + t.Fatal("expected non-nil response") + } + if resp.Data["latest_version"] != 3 { + t.Fatalf("incorrect latest_version found, got: %d, want: %d", resp.Data["latest_version"], 3) + } + req.Path = "keys/test2" + resp, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp == nil { + t.Fatal("expected non-nil response") + } + if resp.Data["latest_version"] != 2 { + t.Fatalf("incorrect latest_version found, got: %d, want: %d", resp.Data["latest_version"], 2) + } + + time.Sleep(time.Minute * 10) + err = b.periodicFunc(context.Background(), &logical.Request{Storage: storage}) + if err != nil { + t.Fatal(err) + } + + // Check that the second key auto-rotated when it reached an hour old + req.Path = "keys/test2" + resp, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp == nil { + t.Fatal("expected non-nil response") + } + if resp.Data["latest_version"] != 3 { + t.Fatalf("incorrect latest_version found, got: %d, want: %d", resp.Data["latest_version"], 3) + } + }) +} + func TestTransit_AEAD(t *testing.T) { testTransit_AEAD(t, "aes128-gcm96") testTransit_AEAD(t, "aes256-gcm96") diff --git a/changelog/_14463.txt b/changelog/_14463.txt new file mode 100644 index 0000000000..e19fd38a69 --- /dev/null +++ b/changelog/_14463.txt @@ -0,0 +1,3 @@ +```release-note:bugfix +secrets/transit: added granular autorotation to transit keys, prevents a key missing it's scheduled autorotation by an hour. +``` \ No newline at end of file