Skip to content

Commit

Permalink
Make sure we really use thread_rng everywhere (#341)
Browse files Browse the repository at this point in the history
We already rely on rand::thread_rng everywhere in libsignal-service-rs but the ServiceCipher and MessageSender inherited the parameter from a while ago.
  • Loading branch information
gferon authored Nov 2, 2024
1 parent 09fe094 commit b0fcd8b
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 31 deletions.
17 changes: 9 additions & 8 deletions src/account_manager.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use base64::prelude::*;
use phonenumber::PhoneNumber;
use rand::Rng;
use reqwest::Method;
use std::collections::HashMap;
use std::convert::{TryFrom, TryInto};
Expand Down Expand Up @@ -641,7 +642,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, 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,
Expand All @@ -652,11 +653,11 @@ impl AccountManager {
&mut self,
aci_protocol_store: &mut Aci,
pni_protocol_store: &mut Pni,
mut sender: MessageSender<AciOrPni, R>,
mut sender: MessageSender<AciOrPni>,
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?;

Expand Down Expand Up @@ -713,20 +714,20 @@ impl AccountManager {
crate::pre_keys::replenish_pre_keys(
pni_protocol_store,
&pni_identity_key_pair,
csprng,
&mut csprng,
true,
0,
0,
)
.await?
} else {
// Generate a signed prekey
let signed_pre_key_pair = KeyPair::generate(csprng);
let signed_pre_key_pair = KeyPair::generate(&mut 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 csprng,
)?;

let signed_prekey_record = SignedPreKeyRecord::new(
Expand Down Expand Up @@ -754,7 +755,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 csprng);
if !pni_registration_ids.iter().any(|(_k, v)| *v == regid) {
break regid;
}
Expand Down Expand Up @@ -802,7 +803,7 @@ impl AccountManager {
e164.format().mode(phonenumber::Mode::E164).to_string(),
),
}),
padding: Some(random_length_padding(csprng, 512)),
padding: Some(random_length_padding(&mut csprng, 512)),
..SyncMessage::default()
};
let content: ContentBody = msg.into();
Expand Down
17 changes: 6 additions & 11 deletions src/cipher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use libsignal_protocol::{
SignalProtocolError, SignedPreKeyStore, Timestamp,
};
use prost::Message;
use rand::{CryptoRng, Rng};
use uuid::Uuid;

use crate::{
Expand All @@ -29,15 +28,14 @@ use crate::{
///
/// Equivalent of SignalServiceCipher in Java.
#[derive(Clone)]
pub struct ServiceCipher<S, R> {
pub struct ServiceCipher<S> {
protocol_store: S,
csprng: R,
trust_root: PublicKey,
local_uuid: Uuid,
local_device_id: u32,
}

impl<S, R> fmt::Debug for ServiceCipher<S, R> {
impl<S> fmt::Debug for ServiceCipher<S> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("ServiceCipher")
.field("protocol_store", &"...")
Expand Down Expand Up @@ -70,21 +68,18 @@ fn debug_envelope(envelope: &Envelope) -> String {
}
}

impl<S, R> ServiceCipher<S, R>
impl<S> ServiceCipher<S>
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,
Expand Down Expand Up @@ -180,7 +175,7 @@ where
&mut self.protocol_store.clone(),
&self.protocol_store.clone(),
&mut self.protocol_store.clone(),
&mut self.csprng,
&mut rand::thread_rng(),
)
.await?
.as_slice()
Expand Down Expand Up @@ -236,7 +231,7 @@ where
&sender,
&mut self.protocol_store.clone(),
&mut self.protocol_store.clone(),
&mut self.csprng,
&mut rand::thread_rng(),
)
.await?
.as_slice()
Expand Down Expand Up @@ -354,7 +349,7 @@ where
&mut self.protocol_store.clone(),
&mut self.protocol_store,
SystemTime::now(),
&mut self.csprng,
&mut rand::thread_rng(),
)
.await?;

Expand Down
19 changes: 7 additions & 12 deletions src/sender.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ 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;
Expand Down Expand Up @@ -83,12 +82,11 @@ pub struct AttachmentSpec {
}

#[derive(Clone)]
pub struct MessageSender<S, R> {
pub struct MessageSender<S> {
identified_ws: SignalWebSocket,
unidentified_ws: SignalWebSocket,
service: PushService,
cipher: ServiceCipher<S, R>,
csprng: R,
cipher: ServiceCipher<S>,
protocol_store: S,
local_aci: ServiceAddress,
local_pni: ServiceAddress,
Expand Down Expand Up @@ -150,18 +148,16 @@ pub struct EncryptedMessages {
used_identity_key: IdentityKey,
}

impl<S, R> MessageSender<S, R>
impl<S> MessageSender<S>
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<S, R>,
csprng: R,
cipher: ServiceCipher<S>,
protocol_store: S,
local_aci: impl Into<ServiceAddress>,
local_pni: impl Into<ServiceAddress>,
Expand All @@ -174,7 +170,6 @@ where
identified_ws,
unidentified_ws,
cipher,
csprng,
protocol_store,
local_aci: local_aci.into(),
local_pni: local_pni.into(),
Expand Down Expand Up @@ -625,7 +620,7 @@ where
&mut self.protocol_store,
&pre_key,
SystemTime::now(),
&mut self.csprng,
&mut rand::thread_rng(),
)
.await
.map_err(|e| {
Expand Down Expand Up @@ -838,7 +833,7 @@ where
.expect("PNI key set when PNI signature requested")
.sign_alternate_identity(
self.aci_identity.identity_key(),
&mut self.csprng,
&mut rand::thread_rng(),
)?;
Ok(crate::proto::PniSignatureMessage {
pni: Some(self.local_pni.uuid.as_bytes().to_vec()),
Expand Down Expand Up @@ -1025,7 +1020,7 @@ where
&mut self.protocol_store,
&pre_key_bundle,
SystemTime::now(),
&mut self.csprng,
&mut rand::thread_rng(),
)
.await?;
}
Expand Down

0 comments on commit b0fcd8b

Please sign in to comment.