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

RFC 0023: need to send DID Doc as bytes, not as data structure #243

Closed
dhh1128 opened this issue Oct 4, 2019 · 22 comments
Closed

RFC 0023: need to send DID Doc as bytes, not as data structure #243

dhh1128 opened this issue Oct 4, 2019 · 22 comments
Labels
DIDComm Related enhancement New feature or request

Comments

@dhh1128
Copy link
Contributor

dhh1128 commented Oct 4, 2019

The DID exchange protocol (and 0160 Connection Protocol) say that as part of exchanging DIDs, the parties should send one another their DID docs. This is done by populating a JSON object called "DIDDoc" (0160) or "did_doc" (0023). All existing impls assume that the value of this JSON key is a JSON object which is the DID Doc.

If you are using a public DID, this is not necessary; you can just send your public DID, and the other party can resolve it to get the DID doc.

If you are using peer DIDs, this mechanism needs to be tweaked slightly, because peer DID doc integrity is checked by computing a hash of the raw bytes of the doc, NOT by testing that the data structure built from JSON matches a fingerprint. By doing it this way, peer DIDs avoids the entire problem of JSON canonicalization--but it means that if a space or an indent or order of keys is changed between the peer DID doc of the sender, and the peer DID doc as reconstituted by the receiver during DID exchange, the sender and receiver won't agree on state.

I suggest that we change the protocols so they use the attachment mechanism instead. The bytes of the DID doc would then be streamed with base64 encoding, instead of just the JSON structure of the DID doc. The name of the key in the JSON of the message would become "did_doc~attach".

This is actually a positive change for several reasons. We couldn't do it when we wrote the connection protocol, because attachments were not yet defined. But now that we have attachments, attachments make DID doc content transmission more standard instead of one-off, and they increase the flexibility of how DID doc content is fetched (e.g., we could attach by reference with a hash, in addition to attaching inline). And perhaps most importantly, it is probably wrong to assume that we can transmit a DID Doc as JSON without breaking a DID method's versioning strategy; I think the BTCR and IPFS methods may have similar constraints to the one from did:peer that triggered this ticket.

^ @andrewwhitehead @TelegramSam @swcurran

@llorllale
Copy link
Contributor

Looks like the sync_connection RFC also needs an update to make this explicit.

@dhh1128
Copy link
Contributor Author

dhh1128 commented Oct 4, 2019

Agreed. Thank you for the note. I will be updating that RFC anyway, based on some feedback from IIW conversations.

@kdenhartog
Copy link
Contributor

This feels like a much better approach then Base64url encoding everything. It means we get lighter weight responses and less time is spent encoding and decoding. I'm heavily in favor of this.

@llorllale
Copy link
Contributor

llorllale commented Oct 8, 2019

@dhh1128 @kdenhartog looks like the proof section might be removed from the DID spec: w3c/did-core#26

@llorllale
Copy link
Contributor

llorllale commented Oct 10, 2019

@kdenhartog on the 2019-09-25-B call at around the 1:01:14 mark (recording) you were proposing to merge the did-exchange RFC with RFC0066 and RFC0019. Did I interpret your thoughts correctly? Have you changed your mind?

Merging this RFC with 0066 would mean we would not implement the proposal presented in this issue.

@llorllale
Copy link
Contributor

@dhh1128 on another note:

as part of exchanging DIDs, the parties should send one another their DID docs. This is done by populating a JSON object called "DIDDoc" (0160) or "did_doc" (0023). All existing impls assume that the value of this JSON key is a JSON object which is the DID Doc.

The did-exchange response message is actually using the signature decorator. The request message could be changed to do the same. The signature decorator already does something similar to what you're proposing (and adds non-repudiation on top), but probably needs a little tweaking.

@andrewwhitehead
Copy link
Member

andrewwhitehead commented Oct 10, 2019

Would it make sense to combine the signature decorator semantics into the attachment decorator (optionally depending on the context)? This would allow signing of non-JSON-formatted data. It could also include something like an MD5/SHA hash of external data.

@dhh1128
Copy link
Contributor Author

dhh1128 commented Oct 10, 2019

@andrewwhitehead That's brilliant, IMO! It makes the ~sig a lot easier to explain to people who are coming at this from a perspective where they expect the whole message to be signed.

