feat(vault): restrict zone-b workflow DON GetSecrets to allowlisted owners#23145
feat(vault): restrict zone-b workflow DON GetSecrets to allowlisted owners#23145prashantkumar1982 wants to merge 15 commits into
Conversation
…wners Block GetSecrets reads originating from a zone-b workflow DON unless the calling workflow owner is allowlisted. Enforced in Capability.Execute before the OCR queue, so non-allowlisted owners get a deterministic rejection and zone-a / gateway paths are unaffected. - zoneBRestrictor (new file) owns the two gate limiters and the check; zone membership is resolved authoritatively via CapabilitiesRegistry.DONByID on the caller's WorkflowDonID, never from caller-supplied zone data. - Implement DONByID on LocalRegistry and delegate through the concrete Registry. - Fails closed if the caller DON cannot be resolved. Note: trusting WorkflowDonID assumes the transport binds it to the authenticated caller DON; see follow-up transport-layer binding change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
👋 prashantkumar1982, 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! |
|
✅ No conflicts with other open PRs targeting |
|
I see you updated files related to
|
CORA - Analysis SkippedReason: The number of code owners (4) is less than the minimum required (5) and/or the number of CODEOWNERS entries with changed files (4) is less than the minimum required (2). |
… into vault-zone-b-restriction
|
/vault-audit |
|
❌ Vault audit failed to run for |
|
/vault-audit model=haiku |
|
✅ Vault audit complete for 📋 View the full report and findings — private tracking issue, visible to chainlink team members only. Resolve blocking findings there with |
|
/vault-audit |
|
📋 View the full report and findings — private tracking issue, visible to chainlink team members only. Resolve blocking findings there with |
… into vault-zone-b-restriction
Set RequestMetadata.WorkflowDonID to the local (calling) workflow DON ID for the vault GetSecrets and confidential-workflows capability requests, gated on the RemoteExecutableWorkflowDONBindingEnabled setting so the remote executable server's binding check accepts the request. Gating on the dynamic setting (rather than setting unconditionally) keeps the F+1 aggregated request payloads byte-identical across a mixed-binary rollout. Any failure reading the setting or resolving the local node fails the whole call rather than sending a zero WorkflowDonID. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
/vault-audit |
|
📋 View the full report and findings — private tracking issue, visible to chainlink team members only. Resolve blocking findings there with |
Add a new local CRE topology that enables RemoteExecutableWorkflowDONBindingEnabled on the workflow (caller) and capabilities (vault remote-executable server) nodes, so the DON2DON WorkflowDonID binding check is exercised end-to-end by the existing Vault secrets smoke flow. Selected via TOPOLOGY_NAME containing "vault-workflow-don-binding-enabled". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rename the inner LocalNode error to lnErr in confidential_module.go and secrets.go so it no longer shadows the outer err declaration (govet shadow). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
MUST-2: cache last-known zone-b membership per WorkflowDonID so a transient
nil/empty capabilities-registry snapshot (startup or empty sync) does not fail
every vault GetSecrets read DON-wide. Only a never-before-resolved DON fails
closed. The zone-b restrictor now takes a logger and warns on cache fallback.
MUST-3: add a test with a mixed-case ("Zone-B") registry family so the
case-insensitive family match has CI coverage — a regression to an exact (==)
comparison would silently disable the zone-b check and now fails this test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
/vault-audit |
|
📋 View the full report and findings — private tracking issue, visible to chainlink team members only. Resolve blocking findings there with |
Split the registry-outage test into subtests and add the missing zone-b case: a previously-resolved zone-b DON must remain denied (allowlist enforced from cache) during a registry outage, not slip through via a resolve failure. Covers both cache directions plus the cold-DON fail-closed path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a test proving a zone-b caller supplying a 0x-prefixed, mixed-case owner is allowed against an allowlist keyed by the normalized (0x-stripped, lowercased) address. Guards against a regression to bare string comparison, which would silently deny every standard-format allowlisted caller. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add an Execute()-level test for the allow path: an allowlisted zone-b owner passes the gate and reaches the OCR handler (times out with no oracle in the unit test) instead of being denied. Exercises the CRE-context population wiring that the direct enforce() tests bypass, so a breakage there is now detectable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
/vault-audit |
|
📋 View the full report and findings — private tracking issue, visible to chainlink team members only. Resolve blocking findings there with |
|
3 participants
What
Block
GetSecretsreads originating from a zone-b workflow DON unless the calling workflow owner is allowlisted. Enforced inCapability.Executebefore the OCR queue, so non-allowlisted owners get a deterministic rejection and zone-a / gateway paths are unaffected.zoneBRestrictor(own file) owns the two gate limiters and the check.CapabilitiesRegistry.DONByIDon the caller'sWorkflowDonIDand itsFamilies— never from caller-supplied zone data. FragileDON.Namematching is not used.DONByIDimplemented onLocalRegistryand delegated through the concreteRegistry.Gated by
VaultZoneBWorkflowGetSecretsRestrictEnabled(global, default off); allowlist via owner-scopedPerOwner.VaultZoneBGetSecretsAllowed.Dependencies
DONByID+ the cresettings gates). CI will fail until that merges and thechainlink-commonversion is bumped here (local dev used areplacedirective, intentionally not committed).Security note
Trusting
WorkflowDonIDassumes the transport binds it to the authenticated caller DON. A colluding zone-b DON could otherwise spoof it. The complementary transport-layer binding is a separate change (chainlink-common#2257 + the corresponding chainlink PR) and should be enabled in the same rollout.https://smartcontract-it.atlassian.net/browse/PRIV-593