From c2ad298963c1db6f687b671f87dedf258e798172 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 20 Aug 2020 15:40:49 +0200 Subject: [PATCH] crypto: Check that the user ids match for the cross signing keys. --- matrix_sdk_crypto/src/machine.rs | 24 ++++++++++++++++++++++-- matrix_sdk_crypto/src/user_identity.rs | 20 ++++++++++++++++++-- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index fb9e87f0..c36e1ab1 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -552,6 +552,18 @@ impl OlmMachine { } else if user_id == self.user_id() { if let Some(s) = response.user_signing_keys.get(user_id) { let user_signing = UserSigningPubkey::from(s); + + if master_key.user_id() != user_id + || self_signing.user_id() != user_id + || user_signing.user_id() != user_id + { + warn!( + "User id missmatch in one of the cross signing keys for user {}", + user_id + ); + continue; + } + OwnUserIdentity::new(master_key, self_signing, user_signing) .map(UserIdentities::Own) } else { @@ -563,7 +575,15 @@ impl OlmMachine { continue; } } else { - UserIdentity::new(master_key, self_signing).map(UserIdentities::Other) + if master_key.user_id() != user_id || self_signing.user_id() != user_id { + warn!( + "User id missmatch in one of the cross signing keys for user {}", + user_id + ); + continue; + } else { + UserIdentity::new(master_key, self_signing).map(UserIdentities::Other) + } }; match identity { @@ -577,7 +597,7 @@ impl OlmMachine { } Err(e) => { warn!( - "Coulnd't update or create new user identity for {}: {:?}", + "Couldn't update or create new user identity for {}: {:?}", user_id, e ); continue; diff --git a/matrix_sdk_crypto/src/user_identity.rs b/matrix_sdk_crypto/src/user_identity.rs index 2a1206ca..b6c4159a 100644 --- a/matrix_sdk_crypto/src/user_identity.rs +++ b/matrix_sdk_crypto/src/user_identity.rs @@ -44,7 +44,6 @@ pub struct UserSigningPubkey(Arc); impl PartialEq for MasterPubkey { fn eq(&self, other: &MasterPubkey) -> bool { self.0.user_id == other.0.user_id && self.0.keys == other.0.keys - // TODO check the usage once `KeyUsage` gets PartialEq. } } @@ -105,6 +104,11 @@ impl<'a> CrossSigningSubKeys<'a> { } impl MasterPubkey { + /// Get the user id of the master key's owner. + pub fn user_id(&self) -> &UserId { + &self.0.user_id + } + /// Get the master key with the given key id. /// /// # Arguments @@ -133,6 +137,8 @@ impl MasterPubkey { .next() .ok_or(SignatureError::MissingSigningKey)?; + let key_id = DeviceKeyId::try_from(key_id.as_str())?; + // FIXME `KeyUsage is missing PartialEq. // if self.0.usage.contains(&KeyUsage::Master) { // return Err(SignatureError::MissingSigningKey); @@ -145,7 +151,7 @@ impl MasterPubkey { verify_json( &self.0.user_id, - &DeviceKeyId::try_from(key_id.as_str())?, + &key_id, key, &mut to_value(subkey.cross_signing_key()).map_err(|_| SignatureError::NotAnObject)?, ) @@ -153,6 +159,11 @@ impl MasterPubkey { } impl UserSigningPubkey { + /// Get the user id of the user signing key's owner. + pub fn user_id(&self) -> &UserId { + &self.0.user_id + } + /// Check if the given master key is signed by this user signing key. /// /// # Arguments @@ -182,6 +193,11 @@ impl UserSigningPubkey { } impl SelfSigningPubkey { + /// Get the user id of the self signing key's owner. + pub fn user_id(&self) -> &UserId { + &self.0.user_id + } + /// Check if the given device is signed by this self signing key. /// /// # Arguments