Skip to content
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

Make sure we really use thread_rng everywhere #341

Merged
merged 1 commit into from
Nov 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading