w3c-ccg / did-method-key

DID (Decentralized Identifier) did:key method for embedding keys in DID urls
https://w3c-ccg.github.io/did-method-key
Other
18 stars 12 forks source link

Support for did param "signed-ietf-json-patch" #17

Closed OR13 closed 2 years ago

OR13 commented 4 years ago

did:key did document representations are currently immutable, and can not be used for testing various cases where services / other keys are needed. This means did:key does not work with DIDComm, and did key cannot be bi-directionally linked to a domain with well known did configuration.

propose we add support for signed-ietf-json-patch:

https://github.com/decentralized-identity/did-spec-extensions/blob/master/parameters/signed-ietf-json-patch.md

This would allow did key to be used with didcomm.

because it would allow did key resolvers to be configured to return did documents that contained service endpoints.

tmarkovski commented 3 years ago

Bumping this, as we're considering adding support for this as tracked by https://github.com/decentralized-identity/did-key.rs/issues/8

Primary motivation is to allow communicating service related data in offline use cases, like BLE addresses and parameters using DIDComm, but not limited to that transport.

What should the spec define in terms of implementers guidance? Is the patch allowed to remove a verification method? Add other keys? Should it be restricted to managing service related data only, considering the keys are immutable?

OR13 commented 3 years ago

We are also considering support for this, as tracked by https://github.com/transmute-industries/did-key.js/issues/72

Similar use case, we want to be able to send a did url that contains a did-key + a service.

Right now, this is a major reason to use did:web, or other did methods over did:key (lack of service support is a major missing feature)

OR13 commented 3 years ago

ping @dlongley @dmitrizagidulin @tplooker

OR13 commented 3 years ago

did key remains incompatible with didcomm, use did:web instead of did key.

tmarkovski commented 2 years ago

@OR13 Can you please comment a little on the incompatibility with didcomm that was discovered?

OR13 commented 2 years ago

@tmarkovski AFAIK, didcomm requires the service endpoint definition in the did document... did key does not support service.

tmarkovski commented 2 years ago

Thanks, I was wondering if there's a different issue. This extension aims to solve that problem. We'll be taking a first pass at adding support for this in the rust implementation.

OR13 commented 2 years ago

it won't work... see https://github.com/OR13/did-params-and-you

Specifically the credentials would be bound to a different identifier.

https://github.com/OR13/did-params-and-you/blob/master/examples/case-2-illegal-working.json

the issuer.id needs to match the didDocument.id... which is forbidden from being a DID URL.

tmarkovski commented 2 years ago

What about use case outside credentials? DIDComm would utilize this mostly for communicating service endpoint to complete key exchange. Issuance of credentials doesn't need to happen using a patched DID Document, if someone chooses to use the method for that purpose (which should be discouraged anyway).

dlongley commented 2 years ago

I think the right solution to this problem is for DIDComm to support other mechanisms for specifying service endpoints, such as using VCs -- that can have (potentially very short) expiration periods on them. Expiring this sort of information is important so that there's no confusion over values that are no longer valid. It's also important that no new "patch" or additional update is required in order to know that something has become invalid. This sort of information may be omitted to create vulnerabilities -- so the same piece of information that is used to declare the presence of a service endpoint should also indicate when that information expires.

Otherwise, IMO, you need some kind of VDR to make this sort of thing work safely -- and did key is specifically designed to work without one.

tmarkovski commented 2 years ago

I agree, DIDComm is limited in how service information can be communicated, and seem to have adopted the did:peer way of doing this through message exchange. I should be able to specify did:example:alice#service-3 to my negotiating party, when I have a list of multiple services in my DID Doc. Dereferencing can play a bigger role here.

On an unrelated note, any objections to simplifying signed-ietf-json-patch parameter to just patch and updating the extension? The reason for this is mostly for use with QR codes which favor shorter character sequences. If there aren't any strong objections, I can make a PR and continue the discussion there.

OR13 commented 2 years ago

I think DIDComm becomes nearly useless if it only supports 1 DID Method... But that does not mean that it will support all DID Methods... I think that DIDComm will never support DID Key.

the place to make the change to did extensions is here: https://w3c.github.io/did-spec-registries/#signedIetfJsonPatch-param PRs are always welcome :)

tmarkovski commented 2 years ago

DIDComm should support any DIDs within the scope of the DID Core spec, or risk becoming DIDPeerComm. DID Key does have a service entry through the use of this patch, and that does not change the controller of the DID, since the output from the dereferencing of the DID URI is going to be a DID Document with service entry, i.e. controller of did:example:alice#service-1 is still did:example:alice.

I understand this DIDComm issue is more political and not technical at this point.

OR13 commented 2 years ago

DID Key does have a service entry through the use of this patch, and that does not change the controller of the DID, since the output from the dereferencing of the DID URI is going to be a DID Document with service entry, i.e. controller of did:example:alice#service-1 is still did:example:alice.

I answered this question, above here: https://github.com/w3c-ccg/did-method-key/issues/17#issuecomment-1026122838

Please don't assert that "did:key" supports DIDComm or "ietf-json-patch"... I don't think that is true or will become true without changes to the "did:key" specification.

You are welcome to open a PR here, to try and make it true, but be careful saying it's true without that PR having been merged.

OR13 commented 2 years ago

The reason I opened this issue originally was to open the door to such a PR, but as I noted in the examples above.

I have become convinced that is not supported today, and will not work in the future.

Perhaps we should close this issue with the note that "did:key" does not support "signed-ietf-json-patch".

To make it clearer for future readers.

dlongley commented 2 years ago

+1 to close and not support signed-ietf-json-patch. I don't think it solves use cases safely.

OR13 commented 2 years ago

Until further notice, did:key will not support signed-ietf-json-patch. If you don't agree, please open a PR that addresses adding support for it to the did key method spec.

tmarkovski commented 2 years ago

I think it's premature to close this based on the current compatibility with DIDComm. Verbiage and extension descriptions on how to handle URL parameters can be clarified both in DID Core and in the extension itself. I still think there's value in supporting this for did:key.

OR13 commented 2 years ago

@tmarkovski I can reopen and assign you if you like?

Next step would be a PR on this repo to describe how to support the feature, and you would have the ball for that PR.

tmarkovski commented 2 years ago

I'm working on one, will just submit a PR when ready. I think I gathered all the issues/discussions related to this over the past couple of years and will address them in the PR.