From 81b127b6e78d24dfd0b8f237066d91a37bbbeb40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 2 Sep 2020 11:45:35 +0200 Subject: [PATCH] crypto: Modify all the pickling logic so we return serializeable structs. --- matrix_sdk_crypto/src/olm/account.rs | 4 +- matrix_sdk_crypto/src/olm/group_sessions.rs | 59 ++++++++++++--- matrix_sdk_crypto/src/olm/mod.rs | 6 +- matrix_sdk_crypto/src/olm/session.rs | 77 ++++++++++++++----- matrix_sdk_crypto/src/store/sqlite.rs | 83 +++++++++++---------- 5 files changed, 155 insertions(+), 74 deletions(-) diff --git a/matrix_sdk_crypto/src/olm/account.rs b/matrix_sdk_crypto/src/olm/account.rs index d199f81d..0292afdd 100644 --- a/matrix_sdk_crypto/src/olm/account.rs +++ b/matrix_sdk_crypto/src/olm/account.rs @@ -67,9 +67,9 @@ pub struct Account { uploaded_signed_key_count: Arc, } -#[derive(Debug, Clone, Serialize, Deserialize)] /// A typed representation of a base64 encoded string containing the account /// pickle. +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct AccountPickle(String); impl AccountPickle { @@ -85,11 +85,11 @@ impl From for AccountPickle { } } -#[derive(Debug, Clone, Serialize, Deserialize)] /// A pickled version of an `Account`. /// /// Holds all the information that needs to be stored in a database to restore /// an account. +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct PickledAccount { /// The user id of the account owner. pub user_id: UserId, diff --git a/matrix_sdk_crypto/src/olm/group_sessions.rs b/matrix_sdk_crypto/src/olm/group_sessions.rs index e314e912..8781d751 100644 --- a/matrix_sdk_crypto/src/olm/group_sessions.rs +++ b/matrix_sdk_crypto/src/olm/group_sessions.rs @@ -37,7 +37,7 @@ use olm_rs::{ errors::OlmGroupSessionError, inbound_group_session::OlmInboundGroupSession, outbound_group_session::OlmOutboundGroupSession, PicklingMode, }; -use serde::Serialize; +use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; use zeroize::Zeroize; @@ -154,8 +154,15 @@ impl InboundGroupSession { /// /// * `pickle_mode` - The mode that was used to pickle the group session, /// either an unencrypted mode or an encrypted using passphrase. - pub async fn pickle(&self, pickle_mode: PicklingMode) -> String { - self.inner.lock().await.pickle(pickle_mode) + pub async fn pickle(&self, pickle_mode: PicklingMode) -> PickledInboundGroupSession { + let pickle = self.inner.lock().await.pickle(pickle_mode); + + PickledInboundGroupSession { + pickle: InboundGroupSessionPickle::from(pickle), + sender_key: self.sender_key.to_string(), + signing_key: self.signing_key.to_string(), + room_id: (&*self.room_id).clone(), + } } /// Restore a Session from a previously pickled string. @@ -178,21 +185,18 @@ impl InboundGroupSession { /// /// * `room_id` - The id of the room that the session is used in. pub fn from_pickle( - pickle: String, + pickle: PickledInboundGroupSession, pickle_mode: PicklingMode, - sender_key: String, - signing_key: String, - room_id: RoomId, ) -> Result { - let session = OlmInboundGroupSession::unpickle(pickle, pickle_mode)?; + let session = OlmInboundGroupSession::unpickle(pickle.pickle.0, pickle_mode)?; let session_id = session.session_id(); Ok(InboundGroupSession { inner: Arc::new(Mutex::new(session)), session_id: Arc::new(session_id), - sender_key: Arc::new(sender_key), - signing_key: Arc::new(signing_key), - room_id: Arc::new(room_id), + sender_key: Arc::new(pickle.sender_key), + signing_key: Arc::new(pickle.signing_key), + room_id: Arc::new(pickle.room_id), forwarding_chains: Arc::new(Mutex::new(None)), }) } @@ -282,6 +286,39 @@ impl PartialEq for InboundGroupSession { } } +/// A pickled version of an `InboundGroupSession`. +/// +/// Holds all the information that needs to be stored in a database to restore +/// an InboundGroupSession. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct PickledInboundGroupSession { + /// The pickle string holding the InboundGroupSession. + pub pickle: InboundGroupSessionPickle, + /// The public curve25519 key of the account that sent us the session + pub sender_key: String, + /// The public ed25519 key of the account that sent us the session. + pub signing_key: String, + /// The id of the room that the session is used in. + pub room_id: RoomId, +} + +/// The typed representation of a base64 encoded string of the GroupSession pickle. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct InboundGroupSessionPickle(String); + +impl From for InboundGroupSessionPickle { + fn from(pickle_string: String) -> Self { + InboundGroupSessionPickle(pickle_string) + } +} + +impl InboundGroupSessionPickle { + /// Get the string representation of the pickle. + pub fn as_str(&self) -> &str { + &self.0 + } +} + /// Outbound group session. /// /// Outbound group sessions are used to exchange room messages between a group diff --git a/matrix_sdk_crypto/src/olm/mod.rs b/matrix_sdk_crypto/src/olm/mod.rs index 0feafbc1..3694b7c8 100644 --- a/matrix_sdk_crypto/src/olm/mod.rs +++ b/matrix_sdk_crypto/src/olm/mod.rs @@ -22,11 +22,13 @@ mod group_sessions; mod session; pub use account::{Account, AccountPickle, IdentityKeys, PickledAccount}; -pub use group_sessions::{EncryptionSettings, InboundGroupSession}; +pub use group_sessions::{ + EncryptionSettings, InboundGroupSession, InboundGroupSessionPickle, PickledInboundGroupSession, +}; pub(crate) use group_sessions::{GroupSessionKey, OutboundGroupSession}; pub use olm_rs::PicklingMode; pub(crate) use session::OlmMessage; -pub use session::Session; +pub use session::{PickledSession, Session, SessionPickle}; #[cfg(test)] pub(crate) mod test { diff --git a/matrix_sdk_crypto/src/olm/session.rs b/matrix_sdk_crypto/src/olm/session.rs index 01191e75..f8545d58 100644 --- a/matrix_sdk_crypto/src/olm/session.rs +++ b/matrix_sdk_crypto/src/olm/session.rs @@ -20,10 +20,11 @@ use matrix_sdk_common::{ EventType, }, identifiers::{DeviceId, DeviceKeyAlgorithm, UserId}, - instant::Instant, + instant::{Duration, Instant}, locks::Mutex, }; use olm_rs::{errors::OlmSessionError, session::OlmSession, PicklingMode}; +use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; use super::IdentityKeys; @@ -177,8 +178,16 @@ impl Session { /// /// * `pickle_mode` - The mode that was used to pickle the session, either /// an unencrypted mode or an encrypted using passphrase. - pub async fn pickle(&self, pickle_mode: PicklingMode) -> String { - self.inner.lock().await.pickle(pickle_mode) + pub async fn pickle(&self, pickle_mode: PicklingMode) -> PickledSession { + let pickle = self.inner.lock().await.pickle(pickle_mode); + + PickledSession { + pickle: SessionPickle::from(pickle), + sender_key: self.sender_key.to_string(), + // FIXME this should use the duration from the unix epoch. + creation_time: self.creation_time.elapsed(), + last_use_time: self.last_use_time.elapsed(), + } } /// Restore a Session from a previously pickled string. @@ -194,40 +203,35 @@ impl Session { /// /// * `our_idenity_keys` - An clone of the Arc to our own identity keys. /// - /// * `pickle` - The pickled string of the session. + /// * `pickle` - The pickled version of the `Session`. /// /// * `pickle_mode` - The mode that was used to pickle the session, either /// an unencrypted mode or an encrypted using passphrase. - /// - /// * `sender_key` - The public curve25519 key of the account that - /// established the session with us. - /// - /// * `creation_time` - The timestamp that marks when the session was - /// created. - /// - /// * `last_use_time` - The timestamp that marks when the session was - /// last used to encrypt or decrypt an Olm message. - #[allow(clippy::too_many_arguments)] pub fn from_pickle( user_id: Arc, device_id: Arc>, our_identity_keys: Arc, - pickle: String, + pickle: PickledSession, pickle_mode: PicklingMode, - sender_key: String, - creation_time: Instant, - last_use_time: Instant, ) -> Result { - let session = OlmSession::unpickle(pickle, pickle_mode)?; + let session = OlmSession::unpickle(pickle.pickle.0, pickle_mode)?; let session_id = session.session_id(); + // FIXME this should use the UNIX epoch. + let now = Instant::now(); + + let creation_time = now.checked_sub(pickle.creation_time).unwrap(); + // .ok_or(CryptoStoreError::SessionTimestampError)?; + let last_use_time = now.checked_sub(pickle.last_use_time).unwrap(); + // .ok_or(CryptoStoreError::SessionTimestampError)?; + Ok(Session { user_id, device_id, our_identity_keys, inner: Arc::new(Mutex::new(session)), session_id: Arc::new(session_id), - sender_key: Arc::new(sender_key), + sender_key: Arc::new(pickle.sender_key), creation_time: Arc::new(creation_time), last_use_time: Arc::new(last_use_time), }) @@ -239,3 +243,36 @@ impl PartialEq for Session { self.session_id() == other.session_id() } } + +/// A pickled version of a `Session`. +/// +/// Holds all the information that needs to be stored in a database to restore +/// a Session. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct PickledSession { + /// The pickle string holding the Olm Session. + pub pickle: SessionPickle, + /// The curve25519 key of the other user that we share this session with. + pub sender_key: String, + /// The relative time elapsed since the session was created. + pub creation_time: Duration, + /// The relative time elapsed since the session was last used. + pub last_use_time: Duration, +} + +/// The typed representation of a base64 encoded string of the Olm Session pickle. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SessionPickle(String); + +impl From for SessionPickle { + fn from(picle_string: String) -> Self { + SessionPickle(picle_string) + } +} + +impl SessionPickle { + /// Get the string representation of the pickle. + pub fn as_str(&self) -> &str { + &self.0 + } +} diff --git a/matrix_sdk_crypto/src/store/sqlite.rs b/matrix_sdk_crypto/src/store/sqlite.rs index 52b302dc..e07f04cc 100644 --- a/matrix_sdk_crypto/src/store/sqlite.rs +++ b/matrix_sdk_crypto/src/store/sqlite.rs @@ -26,7 +26,7 @@ use matrix_sdk_common::{ identifiers::{ DeviceId, DeviceKeyAlgorithm, DeviceKeyId, EventEncryptionAlgorithm, RoomId, UserId, }, - instant::{Duration, Instant}, + instant::Duration, locks::Mutex, }; use sqlx::{query, query_as, sqlite::SqliteQueryAs, Connect, Executor, SqliteConnection}; @@ -38,14 +38,15 @@ use crate::{ device::{LocalTrust, ReadOnlyDevice}, memory_stores::{DeviceStore, GroupSessionStore, ReadOnlyUserDevices, SessionStore}, olm::{ - Account, AccountPickle, IdentityKeys, InboundGroupSession, PickledAccount, PicklingMode, - Session, + Account, AccountPickle, IdentityKeys, InboundGroupSession, InboundGroupSessionPickle, + PickledAccount, PickledInboundGroupSession, PickledSession, PicklingMode, Session, + SessionPickle, }, user_identity::UserIdentities, }; -/// SQLite based implementation of a `CryptoStore`. #[derive(Clone)] +/// SQLite based implementation of a `CryptoStore`. pub struct SqliteStore { user_id: Arc, device_id: Arc>, @@ -338,7 +339,7 @@ impl SqliteStore { .ok_or(CryptoStoreError::AccountUnset)?; let mut connection = self.connection.lock().await; - let rows: Vec<(String, String, String, String)> = query_as( + let mut rows: Vec<(String, String, String, String)> = query_as( "SELECT pickle, sender_key, creation_time, last_use_time FROM sessions WHERE account_id = ? and sender_key = ?", ) @@ -347,29 +348,27 @@ impl SqliteStore { .fetch_all(&mut *connection) .await?; - let now = Instant::now(); - Ok(rows - .iter() + .drain(..) .map(|row| { - let pickle = &row.0; - let sender_key = &row.1; - let creation_time = now - .checked_sub(serde_json::from_str::(&row.2)?) - .ok_or(CryptoStoreError::SessionTimestampError)?; - let last_use_time = now - .checked_sub(serde_json::from_str::(&row.3)?) - .ok_or(CryptoStoreError::SessionTimestampError)?; + let pickle = row.0; + let sender_key = row.1; + let creation_time = serde_json::from_str::(&row.2)?; + let last_use_time = serde_json::from_str::(&row.3)?; + + let pickle = PickledSession { + pickle: SessionPickle::from(pickle), + last_use_time, + creation_time, + sender_key, + }; Ok(Session::from_pickle( self.user_id.clone(), self.device_id.clone(), account_info.identity_keys.clone(), - pickle.to_string(), + pickle, self.get_pickle_mode(), - sender_key.to_string(), - creation_time, - last_use_time, )?) }) .collect::>>()?) @@ -379,7 +378,7 @@ impl SqliteStore { let account_id = self.account_id().ok_or(CryptoStoreError::AccountUnset)?; let mut connection = self.connection.lock().await; - let rows: Vec<(String, String, String, String)> = query_as( + let mut rows: Vec<(String, String, String, String)> = query_as( "SELECT pickle, sender_key, signing_key, room_id FROM inbound_group_sessions WHERE account_id = ?", ) @@ -388,19 +387,24 @@ impl SqliteStore { .await?; let mut group_sessions = rows - .iter() + .drain(..) .map(|row| { - let pickle = &row.0; - let sender_key = &row.1; - let signing_key = &row.2; - let room_id = &row.3; + let pickle = row.0; + let sender_key = row.1; + let signing_key = row.2; + let room_id = row.3; + + let pickle = PickledInboundGroupSession { + pickle: InboundGroupSessionPickle::from(pickle), + sender_key, + signing_key, + // FIXME remove this unwrap. + room_id: RoomId::try_from(room_id).unwrap(), + }; Ok(InboundGroupSession::from_pickle( - pickle.to_string(), + pickle, self.get_pickle_mode(), - sender_key.to_string(), - signing_key.to_owned(), - RoomId::try_from(room_id.as_str()).unwrap(), )?) }) .collect::>>()?; @@ -754,11 +758,12 @@ impl CryptoStore for SqliteStore { self.lazy_load_sessions(&session.sender_key).await?; self.sessions.add(session.clone()).await; - let session_id = session.session_id(); - let creation_time = serde_json::to_string(&session.creation_time.elapsed())?; - let last_use_time = serde_json::to_string(&session.last_use_time.elapsed())?; let pickle = session.pickle(self.get_pickle_mode()).await; + let session_id = session.session_id(); + let creation_time = serde_json::to_string(&pickle.creation_time)?; + let last_use_time = serde_json::to_string(&pickle.last_use_time)?; + let mut connection = self.connection.lock().await; query( @@ -770,8 +775,8 @@ impl CryptoStore for SqliteStore { .bind(&account_id) .bind(&*creation_time) .bind(&*last_use_time) - .bind(&*session.sender_key) - .bind(&pickle) + .bind(&pickle.sender_key) + .bind(&pickle.pickle.as_str()) .execute(&mut *connection) .await?; } @@ -800,10 +805,10 @@ impl CryptoStore for SqliteStore { ) .bind(session_id) .bind(account_id) - .bind(&*session.sender_key) - .bind(&*session.signing_key) - .bind(&*session.room_id.to_string()) - .bind(&pickle) + .bind(pickle.sender_key) + .bind(pickle.signing_key) + .bind(pickle.room_id.as_str()) + .bind(pickle.pickle.as_str()) .execute(&mut *connection) .await?;