backport of commit 7463055 (#25343)

Co-authored-by: Steven Clark <steven.clark@hashicorp.com>

builtin/logical/transit/backend.go

@@ -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)

builtin/logical/transit/path_decrypt.go

@@ -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)
 }
 

builtin/logical/transit/path_encrypt.go

@@ -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)
 }
 

builtin/logical/transit/path_hmac.go

@@ -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 {

builtin/logical/transit/path_rewrap.go

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

builtin/logical/transit/path_sign_verify.go

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

changelog/25336.txt

@@ -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.
+```