From 7222de466feb7b1fc5a6b8108db797926d84a4d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20F=C3=A9ron?= Date: Sun, 3 Nov 2024 18:18:28 +0100 Subject: [PATCH 1/3] Revert "Make sure we really use thread_rng everywhere (#341)" This reverts commit b0fcd8bef2ba2d5ec684c5bd3dbce62c06b60e62. --- src/account_manager.rs | 17 ++++++++--------- src/cipher.rs | 17 +++++++++++------ src/sender.rs | 19 ++++++++++++------- 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/account_manager.rs b/src/account_manager.rs index c7bbc5ce1..af8fc1c88 100644 --- a/src/account_manager.rs +++ b/src/account_manager.rs @@ -1,6 +1,5 @@ use base64::prelude::*; use phonenumber::PhoneNumber; -use rand::Rng; use reqwest::Method; use std::collections::HashMap; use std::convert::{TryFrom, TryInto}; @@ -642,7 +641,7 @@ impl AccountManager { /// Should be called as the primary device to migrate from pre-PNI to PNI. /// /// This is the equivalent of Android's PnpInitializeDevicesJob or iOS' PniHelloWorldManager. - #[tracing::instrument(skip(self, aci_protocol_store, pni_protocol_store, sender, local_aci), fields(local_aci = %local_aci))] + #[tracing::instrument(skip(self, aci_protocol_store, pni_protocol_store, sender, local_aci, csprng), fields(local_aci = %local_aci))] pub async fn pnp_initialize_devices< // XXX So many constraints here, all imposed by the MessageSender R: rand::Rng + rand::CryptoRng, @@ -653,11 +652,11 @@ impl AccountManager { &mut self, aci_protocol_store: &mut Aci, pni_protocol_store: &mut Pni, - mut sender: MessageSender, + mut sender: MessageSender, local_aci: ServiceAddress, e164: PhoneNumber, + csprng: &mut R, ) -> Result<(), MessageSenderError> { - let mut csprng = rand::thread_rng(); let pni_identity_key_pair = pni_protocol_store.get_identity_key_pair().await?; @@ -714,7 +713,7 @@ impl AccountManager { crate::pre_keys::replenish_pre_keys( pni_protocol_store, &pni_identity_key_pair, - &mut csprng, + csprng, true, 0, 0, @@ -722,12 +721,12 @@ impl AccountManager { .await? } else { // Generate a signed prekey - let signed_pre_key_pair = KeyPair::generate(&mut csprng); + let signed_pre_key_pair = KeyPair::generate(csprng); let signed_pre_key_public = signed_pre_key_pair.public_key; let signed_pre_key_signature = pni_identity_key_pair.private_key().calculate_signature( &signed_pre_key_public.serialize(), - &mut csprng, + csprng, )?; let signed_prekey_record = SignedPreKeyRecord::new( @@ -755,7 +754,7 @@ impl AccountManager { pni_protocol_store.get_local_registration_id().await? } else { loop { - let regid = generate_registration_id(&mut csprng); + let regid = generate_registration_id(csprng); if !pni_registration_ids.iter().any(|(_k, v)| *v == regid) { break regid; } @@ -803,7 +802,7 @@ impl AccountManager { e164.format().mode(phonenumber::Mode::E164).to_string(), ), }), - padding: Some(random_length_padding(&mut csprng, 512)), + padding: Some(random_length_padding(csprng, 512)), ..SyncMessage::default() }; let content: ContentBody = msg.into(); diff --git a/src/cipher.rs b/src/cipher.rs index 645b4aed9..057cc8332 100644 --- a/src/cipher.rs +++ b/src/cipher.rs @@ -13,6 +13,7 @@ use libsignal_protocol::{ SignalProtocolError, SignedPreKeyStore, Timestamp, }; use prost::Message; +use rand::{CryptoRng, Rng}; use uuid::Uuid; use crate::{ @@ -28,14 +29,15 @@ use crate::{ /// /// Equivalent of SignalServiceCipher in Java. #[derive(Clone)] -pub struct ServiceCipher { +pub struct ServiceCipher { protocol_store: S, + csprng: R, trust_root: PublicKey, local_uuid: Uuid, local_device_id: u32, } -impl fmt::Debug for ServiceCipher { +impl fmt::Debug for ServiceCipher { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("ServiceCipher") .field("protocol_store", &"...") @@ -68,18 +70,21 @@ fn debug_envelope(envelope: &Envelope) -> String { } } -impl ServiceCipher +impl ServiceCipher where S: ProtocolStore + SenderKeyStore + SessionStoreExt + Clone, + R: Rng + CryptoRng, { pub fn new( protocol_store: S, + csprng: R, trust_root: PublicKey, local_uuid: Uuid, local_device_id: u32, ) -> Self { Self { protocol_store, + csprng, trust_root, local_uuid, local_device_id, @@ -175,7 +180,7 @@ where &mut self.protocol_store.clone(), &self.protocol_store.clone(), &mut self.protocol_store.clone(), - &mut rand::thread_rng(), + &mut self.csprng, ) .await? .as_slice() @@ -231,7 +236,7 @@ where &sender, &mut self.protocol_store.clone(), &mut self.protocol_store.clone(), - &mut rand::thread_rng(), + &mut self.csprng, ) .await? .as_slice() @@ -349,7 +354,7 @@ where &mut self.protocol_store.clone(), &mut self.protocol_store, SystemTime::now(), - &mut rand::thread_rng(), + &mut self.csprng, ) .await?; diff --git a/src/sender.rs b/src/sender.rs index ae9d2cfec..460e45900 100644 --- a/src/sender.rs +++ b/src/sender.rs @@ -5,6 +5,7 @@ use libsignal_protocol::{ process_prekey_bundle, DeviceId, IdentityKey, IdentityKeyPair, ProtocolStore, SenderCertificate, SenderKeyStore, SignalProtocolError, }; +use rand::{CryptoRng, Rng}; use tracing::{debug, error, info, trace, warn}; use tracing_futures::Instrument; use uuid::Uuid; @@ -82,11 +83,12 @@ pub struct AttachmentSpec { } #[derive(Clone)] -pub struct MessageSender { +pub struct MessageSender { identified_ws: SignalWebSocket, unidentified_ws: SignalWebSocket, service: PushService, - cipher: ServiceCipher, + cipher: ServiceCipher, + csprng: R, protocol_store: S, local_aci: ServiceAddress, local_pni: ServiceAddress, @@ -148,16 +150,18 @@ pub struct EncryptedMessages { used_identity_key: IdentityKey, } -impl MessageSender +impl MessageSender where S: ProtocolStore + SenderKeyStore + SessionStoreExt + Sync + Clone, + R: Rng + CryptoRng, { #[allow(clippy::too_many_arguments)] pub fn new( identified_ws: SignalWebSocket, unidentified_ws: SignalWebSocket, service: PushService, - cipher: ServiceCipher, + cipher: ServiceCipher, + csprng: R, protocol_store: S, local_aci: impl Into, local_pni: impl Into, @@ -170,6 +174,7 @@ where identified_ws, unidentified_ws, cipher, + csprng, protocol_store, local_aci: local_aci.into(), local_pni: local_pni.into(), @@ -620,7 +625,7 @@ where &mut self.protocol_store, &pre_key, SystemTime::now(), - &mut rand::thread_rng(), + &mut self.csprng, ) .await .map_err(|e| { @@ -833,7 +838,7 @@ where .expect("PNI key set when PNI signature requested") .sign_alternate_identity( self.aci_identity.identity_key(), - &mut rand::thread_rng(), + &mut self.csprng, )?; Ok(crate::proto::PniSignatureMessage { pni: Some(self.local_pni.uuid.as_bytes().to_vec()), @@ -1020,7 +1025,7 @@ where &mut self.protocol_store, &pre_key_bundle, SystemTime::now(), - &mut rand::thread_rng(), + &mut self.csprng, ) .await?; } From b59f8add0ee86beda050564fb561b88a9a814037 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20F=C3=A9ron?= Date: Sun, 3 Nov 2024 18:57:12 +0100 Subject: [PATCH 2/3] Make sure we use the provided CSPRNG everywhere relevant --- src/account_manager.rs | 61 ++++++++++++++++++------------------- src/groups_v2/manager.rs | 29 ++++++------------ src/master_key.rs | 9 +++--- src/profile_cipher.rs | 35 +++++++++++---------- src/proto.rs | 9 +++--- src/provisioning/cipher.rs | 21 ++++++------- src/provisioning/mod.rs | 2 +- src/provisioning/pipe.rs | 10 +++--- src/push_service/profile.rs | 5 +-- src/sender.rs | 31 +++++++++---------- 10 files changed, 101 insertions(+), 111 deletions(-) diff --git a/src/account_manager.rs b/src/account_manager.rs index af8fc1c88..09f02e012 100644 --- a/src/account_manager.rs +++ b/src/account_manager.rs @@ -1,5 +1,6 @@ use base64::prelude::*; use phonenumber::PhoneNumber; +use rand::{CryptoRng, Rng}; use reqwest::Method; use std::collections::HashMap; use std::convert::{TryFrom, TryInto}; @@ -51,7 +52,8 @@ use crate::{ type Aes256Ctr128BE = ctr::Ctr128BE; -pub struct AccountManager { +pub struct AccountManager { + csprng: R, service: PushService, profile_key: Option, } @@ -72,9 +74,14 @@ pub struct Profile { pub avatar: Option, } -impl AccountManager { - pub fn new(service: PushService, profile_key: Option) -> Self { +impl AccountManager { + pub fn new( + csprng: R, + service: PushService, + profile_key: Option, + ) -> Self { Self { + csprng, service, profile_key, } @@ -87,15 +94,11 @@ impl AccountManager { /// /// Equivalent to Java's RefreshPreKeysJob #[allow(clippy::too_many_arguments)] - #[tracing::instrument(skip(self, protocol_store, csprng))] - pub async fn update_pre_key_bundle< - R: rand::Rng + rand::CryptoRng, - P: PreKeysStore, - >( + #[tracing::instrument(skip(self, protocol_store))] + pub async fn update_pre_key_bundle( &mut self, protocol_store: &mut P, service_id_type: ServiceIdType, - csprng: &mut R, use_last_resort_key: bool, ) -> Result<(), ServiceError> { let prekey_status = match self @@ -154,7 +157,7 @@ impl AccountManager { crate::pre_keys::replenish_pre_keys( protocol_store, &identity_key_pair, - csprng, + &mut self.csprng, use_last_resort_key && !has_last_resort_key, PRE_KEY_BATCH_SIZE, PRE_KEY_BATCH_SIZE, @@ -343,7 +346,7 @@ impl AccountManager { let cipher = ProvisioningCipher::from_public(pub_key); - let encrypted = cipher.encrypt(msg)?; + let encrypted = cipher.encrypt(&mut self.csprng, msg)?; self.send_provisioning_message(ephemeral_id, encrypted) .await?; Ok(()) @@ -379,12 +382,10 @@ impl AccountManager { } pub async fn register_account< - R: rand::Rng + rand::CryptoRng, Aci: PreKeysStore + IdentityKeyStore, Pni: PreKeysStore + IdentityKeyStore, >( &mut self, - csprng: &mut R, registration_method: RegistrationMethod<'_>, account_attributes: AccountAttributes, aci_protocol_store: &mut Aci, @@ -408,7 +409,7 @@ impl AccountManager { ) = crate::pre_keys::replenish_pre_keys( aci_protocol_store, &aci_identity_key_pair, - csprng, + &mut self.csprng, true, 0, 0, @@ -423,7 +424,7 @@ impl AccountManager { ) = crate::pre_keys::replenish_pre_keys( pni_protocol_store, &pni_identity_key_pair, - csprng, + &mut self.csprng, true, 0, 0, @@ -504,7 +505,7 @@ impl AccountManager { .retrieve_profile_by_id(address, Some(profile_key)) .await?; - let profile_cipher = ProfileCipher::from(profile_key); + let profile_cipher = ProfileCipher::new(&mut self.csprng, profile_key); Ok(encrypted_profile.decrypt(profile_cipher)?) } @@ -527,7 +528,8 @@ impl AccountManager { ) -> Result, ProfileManagerError> { let profile_key = self.profile_key.expect("set profile key in AccountManager"); - let profile_cipher = ProfileCipher::from(profile_key); + let mut profile_cipher = + ProfileCipher::new(&mut self.csprng, profile_key); // Profile encryption let name = profile_cipher.encrypt_name(name.as_ref())?; @@ -576,11 +578,8 @@ impl AccountManager { device_name: &str, public_key: &IdentityKey, ) -> Result<(), ServiceError> { - let encrypted_device_name = encrypt_device_name( - &mut rand::thread_rng(), - device_name, - public_key, - )?; + let encrypted_device_name = + encrypt_device_name(&mut self.csprng, device_name, public_key)?; #[derive(Serialize)] #[serde(rename_all = "camelCase")] @@ -641,10 +640,9 @@ impl AccountManager { /// Should be called as the primary device to migrate from pre-PNI to PNI. /// /// This is the equivalent of Android's PnpInitializeDevicesJob or iOS' PniHelloWorldManager. - #[tracing::instrument(skip(self, aci_protocol_store, pni_protocol_store, sender, local_aci, csprng), fields(local_aci = %local_aci))] + #[tracing::instrument(skip(self, aci_protocol_store, pni_protocol_store, sender, local_aci), fields(local_aci = %local_aci))] pub async fn pnp_initialize_devices< // XXX So many constraints here, all imposed by the MessageSender - R: rand::Rng + rand::CryptoRng, Aci: PreKeysStore + SessionStoreExt, Pni: PreKeysStore, AciOrPni: ProtocolStore + SenderKeyStore + SessionStoreExt + Sync + Clone, @@ -655,7 +653,6 @@ impl AccountManager { mut sender: MessageSender, local_aci: ServiceAddress, e164: PhoneNumber, - csprng: &mut R, ) -> Result<(), MessageSenderError> { let pni_identity_key_pair = pni_protocol_store.get_identity_key_pair().await?; @@ -713,7 +710,7 @@ impl AccountManager { crate::pre_keys::replenish_pre_keys( pni_protocol_store, &pni_identity_key_pair, - csprng, + &mut self.csprng, true, 0, 0, @@ -721,16 +718,16 @@ impl AccountManager { .await? } else { // Generate a signed prekey - let signed_pre_key_pair = KeyPair::generate(csprng); + let signed_pre_key_pair = KeyPair::generate(&mut self.csprng); let signed_pre_key_public = signed_pre_key_pair.public_key; let signed_pre_key_signature = pni_identity_key_pair.private_key().calculate_signature( &signed_pre_key_public.serialize(), - csprng, + &mut self.csprng, )?; let signed_prekey_record = SignedPreKeyRecord::new( - csprng.gen_range::(0..0xFFFFFF).into(), + self.csprng.gen_range::(0..0xFFFFFF).into(), Timestamp::now(), &signed_pre_key_pair, &signed_pre_key_signature, @@ -739,7 +736,7 @@ impl AccountManager { // Generate a last-resort Kyber prekey let kyber_pre_key_record = KyberPreKeyRecord::generate( kem::KeyType::Kyber1024, - csprng.gen_range::(0..0xFFFFFF).into(), + self.csprng.gen_range::(0..0xFFFFFF).into(), pni_identity_key_pair.private_key(), )?; ( @@ -754,7 +751,7 @@ impl AccountManager { pni_protocol_store.get_local_registration_id().await? } else { loop { - let regid = generate_registration_id(csprng); + let regid = generate_registration_id(&mut self.csprng); if !pni_registration_ids.iter().any(|(_k, v)| *v == regid) { break regid; } @@ -802,7 +799,7 @@ impl AccountManager { e164.format().mode(phonenumber::Mode::E164).to_string(), ), }), - padding: Some(random_length_padding(csprng, 512)), + padding: Some(random_length_padding(&mut self.csprng, 512)), ..SyncMessage::default() }; let content: ContentBody = msg.into(); diff --git a/src/groups_v2/manager.rs b/src/groups_v2/manager.rs index 567c0edea..3d8cf9839 100644 --- a/src/groups_v2/manager.rs +++ b/src/groups_v2/manager.rs @@ -16,7 +16,7 @@ use base64::prelude::*; use bytes::Bytes; use chrono::{Days, NaiveDate, NaiveTime, Utc}; use futures::AsyncReadExt; -use rand::RngCore; +use rand::{CryptoRng, Rng}; use reqwest::Method; use serde::Deserialize; use zkgroup::{ @@ -152,8 +152,9 @@ impl GroupsManager { } } - pub async fn get_authorization_for_today( + pub async fn get_authorization_for_today( &mut self, + csprng: &mut R, group_secret_params: GroupSecretParams, ) -> Result { let (today, today_plus_7_days) = current_days_seconds(); @@ -190,14 +191,16 @@ impl GroupsManager { }; self.get_authorization_string( + csprng, group_secret_params, auth_credential_response.clone(), today, ) } - fn get_authorization_string( + fn get_authorization_string( &self, + csprng: &mut R, group_secret_params: GroupSecretParams, credential_response: AuthCredentialWithPniResponse, today: u64, @@ -219,7 +222,7 @@ impl GroupsManager { })?; let mut random_bytes = [0u8; 32]; - rand::thread_rng().fill_bytes(&mut random_bytes); + csprng.fill_bytes(&mut random_bytes); let auth_credential_presentation = self .server_public_params @@ -241,21 +244,9 @@ impl GroupsManager { Ok(HttpAuth { username, password }) } - #[deprecated = "please use fetch_encrypted_group and decrypt_group separately, which hide more of the implementation details"] - pub async fn get_group( - &mut self, - group_secret_params: GroupSecretParams, - credentials: HttpAuth, - ) -> Result { - let encrypted_group = self.push_service.get_group(credentials).await?; - let decrypted_group = GroupOperations::new(group_secret_params) - .decrypt_group(encrypted_group)?; - - Ok(decrypted_group) - } - - pub async fn fetch_encrypted_group( + pub async fn fetch_encrypted_group( &mut self, + csprng: &mut R, master_key_bytes: &[u8], ) -> Result { let group_master_key = GroupMasterKey::new( @@ -266,7 +257,7 @@ impl GroupsManager { let group_secret_params = GroupSecretParams::derive_from_master_key(group_master_key); let authorization = self - .get_authorization_for_today(group_secret_params) + .get_authorization_for_today(csprng, group_secret_params) .await?; self.push_service.get_group(authorization).await } diff --git a/src/master_key.rs b/src/master_key.rs index 40e60a27a..ef3c4039e 100644 --- a/src/master_key.rs +++ b/src/master_key.rs @@ -1,3 +1,5 @@ +use rand::{CryptoRng, Rng}; + const MASTER_KEY_LEN: usize = 32; const STORAGE_KEY_LEN: usize = 32; @@ -7,13 +9,10 @@ pub struct MasterKey { } impl MasterKey { - pub fn generate() -> Self { - use rand::Rng; - + pub fn generate(csprng: &mut R) -> Self { // Create random bytes - let mut rng = rand::thread_rng(); let mut inner = [0_u8; MASTER_KEY_LEN]; - rng.fill(&mut inner); + csprng.fill(&mut inner); Self { inner } } diff --git a/src/profile_cipher.rs b/src/profile_cipher.rs index a568b09d6..c711f2076 100644 --- a/src/profile_cipher.rs +++ b/src/profile_cipher.rs @@ -1,6 +1,7 @@ use std::convert::TryInto; use aes_gcm::{aead::Aead, AeadCore, AeadInPlace, Aes256Gcm, KeyInit}; +use rand::{CryptoRng, Rng}; use zkgroup::profiles::ProfileKey; use crate::profile_name::ProfileName; @@ -25,16 +26,11 @@ use crate::profile_name::ProfileName; /// let decrypted = cipher.decrypt_name(&encrypted).unwrap().unwrap(); /// assert_eq!(decrypted.as_ref(), name); /// ``` -pub struct ProfileCipher { +pub struct ProfileCipher { + csprng: R, profile_key: ProfileKey, } -impl From for ProfileCipher { - fn from(profile_key: ProfileKey) -> Self { - ProfileCipher { profile_key } - } -} - const NAME_PADDED_LENGTH_1: usize = 53; const NAME_PADDED_LENGTH_2: usize = 257; const NAME_PADDING_BRACKETS: &[usize] = @@ -77,20 +73,27 @@ fn pad_plaintext( Ok(len) } -impl ProfileCipher { +impl ProfileCipher { + pub fn new(csprng: R, profile_key: ProfileKey) -> Self { + Self { + csprng, + profile_key, + } + } + pub fn into_inner(self) -> ProfileKey { self.profile_key } fn pad_and_encrypt( - &self, + &mut self, mut bytes: Vec, padding_brackets: &[usize], ) -> Result, ProfileCipherError> { let _len = pad_plaintext(&mut bytes, padding_brackets)?; let cipher = Aes256Gcm::new(&self.profile_key.get_bytes().into()); - let nonce = Aes256Gcm::generate_nonce(rand::thread_rng()); + let nonce = Aes256Gcm::generate_nonce(&mut self.csprng); cipher .encrypt_in_place(&nonce, b"", &mut bytes) @@ -137,7 +140,7 @@ impl ProfileCipher { } pub fn encrypt_name<'inp>( - &self, + &mut self, name: impl std::borrow::Borrow>, ) -> Result, ProfileCipherError> { let name = name.borrow(); @@ -157,7 +160,7 @@ impl ProfileCipher { } pub fn encrypt_about( - &self, + &mut self, about: String, ) -> Result, ProfileCipherError> { let bytes = about.into_bytes(); @@ -177,7 +180,7 @@ impl ProfileCipher { } pub fn encrypt_emoji( - &self, + &mut self, emoji: String, ) -> Result, ProfileCipherError> { let bytes = emoji.into_bytes(); @@ -222,7 +225,7 @@ mod tests { let mut rng = rand::thread_rng(); let some_randomness = rng.gen(); let profile_key = ProfileKey::generate(some_randomness); - let cipher = ProfileCipher::from(profile_key); + let mut cipher = ProfileCipher::new(rng, profile_key); for name in &names { let profile_name = ProfileName::<&str> { given_name: name, @@ -247,7 +250,7 @@ mod tests { let mut rng = rand::thread_rng(); let some_randomness = rng.gen(); let profile_key = ProfileKey::generate(some_randomness); - let cipher = ProfileCipher::from(profile_key); + let mut cipher = ProfileCipher::new(rng, profile_key); for &about in &abouts { let encrypted = cipher.encrypt_about(about.into()).unwrap(); @@ -264,7 +267,7 @@ mod tests { let mut rng = rand::thread_rng(); let some_randomness = rng.gen(); let profile_key = ProfileKey::generate(some_randomness); - let cipher = ProfileCipher::from(profile_key); + let mut cipher = ProfileCipher::new(rng, profile_key); for &emoji in &emojii { let encrypted = cipher.encrypt_emoji(emoji.into()).unwrap(); diff --git a/src/proto.rs b/src/proto.rs index f24d08037..4d678e727 100644 --- a/src/proto.rs +++ b/src/proto.rs @@ -1,6 +1,6 @@ #![allow(clippy::all)] -use rand::{Rng, RngCore}; +use rand::{CryptoRng, Rng}; include!(concat!(env!("OUT_DIR"), "/signalservice.rs")); include!(concat!(env!("OUT_DIR"), "/signal.rs")); @@ -69,11 +69,10 @@ impl WebSocketResponseMessage { } impl SyncMessage { - pub fn with_padding() -> Self { - let mut rng = rand::thread_rng(); - let random_size = rng.gen_range(1..=512); + pub fn with_padding(csprng: &mut R) -> Self { + let random_size = csprng.gen_range(1..=512); let mut padding: Vec = vec![0; random_size]; - rng.fill_bytes(&mut padding); + csprng.fill_bytes(&mut padding); Self { padding: Some(padding), diff --git a/src/provisioning/cipher.rs b/src/provisioning/cipher.rs index 7143385bf..526dc372c 100644 --- a/src/provisioning/cipher.rs +++ b/src/provisioning/cipher.rs @@ -7,7 +7,7 @@ use bytes::Bytes; use hmac::{Hmac, Mac}; use libsignal_protocol::{KeyPair, PublicKey}; use prost::Message; -use rand::Rng; +use rand::{CryptoRng, Rng}; use sha2::Sha256; pub use crate::proto::{ProvisionEnvelope, ProvisionMessage}; @@ -55,11 +55,10 @@ pub struct ProvisioningCipher { impl ProvisioningCipher { /// Generate a random key pair - pub fn generate(rng: &mut R) -> Result - where - R: rand::Rng + rand::CryptoRng, - { - let key_pair = libsignal_protocol::KeyPair::generate(rng); + pub fn generate( + mut csprng: R, + ) -> Result { + let key_pair = libsignal_protocol::KeyPair::generate(&mut csprng); Ok(Self { key_material: CipherMode::DecryptAndEncrypt(key_pair), }) @@ -81,14 +80,14 @@ impl ProvisioningCipher { self.key_material.public() } - pub fn encrypt( + pub fn encrypt( &self, + csprng: &mut R, msg: ProvisionMessage, ) -> Result { let msg = msg.encode_to_vec(); - let mut rng = rand::thread_rng(); - let our_key_pair = libsignal_protocol::KeyPair::generate(&mut rng); + let our_key_pair = libsignal_protocol::KeyPair::generate(csprng); let agreement = our_key_pair.calculate_agreement(self.public_key())?; let mut shared_secrets = [0; 64]; @@ -98,7 +97,7 @@ impl ProvisioningCipher { let aes_key = &shared_secrets[0..32]; let mac_key = &shared_secrets[32..]; - let iv: [u8; IV_LENGTH] = rng.gen(); + let iv: [u8; IV_LENGTH] = csprng.gen(); let cipher = cbc::Encryptor::::new(aes_key.into(), &iv.into()); let ciphertext = cipher.encrypt_padded_vec_mut::(&msg); @@ -197,7 +196,7 @@ mod tests { ); let msg = ProvisionMessage::default(); - let encrypted = encrypt_cipher.encrypt(msg.clone())?; + let encrypted = encrypt_cipher.encrypt(&mut rng, msg.clone())?; assert!(matches!( encrypt_cipher.decrypt(encrypted.clone()), diff --git a/src/provisioning/mod.rs b/src/provisioning/mod.rs index f1c16c589..cac4c1015 100644 --- a/src/provisioning/mod.rs +++ b/src/provisioning/mod.rs @@ -158,7 +158,7 @@ pub async fn link_device< let registration_id = csprng.gen_range(1..256); let pni_registration_id = csprng.gen_range(1..256); - let provisioning_pipe = ProvisioningPipe::from_socket(ws)?; + let provisioning_pipe = ProvisioningPipe::from_socket(csprng, ws)?; let provision_stream = provisioning_pipe.stream(); pin_mut!(provision_stream); diff --git a/src/provisioning/pipe.rs b/src/provisioning/pipe.rs index 0842ec895..594049ab1 100644 --- a/src/provisioning/pipe.rs +++ b/src/provisioning/pipe.rs @@ -7,6 +7,7 @@ use futures::{ }, prelude::*, }; +use rand::{CryptoRng, Rng}; use url::Url; pub use crate::proto::{ProvisionEnvelope, ProvisionMessage}; @@ -34,12 +35,13 @@ pub enum ProvisioningStep { } impl ProvisioningPipe { - pub fn from_socket(ws: SignalWebSocket) -> Result { + pub fn from_socket( + csprng: &mut R, + ws: SignalWebSocket, + ) -> Result { Ok(ProvisioningPipe { ws, - provisioning_cipher: ProvisioningCipher::generate( - &mut rand::thread_rng(), - )?, + provisioning_cipher: ProvisioningCipher::generate(csprng)?, }) } diff --git a/src/push_service/profile.rs b/src/push_service/profile.rs index c14b59b91..c427d7115 100644 --- a/src/push_service/profile.rs +++ b/src/push_service/profile.rs @@ -1,3 +1,4 @@ +use rand::{CryptoRng, Rng}; use reqwest::Method; use serde::{Deserialize, Serialize}; use zkgroup::profiles::{ProfileKeyCommitment, ProfileKeyVersion}; @@ -38,9 +39,9 @@ pub struct SignalServiceProfile { } impl SignalServiceProfile { - pub fn decrypt( + pub fn decrypt( &self, - profile_cipher: crate::profile_cipher::ProfileCipher, + profile_cipher: crate::profile_cipher::ProfileCipher, ) -> Result { // Profile decryption let name = self diff --git a/src/sender.rs b/src/sender.rs index 460e45900..8fb959955 100644 --- a/src/sender.rs +++ b/src/sender.rs @@ -196,12 +196,10 @@ where let len = contents.len(); // Encrypt let (key, iv) = { - use rand::RngCore; let mut key = [0u8; 64]; let mut iv = [0u8; 16]; - // thread_rng is guaranteed to be cryptographically secure - rand::thread_rng().fill_bytes(&mut key); - rand::thread_rng().fill_bytes(&mut iv); + self.csprng.fill_bytes(&mut key); + self.csprng.fill_bytes(&mut iv); (key, iv) }; @@ -371,8 +369,8 @@ where // only send a sync message when sending to self and skip the rest of the process if message_to_self && is_multi_device { debug!("sending note to self"); - let sync_message = - Self::create_multi_device_sent_transcript_content( + let sync_message = self + .create_multi_device_sent_transcript_content( Some(recipient), content_body, timestamp, @@ -414,8 +412,8 @@ where if needs_sync || is_multi_device { debug!("sending multi-device sync message"); - let sync_message = - Self::create_multi_device_sent_transcript_content( + let sync_message = self + .create_multi_device_sent_transcript_content( Some(recipient), content_body, timestamp, @@ -488,8 +486,8 @@ where // we only need to send a synchronization message once if needs_sync_in_results || self.is_multi_device().await { - let sync_message = - Self::create_multi_device_sent_transcript_content( + let sync_message = self + .create_multi_device_sent_transcript_content( None, content_body, timestamp, @@ -703,7 +701,7 @@ where blob: Some(ptr), complete: Some(complete), }), - ..SyncMessage::with_padding() + ..SyncMessage::with_padding(&mut self.csprng) }; self.send_message( @@ -728,7 +726,7 @@ where ) -> Result<(), MessageSenderError> { let msg = SyncMessage { configuration: Some(configuration), - ..SyncMessage::with_padding() + ..SyncMessage::with_padding(&mut self.csprng) }; let ts = Utc::now().timestamp_millis() as u64; @@ -775,7 +773,7 @@ where let msg = SyncMessage { message_request_response, - ..SyncMessage::with_padding() + ..SyncMessage::with_padding(&mut self.csprng) }; let ts = Utc::now().timestamp_millis() as u64; @@ -794,7 +792,7 @@ where ) -> Result<(), MessageSenderError> { let msg = SyncMessage { keys: Some(keys), - ..SyncMessage::with_padding() + ..SyncMessage::with_padding(&mut self.csprng) }; let ts = Utc::now().timestamp_millis() as u64; @@ -819,7 +817,7 @@ where request: Some(sync_message::Request { r#type: Some(request_type.into()), }), - ..SyncMessage::with_padding() + ..SyncMessage::with_padding(&mut self.csprng) }; let ts = Utc::now().timestamp_millis() as u64; @@ -1041,6 +1039,7 @@ where } fn create_multi_device_sent_transcript_content<'a>( + &mut self, recipient: Option<&ServiceAddress>, content_body: ContentBody, timestamp: u64, @@ -1088,7 +1087,7 @@ where unidentified_status, ..Default::default() }), - ..SyncMessage::with_padding() + ..SyncMessage::with_padding(&mut self.csprng) }) } } From 2d9088ec3a2ff53f80d27d6944c8645846f980d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20F=C3=A9ron?= Date: Sun, 3 Nov 2024 21:23:27 +0100 Subject: [PATCH 3/3] Don't store csprng instances in structs anymore --- src/account_manager.rs | 78 +++++++++++++++++--------------- src/cipher.rs | 34 +++++++------- src/pre_keys.rs | 8 ++-- src/profile_cipher.rs | 88 +++++++++++++++++++++++++------------ src/provisioning/cipher.rs | 15 ++----- src/provisioning/mod.rs | 6 +-- src/provisioning/pipe.rs | 12 ++--- src/push_service/profile.rs | 36 +-------------- src/sender.rs | 11 +++-- 9 files changed, 144 insertions(+), 144 deletions(-) diff --git a/src/account_manager.rs b/src/account_manager.rs index 09f02e012..aca1bf6cb 100644 --- a/src/account_manager.rs +++ b/src/account_manager.rs @@ -52,8 +52,7 @@ use crate::{ type Aes256Ctr128BE = ctr::Ctr128BE; -pub struct AccountManager { - csprng: R, +pub struct AccountManager { service: PushService, profile_key: Option, } @@ -74,14 +73,9 @@ pub struct Profile { pub avatar: Option, } -impl AccountManager { - pub fn new( - csprng: R, - service: PushService, - profile_key: Option, - ) -> Self { +impl AccountManager { + pub fn new(service: PushService, profile_key: Option) -> Self { Self { - csprng, service, profile_key, } @@ -94,9 +88,10 @@ impl AccountManager { /// /// Equivalent to Java's RefreshPreKeysJob #[allow(clippy::too_many_arguments)] - #[tracing::instrument(skip(self, protocol_store))] - pub async fn update_pre_key_bundle( + #[tracing::instrument(skip(self, csprng, protocol_store))] + pub async fn update_pre_key_bundle( &mut self, + csprng: &mut R, protocol_store: &mut P, service_id_type: ServiceIdType, use_last_resort_key: bool, @@ -156,8 +151,8 @@ impl AccountManager { let (pre_keys, signed_pre_key, pq_pre_keys, pq_last_resort_key) = crate::pre_keys::replenish_pre_keys( protocol_store, + csprng, &identity_key_pair, - &mut self.csprng, use_last_resort_key && !has_last_resort_key, PRE_KEY_BATCH_SIZE, PRE_KEY_BATCH_SIZE, @@ -283,8 +278,9 @@ impl AccountManager { /// ```java /// TextSecurePreferences.setIsUnidentifiedDeliveryEnabled(context, false); /// ``` - pub async fn link_device( + pub async fn link_device( &mut self, + csprng: &mut R, url: url::Url, aci_identity_store: &dyn IdentityKeyStore, pni_identity_store: &dyn IdentityKeyStore, @@ -346,7 +342,7 @@ impl AccountManager { let cipher = ProvisioningCipher::from_public(pub_key); - let encrypted = cipher.encrypt(&mut self.csprng, msg)?; + let encrypted = cipher.encrypt(csprng, msg)?; self.send_provisioning_message(ephemeral_id, encrypted) .await?; Ok(()) @@ -382,10 +378,12 @@ impl AccountManager { } pub async fn register_account< + R: Rng + CryptoRng, Aci: PreKeysStore + IdentityKeyStore, Pni: PreKeysStore + IdentityKeyStore, >( &mut self, + csprng: &mut R, registration_method: RegistrationMethod<'_>, account_attributes: AccountAttributes, aci_protocol_store: &mut Aci, @@ -408,8 +406,8 @@ impl AccountManager { aci_last_resort_kyber_prekey, ) = crate::pre_keys::replenish_pre_keys( aci_protocol_store, + csprng, &aci_identity_key_pair, - &mut self.csprng, true, 0, 0, @@ -423,8 +421,8 @@ impl AccountManager { pni_last_resort_kyber_prekey, ) = crate::pre_keys::replenish_pre_keys( pni_protocol_store, + csprng, &pni_identity_key_pair, - &mut self.csprng, true, 0, 0, @@ -470,15 +468,19 @@ impl AccountManager { /// ``` /// in which the `retain_avatar` parameter sets whether to remove (`false`) or retain (`true`) the /// currently set avatar. - pub async fn upload_versioned_profile_without_avatar>( + pub async fn upload_versioned_profile_without_avatar< + R: Rng + CryptoRng, + S: AsRef, + >( &mut self, aci: libsignal_protocol::Aci, name: ProfileName, about: Option, about_emoji: Option, retain_avatar: bool, + csprng: &mut R, ) -> Result<(), ProfileManagerError> { - self.upload_versioned_profile::>, _>( + self.upload_versioned_profile::>, _, _>( aci, name, about, @@ -488,6 +490,7 @@ impl AccountManager { } else { AvatarWrite::NoAvatar }, + csprng, ) .await?; Ok(()) @@ -505,8 +508,8 @@ impl AccountManager { .retrieve_profile_by_id(address, Some(profile_key)) .await?; - let profile_cipher = ProfileCipher::new(&mut self.csprng, profile_key); - Ok(encrypted_profile.decrypt(profile_cipher)?) + let profile_cipher = ProfileCipher::new(profile_key); + Ok(profile_cipher.decrypt(encrypted_profile)?) } /// Upload a profile @@ -517,6 +520,7 @@ impl AccountManager { pub async fn upload_versioned_profile< 's, C: std::io::Read + Send + 's, + R: Rng + CryptoRng, S: AsRef, >( &mut self, @@ -525,18 +529,18 @@ impl AccountManager { about: Option, about_emoji: Option, avatar: AvatarWrite<&'s mut C>, + csprng: &mut R, ) -> Result, ProfileManagerError> { let profile_key = self.profile_key.expect("set profile key in AccountManager"); - let mut profile_cipher = - ProfileCipher::new(&mut self.csprng, profile_key); + let profile_cipher = ProfileCipher::new(profile_key); // Profile encryption - let name = profile_cipher.encrypt_name(name.as_ref())?; + let name = profile_cipher.encrypt_name(name.as_ref(), csprng)?; let about = about.unwrap_or_default(); - let about = profile_cipher.encrypt_about(about)?; + let about = profile_cipher.encrypt_about(about, csprng)?; let about_emoji = about_emoji.unwrap_or_default(); - let about_emoji = profile_cipher.encrypt_emoji(about_emoji)?; + let about_emoji = profile_cipher.encrypt_emoji(about_emoji, csprng)?; // If avatar -> upload if matches!(avatar, AvatarWrite::NewAvatar(_)) { @@ -573,13 +577,14 @@ impl AccountManager { } /// Update (encrypted) device name - pub async fn update_device_name( + pub async fn update_device_name( &mut self, device_name: &str, public_key: &IdentityKey, + csprng: &mut R, ) -> Result<(), ServiceError> { let encrypted_device_name = - encrypt_device_name(&mut self.csprng, device_name, public_key)?; + encrypt_device_name(csprng, device_name, public_key)?; #[derive(Serialize)] #[serde(rename_all = "camelCase")] @@ -640,9 +645,9 @@ impl AccountManager { /// Should be called as the primary device to migrate from pre-PNI to PNI. /// /// This is the equivalent of Android's PnpInitializeDevicesJob or iOS' PniHelloWorldManager. - #[tracing::instrument(skip(self, aci_protocol_store, pni_protocol_store, sender, local_aci), fields(local_aci = %local_aci))] + #[tracing::instrument(skip(self, aci_protocol_store, pni_protocol_store, sender, local_aci, csprng), fields(local_aci = %local_aci))] pub async fn pnp_initialize_devices< - // XXX So many constraints here, all imposed by the MessageSender + R: Rng + CryptoRng, Aci: PreKeysStore + SessionStoreExt, Pni: PreKeysStore, AciOrPni: ProtocolStore + SenderKeyStore + SessionStoreExt + Sync + Clone, @@ -653,6 +658,7 @@ impl AccountManager { mut sender: MessageSender, local_aci: ServiceAddress, e164: PhoneNumber, + csprng: &mut R, ) -> Result<(), MessageSenderError> { let pni_identity_key_pair = pni_protocol_store.get_identity_key_pair().await?; @@ -709,8 +715,8 @@ impl AccountManager { ) = if local_device_id == DEFAULT_DEVICE_ID { crate::pre_keys::replenish_pre_keys( pni_protocol_store, + csprng, &pni_identity_key_pair, - &mut self.csprng, true, 0, 0, @@ -718,16 +724,16 @@ impl AccountManager { .await? } else { // Generate a signed prekey - let signed_pre_key_pair = KeyPair::generate(&mut self.csprng); + let signed_pre_key_pair = KeyPair::generate(csprng); let signed_pre_key_public = signed_pre_key_pair.public_key; let signed_pre_key_signature = pni_identity_key_pair.private_key().calculate_signature( &signed_pre_key_public.serialize(), - &mut self.csprng, + csprng, )?; let signed_prekey_record = SignedPreKeyRecord::new( - self.csprng.gen_range::(0..0xFFFFFF).into(), + csprng.gen_range::(0..0xFFFFFF).into(), Timestamp::now(), &signed_pre_key_pair, &signed_pre_key_signature, @@ -736,7 +742,7 @@ impl AccountManager { // Generate a last-resort Kyber prekey let kyber_pre_key_record = KyberPreKeyRecord::generate( kem::KeyType::Kyber1024, - self.csprng.gen_range::(0..0xFFFFFF).into(), + csprng.gen_range::(0..0xFFFFFF).into(), pni_identity_key_pair.private_key(), )?; ( @@ -751,7 +757,7 @@ impl AccountManager { pni_protocol_store.get_local_registration_id().await? } else { loop { - let regid = generate_registration_id(&mut self.csprng); + let regid = generate_registration_id(csprng); if !pni_registration_ids.iter().any(|(_k, v)| *v == regid) { break regid; } @@ -799,7 +805,7 @@ impl AccountManager { e164.format().mode(phonenumber::Mode::E164).to_string(), ), }), - padding: Some(random_length_padding(&mut self.csprng, 512)), + padding: Some(random_length_padding(csprng, 512)), ..SyncMessage::default() }; let content: ContentBody = msg.into(); diff --git a/src/cipher.rs b/src/cipher.rs index 057cc8332..fb5a58030 100644 --- a/src/cipher.rs +++ b/src/cipher.rs @@ -29,19 +29,17 @@ use crate::{ /// /// Equivalent of SignalServiceCipher in Java. #[derive(Clone)] -pub struct ServiceCipher { +pub struct ServiceCipher { protocol_store: S, - csprng: R, trust_root: PublicKey, local_uuid: Uuid, local_device_id: u32, } -impl fmt::Debug for ServiceCipher { +impl fmt::Debug for ServiceCipher { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("ServiceCipher") .field("protocol_store", &"...") - .field("csprng", &"...") .field("trust_root", &"...") .field("local_uuid", &self.local_uuid) .field("local_device_id", &self.local_device_id) @@ -70,21 +68,18 @@ fn debug_envelope(envelope: &Envelope) -> String { } } -impl ServiceCipher +impl ServiceCipher where S: ProtocolStore + SenderKeyStore + SessionStoreExt + Clone, - R: Rng + CryptoRng, { pub fn new( protocol_store: S, - csprng: R, trust_root: PublicKey, local_uuid: Uuid, local_device_id: u32, ) -> Self { Self { protocol_store, - csprng, trust_root, local_uuid, local_device_id, @@ -94,13 +89,14 @@ where /// Opens ("decrypts") an envelope. /// /// Envelopes may be empty, in which case this method returns `Ok(None)` - #[tracing::instrument(skip(envelope), fields(envelope = debug_envelope(&envelope)))] - pub async fn open_envelope( + #[tracing::instrument(skip(envelope, csprng), fields(envelope = debug_envelope(&envelope)))] + pub async fn open_envelope( &mut self, envelope: Envelope, + csprng: &mut R, ) -> Result, ServiceError> { if envelope.content.is_some() { - let plaintext = self.decrypt(&envelope).await?; + let plaintext = self.decrypt(&envelope, csprng).await?; let message = crate::proto::Content::decode(plaintext.data.as_slice())?; if let Some(bytes) = message.sender_key_distribution_message { @@ -126,10 +122,11 @@ where /// Triage of legacy messages happens inside this method, as opposed to the /// Java implementation, because it makes the borrow checker and the /// author happier. - #[tracing::instrument(skip(envelope), fields(envelope = debug_envelope(envelope)))] - async fn decrypt( + #[tracing::instrument(skip(envelope, csprng), fields(envelope = debug_envelope(envelope)))] + async fn decrypt( &mut self, envelope: &Envelope, + csprng: &mut R, ) -> Result { let ciphertext = if let Some(msg) = envelope.content.as_ref() { msg @@ -180,7 +177,7 @@ where &mut self.protocol_store.clone(), &self.protocol_store.clone(), &mut self.protocol_store.clone(), - &mut self.csprng, + csprng, ) .await? .as_slice() @@ -236,7 +233,7 @@ where &sender, &mut self.protocol_store.clone(), &mut self.protocol_store.clone(), - &mut self.csprng, + csprng, ) .await? .as_slice() @@ -319,18 +316,19 @@ where } #[tracing::instrument( - skip(address, unidentified_access, content), + skip(address, unidentified_access, content, csprng), fields( address = %address, with_unidentified_access = unidentified_access.is_some(), content_length = content.len(), ) )] - pub(crate) async fn encrypt( + pub(crate) async fn encrypt( &mut self, address: &ProtocolAddress, unidentified_access: Option<&SenderCertificate>, content: &[u8], + csprng: &mut R, ) -> Result { let session_record = self .protocol_store @@ -354,7 +352,7 @@ where &mut self.protocol_store.clone(), &mut self.protocol_store, SystemTime::now(), - &mut self.csprng, + csprng, ) .await?; diff --git a/src/pre_keys.rs b/src/pre_keys.rs index cef57d5e1..4af0067da 100644 --- a/src/pre_keys.rs +++ b/src/pre_keys.rs @@ -12,6 +12,7 @@ use libsignal_protocol::{ SignedPreKeyRecord, SignedPreKeyStore, Timestamp, }; +use rand::{CryptoRng, Rng}; use serde::{Deserialize, Serialize}; use tracing::Instrument; @@ -175,13 +176,10 @@ pub(crate) const PRE_KEY_MINIMUM: u32 = 10; pub(crate) const PRE_KEY_BATCH_SIZE: u32 = 100; pub(crate) const PRE_KEY_MEDIUM_MAX_VALUE: u32 = 0xFFFFFF; -pub(crate) async fn replenish_pre_keys< - R: rand::Rng + rand::CryptoRng, - P: PreKeysStore, ->( +pub(crate) async fn replenish_pre_keys( protocol_store: &mut P, - identity_key_pair: &IdentityKeyPair, csprng: &mut R, + identity_key_pair: &IdentityKeyPair, use_last_resort_key: bool, pre_key_count: u32, kyber_pre_key_count: u32, diff --git a/src/profile_cipher.rs b/src/profile_cipher.rs index c711f2076..7da962eab 100644 --- a/src/profile_cipher.rs +++ b/src/profile_cipher.rs @@ -4,7 +4,9 @@ use aes_gcm::{aead::Aead, AeadCore, AeadInPlace, Aes256Gcm, KeyInit}; use rand::{CryptoRng, Rng}; use zkgroup::profiles::ProfileKey; -use crate::profile_name::ProfileName; +use crate::{ + profile_name::ProfileName, push_service::SignalServiceProfile, Profile, +}; /// Encrypt and decrypt a [`ProfileName`] and other profile information. /// @@ -26,8 +28,7 @@ use crate::profile_name::ProfileName; /// let decrypted = cipher.decrypt_name(&encrypted).unwrap().unwrap(); /// assert_eq!(decrypted.as_ref(), name); /// ``` -pub struct ProfileCipher { - csprng: R, +pub struct ProfileCipher { profile_key: ProfileKey, } @@ -73,27 +74,25 @@ fn pad_plaintext( Ok(len) } -impl ProfileCipher { - pub fn new(csprng: R, profile_key: ProfileKey) -> Self { - Self { - csprng, - profile_key, - } +impl ProfileCipher { + pub fn new(profile_key: ProfileKey) -> Self { + Self { profile_key } } pub fn into_inner(self) -> ProfileKey { self.profile_key } - fn pad_and_encrypt( - &mut self, + fn pad_and_encrypt( + &self, mut bytes: Vec, padding_brackets: &[usize], + csprng: &mut R, ) -> Result, ProfileCipherError> { let _len = pad_plaintext(&mut bytes, padding_brackets)?; let cipher = Aes256Gcm::new(&self.profile_key.get_bytes().into()); - let nonce = Aes256Gcm::generate_nonce(&mut self.csprng); + let nonce = Aes256Gcm::generate_nonce(csprng); cipher .encrypt_in_place(&nonce, b"", &mut bytes) @@ -132,6 +131,35 @@ impl ProfileCipher { Ok(plaintext) } + pub fn decrypt( + &self, + encrypted_profile: SignalServiceProfile, + ) -> Result { + let name = encrypted_profile + .name + .as_ref() + .map(|data| self.decrypt_name(data)) + .transpose()? + .flatten(); + let about = encrypted_profile + .about + .as_ref() + .map(|data| self.decrypt_about(data)) + .transpose()?; + let about_emoji = encrypted_profile + .about_emoji + .as_ref() + .map(|data| self.decrypt_emoji(data)) + .transpose()?; + + Ok(Profile { + name, + about, + about_emoji, + avatar: encrypted_profile.avatar, + }) + } + pub fn decrypt_avatar( &self, bytes: &[u8], @@ -139,13 +167,14 @@ impl ProfileCipher { self.decrypt_and_unpad(bytes) } - pub fn encrypt_name<'inp>( - &mut self, + pub fn encrypt_name<'inp, R: Rng + CryptoRng>( + &self, name: impl std::borrow::Borrow>, + csprng: &mut R, ) -> Result, ProfileCipherError> { let name = name.borrow(); let bytes = name.serialize(); - self.pad_and_encrypt(bytes, NAME_PADDING_BRACKETS) + self.pad_and_encrypt(bytes, NAME_PADDING_BRACKETS, csprng) } pub fn decrypt_name( @@ -159,12 +188,13 @@ impl ProfileCipher { Ok(ProfileName::::deserialize(&plaintext)?) } - pub fn encrypt_about( - &mut self, + pub fn encrypt_about( + &self, about: String, + csprng: &mut R, ) -> Result, ProfileCipherError> { let bytes = about.into_bytes(); - self.pad_and_encrypt(bytes, ABOUT_PADDING_BRACKETS) + self.pad_and_encrypt(bytes, ABOUT_PADDING_BRACKETS, csprng) } pub fn decrypt_about( @@ -179,12 +209,13 @@ impl ProfileCipher { Ok(std::str::from_utf8(&plaintext)?.into()) } - pub fn encrypt_emoji( - &mut self, + pub fn encrypt_emoji( + &self, emoji: String, + csprng: &mut R, ) -> Result, ProfileCipherError> { let bytes = emoji.into_bytes(); - self.pad_and_encrypt(bytes, &[EMOJI_PADDED_LENGTH]) + self.pad_and_encrypt(bytes, &[EMOJI_PADDED_LENGTH], csprng) } pub fn decrypt_emoji( @@ -225,14 +256,15 @@ mod tests { let mut rng = rand::thread_rng(); let some_randomness = rng.gen(); let profile_key = ProfileKey::generate(some_randomness); - let mut cipher = ProfileCipher::new(rng, profile_key); + let cipher = ProfileCipher::new(profile_key); for name in &names { let profile_name = ProfileName::<&str> { given_name: name, family_name: None, }; assert_eq!(profile_name.serialize().len(), name.len()); - let encrypted = cipher.encrypt_name(&profile_name).unwrap(); + let encrypted = + cipher.encrypt_name(&profile_name, &mut rng).unwrap(); let decrypted = cipher.decrypt_name(encrypted).unwrap().unwrap(); assert_eq!(decrypted.as_ref(), profile_name); @@ -250,10 +282,11 @@ mod tests { let mut rng = rand::thread_rng(); let some_randomness = rng.gen(); let profile_key = ProfileKey::generate(some_randomness); - let mut cipher = ProfileCipher::new(rng, profile_key); + let cipher = ProfileCipher::new(profile_key); for &about in &abouts { - let encrypted = cipher.encrypt_about(about.into()).unwrap(); + let encrypted = + cipher.encrypt_about(about.into(), &mut rng).unwrap(); let decrypted = cipher.decrypt_about(encrypted).unwrap(); assert_eq!(decrypted, about); @@ -267,10 +300,11 @@ mod tests { let mut rng = rand::thread_rng(); let some_randomness = rng.gen(); let profile_key = ProfileKey::generate(some_randomness); - let mut cipher = ProfileCipher::new(rng, profile_key); + let cipher = ProfileCipher::new(profile_key); for &emoji in &emojii { - let encrypted = cipher.encrypt_emoji(emoji.into()).unwrap(); + let encrypted = + cipher.encrypt_emoji(emoji.into(), &mut rng).unwrap(); let decrypted = cipher.decrypt_emoji(encrypted).unwrap(); assert_eq!(decrypted, emoji); diff --git a/src/provisioning/cipher.rs b/src/provisioning/cipher.rs index 526dc372c..b0ec8ab61 100644 --- a/src/provisioning/cipher.rs +++ b/src/provisioning/cipher.rs @@ -54,16 +54,6 @@ pub struct ProvisioningCipher { } impl ProvisioningCipher { - /// Generate a random key pair - pub fn generate( - mut csprng: R, - ) -> Result { - let key_pair = libsignal_protocol::KeyPair::generate(&mut csprng); - Ok(Self { - key_material: CipherMode::DecryptAndEncrypt(key_pair), - }) - } - pub fn from_public(key: PublicKey) -> Self { Self { key_material: CipherMode::EncryptOnly(key), @@ -185,8 +175,9 @@ mod tests { #[test] fn encrypt_provisioning_roundtrip() -> anyhow::Result<()> { let mut rng = rand::thread_rng(); - let cipher = ProvisioningCipher::generate(&mut rng)?; - let encrypt_cipher = + let key_pair = KeyPair::generate(&mut rng); + let cipher = ProvisioningCipher::from_key_pair(key_pair); + let encrypt_cipher: ProvisioningCipher = ProvisioningCipher::from_public(*cipher.public_key()); assert_eq!( diff --git a/src/provisioning/mod.rs b/src/provisioning/mod.rs index cac4c1015..224069305 100644 --- a/src/provisioning/mod.rs +++ b/src/provisioning/mod.rs @@ -158,7 +158,7 @@ pub async fn link_device< let registration_id = csprng.gen_range(1..256); let pni_registration_id = csprng.gen_range(1..256); - let provisioning_pipe = ProvisioningPipe::from_socket(csprng, ws)?; + let provisioning_pipe = ProvisioningPipe::from_socket(ws, csprng); let provision_stream = provisioning_pipe.stream(); pin_mut!(provision_stream); @@ -237,8 +237,8 @@ pub async fn link_device< aci_pq_last_resort_pre_key, ) = crate::pre_keys::replenish_pre_keys( aci_store, - &aci_key_pair, csprng, + &aci_key_pair, true, 0, 0, @@ -257,8 +257,8 @@ pub async fn link_device< pni_pq_last_resort_pre_key, ) = crate::pre_keys::replenish_pre_keys( pni_store, - &pni_key_pair, csprng, + &pni_key_pair, true, 0, 0, diff --git a/src/provisioning/pipe.rs b/src/provisioning/pipe.rs index 594049ab1..89dae9e57 100644 --- a/src/provisioning/pipe.rs +++ b/src/provisioning/pipe.rs @@ -7,6 +7,7 @@ use futures::{ }, prelude::*, }; +use libsignal_protocol::KeyPair; use rand::{CryptoRng, Rng}; use url::Url; @@ -36,13 +37,14 @@ pub enum ProvisioningStep { impl ProvisioningPipe { pub fn from_socket( - csprng: &mut R, ws: SignalWebSocket, - ) -> Result { - Ok(ProvisioningPipe { + csprng: &mut R, + ) -> Self { + let key_pair = KeyPair::generate(csprng); + ProvisioningPipe { ws, - provisioning_cipher: ProvisioningCipher::generate(csprng)?, - }) + provisioning_cipher: ProvisioningCipher::from_key_pair(key_pair), + } } /// Worker task that diff --git a/src/push_service/profile.rs b/src/push_service/profile.rs index c427d7115..1a08b91b7 100644 --- a/src/push_service/profile.rs +++ b/src/push_service/profile.rs @@ -1,4 +1,3 @@ -use rand::{CryptoRng, Rng}; use reqwest::Method; use serde::{Deserialize, Serialize}; use zkgroup::profiles::{ProfileKeyCommitment, ProfileKeyVersion}; @@ -6,10 +5,9 @@ use zkgroup::profiles::{ProfileKeyCommitment, ProfileKeyVersion}; use crate::{ configuration::Endpoint, content::ServiceError, - profile_cipher::ProfileCipherError, push_service::AvatarWrite, utils::{serde_base64, serde_optional_base64}, - Profile, ServiceAddress, + ServiceAddress, }; use super::{DeviceCapabilities, HttpAuthOverride, PushService, ReqwestExt}; @@ -38,38 +36,6 @@ pub struct SignalServiceProfile { pub capabilities: DeviceCapabilities, } -impl SignalServiceProfile { - pub fn decrypt( - &self, - profile_cipher: crate::profile_cipher::ProfileCipher, - ) -> Result { - // Profile decryption - let name = self - .name - .as_ref() - .map(|data| profile_cipher.decrypt_name(data)) - .transpose()? - .flatten(); - let about = self - .about - .as_ref() - .map(|data| profile_cipher.decrypt_about(data)) - .transpose()?; - let about_emoji = self - .about_emoji - .as_ref() - .map(|data| profile_cipher.decrypt_emoji(data)) - .transpose()?; - - Ok(Profile { - name, - about, - about_emoji, - avatar: self.avatar.clone(), - }) - } -} - #[derive(Debug, Serialize)] #[serde(rename_all = "camelCase")] struct SignalServiceProfileWrite<'s> { diff --git a/src/sender.rs b/src/sender.rs index 8fb959955..ea7cd2f9f 100644 --- a/src/sender.rs +++ b/src/sender.rs @@ -87,7 +87,7 @@ pub struct MessageSender { identified_ws: SignalWebSocket, unidentified_ws: SignalWebSocket, service: PushService, - cipher: ServiceCipher, + cipher: ServiceCipher, csprng: R, protocol_store: S, local_aci: ServiceAddress, @@ -160,7 +160,7 @@ where identified_ws: SignalWebSocket, unidentified_ws: SignalWebSocket, service: PushService, - cipher: ServiceCipher, + cipher: ServiceCipher, csprng: R, protocol_store: S, local_aci: impl Into, @@ -1031,7 +1031,12 @@ where let message = self .cipher - .encrypt(&recipient_protocol_address, unidentified_access, content) + .encrypt( + &recipient_protocol_address, + unidentified_access, + content, + &mut self.csprng, + ) .instrument(tracing::trace_span!("encrypting message")) .await?;