Merge remote-tracking branch 'remotes/from/ce/release/2.x.x' into release/2.x.x

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

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")

changelog/_11823.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+config/listener: logs warnings on invalid x-forwarded-for configurations.
+```

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.
+```

internalshared/configutil/listener.go

@@ -19,6 +19,7 @@ import (
 	"github.com/hashicorp/go-sockaddr/template"
 	"github.com/hashicorp/hcl"
 	"github.com/hashicorp/hcl/hcl/ast"
+	"github.com/hashicorp/hcl/hcl/token"
 	"github.com/hashicorp/vault/helper/namespace"
 )
 
@@ -203,6 +204,7 @@ func (l *Listener) GoString() string {
 
 func (l *Listener) Validate(path string) []ConfigError {
 	results := append(ValidateUnusedFields(l.UnusedKeys, path), ValidateUnusedFields(l.Telemetry.UnusedKeys, path)...)
+	results = append(results, l.validateForwardedForSettings(path)...)
 	return append(results, ValidateUnusedFields(l.Profiling.UnusedKeys, path)...)
 }
 
@@ -663,6 +665,80 @@ func (l *Listener) parseForwardedForSettings() error {
 	return nil
 }
 
+func (l *Listener) validateForwardedForSettings(fileName string) []ConfigError {
+	configErrors := []ConfigError{}
+
+	if len(l.XForwardedForAuthorizedAddrs) == 0 {
+		if l.XForwardedForRejectNotAuthorizedRaw != nil {
+			configErrors = append(configErrors, ConfigError{
+				"x_forwarded_for_authorized_addrs is not set, so forwarding is not enabled but x_forwarded_for_reject_not_authorized was set",
+				token.Pos{
+					Filename: fileName,
+				},
+			})
+		}
+		if l.XForwardedForRejectNotPresentRaw != nil {
+			configErrors = append(configErrors, ConfigError{
+				"x_forwarded_for_authorized_addrs is not set, so forwarding is not enabled but x_forwarded_for_reject_not_present was set",
+				token.Pos{
+					Filename: fileName,
+				},
+			})
+		}
+		if l.XForwardedForHopSkipsRaw != nil {
+			configErrors = append(configErrors, ConfigError{
+				"x_forwarded_for_authorized_addrs is not set, so forwarding is not enabled but x_forwarded_for_hops_skips was set",
+				token.Pos{
+					Filename: fileName,
+				},
+			})
+		}
+		if l.XForwardedForClientCertHeader != "" {
+			configErrors = append(configErrors, ConfigError{
+				"x_forwarded_for_authorized_addrs is not set, so forwarding is not enabled but x_forwarded_for_client_cert_header was set",
+				token.Pos{
+					Filename: fileName,
+				},
+			})
+		}
+		if l.XForwardedForClientCertHeaderDecoders != "" {
+			configErrors = append(configErrors, ConfigError{
+				"x_forwarded_for_authorized_addrs is not set, so forwarding is not enabled but x_forwarded_for_client_cert_header_decoders was set",
+				token.Pos{
+					Filename: fileName,
+				},
+			})
+		}
+	}
+
+	if l.XForwardedForClientCertHeader == "" && l.XForwardedForClientCertHeaderDecoders != "" {
+		configErrors = append(configErrors, ConfigError{
+			"x_forwarded_for_client_cert_header_decoders was set, but not x_forwarded_for_client_cert_header was set to be decoded",
+			token.Pos{
+				Filename: fileName,
+			},
+		})
+	}
+	if l.XForwardedForClientCertHeader != "" && l.XForwardedForClientCertHeaderDecoders == "" {
+		configErrors = append(configErrors, ConfigError{
+			"x_forwarded_for_client_cert_header was set, but no x_forwarded_for_client_cert_header_decoders were set to decode that header",
+			token.Pos{
+				Filename: fileName,
+			},
+		})
+	}
+	if strings.Contains(l.XForwardedForClientCertHeaderDecoders, "DER") && !strings.HasSuffix(l.XForwardedForClientCertHeaderDecoders, "DER") {
+		configErrors = append(configErrors, ConfigError{
+			"x_forwarded_for_client_cert_header_decoders DER decoder actually decodes PEM into DER, and therefore can only function as the final decoder",
+			token.Pos{
+				Filename: fileName,
+			},
+		})
+	}
+
+	return configErrors
+}
+
 // parseTelemetrySettings attempts to parse the raw listener telemetry settings.
 // The state of the listener will be modified, raw data will be cleared upon
 // successful parsing.