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

[Partial revert] Make sure we use the provided CSPRNG everywhere #342

Merged
merged 4 commits into from
Nov 5, 2024
Merged

[Partial revert] Make sure we use the provided CSPRNG everywhere #342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7222de4
Revert "Make sure we really use thread_rng everywhere (#341)"
gferon Nov 3, 2024
b59f8ad
Make sure we use the provided CSPRNG everywhere relevant
gferon Nov 3, 2024
2d9088e
Don't store csprng instances in structs anymore
gferon Nov 3, 2024
3b19497
Merge remote-tracking branch 'origin/main' into rng-template-param
gferon Nov 5, 2024
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
76 changes: 39 additions & 37 deletions src/account_manager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use base64::prelude::*;
use phonenumber::PhoneNumber;
use rand::Rng;
use rand::{CryptoRng, Rng};
use reqwest::Method;
use std::collections::HashMap;
use std::convert::{TryFrom, TryInto};
Expand Down Expand Up @@ -88,16 +88,13 @@ 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, csprng, protocol_store))]
pub async fn update_pre_key_bundle<R: Rng + CryptoRng, P: PreKeysStore>(
&mut self,
protocol_store: &mut P,
service_id_kind: ServiceIdKind,
csprng: &mut R,
use_last_resort_key: bool,
csprng: &mut R,
) -> Result<(), ServiceError> {
let prekey_status = match self
.service
Expand Down Expand Up @@ -154,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,
&identity_key_pair,
csprng,
&identity_key_pair,
use_last_resort_key && !has_last_resort_key,
PRE_KEY_BATCH_SIZE,
PRE_KEY_BATCH_SIZE,
Expand Down Expand Up @@ -281,8 +278,9 @@ impl AccountManager {
/// ```java
/// TextSecurePreferences.setIsUnidentifiedDeliveryEnabled(context, false);
/// ```
pub async fn link_device(
pub async fn link_device<R: Rng + CryptoRng>(
&mut self,
csprng: &mut R,
url: url::Url,
aci_identity_store: &dyn IdentityKeyStore,
pni_identity_store: &dyn IdentityKeyStore,
Expand Down Expand Up @@ -344,7 +342,7 @@ impl AccountManager {

let cipher = ProvisioningCipher::from_public(pub_key);

let encrypted = cipher.encrypt(msg)?;
let encrypted = cipher.encrypt(csprng, msg)?;
self.send_provisioning_message(ephemeral_id, encrypted)
.await?;
Ok(())
Expand Down Expand Up @@ -380,7 +378,7 @@ impl AccountManager {
}

pub async fn register_account<
R: rand::Rng + rand::CryptoRng,
R: Rng + CryptoRng,
Aci: PreKeysStore + IdentityKeyStore,
Pni: PreKeysStore + IdentityKeyStore,
>(
Expand Down Expand Up @@ -408,8 +406,8 @@ impl AccountManager {
aci_last_resort_kyber_prekey,
) = crate::pre_keys::replenish_pre_keys(
aci_protocol_store,
&aci_identity_key_pair,
csprng,
&aci_identity_key_pair,
true,
0,
0,
Expand All @@ -423,8 +421,8 @@ impl AccountManager {
pni_last_resort_kyber_prekey,
) = crate::pre_keys::replenish_pre_keys(
pni_protocol_store,
&pni_identity_key_pair,
csprng,
&pni_identity_key_pair,
true,
0,
0,
Expand Down Expand Up @@ -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<S: AsRef<str>>(
pub async fn upload_versioned_profile_without_avatar<
R: Rng + CryptoRng,
S: AsRef<str>,
>(
&mut self,
aci: libsignal_protocol::Aci,
name: ProfileName<S>,
about: Option<String>,
about_emoji: Option<String>,
retain_avatar: bool,
csprng: &mut R,
) -> Result<(), ProfileManagerError> {
self.upload_versioned_profile::<std::io::Cursor<Vec<u8>>, _>(
self.upload_versioned_profile::<std::io::Cursor<Vec<u8>>, _, _>(
aci,
name,
about,
Expand All @@ -488,6 +490,7 @@ impl AccountManager {
} else {
AvatarWrite::NoAvatar
},
csprng,
)
.await?;
Ok(())
Expand All @@ -505,8 +508,8 @@ impl AccountManager {
.retrieve_profile_by_id(address, Some(profile_key))
.await?;

let profile_cipher = ProfileCipher::from(profile_key);
Ok(encrypted_profile.decrypt(profile_cipher)?)
let profile_cipher = ProfileCipher::new(profile_key);
Ok(profile_cipher.decrypt(encrypted_profile)?)
}

/// Upload a profile
Expand All @@ -517,6 +520,7 @@ impl AccountManager {
pub async fn upload_versioned_profile<
's,
C: std::io::Read + Send + 's,
R: Rng + CryptoRng,
S: AsRef<str>,
>(
&mut self,
Expand All @@ -525,17 +529,18 @@ impl AccountManager {
about: Option<String>,
about_emoji: Option<String>,
avatar: AvatarWrite<&'s mut C>,
csprng: &mut R,
) -> Result<Option<String>, ProfileManagerError> {
let profile_key =
self.profile_key.expect("set profile key in AccountManager");
let profile_cipher = ProfileCipher::from(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(_)) {
Expand Down Expand Up @@ -572,16 +577,14 @@ impl AccountManager {
}

/// Update (encrypted) device name
pub async fn update_device_name(
pub async fn update_device_name<R: Rng + CryptoRng>(
&mut self,
device_name: &str,
public_key: &IdentityKey,
csprng: &mut R,
) -> Result<(), ServiceError> {
let encrypted_device_name = encrypt_device_name(
&mut rand::thread_rng(),
device_name,
public_key,
)?;
let encrypted_device_name =
encrypt_device_name(csprng, device_name, public_key)?;

#[derive(Serialize)]
#[serde(rename_all = "camelCase")]
Expand Down Expand Up @@ -642,22 +645,21 @@ 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.service_id_string()))]
#[tracing::instrument(skip(self, aci_protocol_store, pni_protocol_store, sender, local_aci, csprng), fields(local_aci = local_aci.service_id_string()))]
pub async fn pnp_initialize_devices<
// XXX So many constraints here, all imposed by the MessageSender
R: rand::Rng + rand::CryptoRng,
R: Rng + CryptoRng,
AciStore: PreKeysStore + SessionStoreExt,
PniStore: PreKeysStore,
AciOrPni: ProtocolStore + SenderKeyStore + SessionStoreExt + Sync + Clone,
>(
&mut self,
aci_protocol_store: &mut AciStore,
pni_protocol_store: &mut PniStore,
mut sender: MessageSender<AciOrPni>,
mut sender: MessageSender<AciOrPni, R>,
local_aci: Aci,
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,21 +715,21 @@ impl AccountManager {
) = if local_device_id == DEFAULT_DEVICE_ID {
crate::pre_keys::replenish_pre_keys(
pni_protocol_store,
csprng,
&pni_identity_key_pair,
&mut csprng,
true,
0,
0,
)
.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(
Expand Down Expand Up @@ -755,7 +757,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;
}
Expand Down Expand Up @@ -803,7 +805,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();
Expand Down
25 changes: 14 additions & 11 deletions src/cipher.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use libsignal_protocol::{
SignalMessage, SignalProtocolError, SignedPreKeyStore, Timestamp,
};
use prost::Message;
use rand::{CryptoRng, Rng};
use uuid::Uuid;

use crate::{
Expand All @@ -39,7 +40,6 @@ impl<S> fmt::Debug for ServiceCipher<S> {
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)
Expand Down Expand Up @@ -89,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<R: Rng + CryptoRng>(
&mut self,
envelope: Envelope,
csprng: &mut R,
) -> Result<Option<Content>, 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 {
Expand All @@ -121,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<R: Rng + CryptoRng>(
&mut self,
envelope: &Envelope,
csprng: &mut R,
) -> Result<Plaintext, ServiceError> {
let ciphertext = if let Some(msg) = envelope.content.as_ref() {
msg
Expand Down Expand Up @@ -175,7 +177,7 @@ where
&mut self.protocol_store.clone(),
&self.protocol_store.clone(),
&mut self.protocol_store.clone(),
&mut rand::thread_rng(),
csprng,
)
.await?
.as_slice()
Expand Down Expand Up @@ -231,7 +233,7 @@ where
&sender,
&mut self.protocol_store.clone(),
&mut self.protocol_store.clone(),
&mut rand::thread_rng(),
csprng,
)
.await?
.as_slice()
Expand Down Expand Up @@ -317,18 +319,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<R: Rng + CryptoRng>(
&mut self,
address: &ProtocolAddress,
unidentified_access: Option<&SenderCertificate>,
content: &[u8],
csprng: &mut R,
) -> Result<OutgoingPushMessage, ServiceError> {
let session_record = self
.protocol_store
Expand All @@ -352,7 +355,7 @@ where
&mut self.protocol_store.clone(),
&mut self.protocol_store,
SystemTime::now(),
&mut rand::thread_rng(),
csprng,
)
.await?;

Expand Down
Loading
Loading