backport of commit 7463055

builtin/logical/transit/backend.go

@@ -241,6 +241,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

@@ -178,6 +178,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 {
@@ -237,8 +238,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}
 			}
@@ -250,8 +249,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

@@ -454,6 +454,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
@@ -541,8 +542,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}
 			}
@@ -564,8 +563,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

@@ -129,6 +129,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:
@@ -138,23 +139,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
 	}
 
@@ -165,18 +162,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
 		}
 
@@ -226,8 +220,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 {
@@ -275,10 +267,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
 	}
 
@@ -289,12 +281,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 {
@@ -391,8 +381,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

@@ -142,6 +142,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 {
@@ -161,7 +162,6 @@ func (b *backend) pathRewrapWrite(ctx context.Context, req *logical.Request, d *
 				batchResponseItems[i].Error = err.Error()
 				continue
 			default:
-				p.Unlock()
 				return nil, err
 			}
 		}
@@ -177,16 +177,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)
 		}
 
@@ -210,7 +207,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{}{
@@ -223,7 +219,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

@@ -353,9 +353,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
 	}
 
@@ -371,12 +371,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 {
@@ -389,7 +387,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"]
@@ -477,7 +474,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
 			}
@@ -495,7 +491,6 @@ func (b *backend) pathSignWrite(ctx context.Context, req *logical.Request, d *fr
 		}
 	}
 
-	p.Unlock()
 	return resp, nil
 }
 
@@ -611,9 +606,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
 	}
 
@@ -718,7 +713,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
 			}
@@ -729,7 +723,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.
+```