Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix definition of controller and add verification method binding section #126

Closed
wants to merge 3 commits into from

Conversation

msporny
Copy link
Member

@msporny msporny commented Nov 17, 2024

This PR is an attempt to address issue #119 by revising the definition of controller and adding a section on verification method binding.


Preview | Diff

Copy link
Contributor

@David-Chadwick David-Chadwick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think we have a problem with the definition. If the controller of the controller document can do anything to it (i.e. perform any action on the controller document) then the controller of a verification method should be able to perform any action on the verification method. However anyone can use/read the controller document and anyone can use the verification method e.g. verify a digital signature with the published public key. But this is not what the definition currently says (or implies) prior to my edit. Note what the definition of verification method is "A set of parameters that can be used together with a process to independently verify a proof. "

An entity that is [=authorized=] to perform an action with a specific resource,
such as update a [=controller document=] or use a cryptographic key to generate
a digital signature.
An entity that is [=authorized=] to perform an action associated with a specific
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
An entity that is [=authorized=] to perform an action associated with a specific
An entity that is [=authorized=] to perform any action on the associated

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An entity may be authorized to perform SOME actions but not necessarily ANY actions. This suggestion should not be incorporated.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed. the action is, in fact, limited by the context. For example, the verification method controller can't update the verification method as listed in the CID, but can create proofs that satisfy the method. "Any" action is too broad.

such as update a [=controller document=] or use a cryptographic key to generate
a digital signature.
An entity that is [=authorized=] to perform an action associated with a specific
resource such as updating a [=controller document=] or generating a digital
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
resource such as updating a [=controller document=] or generating a digital
resource such as updating the associated [=controller document=] or updating the associated verification method

Copy link
Member

@TallTed TallTed Nov 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I agree with @David-Chadwick's suggestion, but if it is accepted, it should include the markup and full-stop below --

Suggested change
resource such as updating a [=controller document=] or generating a digital
resource such as updating the associated [=controller document=] or updating
the associated [=verification method=].

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This edit continues the error of the previous suggestion. Verification Method controllers CANNOT update the associated verification method. Only CID document controllers can update the method.

a digital signature.
An entity that is [=authorized=] to perform an action associated with a specific
resource such as updating a [=controller document=] or generating a digital
signature that can be verified using a [=verification method=].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
signature that can be verified using a [=verification method=].

Comment on lines +2915 to +2921
<p>
Implementers ensure that a [=verification method=] is bound to a particular
[=controller=] by going from the expression of the [=verification method=] to
the [=controller document=], and then ensuring that the [=controller document=]
also contains a reference to the [=verification method=]. This process is
described in the algorithm for <a href="#retrieve-verification-method">
retrieving a verification method</a>.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for having copied the section from the DI spec.

However... I still have a problem. This section refers to the algorithmic section for the details on how to find the controller document. That section says:

Let controllerDocumentUrl be the result of parsing vmIdentifier according to the rules of the URL scheme and extracting the primary resource identifier (without the fragment identifier).

The only way I can interpret this sentence is that the URL of a VM is a URL with a fragment, and the controller document's URL is retrieved by removing the fragment.

Is this restriction intentional? If so, the specification of the verification method in §2.2. must make this restriction clear for the id property. Also, if it is indeed intentional, this will not hold up if ever we want to use this specification with Linked Data where URL-s are considered to be opaque, and such structure would be frowned upon; this part of the spec should be rewritten.

If it is not intentional, then... I am not sure. Would we have to say that the exact approach taken to get from the VM to a CD is implementation dependent? Application dependent? I.e., should we have to say that, in a VC setting, the VM MUST be part of the same JSON-LD document as its CD, hence the usage of a fragment ID, otherwise this is up to the implementation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just syncing up with the similar discussion in another thread:

Note that the additional property I propose (linking the VM to the CD) is absolutely optional and does not contradict with what you say; you also talk in conditional. The only reason I think the property would be useful is because I presume it would simplify a number of business rules. But, again, if we decide it is too late for that, be it.

Yeah, I do believe it's too late for that but it could perhaps be further debated/bikeshedded/sorted and tested in a future minor release. But it's not too late for us to add some non-normative text somewhere as needed.

