Skip to content

Commit

Permalink
backport of commit 7463055 (#25343)
Browse files Browse the repository at this point in the history
Co-authored-by: Steven Clark <[email protected]>
  • Loading branch information
1 parent 2bdb24a commit 7620b53
Show file tree
Hide file tree
Showing 7 changed files with 11 additions and 37 deletions.
1 change: 1 addition & 0 deletions builtin/logical/transit/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 1 addition & 4 deletions builtin/logical/transit/path_decrypt.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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}
}
Expand All @@ -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)
}

Expand Down
5 changes: 1 addition & 4 deletions builtin/logical/transit/path_encrypt.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}
}
Expand All @@ -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)
}

Expand Down
16 changes: 2 additions & 14 deletions builtin/logical/transit/path_hmac.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
}

Expand All @@ -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
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
7 changes: 1 addition & 6 deletions builtin/logical/transit/path_rewrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
}
Expand All @@ -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)
}

Expand All @@ -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{}{
Expand All @@ -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
}

Expand Down
11 changes: 2 additions & 9 deletions builtin/logical/transit/path_sign_verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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 {
Expand All @@ -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"]
Expand Down Expand Up @@ -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
}
Expand All @@ -509,7 +505,6 @@ func (b *backend) pathSignWrite(ctx context.Context, req *logical.Request, d *fr
}
}

p.Unlock()
return resp, nil
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
Expand All @@ -743,7 +737,6 @@ func (b *backend) pathVerifyWrite(ctx context.Context, req *logical.Request, d *
}
}

p.Unlock()
return resp, nil
}

Expand Down
3 changes: 3 additions & 0 deletions changelog/25336.txt
Original file line number Diff line number Diff line change
@@ -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.
```

0 comments on commit 7620b53

Please sign in to comment.