@dhh1128
Copy link
Contributor Author

dhh1128 commented Oct 10, 2019

@kdenhartog What do you think of Andrew's suggestion?

@llorllale
Copy link
Contributor

I would like to hear more about @andrewwhitehead 's idea, particularly the part about including hashes of external data.

@dhh1128
Copy link
Contributor Author

dhh1128 commented Oct 10, 2019

Attachments already define how to reference external data with a hash. See https://github.com/hyperledger/aries-rfcs/tree/master/concepts/0017-attachments#links. All we would have to do is extend ~attach to have an optional sig property. The ~sig decorator would go away, and we'd say that any time you want to sign something smaller than a full message, you attach it.

@kdenhartog kdenhartog added the enhancement New feature or request label Oct 10, 2019
@kdenhartog
Copy link
Contributor

I find @andrewwhitehead idea a very valuable improvement and would certainly be in favor.

@andrewwhitehead
Copy link
Member

andrewwhitehead commented Oct 11, 2019

I think the combined decorator would look something like this in the DID-exchange protocol?

{
  "@type": "did:sov:BzCbsNYhMrjHiqZDTUASHg;spec/didexchange/1.0/response",
  "@id": "12345678900987654321",
  "~thread": {
    "thid": "<The Thread ID is the Message ID (@id) of the first message in the thread>"
  },
  "connection~attach": {
    "data": {
      "base64": "<base64URL(connection_attribute)>"
    },
    "sig": {
      "type": "ed25519Sha512_single",
      "ts": "<decimal_unix_epoch_timestamp>",
      "signature": "<base64URL(signature(64bit_big_endian_ts || connection_attribute))>",
      "signers": "<signing_verkey>"
    }
  }
}

Note the semantics might be slightly different – at the moment we assume the contents of a ~sig decorator are JSON, and thus can be automatically inlined during message deserialization, while that's not necessarily true for an ~attach decorator.

@sklump Since he's very familiar with the ~attach decorator at the moment

@llorllale
Copy link
Contributor

I agree this sounds like a good idea. What is needed to make this change to the attachments RFC? It's currently in ACCEPTED status.

Pinging @TelegramSam as well since he's one of the authors.

@dhh1128
Copy link
Contributor Author

dhh1128 commented Oct 11, 2019 via email

@TelegramSam
Copy link
Contributor

TelegramSam commented Oct 11, 2019 via email

@sklump
Copy link
Contributor

sklump commented Oct 15, 2019

One tiny question here: the Aries-RFC trend in base64 dialect choice seems to me to be toward base64url rather than base64? The original attachment decorator RFC cites base64 and never base64url, for example.

Is this a conscious choice? I would personally find the Aries-RFC series more pleasing if all RFCs used the same flavour of base64. Plus, the code base, support surface, and API apertures could be that much smaller.

@tmarkovski
Copy link
Contributor

+1 to @sklump's note. We discovered during the credential exchange protocol that not aligning on base64 vs base64URL is a huge point of incompatibility.

@dhh1128
Copy link
Contributor Author

dhh1128 commented Oct 15, 2019

I don't mind coalescing on 1 dialect, but I'd just like to clarify why 2 dialects exist: In the connection protocol, the invitation needs to use base64url encoding because it's going to be embedded in a URL. All other base64 encodings that I know of are inside a JSON message, where encoding for a URL is fine from a consistency POV, but irrelevant from a functional standpoint. So converging on base64url would be driven by one anomaly.

As I said, that's not an argument against coalescing; it's just an explanation of why having 2 wasn't caused by simple sloppiness.

If we do coalesce on a single encoding, we should probably update the DIDComm best practices RFC (74?) with a comment about it.

@dhh1128
Copy link
Contributor Author

dhh1128 commented Oct 23, 2019

Just a note that PR #267 updates the Attachments RFC to use the ~sig decorator, thus allowing a DID doc to be sent as a signed attachment. @TelegramSam is going to update the DID Exchange protocol to account for this.

@kdenhartog
Copy link
Contributor

Discussed on the call today:

It was agreed on the call that we will make the updates discussed in this issue. When a PR is opened, I'll close this issue.

@TelegramSam
Copy link
Contributor

Continue this issue here: decentralized-identity/didcomm-messaging#25

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DIDComm Related enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants