diff --git a/changelog/29176.txt b/changelog/29176.txt new file mode 100644 index 0000000000..a701cd1170 --- /dev/null +++ b/changelog/29176.txt @@ -0,0 +1,3 @@ +```release-note:bug +core: Prevent integer overflows of the barrier key counter on key rotation requests +``` diff --git a/vault/barrier_aes_gcm.go b/vault/barrier_aes_gcm.go index 1144c7e60f..e75b7a78a8 100644 --- a/vault/barrier_aes_gcm.go +++ b/vault/barrier_aes_gcm.go @@ -621,6 +621,11 @@ func (b *AESGCMBarrier) Rotate(ctx context.Context, randomSource io.Reader) (uin term := b.keyring.ActiveTerm() newTerm := term + 1 + if newTerm < term { + // We've rolled over the uint32, don't allow this + return 0, errors.New("failed to generate a new term value due to integer overflow") + } + // Add a new encryption key newKeyring, err := b.keyring.AddKey(&Key{ Term: newTerm, diff --git a/vault/barrier_aes_gcm_test.go b/vault/barrier_aes_gcm_test.go index 1268bf006c..77c5fee7e0 100644 --- a/vault/barrier_aes_gcm_test.go +++ b/vault/barrier_aes_gcm_test.go @@ -8,6 +8,8 @@ import ( "context" "crypto/rand" "encoding/json" + "math" + "strings" "testing" "time" @@ -697,6 +699,45 @@ func TestBarrier_LegacyRotate(t *testing.T) { } } +// TestBarrier_RotateFailsOnOverflow validates that if we ever actually hit the +// barrier key rotation limit, we fail the rotation with an error. +func TestBarrier_RotateFailsOnOverflow(t *testing.T) { + inm, err := inmem.NewInmem(nil, logger) + if err != nil { + t.Fatalf("err: %v", err) + } + b, err := NewAESGCMBarrier(inm, false) + if err != nil { + t.Fatalf("err: %v", err) + } + key, err := b.GenerateKey(rand.Reader) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Initialize the barrier + ctx := context.Background() + err = b.Initialize(ctx, key, nil, rand.Reader) + if err != nil { + t.Fatalf("err: %v", err) + } + err = b.Unseal(ctx, key) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Hack to avoid generating a lot of keys... + b.keyring.activeTerm = math.MaxUint32 + + _, err = b.Rotate(ctx, rand.Reader) + if err == nil { + t.Fatalf("Rotate should fail on overflow but did not") + } + if !strings.Contains(err.Error(), "integer overflow") { + t.Fatalf("Rotate failed but not for the expected reason of integer overflow: %v", err) + } +} + // TestBarrier_persistKeyring_Context checks that we get the right errors if // the context is cancelled or times-out before the first part of persistKeyring // is able to persist the keyring itself (i.e. we don't go on to try and persist