decentralized-identity / sidetree

Sidetree Specification and Reference Implementation
https://identity.foundation/sidetree/spec
Apache License 2.0
437 stars 112 forks source link

Create Payload Results in Non Compliant DID Doc #368

Closed OR13 closed 4 years ago

OR13 commented 4 years ago

Per the did core spec: https://www.w3.org/TR/did-core/#public-keys

The value of the id property (if present) MUST be a URI.

This means that #primary is not a URI, while did:example:123#primary is.

This causes problems when attempting to reference a just created public key by id.

For example when integrating with Encrypted Data Vaults client, or doing anything useful with JSON-LD (which assumes id is a URI per the DID Core spec).

What you want to do is:

did:sidetree:123#primary, but this "Fully qualified public key id" will not be present in the DID Document.

This breaks integrations with verificationMethods, that expect the publicKeyId to be resolvable, such as JSON-LD Signatures, Verifiable Credentials and Authorization Capabilities used in Encrypted Data Vaults.

There are a couple solutions:

1. Change the Sidetree Protocol Spec

We could change the type of create payload to be the actual DID Document.

This would mean it would contain the id property and all verificationMethods and public keys would have fully qualified (valid URI) identifiers.

The client would compute the DID Document without the did present, compute the did, add the did to the id and any id properties that started with a # character. The result would be identical to the DID Document that is returned from the resolver.

The sidetree node would process create payloads by reversing the process above and ensuring that the id property matched the hash of the not-fully qualified create payload.

This option would be best, since it would not require any changes to the universal resolver or client code.

2. Add a Fully Qualifying Transformer to the Universal Resolver

Leave Sidetree DID Documents as is. Implement a resolver option for doing the transformation noted above: for and id property that starts with a # (is not a valid URI), prefix it with the DID.

This option might be helpful if other DID Methods have painted themselves into a corner, but its a patch over a DID Core Spec compliance issue, and I don't think its good.

3. Use a client side transform of a DID Document before returning it.

Any client code wishing to use verificationMethods would transform results returned from the universal resolver, we do this for publicKeyHex to publicKeyJwk here, so that kid field in JWS is calculated from publicKeyHex correctly.....:

https://github.com/decentralized-identity/lds-ecdsa-secp256k1-2019.js/blob/master/packages/lds-ecdsa-secp256k1-2019/src/verify.ts#L66

This is the worst option, as it leaves the Method non compliant, and must be copied everywhere a verificationMethod is used (implemented in many languages, etc...).

OR13 commented 4 years ago

A side effect of this, is that (without the solutions above) an update operation is required before anything that uses JSON-LD can be used, which makes sidetree methods less likely to be adopted by companies / governments that require JSON-LD.

OR13 commented 4 years ago

@csuwildcat @peacekeeper I'd love to discuss this in our next ID&D WG meeting.

cboscolo commented 4 years ago

Would it be possible to change the DID-Core spec so that it does not require the id to be a URI?

Otherwise, every DID method that does not know the DID method-specific identifier until after the initial key material and DID Document metadata is hashed will need to have logic to construct each of these specific public key ids.

BTW, I think the answer to your question is that the DID method MUST be the thing that produces the valid DID Document, it can't be by anything downstream (ie. option 2 and 3).

OR13 commented 4 years ago

I think its not a good idea to change an id from being a URI to not being one... for many reasons.

Your comment is accurate though, this might be a problem for non sidetree methods...

It is possible to transform the fragments in the DID Method resolver as well, thanks for pointing that out.

In other words, for sidetree, the create payload could remain the same, but processing the payload might result in a spec complain fully qualified did doc... I think its better that the client do that work and not the server though... sidetree nodes should be as lazy as possible.

Consider also the case where the client signs the did document before submitting it, anything the server does to the create payload breaks this signature.

cboscolo commented 4 years ago

Wow, this discussion also applied to serviceEndpoint doesn't it?

@OR13 wrote:

I think its not a good idea to change an id from being a URI to not being one... for many reasons.

You may want to enumerate them in the discussion that @csuwildcat just refreshed in the DID-Core list. https://github.com/w3c/did-core/issues/15

IMO, having each of those id be a URI instead of a fragment will result in unnecessary DID Document bloat (for solutions that store the actual DID Document) and DID method complexity when returning DID Document, for those methods that must rewrite each of those URIs.

OR13 commented 4 years ago

Thanks, I have a strong opinion, i've shared it :)

msporny commented 4 years ago

This means that #primary is not a URI, while did:example:123#primary is.

Weeeeell, it all depends on what piece of software is resolving the DID Document and what it's doing when resolving a DID Document. For example, if we assume that DID Documents all have a BASE value, which is the DID itself, then having pure fragment identifiers is fine. If we say that a DID Document MUST have IDs that are URIs and the resolution process ensures that any fragment identifiers is expanded to a URI, then everything is fine. If @cboscolo wants to compress any mention of did:... to a single byte when he stores the document and then the value is expanded when the document is retrieved again, everything works.

I think those that are concerned about document size are confusing the storage and presentation layers. How something is serialized on disk (or to a blockchain) and how a resolver resolves it are two totally separate things.

All that to say, and unless I'm missing something, I think people are flipping out for no reason. :)

OR13 commented 4 years ago

Any DID from the universal resolver that returns id properties that are not URIs must be transformed before use with JSON-LD.

We have to do this with element today to make it work with EDVs out of the box, without creating 2 blockchain transactions, instead of 1.

I'm only interested in what we need to do with a DID Document we get from the universal resolver before we can use another interoperable open standard (like edv / vcs)... my preference is that we don't need to do anything.

msporny commented 4 years ago

my preference is that we don't need to do anything.

That's my preference as well, but here's why we can't have nice things:

Even if we were able to say they MUST be fully qualified URIs, some developer somewhere will commit a fragid-only DID to a ledger, and then your software will fail because clearly their DID Document is broken... they'll complain and bring all their friends with pitchforks to your github repo and threaten to not use your software unless you fix the bug... you'll insist that it's not a bug and it's not your fault that they did the wrong thing... and then some other implementer will come along and support relative fragids in their software, which will do the proper transformation, and then people will move away from your software to this other software that does the thing that the developers want, which will break the entire ecosystem until everyone supports relative fragids and we update the spec 10 years later to finally admit that relative fragids SHOULD NOT be used, but software won't break if they are. We can't have nice things because people just want to use relative fragids and will do so even if it's wrong. That is, unless we can convince all of these communities to not add unnecessary complexity to libraries - happy to skip merrily into that conversation with you, it's happened in so many WGs and it almost always results in relative fragids because 1) people don't understand layering, and 2) it eats up precious WG time when there are other higher priority things, and 3) eventually someone throws a chair at someone and puts out an eye (usually yours, even though you're making a valid point).

I suggest we work with DID resolvers and library authors so that any relative fragids are automatically resolved to their fully qualified form. That enables us to say that a valid DID Document MUST contain fully qualified URIs for all ids and it's up to implementers if they want to use fragid only things in intermediate DID documents. We might have to add something to the DID Core spec to specify the concept of a base URL and how relative URLs are resolved in a DID Document and say you SHOULD NOT use relative URLs, but if you do, resolve them against the BASE_URL (which is the DID).

OR13 commented 4 years ago

Speaking as a Sidetree Method implementer, I'd rather we solve this problem in our protocol, and support anchoring of a signed did document / self referencing create payloads, than punt this to resolver / library developers... Even if the did core spec said we didn't have to be that nice :)

gjgd commented 4 years ago

Allowing the id property to be a fragment does not solve the more general problem that it is currently not possible to reference the did in the create payload.

Another example of this problem is that it is not possible for the client to set the controller property, under the current Sidetree protocol rules. See: https://github.com/decentralized-identity/sidetree/blob/004492d34d34da8f8c9a7d5728c61270c5e6c6bb/lib/core/versions/latest/Operation.ts#L146-L147

But what if the client needs to set the controller to a different DID? As far as I understand it is not possible because of that restriction

Forbidding clients from setting the controller of a key and using fragments as ids are not good solutions. We can't set custom rules for every property that needs self referencing.

cboscolo commented 4 years ago

@msporny wrote:

I think those that are concerned about document size are confusing the storage and presentation layers.

