-
Notifications
You must be signed in to change notification settings - Fork 0
Generate first DH key for invitee in Double ratchet and send out the first message in conversation. #20
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
Conversation
jazzz
left a comment
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.
Overall great fixes.
|
|
||
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a tracking ticket to make sendMessage content Agnostic.
#21
src/content_types/all.nim
Outdated
|
|
||
|
|
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 there a Linter that works well for nim? I've been looking for one
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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