Verify enclave config against onchain DON in confidentialrelay handler (PRIV-458)#22516
Open
nadahalli wants to merge 1 commit into
Open
Verify enclave config against onchain DON in confidentialrelay handler (PRIV-458)#22516nadahalli wants to merge 1 commit into
nadahalli wants to merge 1 commit into
Conversation
…r (PRIV-458) The handler now compares the attested EnclaveConfig in every incoming SecretsRequestParams and CapabilityRequestParams against the local node's WorkflowDON membership and fault tolerance, after Nitro attestation validation succeeds. Closes Sigma Prime CL112-01 on the relay-DON path. The relay DON runs on the same nodes as the workflow DON, so localNode.WorkflowDON.Members is the right comparison target. LocalNode is an O(1) in-memory map lookup populated by the registry syncer on a ~12s tick, so the check stays off the RPC hot path. Up to ~12s staleness applies during DON membership rotations and is acceptable given how rare those events are. Tests cover match-accepts, F mismatch, signers-count mismatch, signer value mismatch, order-independent comparison, and the secrets-get path. Existing tests updated to fill EnclaveConfig in fixtures with a matching WorkflowDON.Members in the mock registry. Bumps chainlink-common to v0.11.2-0.20260518112011-40a8e4cedaa8 to pick up the EnclaveConfig field on confidentialrelay request params (smartcontractkit/chainlink-common#2063). Companion PRs: - smartcontractkit/confidential-compute#329 (pool.go-side check). - smartcontractkit/chainlink-common#2063 (field on params). - smartcontractkit/confidential-compute#330 (enclave fills field).
product-security-plaid-production
Bot
requested review from
cedric-cordenier and
ettec
Contributor
|
👋 nadahalli, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
Contributor
|
I see you updated files related to
|
Contributor
|
✅ No conflicts with other open PRs targeting |
Contributor
There was a problem hiding this comment.
Pull request overview
Risk Rating: MEDIUM — changes add new validation in the confidential relay request handling path (security hardening) and bump a shared dependency (chainlink-common).
This PR hardens the confidential relay handler by verifying that the attested EnclaveConfig in incoming requests matches the node’s locally-synced onchain WorkflowDON membership and fault tolerance, addressing Sigma Prime CL112-01 (PRIV-458).
Changes:
- Add relay-side EnclaveConfig vs WorkflowDON verification for both SecretsGet and CapabilityExec flows.
- Bump
github.com/smartcontractkit/chainlink-commonto pick up the newEnclaveConfigfield on request params. - Update handler tests/fixtures and add dedicated tests for EnclaveConfig verification (match, mismatches, order-independence, secrets-get coverage).
Targeted areas requiring scrupulous human review:
verifyEnclaveConfigMatchesDON: confirm the chosen onchain reference (localNode.WorkflowDON) is correct for all deployments/rotations and that the comparison logic matches the intended security model (especially around signer identity representation).- Request-path behavior during DON membership rotations / registry staleness: ensure the new rejection behavior is operationally acceptable for the gateway/enclave interaction.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| go.mod | Bumps chainlink-common to a newer pseudo-version to include EnclaveConfig in request params. |
| go.sum | Updates module sums for the chainlink-common version bump. |
| core/capabilities/confidentialrelay/handler.go | Adds EnclaveConfig verification against locally synced WorkflowDON members/F; hooks it into SecretsGet and CapabilityExec after attestation hash verification. |
| core/capabilities/confidentialrelay/handler_test.go | Updates fixtures to include EnclaveConfig and adds tests covering accept/reject cases and order-independence. |
Comments suppressed due to low confidence (1)
core/capabilities/confidentialrelay/handler_test.go:475
- This struct literal has formatting/indentation issues (EnclaveConfig and the closing brace are misaligned). Please run gofmt (or adjust indentation) to keep formatting consistent and avoid gofmt-check failures.
params := confidentialrelaytypes.CapabilityRequestParams{
WorkflowID: "wf-1",
Owner: testOwner,
ExecutionID: "32c631d295ef5e32deb99a10ee6804bc4af13855687559d7ff6552ac6dbb2ce1",
ReferenceID: "17",
CapabilityID: "fail-cap@1.0.0",
Payload: base64.StdEncoding.EncodeToString(mustMarshalProto(t, &sdkpb.CapabilityRequest{Id: "fail-cap@1.0.0", Method: "Execute"})),
EnclaveConfig: testEnclaveConfig(),
}
| // the Nitro attestation binds the request hash, but a malicious host | ||
| // can produce a genuinely-attested request over a forged enclave config | ||
| // unless we compare the config value against an onchain reference. | ||
| if err := h.verifyEnclaveConfigMatchesDON(ctx, params.EnclaveConfig); err != nil { |
Comment on lines
333
to
341
| params := confidentialrelaytypes.CapabilityRequestParams{ | ||
| WorkflowID: "wf-1", | ||
| Owner: testOwner, | ||
| ExecutionID: "32c631d295ef5e32deb99a10ee6804bc4af13855687559d7ff6552ac6dbb2ce1", | ||
| ReferenceID: "17", | ||
| CapabilityID: "my-cap@1.0.0", | ||
| Payload: makeCapabilityPayload(t, map[string]any{"key": "val"}), | ||
| EnclaveConfig: testEnclaveConfig(), | ||
| } |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes Sigma Prime CL112-01 on the relay-DON path.
The confidentialrelay handler now compares the attested EnclaveConfig
in every incoming SecretsRequestParams and CapabilityRequestParams
against the local node's WorkflowDON membership and fault tolerance,
after Nitro attestation validation succeeds.
The relay DON runs on the same nodes as the workflow DON, so
localNode.WorkflowDON.Members is the right comparison target. LocalNode
is an O(1) in-memory map lookup populated by the registry syncer on a
~12s tick, so the check stays off the RPC hot path. Up to ~12s
staleness applies during DON membership rotations and is acceptable
given how rare those events are.
Bumps chainlink-common to v0.11.2-0.20260518112011-40a8e4cedaa8 to pick
up the EnclaveConfig field on confidentialrelay request params:
Companion PRs (Stage 2 of PRIV-458):
LOOPP plugin path).
Tests cover match-accepts, F mismatch, signers-count mismatch, signer
value mismatch, order-independent comparison, and the secrets-get
path. Existing handler tests updated to fill EnclaveConfig in fixtures
with a matching WorkflowDON.Members in the mock registry.
Jira: https://smartcontract-it.atlassian.net/browse/PRIV-458