My reasoning is not based on an abstract engineering approach of storage vs. presentation, if it were I'd agree with you. It's that for our sidetree approach, the initial DID Document is cryptographically tied to the DID. Imagine a DID did:foo:1234abc, 1234abc is the output of hash(base64(initial-did-document)). These initial DID Documents are then stored via IPFS which also uses1234abc to lookup and retrieve it. This does not allow us to do the compression you suggest and means the DID Document cannot contain references to the DID since it isn't known yet.

msporny commented 4 years ago

It's that for our sidetree approach, the initial DID Document is cryptographically tied to the DID. Imagine a DID did:foo:1234abc, 1234abc is the output of hash(base64(initial-did-document)).

Then do whatever you want to encode the initial-did-document in whatever way you want to and make your DID Method spec define how you transform your encoded form (which has fragment ids and UTF-8 unicorns embedded in it) to/from its representation on IPFS. If storage size is really an issue, bzip2 the file while you're at it. The document stored on IPFS (storage layer) can be an intermediate DID Document, which does not have to be a fully conforming DID Document (presentation layer).

Why does that not solve your problem?

cboscolo commented 4 years ago

@msporny wrote:

Why does that not solve your problem?

It does. But, so does not embedding fully qualified DID URIs in the DID Document, with less code to write. I was hoping to optimize the spec for simplicity at the DID layer since it bootstraps all the cryptography and wasn't as concerned about making it JSON-LD compliant.

csuwildcat commented 4 years ago

I completely agree with Chris - in an effort to support a more complex data format, we're effectively encumbering DID Methods with a set of hoops that otherwise wouldn't exist. I'm not a JSON-LD hater, but in this case, it's hard to swallow the tradeoffs.

On Wed, Jan 8, 2020, 3:42 PM Chris Boscolo notifications@github.com wrote:

@msporny https://github.com/msporny wrote:

Why does that not solve your problem?

It does. But, so does not embedding fully qualified DID URIs in the DID Document, with less code to write. I was hoping to optimize the spec for simplicity at the DID layer since it bootstraps all the cryptography and wasn't as concerned about making it JSON-LD compliant.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/decentralized-identity/sidetree/issues/368?email_source=notifications&email_token=AABAFSVW43PO4CQRJKH4ARLQ4ZQGJA5CNFSM4KEMGVOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIOMRBI#issuecomment-572311685, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABAFSSDGHMWMRQM5V7VLBDQ4ZQGJANCNFSM4KEMGVOA .

msporny commented 4 years ago

@cboscolo wrote:

wasn't as concerned about making it JSON-LD compliant.

@csuwildcat wrote:

I'm not a JSON-LD hater, but in this case, it's hard to swallow the tradeoffs.

This has nothing to do with JSON-LD, it has to do with the identifiers that are used in the specification, which are URIs. Even if we were to purge JSON-LD from the specification, we'd still have to write spec text about this. If we are going to use URIs, we have to define whether or not we're using fully qualified URIs or relative URIs. If we say that relative URIs are allowed, then we have to specify how relative URIs are resolved to fully qualified URIs.

It's always simpler to use fully qualified URIs. It's the notion that folks want to use relative URIs that's increasing spec complexity.

csuwildcat commented 4 years ago

Can we just specify relative URIs then? This would be immensely helpful imo.

On Wed, Jan 8, 2020, 4:48 PM Manu Sporny notifications@github.com wrote:

@cboscolo https://github.com/cboscolo wrote:

wasn't as concerned about making it JSON-LD compliant.

@csuwildcat https://github.com/csuwildcat wrote:

I'm not a JSON-LD hater, but in this case, it's hard to swallow the tradeoffs.

This has nothing to do with JSON-LD, it has to do with the identifiers that are used in the specification, which are URIs. Even if we were to purge JSON-LD from the specification, we'd still have to write spec text about this. If we are going to use URIs, we have to define whether or not we're using fully qualified URIs or relative URIs. If we say that relative URIs are allowed, then we have to specify how relative URIs are resolved to fully qualified URIs.

It's always simpler to use fully qualified URIs. It's the notion that folks want to use relative URIs that's increasing spec complexity.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/decentralized-identity/sidetree/issues/368?email_source=notifications&email_token=AABAFSTOPPY44T4UGB55CN3Q4ZX5FA5CNFSM4KEMGVOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIOQSKY#issuecomment-572328235, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABAFSVXHW76KEZNTUK55RDQ4ZX5FANCNFSM4KEMGVOA .

thehenrytsai commented 4 years ago

As pointed out earlier by @gjgd, ignoring the id field, the DID Document included in the create operation today already is not "DID core spec compliant" because we can't include the yet-to-be-computed DID in the controller field. But this is abstracted/hidden away from resolution/resolver in our implementation because the Sidetree core transforms this "non-compliant" DID Document into a compliant one by inserting user's DID as the controller field back at the time of resolution.

It is merely a decision to limit scope for v1 to not allow user specify the controller, but we can certainly support it if scenarios call for it.

We could certainly follow the same pattern to return full URI instead of fragments for the id property, we simply did not do so because there was a separate thread a year+ ago on this which seemed to conclude at the time that fragments are fine; and that this is obviously the more "concise" thing to do.

OR13 commented 4 years ago

I hope the did spec does define the concept of relative URIs, but I would till prefer to use fully qualified ones :)

These are the kind of things you notice only after you write software that integrates with a couple DID Methods, and you discover that some methods don't support the same set of features (are better than others).

We can of course update the DID Core spec to make fragids valid, but that doesn't actually help downstream developers.

I agree with @thehenrytsai that we could apply the same approach we are taking for controller and id, and have the server mutate the payload to make the document compliant. But I feel that solution is less elegant than supporting immutable spec compliant create payloads.

Why should the server do anything other than check that the did in the create payload is computed correctly?

Why PREVENT sidetree DID Documents from being created with references to their own did?

Why write extra code on the server that mutates user supplied input or extra code on the client that transforms a did doc?

I'm fine allowing the use of fragids for public keys, as long as I can create a did document with fully qualified ones, that works out of the box with schema.org / Bing / Google Knowledge Graph... and pretty much every other knowledge management system that is standards based.

If we're saying that sidetree did documents are special and will always be created looking different than other did docs, we better have some really good reasons why.

AFAIK, the ONLY reason we REQUIRE all id to be fragments in the create payload today is that its easier to compute the didUniqueSuffix.

This restriction unnecessarily deviates from the did core spec, we should fix it.

Henry, can you see any reason why solution 1 would not expand the capabilities of sidetree did documents and solve this problem while continuing to allow people to use fragments if they want to. Do you have an alternative preferred solution?

gjgd commented 4 years ago

An alternative to Solution 1 that @OR13 proposed would be to use a reserved keyword like this or self for referencing the DID.

That way the client would be able to set self referencing properties like controller or id in the create payload

{
  "id": "self",
  "@context": "https://w3id.org/did/v1",
  "publicKey": [
    {
      "id": "self#primary",
      "type": "Secp256k1VerificationKey2018",
      "controller": "self",
      "publicKeyHex": "0264172edfc134f75b87f1f2b0a9509a1f55f4d511ea903452d5f2b4f2ba81f141"
    },
    {
      "id": "self#recovery",
      "type": "Secp256k1VerificationKey2018",
      "controller": "self",
      "publicKeyHex": "036af8ea690847441dfa46d6f8a9905a78c0c3eee6852c4382397eeff4709dbd74"
    }
  ]
}

Then, the sidetree node would be able to just replace that keyword with the value of the did while processing operations at resolve time.

It's very similar approach to Solution 1, but IMO more intuitive because there is an explicit placeholder for the did.

Also, I would note that there is nothing that says that the content of the create payload has to be a spec compliant did document. Only the output of the resolver should be.

cboscolo commented 4 years ago

<small digression> I do not understand why the decision was made to add a controller field to the DID Document in the first place. Or at a minimum why it wasn't optional and only if the controller is not the DID itself. In most uses of DIDs, the controller is the entity that has the means (via cryptography) to control the DID. Setting the controller to the DID seems unnecessarily redundant. But, that is a discussion for the DID-Core list, not here. ... And I've already used up my quota of negative feedback for the week. </small digression>

If we are unable to convince the people working on DID-Core to remove DID references from within the DID Document, then the approach proposed by @gjgd seems like a good solution. But, I would tighten things a bit more in the following way:

  1. Decorate the self placeholder a little to make it stand out. Perhaps %DID%
  2. Make it clear (and enforced in the code) that substitution only takes place for the value of id and controller fields.
OR13 commented 4 years ago

I dislike the idea of self...

There is no reason why sidetree should be inventing something like that if it is not 100% necessary.

Why use self, and do the same transformation, instead of using the actual did?

The client will use software to construct the initial create payload, I don't think they will notice the difference between self and solution 1, but solution 1 makes what they submit, what they get back, while self makes them different... for no good reason as far as I can see.

OR13 commented 4 years ago

For reference, here is our workaround: https://github.com/decentralized-identity/element/issues/137

cboscolo commented 4 years ago

@OR13 wrote:

I dislike the idea of self...

There is no reason why sidetree should be inventing something like that if it is not 100% necessary.

Every bone in my body wants to agree with this sentiment. But, your approach creates unintended reliance on a specific JSON.stringify() output, which I think is a bad idea™. Imagine how most software will work to compute the DID identifier from the doc with the embedded DIDs such as this javascripty-pseudocode:

did_doc_obj = JSON.parse( did_doc_with_embedded_dids )
stripped_did_doc_obj = removeEmbeddedDIDs( did_doc_obj )

var my_did = "did:sidetree:" + hash(btoa(JSON.stringify(stripped_did_doc_obj))) 

In this snippet, if JSON.stringify does not work identically to the JSON.stringify that the holder used to created the initial DID Document, you will have interop problems.

csuwildcat commented 4 years ago

Can someone please reduce this thread to:

1) The specific negative outcomes the current setup produces 2) the specific changes folks are seeking to make, and where. 3) The impact these changes would have on inbound DID op users/wallet code, resolution code in Sidetree, and to resolving entities who receive the modified output

OR13 commented 4 years ago

@cboscolo JSON.stringify is not deterministic, see: https://www.npmjs.com/package/canonicalize

"Creates crypto safe predictable canocalization of JSON as defined by draft-rundgren-json-canonicalization-scheme"

Sidetree Protocol defines how the didUniqueSuffix is created, today it uses JSON.stringify.

https://github.com/decentralized-identity/sidetree/blob/004492d34d34da8f8c9a7d5728c61270c5e6c6bb/lib/core/versions/latest/Operation.ts#L77

https://github.com/decentralized-identity/sidetree/blob/004492d34d34da8f8c9a7d5728c61270c5e6c6bb/lib/core/versions/latest/Operation.ts#L72

https://github.com/decentralized-identity/sidetree/blob/3613b9823f68499a7c15b3d6aec4f7deb796d47a/lib/core/versions/0.4.0/Operation.ts#L136

Because the encodedPayload relies on JSON strings today...

We could specify that the operations should be canonicalized according to https://tools.ietf.org/html/draft-rundgren-json-canonicalization-scheme

This would address your concern, but is not related to the capability of self referencing create payloads, in other words "Does sidetree support creating a did document which references its own DID?"... the answer to that is currently No, but it could be Yes, and I am arguing that it should be yes, because developers will likely want to create self referencing URIs in DID Documents, and they would currently need to rely on fragids, client side transforms or a second sidetree operation in order to get their did document initialized with self referencing URIs.

@csuwildcat

  1. The specific negative outcomes the current setup produces

Sidetree DID Documents cannot support self referencing URIs, like other DID Methods without 2 blockchain operations (create + update).

https://github.com/OR13/ghdid/blob/master/index.jsonld#L19

Note that did:github:OR13#XJZ3sHC-idqckHQCYQSQBlNYcBNxh8FePiD_KenzZ2o is a URI.

#primary does not identify a resource without modification to the DID Core Spec.

  1. the specific changes folks are seeking to make, and where.

Seeking the ability to create a DID Document with correctly formatted URIs in a single create operation without the need to transform data or extend the did core spec.

I should be able to register keys in the create payload, that are resolvable without transformation, for example:

did:sidetree:didUniqueSuffix#primary

  1. The impact these changes would have on inbound DID op users/wallet code, resolution code in Sidetree, and to resolving entities who receive the modified output

Wallets would calculate the create payload according to the description in solution 1.

Resolvers would not need to transform the resulting DID Document in order to use JSON-LD, integrate with things that require proper URIs... such as encrypted data vaults, Linked Data Proofs, schema.org, etc...

cboscolo commented 4 years ago

