decentralized-identity / peer-did-method-spec

A rich DID method that has no blockchain dependencies. The verifiable data registry is a synchronization protocol between peers.
https://decentralized-identity.github.io/peer-did-method-spec/index.html
Apache License 2.0
27 stars 17 forks source link

fix: corrections and clarifications for did:peer:2 #62

Closed dbluhm closed 8 months ago

dbluhm commented 9 months ago

As the author of the did:peer:4 generation algorithm, I obviously would like to see us all move to that method. However, it seems inevitable that we'll continue to use did:peer:2, at least for a while. To ease interop challenges across did:peer:2 implementations, I propose these clarifications to the did:peer:2 spec.

I believe this should address the concerns raised in #56.

cc @TelegramSam @FabioPinheiro @swcurran @genaris @TimoGlastra

dbluhm commented 9 months ago

A couple of decisions to call out:

The method for generating identifiers for verification methods that I proposed follows the same method used in did:key. This seemed like a fair precedent and least likely to cause confusion for users and implementers of both methods. However, I do note that of the did:peer:2 implementations I'm aware of, no one has used this method yet :sweat_smile: This would require changes to libraries.

I added some clarifications on how multiple services are expressed with did:peer:2. This is done by appending more .Sey... encoded services as opposed to encoding an array into the base64 of a single service element.

I added clarification that services should include ids. But, for backwards compatibility, it makes sense for us to still permit them to be omitted. I added clarification that when multiple services without ids are present in the DID, they should have incrementing integers appended to them to ensure uniqueness.

I added clarification that common string abbreviation should recursively descend through the service object.

I added clarification around what verification method types should be used for a given multibase encoded key, specifically selecting the "2020" variants so we get Ed25519VerificationKey2020 and X25519KeyAgreementKey2020. It likely makes sense to expand the table mapping multicodec prefix to verification method type with additional key types.

I added clarification that when using these verification method types, additional contexts are required in the resolved document.

TimoGlastra commented 9 months ago

The method for generating identifiers for verification methods that I proposed follows the same method used in did:key. This seemed like a fair precedent and least likely to cause confusion for users and implementers of both methods. However, I do note that of the did:peer:2 implementations I'm aware of, no one has used this method yet 😅 This would require changes to libraries.

IN AFJ we use sort of the same method as did:key (but without the z prefix). I think i got this from the spec.

https://github.com/hyperledger/aries-framework-javascript/blob/296955b3a648416ac6b502da05a10001920af222/packages/core/src/modules/dids/methods/peer/peerDidNumAlgo2.ts#L86C27-L86C27

I added clarification around what verification method types should be used for a given multibase encoded key, specifically selecting the "2020" variants so we get Ed25519VerificationKey2020 and X25519KeyAgreementKey2020. It likely makes sense to expand the table mapping multicodec prefix to verification method type with additional key types

This doesn't really matter right? As long as i can resolve the Multibase key to a verification method and can use that, i could use 2018, 2020, or even JsonWebKey2020. You also have MultibaseKey2022 now I think.

I've seen a lot of did:key resolvers that do things slightly different, but they all succeed in extracting the needed the key material.

FabioPinheiro commented 9 months ago

@TimoGlastra the problem is not extracting the key material. Is the name/ref/id to give the key. Since you refer to the key id used to encrypt when sending a message.

I do agree with @swcurran to use a index base name. Do over complicated stuff. I know that no one implemented like that but if I'm going to help change fix libraries I would prefer something simple

Ex: did:peer:2.Ez6LSghwSE437wnDE1pt3X6hVDUQzSjsHzinpX3XFvMjRAm7y.Vz6Mkhh1e5CEYYq6JBUcTZ6Cp2ranCWRrv7Yax3Le4N59R6dd.SeyJ0IjoiZG0iLCJzIjoiaHR0cHM6Ly9hbGljZS5kaWQuZm1ncC5hcHAvIiwiciI6W10sImEiOlsiZGlkY29tbS92MiJdfQ Ez6LSghwSE437wnDE1pt3X6hVDUQzSjsHzinpX3XFvMjRAm7y will be #key-1 whatever that be Vz6Mkhh1e5CEYYq6JBUcTZ6Cp2ranCWRrv7Yax3Le4N59R6dd will be #key-2 whatever that be

