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

Address did:peer:2 spec changes #64

Open
FabioPinheiro opened this issue Oct 27, 2023 · 8 comments
Open

Address did:peer:2 spec changes #64

FabioPinheiro opened this issue Oct 27, 2023 · 8 comments

Comments

@FabioPinheiro
Copy link
Contributor

FabioPinheiro commented Oct 27, 2023

The goal of this ticket is just to track the adoption of the community for the specs change.

There are some spec changes for did:peer:2... that are breaking changes for DID Comm.
In order to achieve interoperability. It's important to fix libraries that implement did:peer:2 and update all applications that depend on those libraries.

I'm pretty sure this list is far from being complete.
Feel free to add to this list. Also, help us contact the maintainers by creating tickets on the respective libraries.


Context

The changes were merged in this commit a5eca6b
Both issues have possible mitigation solutions that allow a smoother migration.
Although previous implementations are considered faulty! Since DID methods have no version.

Issue 1

The first is to fix the encoding to follow the DID Document.
The DID Document's service example in did:peer looks like

"service":{
  ...
  "serviceEndpoint": "https://example.com/endpoint",
  "routingKeys": [...],
  "accept": [...]
}

But it should instead look like

"service":{
  ...
  "serviceEndpoint": {
    "uri":"https://example.com/endpoint",
    "routingKeys": [...],
    "accept": [...]
  }
}

We should make sure that our did:peer used on the applications are encoded correctly.
when decrypting you look for that key

Issue 2

The second problem is about the id of the keys (kid).
It was unspecified before. So each library generates the kid in its own way.

The major problem is that the kid is used on the DID Comm message itself. More specifically the field skid in the message's Protected Header is the kid of the sender and the recipients.header.kid is also the kid recipient. When decrypting you look for that key.
So the agent encrypting and the agent decrypting MUST have the same or an equivalent resolver (the key id needs to be deterministic).

@FabioPinheiro
Copy link
Contributor Author

https://identity.foundation/peer-did-method-spec/
I was expecting the specs web pages to be updated by the GitHub job.
@dbluhm @swcurran do you know if the job failed or do we still need to do something?

@dbluhm
Copy link
Member

dbluhm commented Oct 27, 2023

https://identity.foundation/peer-did-method-spec/ I was expecting the specs web pages to be updated by the GitHub job. @dbluhm @swcurran do you know if the job failed or do we still need to do something?

It should have been updated... good catch, I'll investigate what went wrong!

@dbluhm
Copy link
Member

dbluhm commented Oct 27, 2023

Thanks for putting this issue together, by the way! This is excellent. We'll work on getting the didcomm-demo updated as well as the Indicio Mediator (no public repo to link to at the moment but feel free to add to your list). We will likely update those two projects in tandem.

@mineme0110
Copy link

mineme0110 commented Dec 4, 2023

For sicpa peer-did-jvm library
I have raised a PR which is an initiative to start the work required for the changes in PeerDID Spec
sicpa-dlab/peer-did-jvm#37
This PR doesn't handle all the PeerDID Spec changes required.

@TheTechmage
Copy link

TheTechmage commented Dec 7, 2023

Looking at the list, there's a library under the uniresolver that is missing. The veramo labs depends on a library that handles the actual parsing of DIDs and here's a PR for the fixes aviarytech/did-peer#3

@FabioPinheiro
Copy link
Contributor Author

Thanks @frostyfrog. So now we can update the version of @aviarytech/[email protected] in veramolabs/peer-did-resolver

@mirceanis
Copy link
Member

@FabioPinheiro @frostyfrog I bumped the versions of the libraries in the uniresolver chain of dependencies.
The last step is merging the changes in teh universal resolver repo:
decentralized-identity/universal-resolver#395

@FabioPinheiro
Copy link
Contributor Author

@FabioPinheiro @frostyfrog I bumped the versions of the libraries in the uniresolver chain of dependencies. The last step is merging the changes in teh universal resolver repo: decentralized-identity/universal-resolver#395

That is great. I'm seeing a lot of fix/PRs for this lately.
Let me try to update the list

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants