diff --git a/matrix_sdk_crypto/src/device.rs b/matrix_sdk_crypto/src/device.rs index 3fde9294..0ffe946b 100644 --- a/matrix_sdk_crypto/src/device.rs +++ b/matrix_sdk_crypto/src/device.rs @@ -13,13 +13,14 @@ // limitations under the License. use std::collections::BTreeMap; -#[cfg(test)] use std::convert::TryFrom; use std::mem; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use atomic::Atomic; +use olm_rs::utility::OlmUtility; +use serde_json::{json, Value}; #[cfg(test)] use super::OlmMachine; @@ -27,6 +28,8 @@ use matrix_sdk_common::api::r0::keys::{DeviceKeys, KeyAlgorithm}; use matrix_sdk_common::events::Algorithm; use matrix_sdk_common::identifiers::{DeviceId, UserId}; +use crate::error::SignatureError; + /// A device represents a E2EE capable client of an user. #[derive(Debug, Clone)] pub struct Device { @@ -126,7 +129,9 @@ impl Device { } /// Update a device with a new device keys struct. - pub(crate) fn update_device(&mut self, device_keys: &DeviceKeys) { + pub(crate) fn update_device(&mut self, device_keys: &DeviceKeys) -> Result<(), SignatureError> { + self.verify_device_keys(device_keys)?; + let mut keys = BTreeMap::new(); for (key_id, key) in device_keys.keys.iter() { @@ -148,6 +153,59 @@ impl Device { ); let _ = mem::replace(&mut self.keys, Arc::new(keys)); let _ = mem::replace(&mut self.display_name, display_name); + + Ok(()) + } + + fn is_signed_by_device(&self, json: &mut Value) -> Result<(), SignatureError> { + let signing_key = self.keys.get(&KeyAlgorithm::Ed25519).unwrap(); + + let json_object = json.as_object_mut().ok_or(SignatureError::NotAnObject)?; + let unsigned = json_object.remove("unsigned"); + let signatures = json_object.remove("signatures"); + + let canonical_json = cjson::to_string(json_object)?; + + if let Some(u) = unsigned { + json_object.insert("unsigned".to_string(), u); + } + + // TODO this should be part of ruma-client-api. + let key_id_string = format!("{}:{}", KeyAlgorithm::Ed25519, self.device_id); + + let signatures = signatures.ok_or(SignatureError::NoSignatureFound)?; + let signature_object = signatures + .as_object() + .ok_or(SignatureError::NoSignatureFound)?; + let signature = signature_object + .get(&self.user_id.to_string()) + .ok_or(SignatureError::NoSignatureFound)?; + let signature = signature + .get(key_id_string) + .ok_or(SignatureError::NoSignatureFound)?; + let signature = signature.as_str().ok_or(SignatureError::NoSignatureFound)?; + + let utility = OlmUtility::new(); + + let ret = if utility + .ed25519_verify(signing_key, &canonical_json, signature) + .is_ok() + { + Ok(()) + } else { + Err(SignatureError::VerificationError) + }; + + json_object.insert("signatures".to_string(), signatures); + + ret + } + + pub(crate) fn verify_device_keys( + &self, + device_keys: &DeviceKeys, + ) -> Result<(), SignatureError> { + self.is_signed_by_device(&mut json!(&device_keys)) } /// Mark the device as deleted. @@ -185,8 +243,10 @@ impl From<&OlmMachine> for Device { } } -impl From<&DeviceKeys> for Device { - fn from(device_keys: &DeviceKeys) -> Self { +impl TryFrom<&DeviceKeys> for Device { + type Error = SignatureError; + + fn try_from(device_keys: &DeviceKeys) -> Result { let mut keys = BTreeMap::new(); for (key_id, key) in device_keys.keys.iter() { @@ -194,7 +254,7 @@ impl From<&DeviceKeys> for Device { let _ = keys.insert(key_id, key.clone()); } - Device { + let device = Device { user_id: Arc::new(device_keys.user_id.clone()), device_id: Arc::new(device_keys.device_id.clone()), algorithms: Arc::new(device_keys.algorithms.clone()), @@ -208,7 +268,10 @@ impl From<&DeviceKeys> for Device { ), deleted: Arc::new(AtomicBool::new(false)), trust_state: Arc::new(Atomic::new(TrustState::Unset)), - } + }; + + device.verify_device_keys(device_keys)?; + Ok(device) } } @@ -221,30 +284,27 @@ impl PartialEq for Device { #[cfg(test)] pub(crate) mod test { use serde_json::json; - use std::convert::{From, TryFrom}; + use std::convert::TryFrom; use crate::device::{Device, TrustState}; use matrix_sdk_common::api::r0::keys::{DeviceKeys, KeyAlgorithm}; use matrix_sdk_common::identifiers::UserId; fn device_keys() -> DeviceKeys { - let user_id = UserId::try_from("@alice:example.org").unwrap(); - let device_id = "DEVICEID"; - let device_keys = json!({ "algorithms": vec![ "m.olm.v1.curve25519-aes-sha2", "m.megolm.v1.aes-sha2" ], - "device_id": device_id, - "user_id": user_id.to_string(), + "device_id": "BNYQQWUMXO", + "user_id": "@example:localhost", "keys": { - "curve25519:DEVICEID": "wjLpTLRqbqBzLs63aYaEv2Boi6cFEbbM/sSRQ2oAKk4", - "ed25519:DEVICEID": "nE6W2fCblxDcOFmeEtCHNl8/l8bXcu7GKyAswA4r3mM" + "curve25519:BNYQQWUMXO": "xfgbLIC5WAl1OIkpOzoxpCe8FsRDT6nch7NQsOb15nc", + "ed25519:BNYQQWUMXO": "2/5LWJMow5zhJqakV88SIc7q/1pa8fmkfgAzx72w9G4" }, "signatures": { - user_id.to_string(): { - "ed25519:DEVICEID": "m53Wkbh2HXkc3vFApZvCrfXcX3AI51GsDHustMhKwlv3TuOJMj4wistcOTM8q2+e/Ro7rWFUb9ZfnNbwptSUBA" + "@example:localhost": { + "ed25519:BNYQQWUMXO": "kTwMrbsLJJM/uFGOj/oqlCaRuw7i9p/6eGrTlXjo8UJMCFAetoyWzoMcF35vSe4S6FTx8RJmqX6rM7ep53MHDQ" } }, "unsigned": { @@ -257,13 +317,13 @@ pub(crate) mod test { pub(crate) fn get_device() -> Device { let device_keys = device_keys(); - Device::from(&device_keys) + Device::try_from(&device_keys).unwrap() } #[test] fn create_a_device() { - let user_id = UserId::try_from("@alice:example.org").unwrap(); - let device_id = "DEVICEID"; + let user_id = UserId::try_from("@example:localhost").unwrap(); + let device_id = "BNYQQWUMXO"; let device = get_device(); @@ -277,11 +337,11 @@ pub(crate) mod test { ); assert_eq!( device.get_key(KeyAlgorithm::Curve25519).unwrap(), - "wjLpTLRqbqBzLs63aYaEv2Boi6cFEbbM/sSRQ2oAKk4" + "xfgbLIC5WAl1OIkpOzoxpCe8FsRDT6nch7NQsOb15nc" ); assert_eq!( device.get_key(KeyAlgorithm::Ed25519).unwrap(), - "nE6W2fCblxDcOFmeEtCHNl8/l8bXcu7GKyAswA4r3mM" + "2/5LWJMow5zhJqakV88SIc7q/1pa8fmkfgAzx72w9G4" ); } @@ -297,7 +357,7 @@ pub(crate) mod test { let mut device_keys = device_keys(); device_keys.unsigned.as_mut().unwrap().device_display_name = Some("Alice's work computer".to_owned()); - device.update_device(&device_keys); + device.update_device(&device_keys).unwrap(); assert_eq!( "Alice's work computer", diff --git a/matrix_sdk_crypto/src/error.rs b/matrix_sdk_crypto/src/error.rs index fac57a56..09895a6d 100644 --- a/matrix_sdk_crypto/src/error.rs +++ b/matrix_sdk_crypto/src/error.rs @@ -104,7 +104,10 @@ pub enum EventError { } #[derive(Error, Debug)] -pub(crate) enum SignatureError { +pub enum SignatureError { + #[error("the signing key is missing from the object that signed the message")] + MissingSigningKey, + #[error("the provided JSON value isn't an object")] NotAnObject, diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index a9b0f199..172d047e 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -13,6 +13,7 @@ // limitations under the License. use std::collections::{BTreeMap, HashMap, HashSet}; +use std::convert::TryFrom; use std::convert::TryInto; use std::mem; #[cfg(feature = "sqlite-cryptostore")] @@ -429,9 +430,6 @@ impl OlmMachine { continue; } - // TODO this logic could go into the device struct, especially - // since we're gonna have cross signing identities soon. - if user_id != &device_keys.user_id || device_id != &device_keys.device_id { warn!( "Mismatch in device keys payload of device {} from user {}", @@ -440,46 +438,28 @@ impl OlmMachine { continue; } - let ed_key_id = AlgorithmAndDeviceId(KeyAlgorithm::Ed25519, device_id.to_owned()); - - let signing_key = if let Some(k) = device_keys.keys.get(&ed_key_id) { - k - } else { - warn!( - "Ed25519 identity key wasn't found for user/device {} {}", - user_id, device_id - ); - continue; - }; - - if self - .verify_json(user_id, device_id, signing_key, &mut json!(&device_keys)) - .is_err() - { - warn!( - "Failed to verify the device key signatures for {} {}", - user_id, device_id - ); - continue; - } - let device = self.store.get_device(&user_id, device_id).await?; - let device = if let Some(mut d) = device { - let stored_signing_key = d.get_key(KeyAlgorithm::Ed25519); - - if let Some(stored_signing_key) = stored_signing_key { - if stored_signing_key != signing_key { - warn!("Ed25519 key has changed for {} {}", user_id, device_id); + let device = if let Some(mut device) = device { + if let Err(e) = device.update_device(device_keys) { + warn!( + "Failed to update the device keys for {} {}: {:?}", + user_id, device_id, e + ); + continue; + } + device + } else { + let device = match Device::try_from(device_keys) { + Ok(d) => d, + Err(e) => { + warn!( + "Failed to create a new device for {} {}: {:?}", + user_id, device_id, e + ); continue; } - } - - d.update_device(device_keys); - - d - } else { - let device = Device::from(device_keys); + }; info!("Adding a new device to the device store {:?}", device); device };