-
Notifications
You must be signed in to change notification settings - Fork 73
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
FCP HIPE: Wire Messages (formerly AMES) #43
Conversation
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
Signed-off-by: Stephen Curran <swcurran@gmail.com>
Signed-off-by: Stephen Curran <swcurran@gmail.com>
Signed-off-by: Stephen Curran <swcurran@gmail.com>
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
AMES/README.md
Outdated
|
||
## Serialization Examples | ||
|
||
### JSON Serialization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this an example of Authcrypt, or Anoncrypt, or both? How do they differ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Authcrypt and Anoncrypt only differ 1 but significant way. You're right this should be documented somewhere. I briefly talk about them in this HIPE
Anoncrypt generates an ephemeral key pair, uses the private key with the receivers relationship public key to perform Elliptic Curve Integration Encryption Scheme (ECIES). This means the receiver will not have a cryptographic method to verify who sent it. This offers forward secrecy for the sender. This is used when the sender's public key is not known by the receiver yet but the receivers key is or can be known.
Authcrypt instead of using an ephemeral key pair, uses the senders relationship key pair with the receivers relationship key pair. No forward secrecy is offered. The receiver has a cryptographic method to verify the sender.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm splitting the API for this now. I do mention this in the API aspects below, but to make the API cleaner and to stay consistent with the way we anon/authcrypt now I'm planning to separate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I understand AuthCrypt and AnonCrypt. I was looking for how Kyle was demonstrating which.
AMES/README.md
Outdated
``` | ||
|
||
### Compact Serialization | ||
` <header_json> . <content_encryption_key> . <iv> . <ciphertext> . <tag> ` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we adjust the header format to include a list of recipients. That way, multi-recipient messages can be also presented in compact form.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consolidating to a single serialization format would make the code much cleaner. Both are implemented right now. The format outputted is dependent on the number of keys passed into pack_message at the moment.
AMES/README.md
Outdated
|
||
AMES: This should pass in a json string that follows one of the two serializations provided above. | ||
|
||
my_vk: This should be the verkey that you wish to use to decrypt the message. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the message itself contain a clue as to which verkey is needed to decrypt the message? This would be useful (if not a reason against it), and we should either modify the unpack message to not need this argument, or provide a method that would inspect the message and inform about expected keys prior to calling unpack.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been under the assumption that this was to be handled by a different layer. If there is a 1 -> 1 mapping of DID -> Key owned by agent it could be removed, but currently there's no way to do this because wallet lookups are dependent on verkeys rather than DIDs.
AMES/README.md
Outdated
output: A decrypted message | ||
|
||
### pack_message(plaintext, auth, recv_keys, wallet_handle, my_vk) -> JSON String: | ||
The purpose of this function is to take in a message, encrypt the message for the keys specified and output a JSON string with one of the two serializations identified above. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a compact
boolean argument to explicitly indicate compact or full encoding is more useful, rather than always going compact for one form and full for the other. If an invalid option is requested, return an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think consolidating to one format only would be a better approach rather than adding another boolean on to determine format. I do agree, that the number of keys determining the serialization format is confusing, but it's what I came up with as I was coding it. Definitely worth a refactor.
AMES/README.md
Outdated
|
||
recv_keys: This is a list of verkeys passed as a string. If only 1 key is passed in, then a Compact serialization will be outputed. If > 1 keys are passed in then JSON serialization will be outputted from this function. | ||
|
||
wallet_handle: this is the wallet_handle that contains the key used to encrypt the message (when using authcrypt). It is required even if using anoncrypt. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are both the wallet and the verkey needed here for authcrypt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the function knows which wallet to find the private key in. It's not necessary when anoncrypting because an ephemeral key is being generated on the fly.
AMES/README.md
Outdated
"recipients" : [ | ||
{ | ||
"header" : { | ||
"typ" : "x-b64nacl" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
x-b64nacl
was intended as a temporary typ
value until we defined this format. We should replace this with ames
or something to designate this format. Same for alg
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with changing typ. I'm not sure about changing alg. These are intended to show how the cek is being encrypted. I suspect when I do the refactors to encrypt headers (in order to protect to/from keys) then this information will need to be changed as well.
AMES/README.md
Outdated
### remove_route(did_with_key_frag, wallet_handle): | ||
This function removes a route (did#key, endpoint) from the routing table for an endpoint based upon the provided parameter. It's expected that this function will not return any data upon completion. | ||
|
||
### update_route(DID#key, endpoint, wallet_handle): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method feels very redundant with add_route. One method that serves as an upsert feels like the right answer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I preferred separating them to make the code cleaner under the hood. The wallet is handling these two actions as separate as well, so I wanted to continue this pattern. Thanks, for coming in and taking a look at this. There's some good ideas in the suggested refactors.
Here is the rendered text |
So how is the entire message protected? I didn't see any method to protect the confidentiality and integrity of the headers? Is this done with authcrypt or anon crypt or using the Signal protocol? |
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
@mikelodder7 this is a refactor I haven't gone back in and implemented. I expect this to be handled by the function itself similar to how authcrypt handles all of this as well under the hood. This will need to be implemented before it's accepted. |
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor fixes.
text/AMES/README.md
Outdated
"alg": "Authcrypt", | ||
"recipients": [ | ||
{ | ||
"encrypted_key": anoncrypt(encrypted_cek|sender_vk|nonce) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be anoncrypt(cek|sender_vk|nonce)
? the cek
isn't yet encrypted (which is the point of this line).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed this to say anoncrypt(authcrypted_cek|sender_vk|nonce)
. This is implemented this way because this is using crypto_box
with a combo_box
are both consumed to implement 'authenticated_encrypt' in the libindy/services/crypto/mod.rs file.
text/AMES/README.md
Outdated
], | ||
})" | ||
"iv": <b64URLencode()>, | ||
"ciphertext": <b64URLencode(encrypt({'@type'...}, sym_key)>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why use sym_key
here, when elsewhere it is called cek
?
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
A message pack dependency crept in from building off the authcrypt code in libindy. Please hold off on approving this HIPE until I've found a way to remove this dependency while remaining in alignment with the libindy work. |
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
While this shouldn't hold up moving this HIPE forward (and definitely not the code!), I think there should be more precise examples of the pack() and unpack() interface. What happens in between is important to the code implementers (and is well covered in the HIPE). However, for the users of the capabilities - they really just want to know what they have to pass in to each function, and what is going to be returned. On that topic - a question I have is about unpack - does it just return the message (like the example in "Code for this might look like the following:" shows), or does it return other necessary information that might be needed routing - notably the verkey? BTW - that example is wrong for the received message - the "tmsg" and "jwe" are both used for the same data element., Related question - does the fact that the verkey of the receiver is in the header mean that the "forward" message type is no longer needed? The receiver of a Wire Message could always just get back the verkey of any embedded JWE, and from that, infer (based on routing tables) where the message is to go next. It seems like "Forward" is redundant now that the information is in the JWE. |
I can add this easily. It's documented in the APIs code, but I'll move it to the HIPE as well, so it's easier for others to implement if they wanted to without looking at the code. Thanks for the suggestion.
I'll message you to get the details about the specific example if I can't find it. With regards to the return value, it will return a JSON object that includes a verkey if it's authcrypted. I'll include this in the details that I provide from above.
I do believe that Forward could be redundant at this point because the verkeys are listed in the recipients header and are publicly viewable. The one advantage that a forward message would have over the verkey's would be the use of DIDs, where as with the wire message we chose not to use DIDs to prevent DID resolution issues from popping up. Instead we think it would be better to build some code on top later that will consume pack/unpack and will handle DID resolution seperately. If done right, forward messages could be removed. Given that the wallet doesn't have a built in verkey -> DID solution right now though, I think it's fine to support forwards without causing future breaking changes. |
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
text/AMES/README.md
Outdated
|
||
```json | ||
{ | ||
"message":"Hello World", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe an agent message would be a better example here than a string. Even if abbreviated like {"@type":"" ...}
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
# Wire Messages | ||
[summary]: #summary | ||
|
||
There are two layers of messages that combine to enable **interoperable** self-sovereign identity Agent-to-Agent communication. At the highest level are Agent Messages - messages sent between Identities (sender, recievers) to accomplish some shared goal. For example, establishing a connection between identities, issuing a Verifiable Credential from an Issuer to a Holder or even the simple delivery of a text Instant Message from one person to another. Agent Messages are delivered via the second, lower layer of messaging - Wire. A Wire Message is a wrapper (envelope) around an Agent Message to permit sending the message from one Agent directly to another Agent. An Agent Message going from its Sender to its Receiver may be passed through a number of Agents, and a Wire Message is used for each hop of the journey. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to link here to HIPE 0026-agent-file-format. Note that that HIPE uses the term "Agent Plaintext Message" and "Agent Wire Message"; it does not use "Agent" for one and "Wire" as the contrasting word. We could adjust the text here by saying "At the highest level are agent plaintext messages--messages sent...". Then later we could say, "Agent plaintext messages are delivered via the second..." and "An agent plaintext message going from its sender to its receiver may be passed through a number of agents, and a wire message envelope is used for each hop of the journey."
Signed-off-by: Kyle Den Hartog <kdenhar@gmail.com>
Signed-off-by: Daniel Hardman <daniel.hardman@gmail.com>
Explain for non-Indy audience
HIPE has been at FCP for quite a while. Implementation was merged into Indy SDK today. |
```json | ||
{ | ||
"protected": "eyJlbmMiOiJ4Y2hhY2hhMjBwb2x5MTMwNV9pZXRmIiwidHlwIjoiSldNLzEuMCIsImFsZyI6IkF1dGhjcnlwdCIsInJlY2lwaWVudHMiOlt7ImVuY3J5cHRlZF9rZXkiOiJMNVhEaEgxNVBtX3ZIeFNlcmFZOGVPVEc2UmZjRTJOUTNFVGVWQy03RWlEWnl6cFJKZDhGVzBhNnFlNEpmdUF6IiwiaGVhZGVyIjp7ImtpZCI6IkdKMVN6b1d6YXZRWWZOTDlYa2FKZHJRZWpmenRONFhxZHNpVjRjdDNMWEtMIiwiaXYiOiJhOEltaW5zdFhIaTU0X0otSmU1SVdsT2NOZ1N3RDlUQiIsInNlbmRlciI6ImZ0aW13aWlZUkc3clJRYlhnSjEzQzVhVEVRSXJzV0RJX2JzeERxaVdiVGxWU0tQbXc2NDE4dnozSG1NbGVsTThBdVNpS2xhTENtUkRJNHNERlNnWkljQVZYbzEzNFY4bzhsRm9WMUJkREk3ZmRLT1p6ckticUNpeEtKaz0ifX0seyJlbmNyeXB0ZWRfa2V5IjoiZUFNaUQ2R0RtT3R6UkVoSS1UVjA1X1JoaXBweThqd09BdTVELTJJZFZPSmdJOC1ON1FOU3VsWXlDb1dpRTE2WSIsImhlYWRlciI6eyJraWQiOiJIS1RBaVlNOGNFMmtLQzlLYU5NWkxZajRHUzh1V0NZTUJ4UDJpMVk5Mnp1bSIsIml2IjoiRDR0TnRIZDJyczY1RUdfQTRHQi1vMC05QmdMeERNZkgiLCJzZW5kZXIiOiJzSjdwaXU0VUR1TF9vMnBYYi1KX0pBcHhzYUZyeGlUbWdwWmpsdFdqWUZUVWlyNGI4TVdtRGR0enAwT25UZUhMSzltRnJoSDRHVkExd1Z0bm9rVUtvZ0NkTldIc2NhclFzY1FDUlBaREtyVzZib2Z0d0g4X0VZR1RMMFE9In19XX0=", | ||
"iv": "ZqOrBZiA-RdFMhy2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason why iv
not called nonce as called in libsodium. I see indy-sdk code does not call it nonce either, is that the reason or something else?
The protected data base64URL decodes to this: | ||
```json | ||
{ | ||
"enc": "xchacha20poly1305_ietf", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code we have in indy-sdk seems to be using xsalsa20
and not xchacha20
? Is there a proposed change to xchacha20
? Or does this enc
refer to encryption method of the message
and the method to encrypt will allways remain libsodium's crypto_box
?
FCP HIPE: Wire Messages (formerly AMES) Signed-off-by: Brent <brent.zundel@gmail.com>
This is the JWM work, but it's been renamed to prevent confusion. I'm expecting it to diverge further from the JWE spec some more in order to align with other multiplex designs. It details the format of the serialization and the API that should be used to use this functionality. I expect there to be some discussion and changes in relation to multiplexing described here
rendered text