Skip to content

vault: validate encrypted value size in request validator #21760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
prashantkumar1982 wants to merge 2 commits into
base: release/2.40.0
Choose a base branch
from
+131 −3
Open

vault: validate encrypted value size in request validator #21760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0656354
vault: validate encrypted value size in request validator
prashantkumar1982 Mar 27, 2026
26a1afb
chore: trigger CI
prashantkumar1982 Mar 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion core/capabilities/vault/capability.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,10 @@ func NewCapability(
if err != nil {
return nil, fmt.Errorf("could not create request batch size limiter: %w", err)
}
ciphertextLimiter, err := limits.MakeUpperBoundLimiter(limitsFactory, cresettings.Default.VaultCiphertextSizeLimit)
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Close ciphertext limiter in vault capability lifecycle

NewCapability now creates a ciphertextLimiter, but Capability.Close() still only closes MaxRequestBatchSizeLimiter; this leaves the new limiter open for the lifetime of each start/stop cycle. In deployments where limiters maintain background state (tenant maps/watchers), repeated capability restarts will accumulate leaked resources and stale limit subscriptions. Please close MaxCiphertextLengthLimiter alongside the existing limiter in Close().

Useful? React with 👍 / 👎.

if err != nil {
return nil, fmt.Errorf("could not create ciphertext size limiter: %w", err)
Comment on lines +415 to +417
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Close batch limiter on ciphertext limiter init failure

When MakeUpperBoundLimiter fails here, NewCapability returns without closing the already-created request batch limiter. Since these limiters are closable and may hold settings/watcher state, repeated startup retries can accumulate leaked resources. Close limiter on this error path before returning.

Useful? React with 👍 / 👎.

}
return &Capability{
lggr: logger.Named(lggr, "VaultCapability"),
clock: clock,
Expand All @@ -420,6 +424,6 @@ func NewCapability(
requestAuthorizer: requestAuthorizer,
capabilitiesRegistry: capabilitiesRegistry,
publicKey: publicKey,
RequestValidator: NewRequestValidator(limiter),
RequestValidator: NewRequestValidator(limiter, ciphertextLimiter),
}, nil
}
26 changes: 25 additions & 1 deletion core/capabilities/vault/validator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ import (
"github.com/smartcontractkit/tdh2/go/tdh2/tdh2easy"

vaultcommon "github.com/smartcontractkit/chainlink-common/pkg/capabilities/actions/vault"
pkgconfig "github.com/smartcontractkit/chainlink-common/pkg/config"
"github.com/smartcontractkit/chainlink-common/pkg/settings/limits"
"github.com/smartcontractkit/chainlink/v2/core/capabilities/vault/vaulttypes"
)

type RequestValidator struct {
MaxRequestBatchSizeLimiter limits.BoundLimiter[int]
MaxCiphertextLengthLimiter limits.BoundLimiter[pkgconfig.Size]
}

func (r *RequestValidator) ValidateCreateSecretsRequest(publicKey *tdh2easy.PublicKey, request *vaultcommon.CreateSecretsRequest) error {
Expand Down Expand Up @@ -60,6 +62,9 @@ func (r *RequestValidator) validateWriteRequest(publicKey *tdh2easy.PublicKey, i
if req.EncryptedValue == "" {
return errors.New("secret must have encrypted value set at index " + strconv.Itoa(idx) + ":" + req.Id.String())
}
if err := r.validateCiphertextSize(req.EncryptedValue); err != nil {
return fmt.Errorf("secret encrypted value at index %d is invalid: %w", idx, err)
}
err := EnsureRightLabelOnSecret(publicKey, req.EncryptedValue, req.Id.Owner)
if err != nil {
return errors.New("Encrypted Secret at index [" + strconv.Itoa(idx) + "] doesn't have owner as the label. Error: " + err.Error())
Expand All @@ -75,6 +80,21 @@ func (r *RequestValidator) validateWriteRequest(publicKey *tdh2easy.PublicKey, i
return nil
}

func (r *RequestValidator) validateCiphertextSize(encryptedValue string) error {
rawCiphertext, err := hex.DecodeString(encryptedValue)
if err != nil {
return fmt.Errorf("failed to decode encrypted value: %w", err)
}
if err := r.MaxCiphertextLengthLimiter.Check(context.Background(), pkgconfig.Size(len(rawCiphertext))*pkgconfig.Byte); err != nil {
var errBoundLimited limits.ErrorBoundLimited[pkgconfig.Size]
if errors.As(err, &errBoundLimited) {
return fmt.Errorf("ciphertext size exceeds maximum allowed size: %s", errBoundLimited.Limit)
}
return fmt.Errorf("failed to check ciphertext size limit: %w", err)
}
return nil
}

func (r *RequestValidator) ValidateGetSecretsRequest(request *vaultcommon.GetSecretsRequest) error {
if len(request.Requests) == 0 {
return errors.New("no GetSecret request specified in request")
Expand Down Expand Up @@ -129,9 +149,13 @@ func (r *RequestValidator) ValidateDeleteSecretsRequest(request *vaultcommon.Del
return nil
}

func NewRequestValidator(maxRequestBatchSizeLimiter limits.BoundLimiter[int]) *RequestValidator {
func NewRequestValidator(
maxRequestBatchSizeLimiter limits.BoundLimiter[int],
maxCiphertextLengthLimiter limits.BoundLimiter[pkgconfig.Size],
) *RequestValidator {
return &RequestValidator{
MaxRequestBatchSizeLimiter: maxRequestBatchSizeLimiter,
MaxCiphertextLengthLimiter: maxCiphertextLengthLimiter,
}
}

Expand Down
96 changes: 96 additions & 0 deletions core/capabilities/vault/validator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package vault

import (
"encoding/hex"
"testing"

"github.com/stretchr/testify/require"

vaultcommon "github.com/smartcontractkit/chainlink-common/pkg/capabilities/actions/vault"
pkgconfig "github.com/smartcontractkit/chainlink-common/pkg/config"
"github.com/smartcontractkit/chainlink-common/pkg/settings/limits"
)

func TestRequestValidator_CiphertextSizeLimit(t *testing.T) {
validator := NewRequestValidator(
limits.NewUpperBoundLimiter(10),
limits.NewUpperBoundLimiter[pkgconfig.Size](10*pkgconfig.Byte),
)

id := &vaultcommon.SecretIdentifier{
Key: "key",
Namespace: "namespace",
Owner: "0x1111111111111111111111111111111111111111",
}

tests := []struct {
name string
call func(*testing.T, *RequestValidator, string) error
value string
errSubstr string
}{
{
name: "create accepts ciphertext at the limit",
call: func(t *testing.T, validator *RequestValidator, value string) error {
return validator.ValidateCreateSecretsRequest(nil, &vaultcommon.CreateSecretsRequest{
RequestId: "request-id",
EncryptedSecrets: []*vaultcommon.EncryptedSecret{
{Id: id, EncryptedValue: value},
},
})
},
value: hex.EncodeToString(make([]byte, 10)),
},
{
name: "create rejects ciphertext above the limit",
call: func(t *testing.T, validator *RequestValidator, value string) error {
return validator.ValidateCreateSecretsRequest(nil, &vaultcommon.CreateSecretsRequest{
RequestId: "request-id",
EncryptedSecrets: []*vaultcommon.EncryptedSecret{
{Id: id, EncryptedValue: value},
},
})
},
value: hex.EncodeToString(make([]byte, 11)),
errSubstr: "ciphertext size exceeds maximum allowed size",
},
{
name: "update accepts ciphertext at the limit",
call: func(t *testing.T, validator *RequestValidator, value string) error {
return validator.ValidateUpdateSecretsRequest(nil, &vaultcommon.UpdateSecretsRequest{
RequestId: "request-id",
EncryptedSecrets: []*vaultcommon.EncryptedSecret{
{Id: id, EncryptedValue: value},
},
})
},
value: hex.EncodeToString(make([]byte, 10)),
},
{
name: "update rejects ciphertext above the limit",
call: func(t *testing.T, validator *RequestValidator, value string) error {
return validator.ValidateUpdateSecretsRequest(nil, &vaultcommon.UpdateSecretsRequest{
RequestId: "request-id",
EncryptedSecrets: []*vaultcommon.EncryptedSecret{
{Id: id, EncryptedValue: value},
},
})
},
value: hex.EncodeToString(make([]byte, 11)),
errSubstr: "ciphertext size exceeds maximum allowed size",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := tt.call(t, validator, tt.value)
if tt.errSubstr == "" {
require.NoError(t, err)
return
}

require.Error(t, err)
require.ErrorContains(t, err, tt.errSubstr)
})
}
}
6 changes: 5 additions & 1 deletion core/services/gateway/handlers/vault/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,10 @@ func NewHandler(methodConfig json.RawMessage, donConfig *config.DONConfig, don g
if err != nil {
return nil, fmt.Errorf("could not create request batch size limiter: %w", err)
}
ciphertextLimiter, err := limits.MakeUpperBoundLimiter(limitsFactory, cresettings.Default.VaultCiphertextSizeLimit)
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Close ciphertext limiter when vault gateway handler stops

NewHandler now allocates a ciphertextLimiter, but handler.Close() still only closes writeMethodsEnabled and MaxRequestBatchSizeLimiter. That means every handler restart can leak the newly added limiter instance and any associated internal state, which is especially problematic for long-running nodes that reload handlers. Add MaxCiphertextLengthLimiter.Close() to the shutdown join.

Useful? React with 👍 / 👎.

if err != nil {
return nil, fmt.Errorf("could not create ciphertext size limiter: %w", err)
Comment on lines +200 to +202
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Close batch limiter if ciphertext limiter creation fails

This early return leaks the request batch limiter created just above when ciphertext limiter construction errors. In environments that retry handler initialization, this can leave stale limiter resources subscribed across attempts. Close the previously allocated limiter before returning this error.

Useful? React with 👍 / 👎.

}

writeMethodsEnabled, err := limits.MakeGateLimiter(limitsFactory, cresettings.Default.GatewayVaultManagementEnabled)
if err != nil {
Expand All @@ -218,7 +222,7 @@ func NewHandler(methodConfig json.RawMessage, donConfig *config.DONConfig, don g
metrics: metrics,
aggregator: &baseAggregator{capabilitiesRegistry: capabilitiesRegistry},
clock: clock,
RequestValidator: vaultcap.NewRequestValidator(limiter),
RequestValidator: vaultcap.NewRequestValidator(limiter, ciphertextLimiter),
}, nil
}

Expand Down
Loading