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.

Sign up for GitHub

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

fix: rng state reusage #292

Merged
merged 1 commit into from
Nov 9, 2024
Merged

fix: rng state reusage #292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 11 additions & 10 deletions presage/src/manager/confirmation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use libsignal_service::push_service::{
};
use libsignal_service::zkgroup::profiles::ProfileKey;
use libsignal_service::AccountManager;
use rand::RngCore;
use rand::{thread_rng, RngCore};
use tracing::trace;

use crate::manager::registered::RegistrationData;
Expand All @@ -35,13 +35,15 @@ impl<S: Store> Manager<S, Confirmation> {
/// Returns a [registered manager](Manager::load_registered) that you can use
/// to send and receive messages.
pub async fn confirm_verification_code(
mut self,
self,
confirmation_code: impl AsRef<str>,
) -> Result<Manager<S, Registered>, Error<S::Error>> {
trace!("confirming verification code");

let registration_id = generate_registration_id(&mut self.csprng);
let pni_registration_id = generate_registration_id(&mut self.csprng);
let mut rng = thread_rng();

let registration_id = generate_registration_id(&mut rng);
let pni_registration_id = generate_registration_id(&mut rng);

let Confirmation {
signal_servers,
Expand Down Expand Up @@ -75,19 +77,19 @@ impl<S: Store> Manager<S, Confirmation> {

// generate a 52 bytes signaling key
let mut signaling_key = [0u8; 52];
self.csprng.fill_bytes(&mut signaling_key);
rng.fill_bytes(&mut signaling_key);

// generate a 32 bytes profile key
let mut profile_key = [0u8; 32];
self.csprng.fill_bytes(&mut profile_key);
rng.fill_bytes(&mut profile_key);
let profile_key = ProfileKey::generate(profile_key);

// generate new identity keys used in `register_account` and below
self.store
.set_aci_identity_key_pair(IdentityKeyPair::generate(&mut self.csprng))
.set_aci_identity_key_pair(IdentityKeyPair::generate(&mut rng))
.await?;
self.store
.set_pni_identity_key_pair(IdentityKeyPair::generate(&mut self.csprng))
.set_pni_identity_key_pair(IdentityKeyPair::generate(&mut rng))
.await?;

let skip_device_transfer = true;
Expand All @@ -100,7 +102,7 @@ impl<S: Store> Manager<S, Confirmation> {
number: _,
} = account_manager
.register_account(
&mut self.csprng,
&mut rng,
RegistrationMethod::SessionId(&session.id),
AccountAttributes {
signaling_key: Some(signaling_key.to_vec()),
Expand All @@ -126,7 +128,6 @@ impl<S: Store> Manager<S, Confirmation> {
trace!("confirmed! (and registered)");

let mut manager = Manager {
csprng: self.csprng,
store: self.store,
state: Registered::with_data(RegistrationData {
signal_servers: self.state.signal_servers,
Expand Down
6 changes: 2 additions & 4 deletions presage/src/manager/linking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ use libsignal_service::provisioning::{
link_device, NewDeviceRegistration, SecondaryDeviceProvisioning,
};
use rand::distributions::{Alphanumeric, DistString};
use rand::rngs::StdRng;
use rand::{RngCore, SeedableRng};
use rand::{thread_rng, RngCore};
use tracing::info;
use url::Url;

Expand Down Expand Up @@ -68,7 +67,7 @@ impl<S: Store> Manager<S, Linking> {
store.clear_registration().await?;

// generate a random alphanumeric 24 chars password
let mut rng = StdRng::from_entropy();
let mut rng = thread_rng();
let password = Alphanumeric.sample_string(&mut rng, 24);

// generate a 52 bytes signaling key
Expand Down Expand Up @@ -158,7 +157,6 @@ impl<S: Store> Manager<S, Linking> {
);

let mut manager = Manager {
csprng: rng,
store: store.clone(),
state: Registered::with_data(registration_data),
};
Expand Down
4 changes: 0 additions & 4 deletions presage/src/manager/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ mod registration;

use std::fmt;

use rand::rngs::StdRng;

pub use self::confirmation::Confirmation;
pub use self::linking::Linking;
pub use self::registered::{ReceivingMode, Registered, RegistrationData, RegistrationType};
Expand All @@ -27,8 +25,6 @@ pub struct Manager<Store, State> {
store: Store,
/// Part of the manager which is persisted in the store.
state: State,
/// Random number generator
csprng: StdRng,
}

impl<Store, State: fmt::Debug> fmt::Debug for Manager<Store, State> {
Expand Down
44 changes: 18 additions & 26 deletions presage/src/manager/registered.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ use libsignal_service::websocket::SignalWebSocket;
use libsignal_service::zkgroup::groups::{GroupMasterKey, GroupSecretParams};
use libsignal_service::zkgroup::profiles::ProfileKey;
use libsignal_service::{cipher, AccountManager, Profile, ServiceIdExt};
use rand::rngs::StdRng;
use rand::{CryptoRng, Rng, SeedableRng};
use rand::rngs::ThreadRng;
use rand::thread_rng;
use serde::{Deserialize, Serialize};
use sha2::Digest;
use tokio::sync::Mutex;
Expand All @@ -49,7 +49,7 @@ use crate::store::{ContentsStore, Sticker, StickerPack, StickerPackManifest, Sto
use crate::{model::groups::Group, AvatarBytes, Error, Manager};

type ServiceCipher<S> = cipher::ServiceCipher<S>;
type MessageSender<S> = libsignal_service::prelude::MessageSender<S, StdRng>;
type MessageSender<S> = libsignal_service::prelude::MessageSender<S, ThreadRng>;

#[derive(Clone, Debug, PartialEq, Eq)]
pub enum RegistrationType {
Expand Down Expand Up @@ -138,7 +138,6 @@ impl<S: Store> Manager<S, Registered> {
.ok_or(Error::NotYetRegisteredError)?;

let mut manager = Self {
csprng: StdRng::from_entropy(),
store,
state: Registered::with_data(registration_data),
};
Expand Down Expand Up @@ -250,12 +249,14 @@ impl<S: Store> Manager<S, Registered> {
Some(self.state.data.profile_key),
);

let mut rng = thread_rng();

account_manager
.update_pre_key_bundle(
&mut self.store.aci_protocol_store(),
ServiceIdKind::Aci,
true,
&mut self.csprng,
&mut rng,
)
.await?;

Expand All @@ -264,7 +265,7 @@ impl<S: Store> Manager<S, Registered> {
&mut self.store.pni_protocol_store(),
ServiceIdKind::Pni,
true,
&mut self.csprng,
&mut rng,
)
.await?;

Expand All @@ -284,7 +285,7 @@ impl<S: Store> Manager<S, Registered> {
pni_registration_id
} else {
info!("migrating to PNI");
let pni_registration_id = generate_registration_id(&mut StdRng::from_entropy());
let pni_registration_id = generate_registration_id(&mut thread_rng());
self.store.save_registration_data(&self.state.data).await?;
pni_registration_id
};
Expand Down Expand Up @@ -360,7 +361,7 @@ impl<S: Store> Manager<S, Registered> {
request: Some(sync_message::Request {
r#type: Some(sync_message::request::Type::Contacts.into()),
}),
..SyncMessage::with_padding(&mut self.csprng)
..SyncMessage::with_padding(&mut thread_rng())
};

let timestamp = SystemTime::now()
Expand Down Expand Up @@ -493,7 +494,6 @@ impl<S: Store> Manager<S, Registered> {

let mut gm = self.groups_manager()?;
let Some(group) = upsert_group(
&mut self.csprng,
&self.store,
&mut gm,
context.master_key(),
Expand Down Expand Up @@ -609,10 +609,10 @@ impl<S: Store> Manager<S, Registered> {
&mut self,
mode: ReceivingMode,
) -> Result<impl Stream<Item = Content>, Error<S::Error>> {
struct StreamState<Receiver, Store, AciStore, PniStore, R> {
struct StreamState<Receiver, Store, AciStore, PniStore> {
store: Store,
push_service: PushService,
csprng: R,
csprng: ThreadRng,
encrypted_messages: Receiver,
message_receiver: MessageReceiver,
service_cipher_aci: ServiceCipher<AciStore>,
Expand All @@ -626,7 +626,7 @@ impl<S: Store> Manager<S, Registered> {
let init = StreamState {
store: self.store.clone(),
push_service: push_service.clone(),
csprng: self.csprng.clone(),
csprng: thread_rng(),
encrypted_messages: Box::pin(self.receive_messages_encrypted().await?),
message_receiver: MessageReceiver::new(push_service),
service_cipher_aci: self.new_service_cipher_aci(),
Expand Down Expand Up @@ -780,7 +780,6 @@ impl<S: Store> Manager<S, Registered> {
// and the group changes, which are part of the protobuf messages
// this means we kinda need our own internal representation of groups inside of presage?
if let Ok(Some(group)) = upsert_group(
&mut state.csprng,
&state.store,
&mut state.groups_manager,
master_key_bytes,
Expand Down Expand Up @@ -940,14 +939,8 @@ impl<S: Store> Manager<S, Registered> {
let mut sender = self.new_message_sender().await?;

let mut groups_manager = self.groups_manager()?;
let Some(group) = upsert_group(
&mut self.csprng,
&self.store,
&mut groups_manager,
&master_key_bytes,
&0,
)
.await?
let Some(group) =
upsert_group(&self.store, &mut groups_manager, &master_key_bytes, &0).await?
else {
return Err(Error::UnknownGroup);
};
Expand Down Expand Up @@ -1196,7 +1189,7 @@ impl<S: Store> Manager<S, Registered> {
unidentified_websocket,
self.identified_push_service(),
self.new_service_cipher_aci(),
self.csprng.clone(),
thread_rng(),
aci_protocol_store,
self.state.data.service_ids.aci,
self.state.data.service_ids.pni,
Expand Down Expand Up @@ -1275,7 +1268,7 @@ impl<S: Store> Manager<S, Registered> {

account_manager
.link_device(
&mut self.csprng,
&mut thread_rng(),
secondary,
&self.store.aci_protocol_store(),
&self.store.pni_protocol_store(),
Expand Down Expand Up @@ -1324,8 +1317,7 @@ pub enum ReceivingMode {
WaitForContacts,
}

async fn upsert_group<R: Rng + CryptoRng, S: Store>(
csprng: &mut R,
async fn upsert_group<S: Store>(
store: &S,
groups_manager: &mut GroupsManager<InMemoryCredentialsCache>,
master_key_bytes: &[u8],
Expand All @@ -1346,7 +1338,7 @@ async fn upsert_group<R: Rng + CryptoRng, S: Store>(
if upsert_group {
debug!("fetching and saving group");
match groups_manager
.fetch_encrypted_group(csprng, master_key_bytes)
.fetch_encrypted_group(&mut thread_rng(), master_key_bytes)
.await
{
Ok(encrypted_group) => {
Expand Down
6 changes: 2 additions & 4 deletions presage/src/manager/registration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ use libsignal_service::configuration::{ServiceConfiguration, SignalServers};
use libsignal_service::prelude::phonenumber::PhoneNumber;
use libsignal_service::push_service::{PushService, VerificationTransport};
use rand::distributions::{Alphanumeric, DistString};
use rand::rngs::StdRng;
use rand::SeedableRng;
use rand::thread_rng;
use tracing::trace;

use crate::store::Store;
Expand Down Expand Up @@ -82,7 +81,7 @@ impl<S: Store> Manager<S, Registration> {
store.clear_registration().await?;

// generate a random alphanumeric 24 chars password
let mut rng = StdRng::from_entropy();
let mut rng = thread_rng();
let password = Alphanumeric.sample_string(&mut rng, 24);

let service_configuration: ServiceConfiguration = signal_servers.into();
Expand Down Expand Up @@ -136,7 +135,6 @@ impl<S: Store> Manager<S, Registration> {
password,
session_id: session.id,
},
csprng: rng,
};

Ok(manager)
Expand Down
2 changes: 1 addition & 1 deletion presage/src/store.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ pub async fn save_trusted_identity_message<S: Store>(
let thread = Thread::Contact(sender.raw_uuid());
let verified_sync_message = Content {
metadata: Metadata {
sender: sender,
sender,
destination: sender,
sender_device: 0,
server_guid: None,
Expand Down