Generate first DH key for invitee in Double ratchet and send out the first message in conversation.#20
Generate first DH key for invitee in Double ratchet and send out the first message in conversation.#20kaichaosun wants to merge 3 commits into
Conversation
|
|
||
| proc initPrivateV1*(owner: Identity, participant: PublicKey, seedKey: array[32, byte], | ||
| discriminator: string = "default", isSender: bool, deliveryAckCb: proc( | ||
| discriminator: string = "default", inviter: bool, deliveryAckCb: proc( |
There was a problem hiding this comment.
I think moving away from the "isSender" convention makes sense here.
[Sand] Perhaps "Initiator" would be a better fit as it keeps the language consistent.
There was a problem hiding this comment.
When reading the proto definition, I somehow need to think twice to reason the term participant (initiator seems fine though). The term inviter/invitee actually looks a bit clear and straightforward to me.
message InvitePrivateV1 {
bytes initiator = 1;
bytes initiator_ephemeral = 2;
bytes participant = 3;
int32 participant_ephemeral_id= 4;
string discriminator = 5;
}
There was a problem hiding this comment.
Clear unambiguous terms are important. Feel free to suggest new ones here: logos-messaging/specs#80
| topic = convo.getConvoId() | ||
| client.addConversation(convo) | ||
|
|
||
| # TODO send a control frame instead |
There was a problem hiding this comment.
I'm not sure why any frame needs to be sent here.
A Recipient ought to be able to initialize an inbound session and receive messages without revealing activity to the sender
There was a problem hiding this comment.
Not sure if I miss something, here is my thought:
After both run initDoubleratchet
initiator(Alice), dh is id key, dh remote is bob's id pub key, send key is empty
recipient (Bob), dh is a new generated key,, dh remote is alice's id pub key, send key and new root key derived from DH.
Without sending a control message, if alice sends message to bob, alice will use message key derived from empty send key. This seems not what we want.
There was a problem hiding this comment.
Without sending a control message, if alice sends message to bob, alice will use message key derived from empty send key
You are correct in that the DH step requires a key from the remote party before it can occur.
Alice can use bobs static installation key as the initial DHr. This should ultimately be replaced with a signed pre-key equivalent once the work on identity is completed
Requiring a response from Bob to complete initialization, would remove the non-interactive protocol property
| client.addConversation(convo) | ||
|
|
||
| # TODO send a control frame instead | ||
| discard convo.sendMessage(client.ds, initTextFrame("Hello").toContentFrame()) |
There was a problem hiding this comment.
Added a tracking ticket to make sendMessage content Agnostic.
#21
|
|
||
|
|
There was a problem hiding this comment.
Is there a Linter that works well for nim? I've been looking for one
There was a problem hiding this comment.
There is nph (https://github.com/arnetheduck/nph) used in nwaku, seems fine. Let's try it.
|
|
||
| let dhOutputPost = self.dhSelf.dhExchange(self.dhRemote).get() | ||
| (self.rootKey, self.chainKeyRecv) = kdfRoot(self, self.rootKey, dhOutputPost) | ||
| (self.rootKey, self.chainKeySend) = kdfRoot(self, self.rootKey, dhOutputPost) |
There was a problem hiding this comment.
Nice catch.
I'd suggest getting rid of the direct kdf call all togther and use dhRatchetSend which also initializes msgCountSend
There was a problem hiding this comment.
dhRatchetRecv calls dhRatchetSend is pretty confusing, we may need to rename both terms and see how to reuse some code.
There was a problem hiding this comment.
In a correct implementation, the sendingChainKey is not generated until it is needed to minimize key exposure.
dhRatchetRecv: Performs the first part of the ratchet step when receiving DHr
dhRatchetSend: Performs the second part once the client wants to send a message.
One path to clean this up, is to remove keyGeneration and subsequent KeyExchange from this function all together.
|
Closed since most of the fix is already merged in other PRs. |
What needs to be done,
chainKeySendin DH ratchet if remote key is differentconvo.doubleratchet.dhSelf.public == header.dhPublic.decryption failed context: [code = errMessageAuthentication) in ping pong demo