dbluhm commented 9 months ago

I don't have strong feelings about the IDs; however, I agree that just indexing them based on order they appear in the DID seems like the simplest and least "arbitrary" scheme. @TimoGlastra what do you think of going with #key-N style?

This doesn't really matter right? As long as i can resolve the Multibase key to a verification method and can use that, i could use 2018, 2020, or even JsonWebKey2020. You also have MultibaseKey2022 now I think.

I think this is true. How your did:peer:2 resolver decides to decode the document influences internal handling more than it does anything external. I'm okay with relaxing these constraints. I believe the real critical piece is consistent identifiers for the resources in the document and being able to reliably handle references to those resources.

TelegramSam commented 9 months ago

I like this.

swcurran commented 9 months ago

Looks good to me. Really want to get to did:peer:4, so am a little worried about the comment above that the libraries that do did:peer:2 as documented here, but if others think that is reasonable, I won’t argue.

That said, the sooner we deprecate numalgos 0, 1, 2 and 3, the better.

FabioPinheiro commented 9 months ago

I like that key for numalgos 2 gets define in the specs. My only concern is that most implementations that I have seen is using the string that encode the key as the key id. Exdid:peer:2.Ez<abc>.Vz<xyz> will have the key ids #<abc> and #<xyz>.

I would prefer index-based. For being more easy. But this change will be a breaking change for probably ALL libraries. Also will be hard on the applications that depend on those (especially during the change, since not everyone going to upgrade applications at the same time). So I am concerned how long will we take to adopt and make everyone interoperable again.

dbluhm commented 9 months ago

@FabioPinheiro literally every implementation I'm aware of has done it differently, unfortunately. peerdid-python uses characters 1-9 of the multibase/multicodec encoded key. AFJ is using multibase minus the z. Agents tested in the didcomm v2 mediator test suite use the full multibase, as you've described. The didcomm-demo follows peerdid-python (mostly since interacting with a mediator using that library ended up being first integration target).

So I think no matter how we shake it, there's going to need to be changes in one library or another. I'm not sure how best to address that besides continuing to communicate about this interop challenge (which, in my view, is still less problematic after these changes than it was before) and implementing the changes in well known locations like the didcomm v2 mediator test suite, didcomm demo, ACA-Py, AFJ, etc. If we hit those well known projects, I think we should eventually get a majority of the ecosystem by osmosis.

Still, better yet, if implementations don't want to mess around with fixing did:peer:2, we can encourage everyone to push to did:peer:4. I think this is also an acceptable outcome and would consider these clarifications to be more for posterity's sake rather than the real solution to the interop challenges presented by an ambiguous did:peer:2 spec.

dbluhm commented 9 months ago

This doesn't really matter right? As long as i can resolve the Multibase key to a verification method and can use that, i could use 2018, 2020, or even JsonWebKey2020. You also have MultibaseKey2022 now I think.

I've seen a lot of did:key resolvers that do things slightly different, but they all succeed in extracting the needed the key material.

@TimoGlastra I've added a note conceding that exactly how you choose to "resolve" the DID doesn't matter too much as long as it's only used internally and the id is consistent. I think that's the only critical piece of the puzzle here.

dbluhm commented 9 months ago

I've implemented a minimal did:peer:2 library in python demonstrating these fixes: https://github.com/dbluhm/did-peer-2

dbluhm commented 9 months ago

I took a deeper look at what is intended by publicKeyMultibase in the DID Core spec. As written, publicKeyMultibase is simply a multibase encoding of the public key material WITHOUT the multicodec prefix that we're used to seeing in did:key and the serialized representation of the key in did:peer:2. This point has been terribly confused in more than one location (including draft specs at W3C). In order to align with the DID Core spec, if we were to use verification method type Ed25519VerificationKey2020 and X25519VerificationKey2020, we would actually need to strip the multicodec prefix from the serialized key representation and re-encode the key with base58 and prepend with z. In other words, we should no longer see z6Mk on ed25519 keys and just see z followed by random characters.

Given this, I think I will adjust the spec further to represent verification methods using Multikey which will simplify things in two ways. First, there's no need to vary the context depending on the presence of keys of different types. And second, there's no need to transform the key from it's serialized representation to remove the multicodec prefix.