From 46f6226f69f3c6562e01873ae57b8b0b6db024a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20F=C3=A9ron?= Date: Thu, 16 Nov 2023 21:15:19 +0100 Subject: [PATCH] fix: trust new identities (#206) --- presage-cli/src/main.rs | 2 + presage-store-sled/src/lib.rs | 73 ++++++++++++++++++++++------- presage/src/manager/linking.rs | 4 +- presage/src/manager/registration.rs | 4 +- presage/src/store.rs | 54 +++++++++++++++++++-- 5 files changed, 111 insertions(+), 26 deletions(-) diff --git a/presage-cli/src/main.rs b/presage-cli/src/main.rs index 03889ad99..7d5427095 100644 --- a/presage-cli/src/main.rs +++ b/presage-cli/src/main.rs @@ -33,6 +33,7 @@ use presage::{ Manager, }; use presage_store_sled::MigrationConflictStrategy; +use presage_store_sled::OnNewIdentity; use presage_store_sled::SledStore; use tempfile::Builder; use tokio::task; @@ -203,6 +204,7 @@ async fn main() -> anyhow::Result<()> { db_path, args.passphrase, MigrationConflictStrategy::Raise, + OnNewIdentity::Trust, )?; run(args.subcommand, config_store).await } diff --git a/presage-store-sled/src/lib.rs b/presage-store-sled/src/lib.rs index c6e7547e6..a22e831f6 100644 --- a/presage-store-sled/src/lib.rs +++ b/presage-store-sled/src/lib.rs @@ -24,8 +24,8 @@ use presage::libsignal_service::{ session_store::SessionStoreExt, Profile, ServiceAddress, }; -use presage::manager::RegistrationData; use presage::store::{ContentExt, ContentsStore, PreKeyStoreExt, StateStore, Store, Thread}; +use presage::{manager::RegistrationData, proto::verified}; use prost::Message; use serde::{de::DeserializeOwned, Deserialize, Serialize}; use sha2::{Digest, Sha256}; @@ -64,6 +64,8 @@ pub struct SledStore { db: Arc>, #[cfg(feature = "encryption")] cipher: Option>, + /// Whether to trust new identities automatically (for instance, when a somebody's phone has changed) + trust_new_identities: OnNewIdentity, } /// Sometimes Migrations can't proceed without having to drop existing @@ -111,11 +113,18 @@ impl SchemaVersion { } } +#[derive(Debug, Clone)] +pub enum OnNewIdentity { + Reject, + Trust, +} + impl SledStore { #[allow(unused_variables)] fn new( db_path: impl AsRef, passphrase: Option>, + trust_new_identities: OnNewIdentity, ) -> Result { let database = sled::open(db_path)?; @@ -133,25 +142,33 @@ impl SledStore { db: Arc::new(RwLock::new(database)), #[cfg(feature = "encryption")] cipher: cipher.map(Arc::new), + trust_new_identities, }) } pub fn open( db_path: impl AsRef, migration_conflict_strategy: MigrationConflictStrategy, + trust_new_identities: OnNewIdentity, ) -> Result { - Self::open_with_passphrase(db_path, None::<&str>, migration_conflict_strategy) + Self::open_with_passphrase( + db_path, + None::<&str>, + migration_conflict_strategy, + trust_new_identities, + ) } pub fn open_with_passphrase( db_path: impl AsRef, passphrase: Option>, migration_conflict_strategy: MigrationConflictStrategy, + trust_new_identities: OnNewIdentity, ) -> Result { let passphrase = passphrase.as_ref(); migrate(&db_path, passphrase, migration_conflict_strategy)?; - Self::new(db_path, passphrase) + Self::new(db_path, passphrase, trust_new_identities) } #[cfg(feature = "encryption")] @@ -182,6 +199,7 @@ impl SledStore { #[cfg(feature = "encryption")] // use store cipher with a random key cipher: Some(Arc::new(presage_store_cipher::StoreCipher::new())), + trust_new_identities: OnNewIdentity::Reject }) } @@ -297,7 +315,7 @@ fn migrate( let passphrase = passphrase.as_ref(); let run_migrations = move || { - let mut store = SledStore::new(db_path, passphrase)?; + let mut store = SledStore::new(db_path, passphrase, OnNewIdentity::Reject)?; let schema_version = store.schema_version(); for step in schema_version.steps() { match &step { @@ -492,7 +510,7 @@ impl ContentsStore for SledStore { Ok(()) } - fn save_message(&mut self, thread: &Thread, message: Content) -> Result<(), SledStoreError> { + fn save_message(&self, thread: &Thread, message: Content) -> Result<(), SledStoreError> { let ts = message.timestamp(); log::trace!("storing a message with thread: {thread}, timestamp: {ts}",); @@ -968,17 +986,28 @@ impl IdentityKeyStore for SledStore { identity_key: &IdentityKey, ) -> Result { trace!("saving identity"); - self.insert( - SLED_TREE_IDENTITIES, - address.to_string(), - identity_key.serialize(), - ) - .map_err(|e| { - error!("error saving identity for {:?}: {}", address, e); - e.into_signal_error() - })?; + let existed_before = self + .insert( + SLED_TREE_IDENTITIES, + address.to_string(), + identity_key.serialize(), + ) + .map_err(|e| { + error!("error saving identity for {:?}: {}", address, e); + e.into_signal_error() + })?; + + self.save_trusted_identity_message( + address, + *identity_key, + if existed_before { + verified::State::Unverified + } else { + verified::State::Default + }, + ); - Ok(false) + Ok(true) } async fn is_trusted_identity( @@ -998,7 +1027,17 @@ impl IdentityKeyStore for SledStore { warn!("trusting new identity {:?}", address); Ok(true) } - Some(left_identity_key) => Ok(left_identity_key == *right_identity_key), + // when we encounter some identity we know, we need to decide whether we trust it or not + Some(left_identity_key) => { + if left_identity_key == *right_identity_key { + Ok(true) + } else { + match self.trust_new_identities { + OnNewIdentity::Trust => Ok(true), + OnNewIdentity::Reject => Ok(false), + } + } + } } } @@ -1269,7 +1308,7 @@ mod tests { #[quickcheck_async::tokio] async fn test_store_messages(thread: Thread, content: Content) -> anyhow::Result<()> { - let mut db = SledStore::temporary()?; + let db = SledStore::temporary()?; let thread = thread.0; db.save_message(&thread, content_with_timestamp(&content, 1678295210))?; db.save_message(&thread, content_with_timestamp(&content, 1678295220))?; diff --git a/presage/src/manager/linking.rs b/presage/src/manager/linking.rs index d5cb9db7c..d761a72ab 100644 --- a/presage/src/manager/linking.rs +++ b/presage/src/manager/linking.rs @@ -28,12 +28,12 @@ impl Manager { /// use futures::{channel::oneshot, future, StreamExt}; /// use presage::libsignal_service::configuration::SignalServers; /// use presage::Manager; - /// use presage_store_sled::{MigrationConflictStrategy, SledStore}; + /// use presage_store_sled::{MigrationConflictStrategy, OnNewIdentity, SledStore}; /// /// #[tokio::main] /// async fn main() -> Result<(), Box> { /// let store = - /// SledStore::open("/tmp/presage-example", MigrationConflictStrategy::Drop)?; + /// SledStore::open("/tmp/presage-example", MigrationConflictStrategy::Drop, OnNewIdentity::Trust)?; /// /// let (mut tx, mut rx) = oneshot::channel(); /// let (manager, err) = future::join( diff --git a/presage/src/manager/registration.rs b/presage/src/manager/registration.rs index 1bfaf5a2d..e70b7ab83 100644 --- a/presage/src/manager/registration.rs +++ b/presage/src/manager/registration.rs @@ -39,12 +39,12 @@ impl Manager { /// }; /// use presage::manager::RegistrationOptions; /// use presage::Manager; - /// use presage_store_sled::{MigrationConflictStrategy, SledStore}; + /// use presage_store_sled::{MigrationConflictStrategy, OnNewIdentity, SledStore}; /// /// #[tokio::main] /// async fn main() -> Result<(), Box> { /// let store = - /// SledStore::open("/tmp/presage-example", MigrationConflictStrategy::Drop)?; + /// SledStore::open("/tmp/presage-example", MigrationConflictStrategy::Drop, OnNewIdentity::Trust)?; /// /// let manager = Manager::register( /// store, diff --git a/presage/src/store.rs b/presage/src/store.rs index 86be60857..a98a248ac 100644 --- a/presage/src/store.rs +++ b/presage/src/store.rs @@ -1,21 +1,22 @@ //! Traits that are used by the manager for storing the data. -use std::{fmt, ops::RangeBounds}; +use std::{fmt, ops::RangeBounds, time::SystemTime}; use libsignal_service::{ - content::ContentBody, + content::{ContentBody, Metadata}, groups_v2::Group, models::Contact, prelude::{Content, ProfileKey, Uuid, UuidError}, proto::{ sync_message::{self, Sent}, - DataMessage, EditMessage, GroupContextV2, SyncMessage, + verified, DataMessage, EditMessage, GroupContextV2, SyncMessage, Verified, }, - protocol::{ProtocolStore, SenderKeyStore}, + protocol::{IdentityKey, ProtocolAddress, ProtocolStore, SenderKeyStore}, session_store::SessionStoreExt, zkgroup::GroupMasterKeyBytes, Profile, }; +use log::error; use serde::{Deserialize, Serialize}; use crate::manager::RegistrationData; @@ -87,11 +88,54 @@ pub trait ContentsStore { /// Save a message in a [Thread] identified by a timestamp. fn save_message( - &mut self, + &self, thread: &Thread, message: Content, ) -> Result<(), Self::ContentsStoreError>; + /// Saves a message that can show users when the identity of a contact has changed + /// On Signal Android, this is usually displayed as: "Your safety number with XYZ has changed." + fn save_trusted_identity_message( + &self, + protocol_address: &ProtocolAddress, + right_identity_key: IdentityKey, + verified_state: verified::State, + ) { + let Ok(sender) = protocol_address.name().parse() else { + return; + }; + + // TODO: this is a hack to save a message showing that the verification status changed + // It is possibly ok to do it like this, but rebuidling the metadata and content body feels dirty + let thread = Thread::Contact(sender); + let verified_sync_message = Content { + metadata: Metadata { + sender: sender.into(), + sender_device: 0, + timestamp: SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap_or_default() + .as_secs(), + needs_receipt: false, + unidentified_sender: false, + }, + body: SyncMessage { + verified: Some(Verified { + destination_aci: None, + identity_key: Some(right_identity_key.public_key().serialize().to_vec()), + state: Some(verified_state.into()), + null_message: None, + }), + ..Default::default() + } + .into(), + }; + + if let Err(error) = self.save_message(&thread, verified_sync_message) { + error!("failed to save the verified session message in thread: {error}"); + } + } + /// Delete a single message, identified by its received timestamp from a thread. /// Useful when you want to delete a message locally only. fn delete_message(