(see #119 (comment)).

@iherman
Copy link
Member

iherman commented Nov 20, 2024

The issue was discussed in a meeting on 2024-11-20

  • no resolutions were taken
View the transcript

1.7. Fix definition of controller and add verification method binding section (pr controller-document#126)

See github pull request controller-document#126.

Brent Zundel: This is a bigger PR than the others in terms of its scope, not size.
… There has been some discussion, but no approvals.

Manu Sporny: Two things. I want to also queue up an issue on the controller that came up on the DID special topic call after this.
… This has to do with the definition of controller. There is confusion around the controller of a document and controller of verification.
… It would have been a class 4 change, which would have prevented the DID WG from using this.
… Let's just use the definition of controller to say that you have control over a thing.
… Copy verification method binding text over to here. Trying to find the right words to say that a controller controls a thing, be it a controller document or a verification method.

Ivan Herman: There is a PR on the DID spec that proposes additions to the vocabulary.
… I think it's the only PR over there.
… There was another issue that came up in the discussion. If I have the URL for the verification method, how do I find where is the controller document for that verification method?

Dave Longley: "controller" dereferences to controller document.

Ivan Herman: What we have right now in the specification is very much JSON dependent.
… Or VC dependent structure.
… I'm fine merging this PR but the issue discussion should continue.

Joe Andrieu: It may not be well documented, but I believe it's in the verification method itself, it's the URL you dereference to get the controller document.
… I want to support Manu's point that it's too big of a lift at this point. We don't mean that the controller can update the verification method, we do mean it for the document.

David Chadwick: I don't like the definition as it stands. I have proposed a change that makes the definition of controller symmetrical, that has the same meaning as to what it applies to.
… It's the same thing on two different objects that have a controller property.

Ivan Herman: Here is an example where the confusion comes.

Dave Longley: -1 to what Ivan is saying ...

Dave Longley: controller does refer to the controller document that defines the VM.

Ivan Herman: My understanding, Joe, is that the controller property in the verification method refers to a person, you or me, that has the ability to change to whatever is there. It does not refer to the controller document that refers to the verification method.
… We have to document that properly.

Joe Andrieu: The previous response I wanted to make was a modest one. The class 4 restrictions are on the DID spec, not on the controller document spec.

Ivan Herman: It's an unusual situation, but I see Manu's point.

Manu Sporny: On the class 4 changes, it is possible for us to make a pretty drastic change to the controller document, but then there's a discussion around if DID Core is dependent on the controller document, is it a class 4 change for DID Core?
… Some would argue that it is. Changing something that drastic at this point in time, even if it's the right thing to do, would cause ecosystem problems.
… To go back to DavidC, I think we open a can of worms if we do it.
… I think the way that "controller" is used right now, is fairly hand-wavy, high-level way.
… It can mean multiple different things. The concern about ambiguity is valid. We could have used more specific terms.
… We don't want to change it at the property level right now as it's a class 4 change.
… It is possible to interpret "controller" in the way that you're interpreting it. There are other interpretations as well.
… We should be specific. When talking about a controller of a controller document, we're talking about an entity that can change it.
… When talking about a controller of a verification method, we're talking about an entity that change update a value in certain case and that they have access to private key material to generate a signature.
… I don't think we have to make them consistent.
… They mean slightly different things in slightly different contexts.

Dave Longley: My first response is to Ivan. I want to say that the controller on a verification method does refer to a controller document that defines who controls the verification method.
… Second, you can't know definitively who the controller is.
… You can follow the controller to determine if you can use the verification method.

Joe Andrieu: +1 to Dave's comment.

David Chadwick: +1 dlongley.

Manu Sporny: +1 to JoeAndrieu.

Joe Andrieu: In this case, we're saying "Go get this controller document so you can determine the validity of the verification method.".

David Chadwick: We are offering the contents of the associated resource.

Dave Longley: +1 i actually do agree with what DavidC just said.

David Chadwick: My change is trying to say that we're saying the same thing about each.

Ivan Herman: I look at the controller document itself and it defines the controller for a verification method. It doesn't say anything about the controller property, just that it is a URL.

Joe Andrieu: that's the new PR.

Dave Longley: +1 agree that it needs to be said explicitly.

Ivan Herman: I haven't found anywhere in the document that specifies that the controller property of the verification method refers to the controller document.

Joe Andrieu: +1 to doing better and being explicit.

Joe Andrieu: (because only part of it is in the new PR).

Ivan Herman: It's a very different kind of definition. I was one of those that raised the possibility of separating those two terms. I accept that it's too late to do that.

Manu Sporny: yes, +1 to that ^.

Ivan Herman: At least the semantic definition should be clear, and it's not.

Manu Sporny: I can try to update the PRs to do what Ivan is asking for, and review what David has written (as long as others agree w/ that update).

Brent Zundel: We agree that we can't make clarifications or normative changes that affect the downstream DID document.
… The feeling I'm getting is that we're closer than not to language that is satisfactory. Last word to Manu.

Manu Sporny: Ivan is right, the spec doesn't have that language.

Manu Sporny: I don't have a strong opinion as long as others can agree on whatever the revised text it.

Copy link
Collaborator

@selfissued selfissued left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since "This process is described in the algorithm for retrieving a verification method.", it's not clear to me that we also need to talk about it here.

I will approve this PR if the Verification Method Binding addition is removed.

@iherman
Copy link
Member

iherman commented Nov 22, 2024

Since "This process is described in the algorithm for retrieving a verification method.", it's not clear to me that we also need to talk about it here.

At this moment, this would be the only descriptive text referring to that algorithm, which is otherwise just listed in §3 without justification. I believe it makes the spec more readable, having a set of more human-readable text alongside a section with subsections such as Verification Method revocation or rotation.

However, as I said elsewhere, in my current understanding that algorithm is very specific to a particular implementation (based on the removal of the URI fragment) so the reference in this section should say "one of the algorithms" and say that, in general, this is business rule dependent. Which would justify further to keep this section.

We also discussed the fact that the controller property in a VM may be used to retrieve the corresponding CD, but that should make it clearer in the spec. This section might be the right place to do so.

@msporny
Copy link
Member Author

msporny commented Nov 23, 2024

@David-Chadwick also note that PR #116 was merged, which might have addressed your concerns.

@iherman @David-Chadwick and @selfissued -- please resolve the differences you have with each other and propose concrete text to move forward.

I have conflicting guidance from each of you and won't be able to start trying to fix this PR without each of you reconciling your differences in opinion with each other. The alternate to each of you doing so is to close this PR and the issue with a failure to come to consensus on how to change this language.

@iherman
Copy link
Member

iherman commented Nov 24, 2024

My point is:

  • add a sentence to the new section noting that the referred algorithm is one of the possible ways to get to the controller document, and that application areas and/or implementations may decide to choose a different approach
  • add a similar note to the algorithmic part emphasizing that manipulating the URL and its fragment part is an approach that VC-s and DID-s may use? usually use? (I do not know which), but this is not the only way to do this.

@David-Chadwick
Copy link
Contributor

@msporny Unfortunately I find it too difficult to determine precisely what text has been merged in PR #116 as there are too many changes to view.
Regardless of this, I accept @selfissued proposal to my change, so accepting this my proposed text then becomes

An entity that is [=authorized=] to perform an action associated with a specific resource such as updating the associated [=controller document=] or updating the associated [=verification method=].

Copy link
Contributor

@jandrieu jandrieu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, @David-Chadwick's suggestions seem to be based on abilities that are not our intention. Specifically, verification method controllers have no mechanism to update the verification method, they only have mechanisms to generate a proof that satisfies the method. Only document controllers can update the verification method.

An entity that is [=authorized=] to perform an action with a specific resource,
such as update a [=controller document=] or use a cryptographic key to generate
a digital signature.
An entity that is [=authorized=] to perform an action associated with a specific
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed. the action is, in fact, limited by the context. For example, the verification method controller can't update the verification method as listed in the CID, but can create proofs that satisfy the method. "Any" action is too broad.

such as update a [=controller document=] or use a cryptographic key to generate
a digital signature.
An entity that is [=authorized=] to perform an action associated with a specific
resource such as updating a [=controller document=] or generating a digital
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This edit continues the error of the previous suggestion. Verification Method controllers CANNOT update the associated verification method. Only CID document controllers can update the method.

@David-Chadwick
Copy link
Contributor

@jandrieu The controller of a verification method (say A) is in control of it, and the full details of this will be specified in the CID for the verification method A (and not in CID B that references it as a verification method). So that controller (A) is in control of the verification method, but not of the CID (B) that references it as a verification method. A cannot update the CID B that references it because A cannot sign any changes to that CID (B). A has to rely on the controller of the CID B to update the details of the verification method that is controlled by A.
In short, there is the ability for the definitions of the verification method in the two CIDs A and B (i.e. the CID that defines the verification method and the CID that references it as a verification method) to become out of sync. I do not think that this nuance is adequately explained anywhere.

@msporny
Copy link
Member Author

msporny commented Dec 18, 2024

We couldn't come to consensus on this PR, @David-Chadwick will raise another one in an attempt to address issue #119.

@msporny msporny closed this Dec 18, 2024
@msporny msporny added the DO NOT MERGE The PR contains a fundamental dispute. Do not merge the PR. label Dec 18, 2024
@msporny msporny reopened this Dec 18, 2024
@iherman
Copy link
Member

iherman commented Dec 18, 2024

The issue was discussed in a meeting on 2024-12-18

  • no resolutions were taken
View the transcript

2.2. Fix definition of controller and add verification method binding section (pr cid#126)

See github pull request cid#126.

Brent Zundel: PR 126, fix the definition of controller and add verification method binding section. There are changes requested. Let's talk about it. If your changes have been addressed please let us know.

Manu Sporny: I think this was just up to Joe and David to find the proper wording, I think we added wording to the specification, David's wording is slightly different, Joe is disagreeing with that new wording, I think that's where we are. If we can't get to consensus I suggest we close PR 126 and resolve that 119 is either done or won't fix based on differing opinions.

Michael Jones: I had two change requests, one was not to take DavidC's wording change, which hasn't been taken, the other was that this PR did two things, fixed the definition of controller, which I support, but it also added a verification method binding section, which is not related to the purpose of the PR and is extra text that was not in the specifications, I will support merging or closing this PR if that section is removed.

David Chadwick: there was a disagreement because the controller of the verification method is in control of the verification method, but putting text to that effect was objected to by Joe because the verification method cannot be updated in that document. There is a possibility to allow these documents to sync.

Manu Sporny: to selfissued, if we remove the thing you want to remove we are left with just a definition, this continued wordsmithing will go on around the definition of controller. I take DavidC's point, we are searching for language everyone can agree to, it might be easiest to use a new PR to do that. I'll note that the text Ivan wanted me to move over would be removed.
… we can close the PR and try again, and maybe DavidC can raise that PR and we can all look at it.

Ivan Herman: if I understood what DavidC was saying, the problem is that we are all influenced by a particular way a verification method and controller document are used within this community where the verification method is part of the controller document. However, these two can be distinct documents linked by a URL. These settings are different, that is one of the reasons there is disagreement, this is also my issue all along.
… If the two are separated they need to be treated as such, that is not really done in the spec.

See github pull request cid#119.

Brent Zundel: At this point, the suggestion before us is to close 126, and if there is still a desire to continue on tweaking language to address issue 119 a new PR must be raised. My suggestion is that after the call we close 126. If, by the time we return in January, there is not a new PR that will lead us to consensus, we will close 119. Is that acceptable to folks?

David Chadwick: I commit to producing a new PR for issue 119.

Manu Sporny: +1 for the proposed plan forward.

Phillip Long: +1.

@jandrieu
Copy link
Contributor

@jandrieu The controller of a verification method (say A) is in control of it, and the full details of this will be specified in the CID for the verification method A (and not in CID B that references it as a verification method). So that controller (A) is in control of the verification method, but not of the CID (B) that references it as a verification method. A cannot update the CID B that references it because A cannot sign any changes to that CID (B). A has to rely on the controller of the CID B to update the details of the verification method that is controlled by A.

That's correct. Because A is not the controller of B, in any manner. So, one would not expect to be able to anything other than rely on the actually controller of B if/when the key should be updated. Just because CID B has a verification method in it that CID B is referring to does not accord any rights to the controller of A. Only B's controllers can do that. Not A's.

In short, there is the ability for the definitions of the verification method in the two CIDs A and B (i.e. the CID that defines the verification method and the CID that references it as a verification method) to become out of sync. I do not think that this nuance is adequately explained anywhere.

This is a feature, not a bug.

There is no expectation that anyone maintain verification methods in sync.

Each CID is a completely independent authority, each updatable according to its DID method. There's no reason you should expect what is in the CID for A to be somehow similar or even related to the CID for B. You can't even count on the verification methods being used for controlling updates to the CID, because some DID methods simply can't reduce their controller value to a single URL.

If the CID for B lists a verification method that happens to be in A's CID, e.g., { authentication: [ "did:ex:B#key1"] } that is perfectly legal.

In fact, whoever is in control of B doesn't even have to know about it.

This separation of concerns is at the heart of the DID model and should be here as well: each identifier gets is own canonical controlled identifier document with zero dependencies on any other CIDs.

If someone uses an external DID that's legal. How you do that, why you might, and how to manage keys across multiple identifiers is outside the scope of the controlled identifier document. The CID is definitive for one and only one ID. That is true for both A's and B's CIDs.

@David-Chadwick
Copy link
Contributor

@jandrieu

This is a feature, not a bug.

But there is a difference between simply having a pointer to a verification method, as in { authentication: [ "did:ex:B#key1"] } and adding to this who the controller of the verification method is (which is what this issue is about).

I am struggling to understand what the purpose of adding the controller to a verification method is, especially since this information could be wrong (because you say it is a feature of DIDs that wrong information can be broadcast).

It seems to me that we should not allow any information to accompany a verification method other than a pointer to where its definitive information can be found, because any other information that is there may be wrong, and therefore misleading to the user of the DID.

@David-Chadwick
Copy link
Contributor

@jandrieu

In fact, whoever is in control of B doesn't even have to know about it.

True, but I think that the controller of B would be pretty annoyed if the controller of A said that B's DID key was being controlled by someone other than B, such as a criminal.

@msporny
Copy link
Member Author

msporny commented Dec 29, 2024

Closing without merging per #119 (comment).

@msporny msporny closed this Dec 29, 2024
@jandrieu
Copy link
Contributor

jandrieu commented Dec 31, 2024

@jandrieu

In fact, whoever is in control of B doesn't even have to know about it.

True, but I think that the controller of B would be pretty annoyed if the controller of A said that B's DID key was being controlled by someone other than B, such as a criminal.

Indeed they would. Which is why it's good that's not what's happening here.

In this case, controller of A is saying that B's key is the controller of one of its Verification Methods. In fact, that becomes a factually true statement: when A uses B's key, it is telling all CID document observers that proofs (of this verification method type) created by B's key are to be taken as proofs created by A. This is within the authority of A as the controller of CID A's

A isn't saying anything about who controls those key. Rather, it is saying that those keys are legitimate controllers for the specified verification method.

Similarly, you state

I am struggling to understand what the purpose of adding the controller to a verification method is, especially since this information could be wrong (because you say it is a feature of DIDs that wrong information can be broadcast).

This I'm more inclined to agree with. As far as I can tell, the controller value in a verification method does not establish a point of fact. It doesn't change the functionality of any algorithms I'm aware of. It's just a weird statement that, as you point out, could be erroneous.

In fact, if the controller property in the CID is not a point of transclusion (include relationships from these other CID documents--we agreed this is not what was meant), then I fail to see how it creates any value other than, for SOME DID METHODS it is used as the sole invocation method for updating the VDR.

FWIW, this feels like a complete duplicate of the capabilityInvocation property and is both confusing and unnecessary.

@David-Chadwick
Copy link
Contributor

@jandrieu Do I take it that we agree that the controller property when applied to a verification method is of little value, since it is not a statement of fact, but is a mere assertion that could be true or false.

What I do not understand now is your statement "for SOME DID METHODS it is used as the sole invocation method for updating the VDR." To what does "it" refer. I understand how "it" could mean the verification method, but not how "it" could mean the controller property of the verification method, given that the latter can be false.

@David-Chadwick
Copy link
Contributor

@jandrieu A new PR #139 has now been created to replace this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO NOT MERGE The PR contains a fundamental dispute. Do not merge the PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants