Revert some of commit 050ab805a7565c5b0cadb0176023031ee5f0d17b. (#4768)
If we have a panic defer functions are run but unlocks aren't. Since we can't really trust plugins and storage, this backs out the changes for those parts of the request path.
This commit is contained in:
Jeff Mitchell
GitHub
parent
43d9ae5c0a
commit
75eb0f862e
|
|
@ -119,13 +119,12 @@ func (c *Cache) Put(ctx context.Context, entry *Entry) error {
|
|||
|
||||
lock := locksutil.LockForKey(c.locks, entry.Key)
|
||||
lock.Lock()
|
||||
defer lock.Unlock()
|
||||
|
||||
err := c.backend.Put(ctx, entry)
|
||||
if err == nil {
|
||||
c.lru.Add(entry.Key, entry)
|
||||
}
|
||||
|
||||
lock.Unlock()
|
||||
return err
|
||||
}
|
||||
|
||||
|
|
@ -136,21 +135,19 @@ func (c *Cache) Get(ctx context.Context, key string) (*Entry, error) {
|
|||
|
||||
lock := locksutil.LockForKey(c.locks, key)
|
||||
lock.RLock()
|
||||
defer lock.RUnlock()
|
||||
|
||||
// Check the LRU first
|
||||
if raw, ok := c.lru.Get(key); ok {
|
||||
if raw == nil {
|
||||
lock.RUnlock()
|
||||
return nil, nil
|
||||
}
|
||||
lock.RUnlock()
|
||||
return raw.(*Entry), nil
|
||||
}
|
||||
|
||||
// Read from the underlying backend
|
||||
ent, err := c.backend.Get(ctx, key)
|
||||
if err != nil {
|
||||
lock.RUnlock()
|
||||
return nil, err
|
||||
}
|
||||
|
||||
|
|
@ -159,7 +156,6 @@ func (c *Cache) Get(ctx context.Context, key string) (*Entry, error) {
|
|||
c.lru.Add(key, ent)
|
||||
}
|
||||
|
||||
lock.RUnlock()
|
||||
return ent, nil
|
||||
}
|
||||
|
||||
|
|
@ -170,13 +166,12 @@ func (c *Cache) Delete(ctx context.Context, key string) error {
|
|||
|
||||
lock := locksutil.LockForKey(c.locks, key)
|
||||
lock.Lock()
|
||||
defer lock.Unlock()
|
||||
|
||||
err := c.backend.Delete(ctx, key)
|
||||
if err == nil {
|
||||
c.lru.Remove(key)
|
||||
}
|
||||
|
||||
lock.Unlock()
|
||||
return err
|
||||
}
|
||||
|
||||
|
|
@ -201,16 +196,10 @@ func (c *TransactionalCache) Transaction(ctx context.Context, txns []*TxnEntry)
|
|||
// Lock the keys
|
||||
for _, l := range locksutil.LocksForKeys(c.locks, keys) {
|
||||
l.Lock()
|
||||
}
|
||||
|
||||
unlockFunc := func() {
|
||||
for _, l := range locksutil.LocksForKeys(c.locks, keys) {
|
||||
l.Unlock()
|
||||
}
|
||||
defer l.Unlock()
|
||||
}
|
||||
|
||||
if err := c.Transactional.Transaction(ctx, txns); err != nil {
|
||||
unlockFunc()
|
||||
return err
|
||||
}
|
||||
|
||||
|
|
@ -227,6 +216,5 @@ func (c *TransactionalCache) Transaction(ctx context.Context, txns []*TxnEntry)
|
|||
}
|
||||
}
|
||||
|
||||
unlockFunc()
|
||||
return nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -637,16 +637,14 @@ func (b *AESGCMBarrier) updateMasterKeyCommon(key []byte) (*Keyring, error) {
|
|||
func (b *AESGCMBarrier) Put(ctx context.Context, entry *Entry) error {
|
||||
defer metrics.MeasureSince([]string{"barrier", "put"}, time.Now())
|
||||
b.l.RLock()
|
||||
|
||||
defer b.l.RUnlock()
|
||||
if b.sealed {
|
||||
b.l.RUnlock()
|
||||
return ErrBarrierSealed
|
||||
}
|
||||
|
||||
term := b.keyring.ActiveTerm()
|
||||
primary, err := b.aeadForTerm(term)
|
||||
if err != nil {
|
||||
b.l.RUnlock()
|
||||
return err
|
||||
}
|
||||
|
||||
|
|
@ -655,37 +653,29 @@ func (b *AESGCMBarrier) Put(ctx context.Context, entry *Entry) error {
|
|||
Value: b.encrypt(entry.Key, term, primary, entry.Value),
|
||||
SealWrap: entry.SealWrap,
|
||||
}
|
||||
|
||||
err = b.backend.Put(ctx, pe)
|
||||
|
||||
b.l.RUnlock()
|
||||
return err
|
||||
return b.backend.Put(ctx, pe)
|
||||
}
|
||||
|
||||
// Get is used to fetch an entry
|
||||
func (b *AESGCMBarrier) Get(ctx context.Context, key string) (*Entry, error) {
|
||||
defer metrics.MeasureSince([]string{"barrier", "get"}, time.Now())
|
||||
b.l.RLock()
|
||||
|
||||
defer b.l.RUnlock()
|
||||
if b.sealed {
|
||||
b.l.RUnlock()
|
||||
return nil, ErrBarrierSealed
|
||||
}
|
||||
|
||||
// Read the key from the backend
|
||||
pe, err := b.backend.Get(ctx, key)
|
||||
if err != nil {
|
||||
b.l.RUnlock()
|
||||
return nil, err
|
||||
} else if pe == nil {
|
||||
b.l.RUnlock()
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// Decrypt the ciphertext
|
||||
plain, err := b.decryptKeyring(key, pe.Value)
|
||||
if err != nil {
|
||||
b.l.RUnlock()
|
||||
return nil, errwrap.Wrapf("decryption failed: {{err}}", err)
|
||||
}
|
||||
|
||||
|
|
@ -695,8 +685,6 @@ func (b *AESGCMBarrier) Get(ctx context.Context, key string) (*Entry, error) {
|
|||
Value: plain,
|
||||
SealWrap: pe.SealWrap,
|
||||
}
|
||||
|
||||
b.l.RUnlock()
|
||||
return entry, nil
|
||||
}
|
||||
|
||||
|
|
@ -704,16 +692,12 @@ func (b *AESGCMBarrier) Get(ctx context.Context, key string) (*Entry, error) {
|
|||
func (b *AESGCMBarrier) Delete(ctx context.Context, key string) error {
|
||||
defer metrics.MeasureSince([]string{"barrier", "delete"}, time.Now())
|
||||
b.l.RLock()
|
||||
|
||||
defer b.l.RUnlock()
|
||||
if b.sealed {
|
||||
b.l.RUnlock()
|
||||
return ErrBarrierSealed
|
||||
}
|
||||
|
||||
err := b.backend.Delete(ctx, key)
|
||||
|
||||
b.l.RUnlock()
|
||||
return err
|
||||
return b.backend.Delete(ctx, key)
|
||||
}
|
||||
|
||||
// List is used ot list all the keys under a given
|
||||
|
|
@ -721,16 +705,12 @@ func (b *AESGCMBarrier) Delete(ctx context.Context, key string) error {
|
|||
func (b *AESGCMBarrier) List(ctx context.Context, prefix string) ([]string, error) {
|
||||
defer metrics.MeasureSince([]string{"barrier", "list"}, time.Now())
|
||||
b.l.RLock()
|
||||
|
||||
defer b.l.RUnlock()
|
||||
if b.sealed {
|
||||
b.l.RUnlock()
|
||||
return nil, ErrBarrierSealed
|
||||
}
|
||||
|
||||
keys, err := b.backend.List(ctx, prefix)
|
||||
|
||||
b.l.RUnlock()
|
||||
return keys, err
|
||||
return b.backend.List(ctx, prefix)
|
||||
}
|
||||
|
||||
// aeadForTerm returns the AES-GCM AEAD for the given term
|
||||
|
|
@ -873,7 +853,6 @@ func (b *AESGCMBarrier) decryptKeyring(path string, cipher []byte) ([]byte, erro
|
|||
// Encrypt is used to encrypt in-memory for the BarrierEncryptor interface
|
||||
func (b *AESGCMBarrier) Encrypt(ctx context.Context, key string, plaintext []byte) ([]byte, error) {
|
||||
b.l.RLock()
|
||||
|
||||
if b.sealed {
|
||||
b.l.RUnlock()
|
||||
return nil, ErrBarrierSealed
|
||||
|
|
@ -887,7 +866,6 @@ func (b *AESGCMBarrier) Encrypt(ctx context.Context, key string, plaintext []byt
|
|||
}
|
||||
|
||||
ciphertext := b.encrypt(key, term, primary, plaintext)
|
||||
|
||||
b.l.RUnlock()
|
||||
return ciphertext, nil
|
||||
}
|
||||
|
|
@ -895,7 +873,6 @@ func (b *AESGCMBarrier) Encrypt(ctx context.Context, key string, plaintext []byt
|
|||
// Decrypt is used to decrypt in-memory for the BarrierEncryptor interface
|
||||
func (b *AESGCMBarrier) Decrypt(ctx context.Context, key string, ciphertext []byte) ([]byte, error) {
|
||||
b.l.RLock()
|
||||
|
||||
if b.sealed {
|
||||
b.l.RUnlock()
|
||||
return nil, ErrBarrierSealed
|
||||
|
|
|
|||
|
|
@ -254,17 +254,16 @@ func (c *Core) checkToken(ctx context.Context, req *logical.Request, unauth bool
|
|||
// HandleRequest is used to handle a new incoming request
|
||||
func (c *Core) HandleRequest(req *logical.Request) (resp *logical.Response, err error) {
|
||||
c.stateLock.RLock()
|
||||
|
||||
defer c.stateLock.RUnlock()
|
||||
if c.sealed {
|
||||
c.stateLock.RUnlock()
|
||||
return nil, consts.ErrSealed
|
||||
}
|
||||
if c.standby {
|
||||
c.stateLock.RUnlock()
|
||||
return nil, consts.ErrStandby
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithCancel(c.activeContext)
|
||||
defer cancel()
|
||||
|
||||
// Allowing writing to a path ending in / makes it extremely difficult to
|
||||
// understand user intent for the filesystem-like backends (kv,
|
||||
|
|
@ -275,8 +274,6 @@ func (c *Core) HandleRequest(req *logical.Request) (resp *logical.Response, err
|
|||
if strings.HasSuffix(req.Path, "/") &&
|
||||
(req.Operation == logical.UpdateOperation ||
|
||||
req.Operation == logical.CreateOperation) {
|
||||
cancel()
|
||||
c.stateLock.RUnlock()
|
||||
return logical.ErrorResponse("cannot write to a path ending in '/'"), nil
|
||||
}
|
||||
|
||||
|
|
@ -343,8 +340,6 @@ func (c *Core) HandleRequest(req *logical.Request) (resp *logical.Response, err
|
|||
err := jsonutil.DecodeJSON(resp.Data[logical.HTTPRawBody].([]byte), httpResp)
|
||||
if err != nil {
|
||||
c.logger.Error("failed to unmarshal wrapped HTTP response for audit logging", "error", err)
|
||||
cancel()
|
||||
c.stateLock.RUnlock()
|
||||
return nil, ErrInternalError
|
||||
}
|
||||
|
||||
|
|
@ -380,13 +375,9 @@ func (c *Core) HandleRequest(req *logical.Request) (resp *logical.Response, err
|
|||
}
|
||||
if auditErr := c.auditBroker.LogResponse(ctx, logInput, c.auditedHeaders); auditErr != nil {
|
||||
c.logger.Error("failed to audit response", "request_path", req.Path, "error", auditErr)
|
||||
cancel()
|
||||
c.stateLock.RUnlock()
|
||||
return nil, ErrInternalError
|
||||
}
|
||||
|
||||
cancel()
|
||||
c.stateLock.RUnlock()
|
||||
return
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -209,13 +209,14 @@ func (r *Router) MatchingMountByUUID(mountID string) *MountEntry {
|
|||
}
|
||||
|
||||
r.l.RLock()
|
||||
_, raw, ok := r.mountUUIDCache.LongestPrefix(mountID)
|
||||
r.l.RUnlock()
|
||||
|
||||
_, raw, ok := r.mountUUIDCache.LongestPrefix(mountID)
|
||||
if !ok {
|
||||
r.l.RUnlock()
|
||||
return nil
|
||||
}
|
||||
|
||||
r.l.RUnlock()
|
||||
return raw.(*MountEntry)
|
||||
}
|
||||
|
||||
|
|
@ -226,13 +227,14 @@ func (r *Router) MatchingMountByAccessor(mountAccessor string) *MountEntry {
|
|||
}
|
||||
|
||||
r.l.RLock()
|
||||
_, raw, ok := r.mountAccessorCache.LongestPrefix(mountAccessor)
|
||||
r.l.RUnlock()
|
||||
|
||||
_, raw, ok := r.mountAccessorCache.LongestPrefix(mountAccessor)
|
||||
if !ok {
|
||||
r.l.RUnlock()
|
||||
return nil
|
||||
}
|
||||
|
||||
r.l.RUnlock()
|
||||
return raw.(*MountEntry)
|
||||
}
|
||||
|
||||
|
|
@ -406,10 +408,10 @@ func (r *Router) routeCommon(ctx context.Context, req *logical.Request, existenc
|
|||
// Grab a read lock on the route entry, this protects against the backend
|
||||
// being reloaded during a request.
|
||||
re.l.RLock()
|
||||
defer re.l.RUnlock()
|
||||
|
||||
// Filtered mounts will have a nil backend
|
||||
if re.backend == nil {
|
||||
re.l.RUnlock()
|
||||
return logical.ErrorResponse(fmt.Sprintf("no handler for route '%s'", req.Path)), false, false, logical.ErrUnsupportedPath
|
||||
}
|
||||
|
||||
|
|
@ -419,7 +421,6 @@ func (r *Router) routeCommon(ctx context.Context, req *logical.Request, existenc
|
|||
switch req.Operation {
|
||||
case logical.RevokeOperation, logical.RollbackOperation:
|
||||
default:
|
||||
re.l.RUnlock()
|
||||
return logical.ErrorResponse(fmt.Sprintf("no handler for route '%s'", req.Path)), false, false, logical.ErrUnsupportedPath
|
||||
}
|
||||
}
|
||||
|
|
@ -449,7 +450,6 @@ func (r *Router) routeCommon(ctx context.Context, req *logical.Request, existenc
|
|||
// salted ID, so we double-salt what's going to the cubbyhole backend
|
||||
salt, err := r.tokenStoreSaltFunc(ctx)
|
||||
if err != nil {
|
||||
re.l.RUnlock()
|
||||
return nil, false, false, err
|
||||
}
|
||||
req.ClientToken = re.SaltID(salt.SaltID(req.ClientToken))
|
||||
|
|
@ -491,7 +491,7 @@ func (r *Router) routeCommon(ctx context.Context, req *logical.Request, existenc
|
|||
req.SetTokenEntry(nil)
|
||||
|
||||
// Reset the request before returning
|
||||
resetFunc := func() {
|
||||
defer func() {
|
||||
req.Path = originalPath
|
||||
req.MountPoint = mount
|
||||
req.MountType = re.mountEntry.Type
|
||||
|
|
@ -513,13 +513,11 @@ func (r *Router) routeCommon(ctx context.Context, req *logical.Request, existenc
|
|||
req.EntityID = originalEntityID
|
||||
|
||||
req.SetTokenEntry(reqTokenEntry)
|
||||
}
|
||||
}()
|
||||
|
||||
// Invoke the backend
|
||||
if existenceCheck {
|
||||
ok, exists, err := re.backend.HandleExistenceCheck(ctx, req)
|
||||
resetFunc()
|
||||
re.l.RUnlock()
|
||||
return nil, ok, exists, err
|
||||
} else {
|
||||
resp, err := re.backend.HandleRequest(ctx, req)
|
||||
|
|
@ -544,8 +542,6 @@ func (r *Router) routeCommon(ctx context.Context, req *logical.Request, existenc
|
|||
alias.MountAccessor = re.mountEntry.Accessor
|
||||
}
|
||||
}
|
||||
resetFunc()
|
||||
re.l.RUnlock()
|
||||
return resp, false, false, err
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue