-
Notifications
You must be signed in to change notification settings - Fork 1
Key types #56
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
Open
jazzz
wants to merge
11
commits into
main
Choose a base branch
from
jazzz/keys
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+203
−107
Open
Key types #56
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
31d20cd
Add generic SymmetricKey container
jazzz 8eacee5
Rename SecretKey to SymmetricKey32
jazzz fdae248
Update SymmetricKey usage
jazzz bdef2b5
Add PublicKey
jazzz aaecd0b
Update PublicKey uses
jazzz 25a8a03
Add PrivateKey
jazzz 9d7f494
Replace StaticSecret with PrivateKey
jazzz 0302b8c
Fix imports
jazzz 9810e11
Remove InstallKey from constructors
jazzz 9928992
Final integration
jazzz 8f4647c
lint fixes
jazzz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -6,11 +6,10 @@ use chat_proto::logoschat::{ | |||||
| convos::private_v1::{PrivateV1Frame, private_v1_frame::FrameType}, | ||||||
| encryption::{Doubleratchet, EncryptedPayload, encrypted_payload::Encryption}, | ||||||
| }; | ||||||
| use crypto::SecretKey; | ||||||
| use crypto::{PrivateKey, PublicKey, SymmetricKey32}; | ||||||
| use double_ratchets::{Header, InstallationKeyPair, RatchetState}; | ||||||
| use prost::{Message, bytes::Bytes}; | ||||||
| use std::fmt::Debug; | ||||||
| use x25519_dalek::PublicKey; | ||||||
|
|
||||||
| use crate::{ | ||||||
| conversation::{ChatError, ConversationId, Convo, Id}, | ||||||
|
|
@@ -38,8 +37,8 @@ impl Role { | |||||
| struct BaseConvoId([u8; 18]); | ||||||
|
|
||||||
| impl BaseConvoId { | ||||||
| fn new(key: &SecretKey) -> Self { | ||||||
| let base = Blake2bMac::<U18>::new_with_salt_and_personal(key.as_slice(), b"", b"L-PV1-CID") | ||||||
| fn new(key: &SymmetricKey32) -> Self { | ||||||
| let base = Blake2bMac::<U18>::new_with_salt_and_personal(key.as_bytes(), b"", b"L-PV1-CID") | ||||||
| .expect("fixed inputs should never fail"); | ||||||
| Self(base.finalize_fixed().into()) | ||||||
| } | ||||||
|
|
@@ -60,16 +59,16 @@ pub struct PrivateV1Convo { | |||||
| } | ||||||
|
|
||||||
| impl PrivateV1Convo { | ||||||
| pub fn new_initiator(seed_key: SecretKey, remote: PublicKey) -> Self { | ||||||
| pub fn new_initiator(seed_key: SymmetricKey32, remote: PublicKey) -> Self { | ||||||
| let base_convo_id = BaseConvoId::new(&seed_key); | ||||||
| let local_convo_id = base_convo_id.id_for_participant(Role::Initiator); | ||||||
| let remote_convo_id = base_convo_id.id_for_participant(Role::Responder); | ||||||
|
|
||||||
| // TODO: Danger - Fix double-ratchets types to Accept SecretKey | ||||||
| // TODO: Danger - Fix double-ratchets types to Accept SymmetricKey32 | ||||||
| // perhaps update the DH to work with cryptocrate. | ||||||
| // init_sender doesn't take ownership of the key so a reference can be used. | ||||||
| let shared_secret: [u8; 32] = seed_key.as_bytes().to_vec().try_into().unwrap(); | ||||||
| let dr_state = RatchetState::init_sender(shared_secret, remote); | ||||||
| let shared_secret: [u8; 32] = seed_key.DANGER_to_bytes(); | ||||||
| let dr_state = RatchetState::init_sender(shared_secret, *remote); | ||||||
|
|
||||||
| Self { | ||||||
| local_convo_id, | ||||||
|
|
@@ -78,16 +77,17 @@ impl PrivateV1Convo { | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| pub fn new_responder( | ||||||
| seed_key: SecretKey, | ||||||
| dh_self: InstallationKeyPair, // TODO: (P3) Rename; This accepts a Ephemeral key in most cases | ||||||
| ) -> Self { | ||||||
| pub fn new_responder(seed_key: SymmetricKey32, dh_self: &PrivateKey) -> Self { | ||||||
| let base_convo_id = BaseConvoId::new(&seed_key); | ||||||
| let local_convo_id = base_convo_id.id_for_participant(Role::Responder); | ||||||
| let remote_convo_id = base_convo_id.id_for_participant(Role::Initiator); | ||||||
|
|
||||||
| // TODO: Danger - Fix double-ratchets types to Accept SecretKey | ||||||
| let dr_state = RatchetState::init_receiver(seed_key.as_bytes().to_owned(), dh_self); | ||||||
| // TODO: (P3) Rename; This accepts a Ephemeral key in most cases | ||||||
| let dh_self_installation_keypair = | ||||||
| InstallationKeyPair::from_secret_bytes(dh_self.DANGER_to_bytes()); | ||||||
| // TODO: Danger - Fix double-ratchets types to Accept SymmetricKey32 | ||||||
| let dr_state = | ||||||
| RatchetState::init_receiver(seed_key.DANGER_to_bytes(), dh_self_installation_keypair); | ||||||
|
|
||||||
| Self { | ||||||
| local_convo_id, | ||||||
|
|
@@ -135,7 +135,7 @@ impl PrivateV1Convo { | |||||
|
|
||||||
| // Build the Header that DR impl expects | ||||||
| let header = Header { | ||||||
| dh_pub, | ||||||
| dh_pub: *dh_pub, | ||||||
| msg_num: dr_header.msg_num, | ||||||
| prev_chain_len: dr_header.prev_chain_len, | ||||||
| }; | ||||||
|
|
@@ -221,27 +221,24 @@ impl Debug for PrivateV1Convo { | |||||
|
|
||||||
| #[cfg(test)] | ||||||
| mod tests { | ||||||
| use x25519_dalek::StaticSecret; | ||||||
| use crypto::PrivateKey; | ||||||
|
|
||||||
| use super::*; | ||||||
|
|
||||||
| #[test] | ||||||
| fn test_encrypt_roundtrip() { | ||||||
| let saro = StaticSecret::random(); | ||||||
| let raya = StaticSecret::random(); | ||||||
| let saro = PrivateKey::random(); | ||||||
| let raya = PrivateKey::random(); | ||||||
|
|
||||||
| let pub_raya = PublicKey::from(&raya); | ||||||
|
|
||||||
| let seed_key = saro.diffie_hellman(&pub_raya); | ||||||
| let seed_key = saro.diffie_hellman(&pub_raya).DANGER_to_bytes(); | ||||||
| let seed_key_saro = SymmetricKey32::from(seed_key); | ||||||
| let seed_key_raya = SymmetricKey32::from(seed_key); | ||||||
| let send_content_bytes = vec![0, 2, 4, 6, 8]; | ||||||
| let mut sr_convo = | ||||||
| PrivateV1Convo::new_initiator(SecretKey::from(seed_key.to_bytes()), pub_raya); | ||||||
|
|
||||||
| let installation_key_pair = InstallationKeyPair::from(raya); | ||||||
| let mut rs_convo = PrivateV1Convo::new_responder( | ||||||
| SecretKey::from(seed_key.to_bytes()), | ||||||
| installation_key_pair, | ||||||
| ); | ||||||
| let mut sr_convo = PrivateV1Convo::new_initiator(seed_key_saro, pub_raya); | ||||||
| let mut rs_convo = | ||||||
| PrivateV1Convo::new_responder(SymmetricKey32::from(seed_key_raya), &raya); | ||||||
|
||||||
| PrivateV1Convo::new_responder(SymmetricKey32::from(seed_key_raya), &raya); | |
| PrivateV1Convo::new_responder(seed_key_raya, &raya); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
PrivateV1Convo::new_responderconvertsdh_selfinto raw secret bytes viaDANGER_to_bytes()to build anInstallationKeyPair, which creates an extra copy of secret material that isn’t obviously zeroized. Consider adding/using an API that can constructInstallationKeyPairwithout exposing raw bytes (or ensures the temporary byte buffer is zeroized).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.
Correct. The plan is to remove InstallationKeypair all together