Closed dhh1128 closed 4 years ago
Looks like the sync_connection
RFC also needs an update to make this explicit.
Agreed. Thank you for the note. I will be updating that RFC anyway, based on some feedback from IIW conversations.
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.
@dhh1128 @kdenhartog looks like the proof
section might be removed from the DID spec: https://github.com/w3c/did-spec/pull/26
@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.
@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.
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.
@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.
@kdenhartog What do you think of Andrew's suggestion?
I would like to hear more about @andrewwhitehead 's idea, particularly the part about including hashes of external data.
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.
I find @andrewwhitehead idea a very valuable improvement and would certainly be in favor.
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
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.
Since all we would be changing about the attachment is adding an optional field, we are making a change while retaining backward compatibility. Therefore I think we could do it without disturbing the accepted status just by raising a pull request
On Fri, Oct 11, 2019, 7:42 AM George Aristy notifications@github.com wrote:
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 https://github.com/TelegramSam as well since he's one of the authors.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/hyperledger/aries-rfcs/issues/243?email_source=notifications&email_token=AAQ3JCFVURYPQT5SOHCUWHTQOB7DNA5CNFSM4I5SC4L2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBABCVQ#issuecomment-541069654, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQ3JCC2JOZ7DHL2E4PWTJTQOB7DNANCNFSM4I5SC4LQ .
I'm in favor of this change. It actually solves the canonicalization problem for signatures generally, and the complexity of signed portions of a message gets simpler.
Sam
On Fri, Oct 11, 2019, 8:44 AM Daniel Hardman notifications@github.com wrote:
Since all we would be changing about the attachment is adding an optional field, we are making a change while retaining backward compatibility. Therefore I think we could do it without disturbing the accepted status just by raising a pull request
On Fri, Oct 11, 2019, 7:42 AM George Aristy notifications@github.com wrote:
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 https://github.com/TelegramSam as well since he's one of the authors.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub < https://github.com/hyperledger/aries-rfcs/issues/243?email_source=notifications&email_token=AAQ3JCFVURYPQT5SOHCUWHTQOB7DNA5CNFSM4I5SC4L2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBABCVQ#issuecomment-541069654 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAQ3JCC2JOZ7DHL2E4PWTJTQOB7DNANCNFSM4I5SC4LQ
.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/hyperledger/aries-rfcs/issues/243?email_source=notifications&email_token=AADESD2UZ6RZ3ZW4HL7S5NTQOB7L3A5CNFSM4I5SC4L2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBABJFA#issuecomment-541070484, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADESDZKKSNVY7IVZULUXH3QOB7L3ANCNFSM4I5SC4LQ .
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.
+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.
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.
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.
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.
Continue this issue here: https://github.com/decentralized-identity/didcomm-messaging/issues/25
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