@OR13 wrote:

Because the encodedPayload relies on JSON strings today...

Uhg! When I first read the spec, I thought hashes were always computed from the "originator of the JSON" by first base64-encoding, then computing a hash. Relying on a specific implementation of JSON-stringification is a bad dependency to pick up, IMO.

csuwildcat commented 4 years ago

Few things:

  1. Chris/Orie: I am almost positive the hash in a Sidetree DID is the hash of the base64 of the initial DID doc, to prevent such parser-dependent issues. (@thehenrytsai, can you confirm this is still true?)

  2. Orie: How are we going to generate initial DID documents with the ID inside the doc, given the ID includes the hash of the base64 of the initial DID document itself? Are you proposing a scheme where we introduce some sort of property removal rule where the ID itself in DID docs is not part of the actual doc when it is hashed?

OR13 commented 4 years ago

2. Orie: How are we going to generate initial DID documents with the ID inside the doc, given the ID includes the hash of the base64 of the initial DID document itself? Are you proposing a scheme where we introduce some sort of property removal rule where the ID itself in DID docs is not part of the actual doc when it is hashed?

The client would first construct the create payload that they use today:

{
  "@context": "https://w3id.org/did/v1",
  "publicKey": [
    {
      "id": "#primary",
      "usage": "signing",
      "publicKeyHex": "0229ca1ac0e354e1c0c69720adbf9ae6ecb941ad44ab426cc873db7b6f18d3adc0",
      "type": "Secp256k1VerificationKey2018"
    },
    {
      "publicKeyHex": "034faa195be2899e06051097d4c0346d20335257fe316a01861b3fff7a52fb0466",
      "type": "Secp256k1VerificationKey2018",
      "id": "#recovery",
      "usage": "recovery"
    },
    {
      "id": "#edv",
      "usage": "signing",
      "type": "Ed25519VerificationKey2018",
      "publicKeyBase58": "FH9NVKGiJNt3AD94qwxwSUjAEJeKZU4FhN8EPaemnppV"
    }
  ],
  "assertionMethod": ["#edv"],
  "capabilityInvocation": ["#edv"],
  "authentication": ["#edv"],
  "capabilityDelegation": ["#edv"],
  "keyAgreement": [
    {
      "type": "X25519KeyAgreementKey2019",
      "publicKeyBase58": "4n6Shuvq7sWeYy6QjJjzRPWkgUXpJAyMefvSQd5rBgZT",
      "id": "#keyAgreement",
      "usage": "signing"
    }
  ]
}

Next, since the client knows what the DID will be, the client would compute the DID.

"id": "did:elem:EiAcNS3PHOIDjWLGpIuno0rRtd_WmmdNGgqT6A8jFL8grQ",

Then the client would add this DID to all the places that it wants to, in order to be spec compliant, such as controller and id resulting in:

{
  "@context": "https://w3id.org/did/v1",
  "id": "did:elem:EiAcNS3PHOIDjWLGpIuno0rRtd_WmmdNGgqT6A8jFL8grQ",
  "publicKey": [
    {
      "id": "did:elem:EiAcNS3PHOIDjWLGpIuno0rRtd_WmmdNGgqT6A8jFL8grQ#primary",
      "controller": "did:elem:EiAcNS3PHOIDjWLGpIuno0rRtd_WmmdNGgqT6A8jFL8grQ",
      "usage": "signing",
      "type": "Secp256k1VerificationKey2018",
      "publicKeyHex": "0229ca1ac0e354e1c0c69720adbf9ae6ecb941ad44ab426cc873db7b6f18d3adc0"
    },
    {
      "id": "did:elem:EiAcNS3PHOIDjWLGpIuno0rRtd_WmmdNGgqT6A8jFL8grQ#recovery",
      "controller": "did:elem:EiAcNS3PHOIDjWLGpIuno0rRtd_WmmdNGgqT6A8jFL8grQ",
      "usage": "recovery",
      "type": "Secp256k1VerificationKey2018",
      "publicKeyHex": "034faa195be2899e06051097d4c0346d20335257fe316a01861b3fff7a52fb0466"
    },
    {
      "id": "did:elem:EiAcNS3PHOIDjWLGpIuno0rRtd_WmmdNGgqT6A8jFL8grQ#edv",
      "controller": "did:elem:EiAcNS3PHOIDjWLGpIuno0rRtd_WmmdNGgqT6A8jFL8grQ",
      "usage": "signing",
      "type": "Ed25519VerificationKey2018",
      "publicKeyBase58": "FH9NVKGiJNt3AD94qwxwSUjAEJeKZU4FhN8EPaemnppV"
    }
  ],
  "assertionMethod": [
    "did:elem:EiAcNS3PHOIDjWLGpIuno0rRtd_WmmdNGgqT6A8jFL8grQ#edv"
  ],
  "capabilityInvocation": [
    "did:elem:EiAcNS3PHOIDjWLGpIuno0rRtd_WmmdNGgqT6A8jFL8grQ#edv"
  ],
  "authentication": [
    "did:elem:EiAcNS3PHOIDjWLGpIuno0rRtd_WmmdNGgqT6A8jFL8grQ#edv"
  ],
  "capabilityDelegation": [
    "did:elem:EiAcNS3PHOIDjWLGpIuno0rRtd_WmmdNGgqT6A8jFL8grQ#edv"
  ],
  "keyAgreement": [
    {
      "id": "did:elem:EiAcNS3PHOIDjWLGpIuno0rRtd_WmmdNGgqT6A8jFL8grQ#keyAgreement",
      "controller": "did:elem:EiAcNS3PHOIDjWLGpIuno0rRtd_WmmdNGgqT6A8jFL8grQ",
      "type": "X25519KeyAgreementKey2019",
      "publicKeyBase58": "4n6Shuvq7sWeYy6QjJjzRPWkgUXpJAyMefvSQd5rBgZT",
      "usage": "signing"
    }
  ]
}

Then the client would submit this payload as the create payload to the server... the initial did document will be 100% equal to the initial create payload.

The server would reverse the process, removing did:elem:EiAcNS3PHOIDjWLGpIuno0rRtd_WmmdNGgqT6A8jFL8grQ and deleting the id and controller properties... then computing the DID and confirming that it matches what is in the create payload.

Now a sidetree user can create a DID Document with self referencing valid URIs without performing a second update operation.

msporny commented 4 years ago

@cboscolo JSON.stringify is not deterministic, see: https://www.npmjs.com/package/canonicalize Relying on a specific implementation of JSON-stringification is a bad dependency to pick up, IMO.

I see that DIF is just now hitting the tip of the canonization ice berg, which is good :) -- you hit it before you got production deployments out there. This was us in 2007, scratching our heads on how we'd address this problem.

That folks building software for this ecosystem are unaware of this issue, the history behind it, and how it impacts DID Documents should be deeply concerning. You will not have stable cryptographic signatures until you figure this stuff out (fragment resolution, canonization, etc.). If you solve it the wrong way, you're going to lock the ecosystem into JSON forever.

Yes, if you're going to try to hash anything, you should canonize it first. Keep in mind that while JCS is good for this (we use it in JSON-LD for JSON Literals), you're going to still blow your leg off if you want to canonize on the information model.

That is, if you use JCS, you're stuck with JSON forever, full stop. You will not be able to use any other syntax, which I know folks don't think is an issue, but it will be in 10 years when people want to use something other than JSON (like CBOR). I mean, it's an issue today because some of us want to use JSON-LD, because we saw the problem you guys are seeing now back in 2007 and worked hard to solve it (JSON-LD + RDF Dataset Normalization).

How fragments work with URI resolvers is something that has been hard won over decades. self is a really terrible idea that will break just about every implementation.

@OR13 is on the right track, but you guys are now forever locked to JSON... no CBOR for you without even more machinations and requirements to convert to/from CBOR to JSON to some other format.

