Skip to content
This repository has been archived by the owner on Nov 19, 2024. It is now read-only.

Limit subnet certificate delegations to depth 1 #239

Merged
merged 2 commits into from
Dec 13, 2023
Merged

Conversation

oggy-dfin
Copy link
Member

As discussed in various meetings, the nested certificate delegations allowed by the spec aren't used, but have complicated the reasoning about some features (see discussions around delegations in #163 for example), and stating some properties (like the new subnet read_state requests). But we don't use nested delegations in practice and we don't have any plans to use them in the future. So this PR removes them.

@oggy-dfin oggy-dfin requested a review from a team as a code owner October 13, 2023 12:40
@krpeacock
Copy link

This speaks to a slightly broader issue with the specification, but if there are going to be different rules for a valid subnet delegation, as opposed to what is allowed in a valid delegation chain used for a client identity to make calls to the Internet Computer, we should break down those differences more clearly.

In the actual agent implementation, this change makes delegation logic more complicated, by introducing differences in validation between responses and DelegationChain identities

@oggy-dfin
Copy link
Member Author

oggy-dfin commented Oct 17, 2023

In the actual agent implementation, this change makes delegation logic more complicated, by introducing differences in validation between responses and DelegationChain identities

I don't quite understand the objection here; the validity rules for:

  1. delegation chains for user authentication and for
  2. delegations used in subnet responses and canister signatures

are already quite different. The encoding is different (plain CBOR vs hash trees), the rules for signatures are different (signing the representation-independent hash of a CBOR map vs signing the root has of the hash tree), and the side conditions are also different (how the delegations are signed, and how they are restricted).

Now what has changed with the new read_state requests that refer to subnets instead of canisters is that, even for verifying the responses (and canister signatures), the side conditions are now different. I agree that that can complicate the agent's code, but that change was already done in #191.

@krpeacock
Copy link

I think all I really want is a section in the specification that offers a clear disambiguation about the different "delegation" types and what they should contain. It doesn't conflict with this proposal, but came to mind while I was reading it, based on my recent project that called for a close reading of the spec

oggy-dfin added a commit to oggy-dfin/agent-js that referenced this pull request Dec 12, 2023
This reflects the following change in the interface spec:
dfinity/interface-spec#239
oggy-dfin added a commit to oggy-dfin/agent-js that referenced this pull request Dec 12, 2023
This reflects the following change in the interface spec:
dfinity/interface-spec#239
krpeacock pushed a commit to dfinity/agent-js that referenced this pull request Dec 12, 2023
@oggy-dfin oggy-dfin merged commit e810479 into master Dec 13, 2023
4 checks passed
@oggy-dfin oggy-dfin deleted the limit-delegations branch December 13, 2023 12:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants