diff --git a/sdk/go.mod b/sdk/go.mod index 9ba252a1af..3727dbb0a3 100644 --- a/sdk/go.mod +++ b/sdk/go.mod @@ -95,9 +95,11 @@ require ( github.com/opencontainers/go-digest v1.0.0 // indirect github.com/opencontainers/image-spec v1.1.0-rc2.0.20221005185240-3a7f492d3f1b // indirect github.com/opencontainers/runc v1.1.12 // indirect + github.com/petermattis/goid v0.0.0-20180202154549-b0b1615b78e5 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/rogpeppe/go-internal v1.8.1 // indirect + github.com/sasha-s/go-deadlock v0.2.0 github.com/sirupsen/logrus v1.9.3 // indirect github.com/stretchr/objx v0.5.0 // indirect go.opencensus.io v0.24.0 // indirect diff --git a/sdk/go.sum b/sdk/go.sum index bdfa1fa886..00cd1eda06 100644 --- a/sdk/go.sum +++ b/sdk/go.sum @@ -436,6 +436,8 @@ github.com/opencontainers/runc v1.1.12 h1:BOIssBaW1La0/qbNZHXOOa71dZfZEQOzW7dqQf github.com/opencontainers/runc v1.1.12/go.mod h1:S+lQwSfncpBha7XTy/5lBwWgm5+y5Ma/O44Ekby9FK8= github.com/pascaldekloe/goe v0.1.0 h1:cBOtyMzM9HTpWjXfbbunk26uA6nG3a8n06Wieeh0MwY= github.com/pascaldekloe/goe v0.1.0/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= +github.com/petermattis/goid v0.0.0-20180202154549-b0b1615b78e5 h1:q2e307iGHPdTGp0hoxKjt1H5pDo6utceo3dQVK3I5XQ= +github.com/petermattis/goid v0.0.0-20180202154549-b0b1615b78e5/go.mod h1:jvVRKCrJTQWu0XVbaOlby/2lO20uSCHEMzzplHXte1o= github.com/pierrec/lz4 v2.0.5+incompatible/go.mod h1:pdkljMzZIN41W+lC3N2tnIh5sFi+IEE17M5jbnwPHcY= github.com/pierrec/lz4 v2.6.1+incompatible h1:9UY3+iC23yxF0UfGaYrGplQ+79Rg+h/q9FV9ix19jjM= github.com/pierrec/lz4 v2.6.1+incompatible/go.mod h1:pdkljMzZIN41W+lC3N2tnIh5sFi+IEE17M5jbnwPHcY= diff --git a/sdk/helper/locksutil/locks.go b/sdk/helper/locksutil/locks.go index c7538b63b4..2711098247 100644 --- a/sdk/helper/locksutil/locks.go +++ b/sdk/helper/locksutil/locks.go @@ -7,12 +7,18 @@ import ( "sync" "github.com/hashicorp/vault/sdk/helper/cryptoutil" + "github.com/sasha-s/go-deadlock" ) const ( LockCount = 256 ) +// DeadlockRWMutex is the RW version of DeadlockMutex. +type DeadlockRWMutex struct { + deadlock.RWMutex +} + type LockEntry struct { sync.RWMutex } @@ -36,6 +42,14 @@ func CreateLocks() []*LockEntry { return ret } +func CreateLocksWithDeadlockDetection() []*DeadlockRWMutex { + ret := make([]*DeadlockRWMutex, LockCount) + for i := range ret { + ret[i] = new(DeadlockRWMutex) + } + return ret +} + func LockIndexForKey(key string) uint8 { return uint8(cryptoutil.Blake2b256Hash(key)[0]) } @@ -59,3 +73,23 @@ func LocksForKeys(locks []*LockEntry, keys []string) []*LockEntry { return locksToReturn } + +func LockForKeyWithDeadLockDetection(locks []*DeadlockRWMutex, key string) *DeadlockRWMutex { + return locks[LockIndexForKey(key)] +} + +func LocksForKeysWithDeadLockDetection(locks []*DeadlockRWMutex, keys []string) []*DeadlockRWMutex { + lockIndexes := make(map[uint8]struct{}, len(keys)) + for _, k := range keys { + lockIndexes[LockIndexForKey(k)] = struct{}{} + } + + locksToReturn := make([]*DeadlockRWMutex, 0, len(keys)) + for i, l := range locks { + if _, ok := lockIndexes[uint8(i)]; ok { + locksToReturn = append(locksToReturn, l) + } + } + + return locksToReturn +} diff --git a/vault/barrier.go b/vault/barrier.go index 9cba04eb7b..e2ecd68a91 100644 --- a/vault/barrier.go +++ b/vault/barrier.go @@ -166,6 +166,8 @@ type SecurityBarrier interface { // SecurityBarrier must provide the encryption APIs BarrierEncryptor + + DetectDeadlocks() bool } // BarrierStorage is the storage only interface required for a Barrier. diff --git a/vault/barrier_aes_gcm.go b/vault/barrier_aes_gcm.go index 8e949a967b..1144c7e60f 100644 --- a/vault/barrier_aes_gcm.go +++ b/vault/barrier_aes_gcm.go @@ -23,6 +23,7 @@ import ( "github.com/armon/go-metrics" "github.com/hashicorp/go-secure-stdlib/parseutil" "github.com/hashicorp/go-secure-stdlib/strutil" + "github.com/hashicorp/vault/helper/locking" "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/sdk/physical" @@ -71,7 +72,7 @@ var ( type AESGCMBarrier struct { backend physical.Backend - l sync.RWMutex + l locking.RWMutex sealed bool // keyring is used to maintain all of the encryption keys, including @@ -119,7 +120,7 @@ func (b *AESGCMBarrier) SetRotationConfig(ctx context.Context, rotConfig KeyRota // NewAESGCMBarrier is used to construct a new barrier that uses // the provided physical backend for storage. -func NewAESGCMBarrier(physical physical.Backend) (*AESGCMBarrier, error) { +func NewAESGCMBarrier(physical physical.Backend, detectDeadlocks bool) (*AESGCMBarrier, error) { keyringTimeout := defaultKeyringTimeout keyringTimeoutStr := os.Getenv(bestEffortKeyringTimeoutOverride) if keyringTimeoutStr != "" { @@ -129,8 +130,10 @@ func NewAESGCMBarrier(physical physical.Backend) (*AESGCMBarrier, error) { } keyringTimeout = t } + b := &AESGCMBarrier{ backend: physical, + l: &locking.SyncRWMutex{}, sealed: true, cache: make(map[uint32]cipher.AEAD), currentAESGCMVersionByte: byte(AESGCMVersion2), @@ -139,9 +142,19 @@ func NewAESGCMBarrier(physical physical.Backend) (*AESGCMBarrier, error) { totalLocalEncryptions: atomic.NewInt64(0), bestEffortKeyringTimeout: keyringTimeout, } + if detectDeadlocks { + b.l = &locking.DeadlockRWMutex{} + } return b, nil } +func (b *AESGCMBarrier) DetectDeadlocks() bool { + if _, ok := b.l.(*locking.DeadlockRWMutex); ok { + return true + } + return false +} + // Initialized checks if the barrier has been initialized // and has a root key set. func (b *AESGCMBarrier) Initialized(ctx context.Context) (bool, error) { diff --git a/vault/barrier_aes_gcm_test.go b/vault/barrier_aes_gcm_test.go index 9ed7d6fcb1..1268bf006c 100644 --- a/vault/barrier_aes_gcm_test.go +++ b/vault/barrier_aes_gcm_test.go @@ -28,7 +28,7 @@ func mockBarrier(t testing.TB) (physical.Backend, SecurityBarrier, []byte) { if err != nil { t.Fatalf("err: %v", err) } - b, err := NewAESGCMBarrier(inm) + b, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -45,7 +45,7 @@ func TestAESGCMBarrier_Basic(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - b, err := NewAESGCMBarrier(inm) + b, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -57,7 +57,7 @@ func TestAESGCMBarrier_Rotate(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - b, err := NewAESGCMBarrier(inm) + b, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -69,7 +69,7 @@ func TestAESGCMBarrier_MissingRotateConfig(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - b, err := NewAESGCMBarrier(inm) + b, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -97,11 +97,11 @@ func TestAESGCMBarrier_Upgrade(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - b1, err := NewAESGCMBarrier(inm) + b1, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } - b2, err := NewAESGCMBarrier(inm) + b2, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -113,11 +113,11 @@ func TestAESGCMBarrier_Upgrade_Rekey(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - b1, err := NewAESGCMBarrier(inm) + b1, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } - b2, err := NewAESGCMBarrier(inm) + b2, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -129,7 +129,7 @@ func TestAESGCMBarrier_Rekey(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - b, err := NewAESGCMBarrier(inm) + b, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -143,7 +143,7 @@ func TestAESGCMBarrier_BackwardsCompatible(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - b, err := NewAESGCMBarrier(inm) + b, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -232,7 +232,7 @@ func TestAESGCMBarrier_Confidential(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - b, err := NewAESGCMBarrier(inm) + b, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -272,7 +272,7 @@ func TestAESGCMBarrier_Integrity(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - b, err := NewAESGCMBarrier(inm) + b, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -310,7 +310,7 @@ func TestAESGCMBarrier_MoveIntegrityV1(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - b, err := NewAESGCMBarrier(inm) + b, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -354,7 +354,7 @@ func TestAESGCMBarrier_MoveIntegrityV2(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - b, err := NewAESGCMBarrier(inm) + b, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -398,7 +398,7 @@ func TestAESGCMBarrier_UpgradeV1toV2(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - b, err := NewAESGCMBarrier(inm) + b, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -429,7 +429,7 @@ func TestAESGCMBarrier_UpgradeV1toV2(t *testing.T) { } // Open again as version 2 - b, err = NewAESGCMBarrier(inm) + b, err = NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -453,7 +453,7 @@ func TestEncrypt_Unique(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - b, err := NewAESGCMBarrier(inm) + b, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -489,7 +489,7 @@ func TestInitialize_KeyLength(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - b, err := NewAESGCMBarrier(inm) + b, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -522,7 +522,7 @@ func TestEncrypt_BarrierEncryptor(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - b, err := NewAESGCMBarrier(inm) + b, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -558,7 +558,7 @@ func TestDecrypt_InvalidCipherLength(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - b, err := NewAESGCMBarrier(inm) + b, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -591,7 +591,7 @@ func TestAESGCMBarrier_ReloadKeyring(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - b, err := NewAESGCMBarrier(inm) + b, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -614,7 +614,7 @@ func TestAESGCMBarrier_ReloadKeyring(t *testing.T) { { // Create a second barrier and rotate the keyring - b2, err := NewAESGCMBarrier(inm) + b2, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } @@ -669,7 +669,7 @@ func TestBarrier_LegacyRotate(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - b1, err := NewAESGCMBarrier(inm) + b1, err := NewAESGCMBarrier(inm, false) if err != nil { t.Fatalf("err: %v", err) } // Initialize the barrier @@ -735,7 +735,7 @@ func TestBarrier_persistKeyring_Context(t *testing.T) { // Set up barrier backend, err := inmem.NewInmem(nil, corehelpers.NewTestLogger(t)) require.NoError(t, err) - barrier, err := NewAESGCMBarrier(backend) + barrier, err := NewAESGCMBarrier(backend, false) require.NoError(t, err) key, err := barrier.GenerateKey(rand.Reader) require.NoError(t, err) diff --git a/vault/core.go b/vault/core.go index 69e8d1cdda..66950b091f 100644 --- a/vault/core.go +++ b/vault/core.go @@ -1200,7 +1200,11 @@ func NewCore(conf *CoreConfig) (*Core, error) { } // Construct a new AES-GCM barrier - c.barrier, err = NewAESGCMBarrier(c.physical) + detectDeadlocks := slices.Contains(c.detectDeadlocks, "barrier") + if detectDeadlocks { + c.Logger().Debug("enabling deadlock detection for the barrier") + } + c.barrier, err = NewAESGCMBarrier(c.physical, detectDeadlocks) if err != nil { return nil, fmt.Errorf("barrier setup failed: %w", err) } @@ -1299,8 +1303,8 @@ func NewCore(conf *CoreConfig) (*Core, error) { quotasLogger := conf.Logger.Named("quotas") c.allLoggers = append(c.allLoggers, quotasLogger) - detectDeadlocks := slices.Contains(c.detectDeadlocks, "quotas") - c.quotaManager, err = quotas.NewManager(quotasLogger, c.quotaLeaseWalker, c.metricSink, detectDeadlocks) + detectDeadlocksQuotas := slices.Contains(c.detectDeadlocks, "quotas") + c.quotaManager, err = quotas.NewManager(quotasLogger, c.quotaLeaseWalker, c.metricSink, detectDeadlocksQuotas) if err != nil { return nil, err } diff --git a/vault/core_test.go b/vault/core_test.go index 835e28905a..8eb29f80bd 100644 --- a/vault/core_test.go +++ b/vault/core_test.go @@ -3605,3 +3605,21 @@ func TestBuildUnsealSetupFunctionSlice(t *testing.T) { assert.Equal(t, testcase.expectedLength, len(funcs), testcase.name) } } + +// TestBarrier_DeadlockDetection verifies that the +// DeadlockDetection is correctly enabled and disabled when the core is unsealed +func TestBarrier_DeadlockDetection(t *testing.T) { + testCore := TestCore(t) + testCoreUnsealed(t, testCore) + + if testCore.barrier.DetectDeadlocks() { + t.Fatal("barrierLock has deadlock detection enabled, it shouldn't") + } + + testCore = TestCoreWithDeadlockDetection(t, nil, false) + testCoreUnsealed(t, testCore) + + if !testCore.barrier.DetectDeadlocks() { + t.Fatal("barrierLock doesn't have deadlock detection enabled, it should") + } +} diff --git a/vault/testing.go b/vault/testing.go index cfcb96e629..61dc35695a 100644 --- a/vault/testing.go +++ b/vault/testing.go @@ -146,7 +146,7 @@ func TestCoreWithDeadlockDetection(t testing.T, testSeal Seal, enableRaw bool) * AuditBackends: map[string]audit.Factory{ "file": auditFile.Factory, }, - DetectDeadlocks: "expiration,quotas,statelock", + DetectDeadlocks: "expiration,quotas,statelock,barrier", } return TestCoreWithSealAndUI(t, conf) }