scottk/vault
1
0
Fork 0
mirror of https://github.com/optim-enterprises-bv/vault.git synced 2025-10-29 17:52:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

Transit: Release locks using defer statements (#25336)

Browse Source 
* Transit: Release locks using defer statements

 - Leverage defer statements to Unlock the fetched policy
   to avoid issues with forgetting to manually Unlock during
   each return statement

* Add cl
This commit is contained in:
Steven Clark
2024-02-09 14:06:23 -05:00
committed by GitHub
parent f4248bf16c
commit 7463055f07
7 changed files with 11 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
builtin/logical/transit/backend.go
View File

@@ -248,6 +248,7 @@ func (b *backend) autoRotateKeys(ctx context.Context, req *logical.Request) erro
continue
}
// rotateIfRequired properly acquires/releases the lock on p
err = b.rotateIfRequired(ctx, req, key, p)
if err != nil {
errs = multierror.Append(errs, err)

5
builtin/logical/transit/path_decrypt.go
View File

@@ -184,6 +184,7 @@ func (b *backend) pathDecryptWrite(ctx context.Context, req *logical.Request, d
if !b.System().CachingDisabled() {
p.Lock(false)
}
defer p.Unlock()
successesInBatch := false
for i, item := range batchInputItems {
@@ -243,8 +244,6 @@ func (b *backend) pathDecryptWrite(ctx context.Context, req *logical.Request, d
}
} else {
if batchResponseItems[0].Error != "" {
p.Unlock()
if internalErrorInBatch {
return nil, errutil.InternalError{Err: batchResponseItems[0].Error}
}
@@ -256,8 +255,6 @@ func (b *backend) pathDecryptWrite(ctx context.Context, req *logical.Request, d
}
}
p.Unlock()
return batchRequestResponse(d, resp, req, successesInBatch, userErrorInBatch, internalErrorInBatch)
}

5
builtin/logical/transit/path_encrypt.go
View File

@@ -460,6 +460,7 @@ func (b *backend) pathEncryptWrite(ctx context.Context, req *logical.Request, d
if !b.System().CachingDisabled() {
p.Lock(false)
}
defer p.Unlock()
// Process batch request items. If encryption of any request
// item fails, respectively mark the error in the response
@@ -547,8 +548,6 @@ func (b *backend) pathEncryptWrite(ctx context.Context, req *logical.Request, d
}
} else {
if batchResponseItems[0].Error != "" {
p.Unlock()
if internalErrorInBatch {
return nil, errutil.InternalError{Err: batchResponseItems[0].Error}
}
@@ -570,8 +569,6 @@ func (b *backend) pathEncryptWrite(ctx context.Context, req *logical.Request, d
resp.AddWarning("Attempted creation of the key during the encrypt operation, but it was created beforehand")
}
p.Unlock()
return batchRequestResponse(d, resp, req, successesInBatch, userErrorInBatch, internalErrorInBatch)
}

16
builtin/logical/transit/path_hmac.go
View File

@@ -136,6 +136,7 @@ func (b *backend) pathHMACWrite(ctx context.Context, req *logical.Request, d *fr
if !b.System().CachingDisabled() {
p.Lock(false)
}
defer p.Unlock()
switch {
case ver == 0:
@@ -145,23 +146,19 @@ func (b *backend) pathHMACWrite(ctx context.Context, req *logical.Request, d *fr
case ver == p.LatestVersion:
// Allowed
case p.MinEncryptionVersion > 0 && ver < p.MinEncryptionVersion:
p.Unlock()
return logical.ErrorResponse("cannot generate HMAC: version is too old (disallowed by policy)"), logical.ErrInvalidRequest
}
key, err := p.HMACKey(ver)
if err != nil {
p.Unlock()
return logical.ErrorResponse(err.Error()), logical.ErrInvalidRequest
}
if key == nil && p.Type != keysutil.KeyType_MANAGED_KEY {
p.Unlock()
return nil, fmt.Errorf("HMAC key value could not be computed")
}
hashAlgorithm, ok := keysutil.HashTypeMap[algorithm]
if !ok {
p.Unlock()
return logical.ErrorResponse("unsupported algorithm %q", hashAlgorithm), nil
}
@@ -172,18 +169,15 @@ func (b *backend) pathHMACWrite(ctx context.Context, req *logical.Request, d *fr
if batchInputRaw != nil {
err = mapstructure.Decode(batchInputRaw, &batchInputItems)
if err != nil {
p.Unlock()
return nil, fmt.Errorf("failed to parse batch input: %w", err)
}
if len(batchInputItems) == 0 {
p.Unlock()
return logical.ErrorResponse("missing batch input to process"), logical.ErrInvalidRequest
}
} else {
valueRaw, ok := d.GetOk("input")
if !ok {
p.Unlock()
return logical.ErrorResponse("missing input for HMAC"), logical.ErrInvalidRequest
}
@@ -233,8 +227,6 @@ func (b *backend) pathHMACWrite(ctx context.Context, req *logical.Request, d *fr
response[i].HMAC = retStr
}
p.Unlock()
// Generate the response
resp := &logical.Response{}
if batchInputRaw != nil {
@@ -282,10 +274,10 @@ func (b *backend) pathHMACVerify(ctx context.Context, req *logical.Request, d *f
if !b.System().CachingDisabled() {
p.Lock(false)
}
defer p.Unlock()
hashAlgorithm, ok := keysutil.HashTypeMap[algorithm]
if !ok {
p.Unlock()
return logical.ErrorResponse("unsupported algorithm %q", hashAlgorithm), nil
}
@@ -296,12 +288,10 @@ func (b *backend) pathHMACVerify(ctx context.Context, req *logical.Request, d *f
if batchInputRaw != nil {
err := mapstructure.Decode(batchInputRaw, &batchInputItems)
if err != nil {
p.Unlock()
return nil, fmt.Errorf("failed to parse batch input: %w", err)
}
if len(batchInputItems) == 0 {
p.Unlock()
return logical.ErrorResponse("missing batch input to process"), logical.ErrInvalidRequest
}
} else {
@@ -398,8 +388,6 @@ func (b *backend) pathHMACVerify(ctx context.Context, req *logical.Request, d *f
response[i].Valid = hmac.Equal(retBytes, verBytes)
}
p.Unlock()
// Generate the response
resp := &logical.Response{}
if batchInputRaw != nil {

7
builtin/logical/transit/path_rewrap.go
View File

@@ -148,6 +148,7 @@ func (b *backend) pathRewrapWrite(ctx context.Context, req *logical.Request, d *
if !b.System().CachingDisabled() {
p.Lock(false)
}
defer p.Unlock()
warnAboutNonceUsage := false
for i, item := range batchInputItems {
@@ -167,7 +168,6 @@ func (b *backend) pathRewrapWrite(ctx context.Context, req *logical.Request, d *
batchResponseItems[i].Error = err.Error()
continue
default:
p.Unlock()
return nil, err
}
}
@@ -183,16 +183,13 @@ func (b *backend) pathRewrapWrite(ctx context.Context, req *logical.Request, d *
batchResponseItems[i].Error = err.Error()
continue
case errutil.InternalError:
p.Unlock()
return nil, err
default:
p.Unlock()
return nil, err
}
}
if ciphertext == "" {
p.Unlock()
return nil, fmt.Errorf("empty ciphertext returned for input item %d", i)
}
@@ -216,7 +213,6 @@ func (b *backend) pathRewrapWrite(ctx context.Context, req *logical.Request, d *
}
} else {
if batchResponseItems[0].Error != "" {
p.Unlock()
return logical.ErrorResponse(batchResponseItems[0].Error), logical.ErrInvalidRequest
}
resp.Data = map[string]interface{}{
@@ -229,7 +225,6 @@ func (b *backend) pathRewrapWrite(ctx context.Context, req *logical.Request, d *
resp.AddWarning("A provided nonce value was used within FIPS mode, this violates FIPS 140 compliance.")
}
p.Unlock()
return resp, nil
}

11
builtin/logical/transit/path_sign_verify.go
View File

@@ -367,9 +367,9 @@ func (b *backend) pathSignWrite(ctx context.Context, req *logical.Request, d *fr
if !b.System().CachingDisabled() {
p.Lock(false)
}
defer p.Unlock()
if !p.Type.SigningSupported() {
p.Unlock()
return logical.ErrorResponse(fmt.Sprintf("key type %v does not support signing", p.Type)), logical.ErrInvalidRequest
}
@@ -385,12 +385,10 @@ func (b *backend) pathSignWrite(ctx context.Context, req *logical.Request, d *fr
if batchInputRaw != nil {
err = mapstructure.Decode(batchInputRaw, &batchInputItems)
if err != nil {
p.Unlock()
return nil, fmt.Errorf("failed to parse batch input: %w", err)
}
if len(batchInputItems) == 0 {
p.Unlock()
return logical.ErrorResponse("missing batch input to process"), logical.ErrInvalidRequest
}
} else {
@@ -403,7 +401,6 @@ func (b *backend) pathSignWrite(ctx context.Context, req *logical.Request, d *fr
}
response := make([]batchResponseSignItem, len(batchInputItems))
for i, item := range batchInputItems {
rawInput, ok := item["input"]
@@ -491,7 +488,6 @@ func (b *backend) pathSignWrite(ctx context.Context, req *logical.Request, d *fr
}
} else {
if response[0].Error != "" || response[0].err != nil {
p.Unlock()
if response[0].Error != "" {
return logical.ErrorResponse(response[0].Error), response[0].err
}
@@ -509,7 +505,6 @@ func (b *backend) pathSignWrite(ctx context.Context, req *logical.Request, d *fr
}
}
p.Unlock()
return resp, nil
}
@@ -625,9 +620,9 @@ func (b *backend) pathVerifyWrite(ctx context.Context, req *logical.Request, d *
if !b.System().CachingDisabled() {
p.Lock(false)
}
defer p.Unlock()
if !p.Type.SigningSupported() {
p.Unlock()
return logical.ErrorResponse(fmt.Sprintf("key type %v does not support verification", p.Type)), logical.ErrInvalidRequest
}
@@ -732,7 +727,6 @@ func (b *backend) pathVerifyWrite(ctx context.Context, req *logical.Request, d *
}
} else {
if response[0].Error != "" || response[0].err != nil {
p.Unlock()
if response[0].Error != "" {
return logical.ErrorResponse(response[0].Error), response[0].err
}
@@ -743,7 +737,6 @@ func (b *backend) pathVerifyWrite(ctx context.Context, req *logical.Request, d *
}
}
p.Unlock()
return resp, nil
}

3
changelog/25336.txt Normal file
View File

@@ -0,0 +1,3 @@
```release-note:bug
secrets/transit: When provided an invalid input with hash_algorithm=none, a lock was not released properly before reporting an error leading to deadlocks on a subsequent key configuration update.
```