This whole thread makes me really sad :(

cboscolo commented 4 years ago

The web today mostly functions on JW[T|E|S|M] (yes, 'M' https://tools.ietf.org/html/draft-looker-jwm-01) technology which all rely on base64-encoding the JSON prior to applying crypto. Sidetree follows this paradigm. (Or should have until @OR13 pointed out parts of the code that didn't...) But I think that was an oversight that should be fixed.

Yes, this does forever lock things into JSON. For sidetree, that's pretty much a given.

I'd prefer to see the JSON-Core spec updated so DID Documents didn't have self-referencing elements than to use any of the workarounds discussed in this thread.

msporny commented 4 years ago

technology which all rely on base64-encoding the JSON prior to applying crypto.

Yes, but that assumes zero transformations... it's receive only, JCS is new, most of the existing deployments don't use it. Not all JSON implementations stringify the same. There are edge cases with different implementations of JSON libraries, like how big numbers can be:

https://tools.ietf.org/html/draft-rundgren-json-canonicalization-scheme-16#appendix-D

... anyway, hope everyone is considering all of the corner cases.... haven't seen any of this in the JSON-only specs so far. :(

The web today mostly functions on... JWM

:eyes: ... interesting... what's JWM's relationship to DID Comm?

Yes, this does forever lock things into JSON.

Huh, thought people wanted to support multiple syntaxes? Guess that's not true?

OR13 commented 4 years ago

JWM will be the message encoding format of DIDComm likely... we discussed COSE/JOSE, and for reasons of adoption and simplicity settled on JOSE, but JOSE JWT does not support easy encryption for multiple recipients, so in order to be respectful of existing standards, we are working towards JWM which is almost identical to JWT, but not about "tokens"... instead "messages".

See: https://github.com/mattrglobal/jwm

Does JOSE specify JCS canonization? NO, AFAIK... without canonicalization, there will be a lot of fun 👿 to be had with all credential and did document formats that don't define it.

It also comes up when you are dynamically building objects which are stored in IPFS. This was discussed here: https://github.com/decentralized-identity/sidetree/issues/105 ... canonicalization was mentioned.

Its not too late to add canonicalization to sidetree create payloads which are untrusted user input.

Of course in order to do that we would need to define something beyond just using JOSE....

Ages ago I suggested using linked data signature to construct the sidetree operations, I'd rather read JSON-LD than base64 encoded nondeterministically stringified json objects as a developer.

I think that generating a did document, signing it with a linked data proof, and submitting it as a signed create payload is a reasonable expectation that a security conscious developer might have.

They will likely figure out that they can generate 2 different dids / documents that are both signed by the same keys, and look identical by changing the way that the create payload is stringified before it is encoded as base64url....

OR13 commented 4 years ago

Expect to see this file pretty much everywhere in client software until this issue is resolved: https://github.com/decentralized-identity/lds-ecdsa-secp256k1-2019.js/blob/master/packages/lds-ecdsa-secp256k1-2019/src/convertFragmentsToURIs.ts

OR13 commented 4 years ago

Sounds like we will solve this by transforming the document with fragment ids, as the final step using something like the logic above... while this won't let a user create a did document with service endpoints that contain urls that point to the did... it will solve the broken IRI problem... clients that need to construct URLs will probably end up using template strings or translations services, that convert the did document to contain paths that use the did document id.

OR13 commented 4 years ago

This remains a breaking issue for interop, and causes us to tranform sidetree did documents client side before attempting interop....

using: https://github.com/decentralized-identity/element/blob/master/packages/element-lib/src/func/index.js#L143

we should be doing this as the last step of "resolve"... we should never return a did documet with id properties that are not uris...

OR13 commented 4 years ago

@gjgd can you create a ticket in our code base to track fixing this? and prioritize it asap?

OR13 commented 4 years ago

We had this linked already: https://github.com/decentralized-identity/element/issues/137

OR13 commented 4 years ago

consensus on the call today was that we will implement this as the last step before a resolver returns:

https://github.com/decentralized-identity/element/blob/master/packages/element-lib/src/func/index.js#L143

OR13 commented 4 years ago

We've implemented support for this in element by making all the URIs absolute... if sidetree core decided to use @base we will need to upgrade to that when we implement v1.

I'm assigning @thehenrytsai to this, we should decide how we want to specify that v1 DID Documents are core spec compliant...

  1. Make all id valid URIs (absolute)
  2. Define that sidetree DID Documents use @base.

We implemented solution 1, if you choose to implement solution to, we should make sure that solution 2 is recommended in the sidetree spec... since pretty much nobody is using @base today.

isaacJChen commented 4 years ago

512

OR13 commented 4 years ago

goes in end of resolution.

OR13 commented 4 years ago

@csuwildcat to add a note about using @base and then we can close this.