feat: delegations can require calls to carry a signed sender_info#10452
Closed
aterga wants to merge 2 commits into
Closed
feat: delegations can require calls to carry a signed sender_info#10452aterga wants to merge 2 commits into
aterga wants to merge 2 commits into
Conversation
Adds an optional sender_info_required field to request delegations, covered by the delegation signature. When a delegation in a request's delegation chain sets it to true, update calls authenticated through that chain must carry a (signed and verified) sender_info; calls without one are rejected with the new SenderInfoRequiredByDelegation error. Query and read_state requests are unaffected (read_state cannot even carry a sender_info, and query responses cannot change state). Since the field is part of the delegation's representation-independent hash, the bearer of the delegation cannot lift the requirement by stripping the field: doing so invalidates the delegation signature. For the same reason, replicas that predate this change fail closed on such delegations. Combined with sender_info attribute checking (#10447), this lets issuers like Internet Identity enforce read-only sessions end to end: the delegation forces the certified session attributes to be present on every update call, and the attributes' implicit:permissions = "queries" then causes the call to be rejected. Without this bit, a dapp holding the session key could lift the restriction by omitting the sender_info. https://claude.ai/code/session_01BQNgPJKgxWohrJ6owwJBzz
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces an optional sender_info_required bit on request delegations (covered by the delegation signature / representation-independent hash) and enforces that update calls authenticated via a delegation chain must include a sender_info when any delegation in the chain requires it. This is part of combining the read-only delegation experiments by making “certified attributes must be present” protocol-enforceable.
Changes:
- Add
Delegation.sender_info_required: Option<bool>(hashes as nat 0/1 undersender_info_required) plus constructor/accessor and CBOR coverage. - Thread
DelegationRestrictionsthrough delegation validation and enforcesender_infopresence on the update/callpath (with a newSenderInfoRequiredByDelegationerror). - Add test utilities and ingress-message e2e tests covering required/optional behavior and chain accumulation.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rs/validator/src/ingress_validation.rs | Introduces DelegationRestrictions, propagates sender_info_required through delegation validation, and enforces sender_info presence for update calls. |
| rs/types/types/src/messages/http.rs | Adds sender_info_required to Delegation, constructor/accessor, and includes it in signed-bytes hashing when present. |
| rs/types/types/src/messages/http/tests.rs | Updates Delegation CBOR serialization tests to include sender_info_required (null/bool). |
| rs/validator/tests/ingress_validation.rs | Adds a signed-bytes golden test for delegations with sender_info_required. |
| rs/validator/src/ingress_validation/tests.rs | Updates signature-validation unit tests to assert the returned restrictions include sender_info_required == false by default. |
| rs/validator/http_request_test_utils/src/lib.rs | Extends delegation-chain builders to create delegations with sender_info_required. |
| rs/validator/ingress_message/tests/validate_request.rs | Adds e2e tests for “required ⇒ reject missing sender_info”, “required ⇒ accept valid sender_info”, and “query unaffected”. |
| rs/validator/ingress_message/src/lib.rs | Exposes the new error variant in the ingress-message wrapper crate and its Display text. |
| rs/validator/ingress_message/src/internal/mod.rs | Maps the new ic_validator error variant into the wrapper crate’s error enum. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Avoid duplicating the shared request-content validation on the call path by enforcing the sender_info_required restriction inside validate_request_content behind a parameter, and fix a comment to say query requests rather than query responses. https://claude.ai/code/session_01BQNgPJKgxWohrJ6owwJBzz
Contributor
Author
|
Closing in favor of the more lightweight approach in #10449 (delegations carry the Generated by Claude Code |
pull Bot
pushed a commit
to mikeyhodl/ic
that referenced
this pull request
Jun 24, 2026
…ns (dfinity#10449) # Queries-only permissions on delegations **This is the consolidated read-only-sessions feature** (the previously stacked alternatives dfinity#10447/dfinity#10452, which carried the restriction in certified `sender_info` attributes, were closed in favor of this more lightweight approach). ## What it does Request delegations gain an optional `permissions` field, mirroring the existing `targets` field. It is modeled as a typed enum `DelegationPermissions` whose variants are encoded on the wire (and in the representation-independent hash) as text: * `permissions` **absent** or **`"all"`** (`DelegationPermissions::All`) → the sender can execute all functions — queries, replicated queries, and updates (today's semantics; byte-for-byte identical hash for existing delegations without the field). * **`"queries"`** (`DelegationPermissions::Queries`) → the sender can only execute queries: requests to `/call` endpoints (updates *and* replicated queries) authenticated through such a chain are rejected during ingress validation with the new `RequestValidationError::UpdateCallNotPermittedByDelegation`; query and `read_state` requests remain permitted. * **Any other value** → because the field is a closed enum, an unsupported value fails to deserialize, so the request is **rejected when it is decoded** (before validation). There is no dedicated validator error for it. A restriction anywhere in the chain applies to the whole chain (like `targets`, restrictions only accumulate). The matching interface-spec change (dfinity/developer-docs#292) models the field as the union `Queries | All | Unrestricted`, which this enum mirrors exactly. ## Why this can't be bypassed, and why it's backward compatible * The field is part of the delegation's representation-independent hash, i.e. covered by the delegation signature. A dapp holding the session key **cannot strip the field** — doing so changes the hash and invalidates the signature. * **Old replicas fail closed**: they parse the delegation without the unknown field (serde ignores unknown map keys), recompute the hash without it, and signature verification fails. A restricted delegation is unusable on un-upgraded replicas rather than silently unrestricted. Rollout: replicas first, issuers (Internet Identity) after. * Delegations without the field are completely unaffected, so existing agents, dapps, and issuers need no changes. Agents only need to round-trip the new field once they want to carry restricted delegations; an agent that drops it fails closed. * The enforcement point is `HttpRequestVerifier<SignedIngressContent>` — the same verifier used by the ingress manager for block validation, so a malicious boundary node or replica cannot smuggle a restricted update call into a block. ## Rationale Enables issuers like Internet Identity to sign **read-only delegations**: sessions that can read on the user's behalf (query calls) but cannot change state (update calls), enforced by the protocol regardless of the client's cooperation. II sets this field from a "Read-only mode" checkbox in the authorize flow (II-side changes prepared separately). Historical precedent: the interface spec already extended the delegation map with an optional restriction field once — the `senders` field (added Dec 2021, never implemented by the replica, removed in [interface-spec#246](dfinity/interface-spec#246)). This PR follows the same formal pattern (`verify_delegations` accumulation), but implementation-first. ## Changes * `ic-types`: typed `DelegationPermissions { Queries, All }` enum (serialized as `"queries"`/`"all"`); optional `Delegation.permissions` field included in the signed bytes (`hash_of_map` key `"permissions"`); chainable `with_targets`/`with_permissions` builder methods (replacing the former `new_with_targets`/`new_with_permissions` constructors so a delegation can carry both). * `ic-validator`: `DelegationRestrictions` (targets + queries-only) threaded through delegation-chain validation; update-call rejection via `RequestValidationError::UpdateCallNotPermittedByDelegation`. The `/call` path uses the shared `validate_request_content` (no special-casing) and checks the restriction afterwards. * `ic-validator-ingress-message`: the new error variant mirrored. * `ic-validator-http-request-test-utils`: `delegate_to_with_permissions` chain-builder support. * Tests: signed-bytes golden test for the new field; CBOR encoding + round-trip tests; a decode-rejection test (`delegation_permissions_rejects_unsupported_value`) covering unsupported values including case/whitespace variants; validator-level e2e tests (update-rejected / query-and-read_state-permitted / `"all"`-permitted / restriction-mid-chain / combined targets+permissions); and a system test (`requests_with_delegation_permissions` in `rs/tests/crypto/ingress_verification_test.rs`, tagged `long_test`) exercising the full HTTP path against a real replica. ## Caveats / follow-ups * Calls to query methods submitted via the `/call` path (replicated queries) are rejected for `"queries"` delegations by design — the `"all"` vocabulary makes this explicit. * Interface-spec change in dfinity/developer-docs#292; II-side issuance and agent (`@icp-sdk/core`) round-tripping of the field prepared/pending. `cargo test -p ic-types -p ic-validator -p ic-validator-ingress-message` passes and clippy is clean; downstream consumers (`ic-http-endpoints-public`, `ic-ingress-manager`, `ic-canister-client`, `ic-state-machine-tests`) and both affected system-test binaries compile. https://claude.ai/code/session_01BQNgPJKgxWohrJ6owwJBzz --------- Co-authored-by: Claude <noreply@anthropic.com>
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.
Delegations can require calls to carry a signed
sender_infoThis is step 1 of combining the two read-only-delegation experiments (#10447:
sender_infoattribute checking, #10449: delegation-level permissions): request delegations gain one bit — an optionalsender_info_requiredfield, covered by the delegation signature — that forces update calls authenticated through the chain to carry a (signed and verified)sender_info.Why
The
sender_infoattribute mechanism of #10447 enforcesimplicit:permissions = "queries"when the attributes are present — but the party we want to constrain (the dapp holding the session key) decides whether to attach them, so it can lift the restriction by omitting the blob. This bit closes that hole while keeping all permission semantics in the certified attributes:sender_info, signed by the same issuer underic-sender-info) say what the session may do, e.g.implicit:permissions = "queries"→ update calls rejected per feat: reject update calls when sender_info restricts permissions to queries #10447.Together: an issuer like Internet Identity signs a read-only delegation chain whose update calls must carry attributes that in turn forbid update calls — protocol-enforced, regardless of client cooperation. #10447 is being rebased on top of this PR so the stack forms the complete combined design.
Semantics
false→ today's behavior, byte-for-byte identical hashes for existing delegations; nothing in the ecosystem changes.sender_info_required = trueanywhere in the chain → update calls (/callpath) without asender_infoare rejected with the newRequestValidationError::SenderInfoRequiredByDelegation; calls with one proceed through the existing signature verification of the blob. Requirements only accumulate along the chain (liketargets).sender_infoat all, and queries are not restricted by the permissions attribute anyway.sender_infoitself.Changes
ic-types:Delegation.sender_info_required(optional bool; hashed as nat 0/1 under the keysender_info_required), constructor, accessor; CBOR encoding tests.ic-validator:DelegationRestrictionsthreaded through delegation-chain validation; presence enforcement on the call path; new error variant mirrored inic-validator-ingress-message.ic-validator-http-request-test-utils:delegate_to_with_sender_info_requiredchain-builder support.falseunrestricted / requirement in the middle of a chain.Follow-ups
cargo test -p ic-types -p ic-validator -p ic-validator-ingress-messagepasses and clippy is clean. Bazel was not available in the development container, so CI should confirm the Bazel build.https://claude.ai/code/session_01BQNgPJKgxWohrJ6owwJBzz
Generated by Claude Code