sign and aggregate confidential relay responses#22302
Open
sign and aggregate confidential relay responses#22302
Conversation
product-security-plaid-production
Bot
requested review from
agparadiso,
ettec and
george-dorin
Contributor
There was a problem hiding this comment.
Pull request overview
Risk Rating: HIGH
This PR adds cryptographic signing to confidential relay logical responses (secrets + capability execution results) so the enclave path can verify authenticity on the reverse path, and wires CRE to inject the node’s P2P key as the signing identity.
Changes:
- Injected a P2P-key-backed signer into the confidential relay service/handler and wrapped success + logical error results in signed response types.
- Added focused handler tests that verify returned ed25519 signatures against the returned signer public key.
- Bumped
chainlink-common(for shared signed response wrappers/hash helpers) and updated related Go module dependencies.
Scrupulous human review recommended (high-impact areas):
- PeerID/key selection logic for signing (
confidentialRelayPeerID+P2P.GetOrFirst) to avoid startup failures or unintended key selection in multi-key environments. - Signature payload/hash compatibility with the enclave verifier and
chainlink-common’s expectations (domain separation, hash function, and encoding). - Service startup behavior when keystore is locked/uninitialized (confidential relay now hard-depends on a usable P2P key at start).
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
go.mod |
Bumps chainlink-common, go-plugin, and adds an indirect dependency. |
go.sum |
Updates checksums for bumped dependencies. |
core/services/cre/cre.go |
Wires P2P keystore + peer ID into confidential relay service creation. |
core/capabilities/confidentialrelay/service.go |
Fetches P2P key and injects signer into handler at service start. |
core/capabilities/confidentialrelay/handler.go |
Signs secrets + capability responses and returns signed wrapper types. |
core/capabilities/confidentialrelay/handler_test.go |
Verifies signatures for signed secrets/capability responses. |
.changeset/confidential-relay-response-signatures.md |
Adds a patch changeset entry describing the new signing behavior. |
This reverts commit 0607415.
…onse-signatures # Conflicts: # go.mod # go.sum
Contributor
|
✅ No conflicts with other open PRs targeting |
|
|
mchain0
reviewed
May 5, 2026
| return errors.New("gateway connector not available") | ||
| } | ||
| h, err := NewHandler(s.capRegistry, conn, s.lggr, s.limitsFactory) | ||
| key, err := s.p2pKeystore.GetOrFirst(s.peerID) |
Contributor
There was a problem hiding this comment.
ok - here's startup logic (GetOrFirst(peerID), peerID selection, handler injection), but there are no tests covering success/failure wiring paths
mchain0
reviewed
May 5, 2026
Contributor
|
general notes:
|
This was referenced May 5, 2026
Contributor
Author
Regarding (1), I now have smartcontractkit/chainlink-common#2032 - please review :-) Regarding (2) There are followup PRs coming in core(gateway) and confidential-compute (enclave). |
…onse-signatures # Conflicts: # go.mod # go.sum
nadahalli
changed the title
…e paths
chainlink-common#2032 made SecretsResponseResult.Hash and
CapabilityResponseResult.Hash run params.Validate first and return
([32]byte, error), so a relay node can no longer accidentally sign
over a payload missing fields the canonical hash binds to.
Production callsites:
- core/capabilities/confidentialrelay/handler.go: sign{Secrets,Capability}Response
now wrap any Hash error and return it; the handler's existing error path
surfaces it back to the gateway as a JSON-RPC error so a request with
malformed identity fields (Owner format, ExecutionID length, etc.) is
rejected at signing time rather than silently producing an unbinding
signature.
- core/services/gateway/handlers/confidentialrelay/aggregator.go:
decodeSignedResponse propagates the Hash error so a single node's
response that fails canonical hashing is dropped from quorum counting
in the existing skip-this-response path, without aborting the whole
aggregation.
Also widens the changeset to mention the aggregator change folded in
from PR #22304.
chainlink-common#2032's Validate rejects request params missing any
field the canonical hash binds to (workflow_id, owner, execution_id,
plus the per-method binders) and pins formats: Owner is a 0x-prefixed
20-byte hex Ethereum address, ExecutionID is 32-byte hex with no
prefix, EnclavePublicKey is hex, and SecretIdentifier needs both Key
and Namespace. Hash returns an error if any of these are missing or
malformed, so any test that exercises the real signing or aggregator
path with a minimal {workflow_id} param now hangs the test (gateway
side, aggregator drops responses, quorum never reached) or returns
ErrInternal (relay-DON side, signSecretsResponse / signCapabilityResponse
fail).
Refreshed test fixtures to use canonical valid params:
- gateway aggregator_test: validCapParams / validSecretsParams helpers
threaded through every site that uses the real aggregator.
- gateway handler_test: validCapParamsJSON helper for the three tests
that swap in the real aggregator.
- relay-DON handler_test: a single canonical secretsGetTestParams /
testOwner pair used by the request builder, the response-signature
verification step, and the matching mock vault response (the mock's
EncryptionKey now matches the request's EnclavePublicKey, "aabbcc",
which is a non-trivial hex value that satisfies the new validator).
Subtests that error out before signing (attestation_failure,
not_found, unsupported_method, invalid_params_JSON) are unchanged.
`go test ./core/services/gateway/handlers/confidentialrelay/`: PASS.
`go test ./core/capabilities/confidentialrelay/`: PASS.
…onse-signatures # Conflicts: # go.mod # go.sum
|
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.
What changed
This PR covers both halves of the relay-DON response-signature work for the reverse path back to the enclave:
Per-node signing (was #22302). Each confidential relay node signs the logical relay response it returns toward the enclave path. Adds signed secrets and capability responses from
core/capabilities/confidentialrelay, CRE wiring to resolve the node P2P key and inject a relay-response signer into the confidential relay service, and handler tests that verify the returned ed25519 signatures against the returned signer public key.Gateway aggregation (was #22304). The confidential-relay gateway aggregator now buckets per-node signed responses by their canonical logical hash (via
Hash(params)helpers inchainlink-common) and merges signatures into a single envelope once F+1 unique signers vouch for the same hash. Aggregator dispatches onreq.Methodto handle bothconfidential.secrets.getandconfidential.capability.execute. Transport-level JSON-RPC error responses are dropped from quorum counting per the design doc; only signed logical responses count.A defense-in-depth comment in
aggregator.godescribes the future signature-verification hook for when the gateway has access to the relay-DON signer set; the trust anchor remains the enclave.This PR also bumps
chainlink-commonto the merged relay-signature types from smartcontractkit/chainlink-common#2021.Why
After
chainlink-commondefined the shared signed-response wrappers and hash helpers, signing alone is not deployable: the previous aggregator bucketed by full-envelope digest, which would break once each honest node's envelope started carrying a different signature for identical logical content. Combining signing and aggregation here makes the change deployable end-to-end, so the enclave (PRIV-432, confidential-compute #324) can verify F+1 unique relay signatures over a shared logical payload at E9.These two parts were previously split across #22302 (signing) and #22304 (aggregation, stacked on signing). Per review feedback, they belong together.
Impact
Validation
go test ./core/capabilities/confidentialrelaygo test ./core/services/gateway/handlers/confidentialrelaygo test ./core/services/cre -run TestDoesNotExist