Backport of Fix: non-login requests shouldn't care about login roles for lease quotas. Also fix a potential deadlock into release/1.12.x (#21186)

This commit is contained in:
hc-github-team-secure-vault-core
2023-08-18 09:08:11 -04:00
committed by GitHub
parent 2b2f504d6b
commit c15399a21e
5 changed files with 56 additions and 7 deletions

4
changelog/21110.txt Normal file
View File

@@ -0,0 +1,4 @@
```release-note:bug
core/quotas (enterprise): Fix a case where we were applying login roles to lease count quotas in a non-login context.
Also fix a related potential deadlock.
```

46
helper/locking/lock.go Normal file
View File

@@ -0,0 +1,46 @@
package locking
import (
"sync"
"github.com/sasha-s/go-deadlock"
)
// Common mutex interface to allow either built-in or imported deadlock use
type Mutex interface {
Lock()
Unlock()
}
// Common r/w mutex interface to allow either built-in or imported deadlock use
type RWMutex interface {
Lock()
RLock()
RLocker() sync.Locker
RUnlock()
Unlock()
}
// DeadlockMutex (used when requested via config option `detact_deadlocks`),
// behaves like a sync.Mutex but does periodic checking to see if outstanding
// locks and requests look like a deadlock. If it finds a deadlock candidate it
// will output it prefixed with "POTENTIAL DEADLOCK", as described at
// https://github.com/sasha-s/go-deadlock
type DeadlockMutex struct {
deadlock.Mutex
}
// DeadlockRWMutex is the RW version of DeadlockMutex.
type DeadlockRWMutex struct {
deadlock.RWMutex
}
// Regular sync/mutex.
type SyncMutex struct {
sync.Mutex
}
// DeadlockRWMutex is the RW version of SyncMutex.
type SyncRWMutex struct {
sync.RWMutex
}

View File

@@ -38,6 +38,7 @@ import (
"github.com/hashicorp/vault/audit"
"github.com/hashicorp/vault/command/server"
"github.com/hashicorp/vault/helper/identity/mfa"
"github.com/hashicorp/vault/helper/locking"
"github.com/hashicorp/vault/helper/metricsutil"
"github.com/hashicorp/vault/helper/namespace"
"github.com/hashicorp/vault/helper/osutil"
@@ -337,7 +338,7 @@ type Core struct {
// mountsLock is used to ensure that the mounts table does not
// change underneath a calling function
mountsLock sync.RWMutex
mountsLock locking.DeadlockRWMutex
// mountMigrationTracker tracks past and ongoing remount operations
// against their migration ids
@@ -349,7 +350,7 @@ type Core struct {
// authLock is used to ensure that the auth table does not
// change underneath a calling function
authLock sync.RWMutex
authLock locking.DeadlockRWMutex
// audit is loaded after unseal since it is a protected
// configuration

View File

@@ -16,7 +16,6 @@ import (
"sort"
"strconv"
"strings"
"sync"
"time"
"unicode"
@@ -29,6 +28,7 @@ import (
semver "github.com/hashicorp/go-version"
"github.com/hashicorp/vault/helper/hostutil"
"github.com/hashicorp/vault/helper/identity"
"github.com/hashicorp/vault/helper/locking"
"github.com/hashicorp/vault/helper/logging"
"github.com/hashicorp/vault/helper/metricsutil"
"github.com/hashicorp/vault/helper/monitor"
@@ -1676,7 +1676,7 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string,
return nil, logical.ErrReadOnly
}
var lock *sync.RWMutex
var lock *locking.DeadlockRWMutex
switch {
case strings.HasPrefix(path, credentialRoutePrefix):
lock = &b.Core.authLock

View File

@@ -976,11 +976,9 @@ func (c *Core) handleRequest(ctx context.Context, req *logical.Request) (retResp
}
leaseGenerated := false
loginRole := c.DetermineRoleFromLoginRequest(req.MountPoint, req.Data, ctx)
quotaResp, quotaErr := c.applyLeaseCountQuota(ctx, &quotas.Request{
Path: req.Path,
MountPath: strings.TrimPrefix(req.MountPoint, ns.Path),
Role: loginRole,
NamespacePath: ns.Path,
})
if quotaErr != nil {
@@ -1120,7 +1118,7 @@ func (c *Core) handleRequest(ctx context.Context, req *logical.Request) (retResp
return nil, auth, retErr
}
leaseID, err := registerFunc(ctx, req, resp, loginRole)
leaseID, err := registerFunc(ctx, req, resp, "")
if err != nil {
c.logger.Error("failed to register lease", "request_path", req.Path, "error", err)
retErr = multierror.Append(retErr, ErrInternalError)