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

fix: trust new identities #206

Merged
merged 4 commits into from
Nov 16, 2023
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
2 changes: 2 additions & 0 deletions presage-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -203,6 +204,7 @@ async fn main() -> anyhow::Result<()> {
db_path,
args.passphrase,
MigrationConflictStrategy::Raise,
OnNewIdentity::Trust,
)?;
run(args.subcommand, config_store).await
}
Expand Down
73 changes: 56 additions & 17 deletions presage-store-sled/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -64,6 +64,8 @@ pub struct SledStore {
db: Arc<RwLock<sled::Db>>,
#[cfg(feature = "encryption")]
cipher: Option<Arc<presage_store_cipher::StoreCipher>>,
/// 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
Expand Down Expand Up @@ -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<Path>,
passphrase: Option<impl AsRef<str>>,
trust_new_identities: OnNewIdentity,
) -> Result<Self, SledStoreError> {
let database = sled::open(db_path)?;

Expand All @@ -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<Path>,
migration_conflict_strategy: MigrationConflictStrategy,
trust_new_identities: OnNewIdentity,
) -> Result<Self, SledStoreError> {
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<Path>,
passphrase: Option<impl AsRef<str>>,
migration_conflict_strategy: MigrationConflictStrategy,
trust_new_identities: OnNewIdentity,
) -> Result<Self, SledStoreError> {
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")]
Expand Down Expand Up @@ -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
})
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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}",);

Expand Down Expand Up @@ -968,17 +986,28 @@ impl IdentityKeyStore for SledStore {
identity_key: &IdentityKey,
) -> Result<bool, SignalProtocolError> {
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(
Expand All @@ -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),
}
}
}
}
}

Expand Down Expand Up @@ -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))?;
Expand Down
4 changes: 2 additions & 2 deletions presage/src/manager/linking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ impl<S: Store> Manager<S, Linking> {
/// 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<dyn std::error::Error>> {
/// 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(
Expand Down
4 changes: 2 additions & 2 deletions presage/src/manager/registration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ impl<S: Store> Manager<S, Registration> {
/// };
/// 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<dyn std::error::Error>> {
/// let store =
/// SledStore::open("/tmp/presage-example", MigrationConflictStrategy::Drop)?;
/// SledStore::open("/tmp/presage-example", MigrationConflictStrategy::Drop, OnNewIdentity::Trust)?;
///
/// let manager = Manager::register(
/// store,
Expand Down
54 changes: 49 additions & 5 deletions presage/src/store.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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(
Expand Down
Loading