From de1988265deac2e2eec49d4576af5a5e75760c4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 15 Jul 2020 15:39:56 +0200 Subject: [PATCH] crypto: Move the outbound session creation logic into the account. --- matrix_sdk_crypto/src/error.rs | 24 +++++++++ matrix_sdk_crypto/src/machine.rs | 75 ++++++--------------------- matrix_sdk_crypto/src/olm.rs | 70 +++++++++++++++++++++++-- matrix_sdk_crypto/src/store/sqlite.rs | 2 +- 4 files changed, 108 insertions(+), 63 deletions(-) diff --git a/matrix_sdk_crypto/src/error.rs b/matrix_sdk_crypto/src/error.rs index 09895a6d..4dcc207b 100644 --- a/matrix_sdk_crypto/src/error.rs +++ b/matrix_sdk_crypto/src/error.rs @@ -13,6 +13,7 @@ // limitations under the License. use cjson::Error as CjsonError; +use matrix_sdk_common::identifiers::{DeviceId, UserId}; use olm_rs::errors::{OlmGroupSessionError, OlmSessionError}; use serde_json::Error as SerdeError; use thiserror::Error; @@ -121,6 +122,29 @@ pub enum SignatureError { VerificationError, } +#[derive(Error, Debug)] +pub(crate) enum SessionCreationError { + #[error( + "Failed to create a new Olm session for {0} {1}, the requested \ + one-time key isn't a signed curve key" + )] + OneTimeKeyNotSigned(UserId, DeviceId), + #[error( + "Tried to create a new Olm session for {0} {1}, but the signed \ + one-time key is missing" + )] + OneTimeKeyMissing(UserId, DeviceId), + #[error("Failed to verify the one-time key signatures for {0} {1}: {2:?}")] + InvalidSignature(UserId, DeviceId, SignatureError), + #[error( + "Tried to create an Olm session for {0} {1}, but the device is missing \ + a curve25519 key" + )] + DeviceMissingCurveKey(UserId, DeviceId), + #[error("Error creating new Olm session for {0} {1}: {2:?}")] + OlmError(UserId, DeviceId, OlmSessionError), +} + impl From for SignatureError { fn from(error: CjsonError) -> Self { Self::CanonicalJsonError(error) diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 102fd62b..f0305cfa 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -306,76 +306,35 @@ impl OlmMachine { for (user_id, user_devices) in &response.one_time_keys { for (device_id, key_map) in user_devices { - let device = if let Some(d) = self - .store - .get_device(&user_id, device_id) - .await - .expect("Can't get devices") - { - d - } else { - warn!( - "Tried to create an Olm session for {} {}, but the device is unknown", - user_id, device_id - ); - continue; - }; - - // TODO move this logic into the account, pass the device to the - // account when creating an outbound session. - let one_time_key = if let Some(k) = key_map.values().next() { - match k { - OneTimeKey::SignedKey(k) => k, - OneTimeKey::Key(_) => { + let device: Device = match self.store.get_device(&user_id, device_id).await { + Ok(d) => { + if let Some(d) = d { + d + } else { warn!( - "Tried to create an Olm session for {} {}, but - the requested key isn't a signed curve key", + "Tried to create an Olm session for {} {}, but \ + the device is unknown", user_id, device_id ); continue; } } - } else { - warn!( - "Tried to create an Olm session for {} {}, but the - signed one-time key is missing", - user_id, device_id - ); - continue; - }; - - if let Err(e) = device.verify_one_time_key(&one_time_key) { - warn!( - "Failed to verify the one-time key signatures for {} {}: {:?}", - user_id, device_id, e - ); - continue; - } - - let curve_key = if let Some(k) = device.get_key(KeyAlgorithm::Curve25519) { - k - } else { - warn!( - "Tried to create an Olm session for {} {}, but the - device is missing the curve key", - user_id, device_id - ); - continue; + Err(e) => { + warn!( + "Tried to create an Olm session for {} {}, but \ + can't fetch the device from the store {:?}", + user_id, device_id, e + ); + continue; + } }; info!("Creating outbound Session for {} {}", user_id, device_id); - let session = match self - .account - .create_outbound_session(curve_key, &one_time_key) - .await - { + let session = match self.account.create_outbound_session(device, &key_map).await { Ok(s) => s, Err(e) => { - warn!( - "Error creating new Olm session for {} {}: {}", - user_id, device_id, e - ); + warn!("{:?}", e); continue; } }; diff --git a/matrix_sdk_crypto/src/olm.rs b/matrix_sdk_crypto/src/olm.rs index e1e33a20..e17b2e65 100644 --- a/matrix_sdk_crypto/src/olm.rs +++ b/matrix_sdk_crypto/src/olm.rs @@ -33,7 +33,8 @@ use olm_rs::outbound_group_session::OlmOutboundGroupSession; use olm_rs::session::OlmSession; use olm_rs::PicklingMode; -use crate::error::{EventError, MegolmResult}; +use crate::device::Device; +use crate::error::{EventError, MegolmResult, SessionCreationError}; pub use olm_rs::{ session::{OlmMessage, PreKeyMessage}, utility::OlmUtility, @@ -386,7 +387,7 @@ impl Account { /// /// * `their_one_time_key` - A signed one-time key that the other account /// created and shared with us. - pub(crate) async fn create_outbound_session( + pub(crate) async fn create_outbound_session_helper( &self, their_identity_key: &str, their_one_time_key: &SignedKey, @@ -409,6 +410,67 @@ impl Account { }) } + /// Create a new session with another account given a one-time key and a + /// device. + /// + /// Returns the newly created session or a `OlmSessionError` if creating a + /// session failed. + /// + /// # Arguments + /// * `device` - The other account's device. + /// + /// * `key_map` - A map from the algorithm and device id to the one-time + /// key that the other account created and shared with us. + pub(crate) async fn create_outbound_session( + &self, + device: Device, + key_map: &BTreeMap, + ) -> Result { + let one_time_key = + key_map + .values() + .next() + .ok_or(SessionCreationError::OneTimeKeyMissing( + device.user_id().to_owned(), + device.device_id().to_owned(), + ))?; + + let one_time_key = match one_time_key { + OneTimeKey::SignedKey(k) => k, + OneTimeKey::Key(_) => { + return Err(SessionCreationError::OneTimeKeyNotSigned( + device.user_id().to_owned(), + device.device_id().to_owned(), + )); + } + }; + + device.verify_one_time_key(&one_time_key).map_err(|e| { + SessionCreationError::InvalidSignature( + device.user_id().to_owned(), + device.device_id().to_owned(), + e, + ) + })?; + + let curve_key = device.get_key(KeyAlgorithm::Curve25519).ok_or( + SessionCreationError::DeviceMissingCurveKey( + device.user_id().to_owned(), + device.device_id().to_owned(), + ), + )?; + + self.create_outbound_session_helper(curve_key, &one_time_key) + .await + .map_err(|e| { + SessionCreationError::OlmError( + device.user_id().to_owned(), + device.device_id().to_owned(), + e, + ) + }) + } + /// Create a new session with another account given a pre-key Olm message. /// /// Returns the newly created session or a `OlmSessionError` if creating a @@ -1014,7 +1076,7 @@ pub(crate) mod test { }; let sender_key = bob.identity_keys().curve25519().to_owned(); let session = alice - .create_outbound_session(&sender_key, &one_time_key) + .create_outbound_session_helper(&sender_key, &one_time_key) .await .unwrap(); @@ -1092,7 +1154,7 @@ pub(crate) mod test { }; let mut bob_session = bob - .create_outbound_session(alice_keys.curve25519(), &one_time_key) + .create_outbound_session_helper(alice_keys.curve25519(), &one_time_key) .await .unwrap(); diff --git a/matrix_sdk_crypto/src/store/sqlite.rs b/matrix_sdk_crypto/src/store/sqlite.rs index 2287afe9..bfa26664 100644 --- a/matrix_sdk_crypto/src/store/sqlite.rs +++ b/matrix_sdk_crypto/src/store/sqlite.rs @@ -876,7 +876,7 @@ mod test { }; let sender_key = bob.identity_keys().curve25519().to_owned(); let session = alice - .create_outbound_session(&sender_key, &one_time_key) + .create_outbound_session_helper(&sender_key, &one_time_key) .await .unwrap();