From c5ef3e7c94359e85e3f888860b146813316d27bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 26 May 2022 12:38:11 +0200 Subject: [PATCH 1/5] refactor(crypto): Turn the SignedKeySignatures into a struct --- crates/matrix-sdk-crypto/src/olm/account.rs | 12 +- crates/matrix-sdk-crypto/src/types/mod.rs | 140 ++++++++++++++++++ .../src/types/one_time_keys.rs | 90 +++++------ 3 files changed, 184 insertions(+), 58 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/olm/account.rs b/crates/matrix-sdk-crypto/src/olm/account.rs index bea1c9a85..39cd08104 100644 --- a/crates/matrix-sdk-crypto/src/olm/account.rs +++ b/crates/matrix-sdk-crypto/src/olm/account.rs @@ -883,15 +883,11 @@ impl ReadOnlyAccount { .await .expect("Newly created one-time keys can always be signed"); - let signatures = BTreeMap::from([( + key.signatures_mut().add_signature( self.user_id().to_owned(), - BTreeMap::from([( - DeviceKeyId::from_parts(DeviceKeyAlgorithm::Ed25519, &self.device_id), - signature, - )]), - )]); - - *key.signatures() = signatures; + DeviceKeyId::from_parts(DeviceKeyAlgorithm::Ed25519, self.device_id()), + signature, + ); key } diff --git a/crates/matrix-sdk-crypto/src/types/mod.rs b/crates/matrix-sdk-crypto/src/types/mod.rs index c74ed9666..402e2df9e 100644 --- a/crates/matrix-sdk-crypto/src/types/mod.rs +++ b/crates/matrix-sdk-crypto/src/types/mod.rs @@ -27,6 +27,146 @@ mod cross_signing_key; mod device_keys; mod one_time_keys; +use std::collections::BTreeMap; + pub use cross_signing_key::*; pub use device_keys::*; pub use one_time_keys::*; +use ruma::{DeviceKeyAlgorithm, DeviceKeyId, OwnedDeviceKeyId, OwnedUserId, UserId}; +use serde::{Deserialize, Serialize, Serializer}; +use vodozemac::Ed25519Signature; + +/// An enum over all the signature types. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum Signature { + /// A Ed25519 digital signature. + Ed25519(Ed25519Signature), + /// An unknown digital signature as a base64 encoded string. + Other(String), +} + +impl Signature { + /// Get the Ed25519 signature, if this is one. + pub fn ed25519(&self) -> Option { + if let Self::Ed25519(signature) = &self { + Some(*signature) + } else { + None + } + } + + /// Convert the signature to a base64 encoded string. + pub fn to_base64(&self) -> String { + match self { + Signature::Ed25519(s) => s.to_base64(), + Signature::Other(s) => s.to_owned(), + } + } +} + +impl From for Signature { + fn from(signature: Ed25519Signature) -> Self { + Self::Ed25519(signature) + } +} + +/// Signatures for a signed object. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Signatures(BTreeMap>); + +impl Signatures { + /// Create a new, empty, signatures collection. + pub fn new() -> Self { + Signatures(Default::default()) + } + + /// Add the given signature from the given signer and the given key_id to + /// the collection. + pub fn add_signature( + &mut self, + signer: OwnedUserId, + key_id: OwnedDeviceKeyId, + signature: Ed25519Signature, + ) -> Option { + self.0.entry(signer).or_insert_with(Default::default).insert(key_id, signature.into()) + } + + /// Try to find an Ed25519 signature from the given signer with the given + /// key id. + pub fn get_signature(&self, signer: &UserId, key_id: &DeviceKeyId) -> Option { + self.get(signer)?.get(key_id)?.ed25519() + } + + /// Get the map of signatures that belong to the given user. + pub fn get(&self, signer: &UserId) -> Option<&BTreeMap> { + self.0.get(signer) + } +} + +impl Default for Signatures { + fn default() -> Self { + Self::new() + } +} + +impl IntoIterator for Signatures { + type Item = (OwnedUserId, BTreeMap); + + type IntoIter = + std::collections::btree_map::IntoIter>; + + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} + +impl<'de> Deserialize<'de> for Signatures { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let map: BTreeMap> = + serde::Deserialize::deserialize(deserializer)?; + + Ok(Signatures( + map.into_iter() + .map(|(user, signatures)| { + Ok(( + user, + signatures + .into_iter() + .map(|(key_id, s)| { + let algorithm = key_id.algorithm(); + let signature = match algorithm { + DeviceKeyAlgorithm::Ed25519 => { + Ed25519Signature::from_base64(&s) + .map_err(serde::de::Error::custom)? + .into() + } + _ => Signature::Other(s), + }; + + Ok((key_id, signature)) + }) + .collect::, _>>()?, + )) + }) + .collect::>()?, + )) + } +} + +impl Serialize for Signatures { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + let signatures: BTreeMap<&OwnedUserId, BTreeMap<&OwnedDeviceKeyId, String>> = self + .0 + .iter() + .map(|(u, m)| (u, m.iter().map(|(d, s)| (d, s.to_base64())).collect())) + .collect(); + + serde::Serialize::serialize(&signatures, serializer) + } +} diff --git a/crates/matrix-sdk-crypto/src/types/one_time_keys.rs b/crates/matrix-sdk-crypto/src/types/one_time_keys.rs index f3c609535..6e2452fa1 100644 --- a/crates/matrix-sdk-crypto/src/types/one_time_keys.rs +++ b/crates/matrix-sdk-crypto/src/types/one_time_keys.rs @@ -20,24 +20,22 @@ use std::collections::BTreeMap; -use ruma::{serde::Raw, OwnedDeviceKeyId, OwnedUserId}; +use ruma::serde::Raw; use serde::{Deserialize, Serialize, Serializer}; use serde_json::{value::to_raw_value, Value}; -use vodozemac::{Curve25519PublicKey, Ed25519Signature}; +use vodozemac::Curve25519PublicKey; -/// Signatures for a `SignedKey` object. -pub type SignedKeySignatures = BTreeMap>; +use super::Signatures; /// A key for the SignedCurve25519 algorithm #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct SignedKey { - // /// The Curve25519 key that can be used to establish Olm sessions. + /// The Curve25519 key that can be used to establish Olm sessions. #[serde(deserialize_with = "deserialize_curve_key", serialize_with = "serialize_curve_key")] key: Curve25519PublicKey, /// Signatures for the key object. - #[serde(deserialize_with = "deserialize_signatures", serialize_with = "serialize_signatures")] - signatures: SignedKeySignatures, + signatures: Signatures, /// Is the key considered to be a fallback key. #[serde(default, skip_serializing_if = "Option::is_none", deserialize_with = "double_option")] @@ -67,42 +65,6 @@ where s.serialize_str(&key) } -fn deserialize_signatures<'de, D>(de: D) -> Result -where - D: serde::Deserializer<'de>, -{ - let map: BTreeMap> = - Deserialize::deserialize(de)?; - - map.into_iter() - .map(|(u, m)| { - Ok(( - u, - m.into_iter() - .map(|(d, s)| { - Ok(( - d, - Ed25519Signature::from_base64(&s).map_err(serde::de::Error::custom)?, - )) - }) - .collect::, _>>()?, - )) - }) - .collect::>() -} - -fn serialize_signatures(signatures: &SignedKeySignatures, s: S) -> Result -where - S: Serializer, -{ - let signatures: BTreeMap<&OwnedUserId, BTreeMap<&OwnedDeviceKeyId, String>> = signatures - .iter() - .map(|(u, m)| (u, m.iter().map(|(d, s)| (d, s.to_base64())).collect())) - .collect(); - - signatures.serialize(s) -} - fn double_option<'de, T, D>(de: D) -> Result>, D::Error> where T: Deserialize<'de>, @@ -114,7 +76,7 @@ where impl SignedKey { /// Creates a new `SignedKey` with the given key and signatures. pub fn new(key: Curve25519PublicKey) -> Self { - Self { key, signatures: BTreeMap::new(), fallback: None, other: BTreeMap::new() } + Self { key, signatures: Signatures::new(), fallback: None, other: BTreeMap::new() } } /// Creates a new `SignedKey`, that represents a fallback key, with the @@ -122,7 +84,7 @@ impl SignedKey { pub fn new_fallback(key: Curve25519PublicKey) -> Self { Self { key, - signatures: BTreeMap::new(), + signatures: Signatures::new(), fallback: Some(Some(true)), other: BTreeMap::new(), } @@ -134,7 +96,12 @@ impl SignedKey { } /// Signatures for the key object. - pub fn signatures(&mut self) -> &mut SignedKeySignatures { + pub fn signatures(&self) -> &Signatures { + &self.signatures + } + + /// Signatures for the key object as a mutable borrow. + pub fn signatures_mut(&mut self) -> &mut Signatures { &mut self.signatures } @@ -168,18 +135,24 @@ pub enum OneTimeKey { #[cfg(test)] mod tests { use matches::assert_matches; + use ruma::{device_id, user_id, DeviceKeyAlgorithm, DeviceKeyId}; use serde_json::json; - use vodozemac::Curve25519PublicKey; + use vodozemac::{Curve25519PublicKey, Ed25519Signature}; use super::OneTimeKey; + use crate::types::Signature; #[test] fn serialization() { + let user_id = user_id!("@user:example.com"); + let device_id = device_id!("EGURVBUNJP"); + let json = json!({ "key":"XjhWTCjW7l59pbfx9tlCBQolfnIQWARoKOzjTOPSlWM", "signatures": { - "@user:example.com": { - "ed25519:EGURVBUNJP": "mia28GKixFzOWKJ0h7Bdrdy2fjxiHCsst1qpe467FbW85H61UlshtKBoAXfTLlVfi0FX+/noJ8B3noQPnY+9Cg" + user_id: { + "ed25519:EGURVBUNJP": "mia28GKixFzOWKJ0h7Bdrdy2fjxiHCsst1qpe467FbW85H61UlshtKBoAXfTLlVfi0FX+/noJ8B3noQPnY+9Cg", + "other:EGURVBUNJP": "UnknownSignature" } }, "extra_key": "extra_value" @@ -189,10 +162,27 @@ mod tests { Curve25519PublicKey::from_base64("XjhWTCjW7l59pbfx9tlCBQolfnIQWARoKOzjTOPSlWM") .expect("Can't construct curve key from base64"); + let signature = Ed25519Signature::from_base64( + "mia28GKixFzOWKJ0h7Bdrdy2fjxiHCsst1qpe467FbW85H61UlshtKBoAXfTLlVfi0FX+/noJ8B3noQPnY+9Cg" + ).expect("The signature can always be decoded"); + + let custom_signature = Signature::Other("UnknownSignature".to_owned()); + let key: OneTimeKey = serde_json::from_value(json.clone()).expect("Can't deserialize a valid one-time key"); - assert_matches!(key, OneTimeKey::SignedKey(ref k) if k.key == curve_key); + assert_matches!( + key, + OneTimeKey::SignedKey(ref k) if k.key == curve_key + && k + .signatures() + .get_signature(user_id, &DeviceKeyId::from_parts(DeviceKeyAlgorithm::Ed25519, device_id)) + == Some(signature) + && k + .signatures() + .get(user_id).unwrap().get(&DeviceKeyId::from_parts("other".into(), device_id)) + == Some(&custom_signature) + ); let serialized = serde_json::to_value(key).expect("Can't reserialize a signed key"); From 57560577197c322a9cb9b64d6fbb032834e082a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 26 May 2022 12:56:05 +0200 Subject: [PATCH 2/5] refactor(crypto): Use the new Signatures struct for the DeviceKeys --- crates/matrix-sdk-crypto/src/identities/device.rs | 6 +++--- crates/matrix-sdk-crypto/src/olm/account.rs | 7 ++++--- .../src/olm/signing/pk_signing.rs | 5 +++-- crates/matrix-sdk-crypto/src/types/device_keys.rs | 8 +++++--- crates/matrix-sdk-crypto/src/types/mod.rs | 5 +++++ crates/matrix-sdk-crypto/src/verification/mod.rs | 15 +++++---------- 6 files changed, 25 insertions(+), 21 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/identities/device.rs b/crates/matrix-sdk-crypto/src/identities/device.rs index d843bc1be..8b61ab9db 100644 --- a/crates/matrix-sdk-crypto/src/identities/device.rs +++ b/crates/matrix-sdk-crypto/src/identities/device.rs @@ -32,7 +32,7 @@ use ruma::{ AnyToDeviceEventContent, }, DeviceId, DeviceKeyAlgorithm, DeviceKeyId, EventEncryptionAlgorithm, OwnedDeviceId, - OwnedDeviceKeyId, OwnedUserId, UserId, + OwnedDeviceKeyId, UserId, }; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use serde_json::{json, Value}; @@ -47,7 +47,7 @@ use crate::{ identities::{ReadOnlyOwnUserIdentity, ReadOnlyUserIdentities}, olm::{InboundGroupSession, Session, VerifyJson}, store::{Changes, CryptoStore, DeviceChanges, Result as StoreResult}, - types::{DeviceKey, DeviceKeys, SignedKey}, + types::{DeviceKey, DeviceKeys, Signatures, SignedKey}, verification::VerificationMachine, OutgoingVerificationRequest, ReadOnlyAccount, Sas, ToDeviceRequest, VerificationRequest, }; @@ -433,7 +433,7 @@ impl ReadOnlyDevice { } /// Get a map containing all the device signatures. - pub fn signatures(&self) -> &BTreeMap> { + pub fn signatures(&self) -> &Signatures { &self.inner.signatures } diff --git a/crates/matrix-sdk-crypto/src/olm/account.rs b/crates/matrix-sdk-crypto/src/olm/account.rs index 39cd08104..e9c21ec73 100644 --- a/crates/matrix-sdk-crypto/src/olm/account.rs +++ b/crates/matrix-sdk-crypto/src/olm/account.rs @@ -739,7 +739,7 @@ impl ReadOnlyAccount { (*self.device_id).to_owned(), Self::ALGORITHMS.iter().map(|a| (**a).clone()).collect(), keys, - BTreeMap::new(), + Default::default(), ) } @@ -757,9 +757,10 @@ impl ReadOnlyAccount { .await .expect("Newly created device keys can always be signed"); - device_keys.signatures.entry(self.user_id().to_owned()).or_default().insert( + device_keys.signatures.add_signature( + self.user_id().to_owned(), DeviceKeyId::from_parts(DeviceKeyAlgorithm::Ed25519, &self.device_id), - signature.to_base64(), + signature, ); device_keys diff --git a/crates/matrix-sdk-crypto/src/olm/signing/pk_signing.rs b/crates/matrix-sdk-crypto/src/olm/signing/pk_signing.rs index 5e02da1be..fd437feb2 100644 --- a/crates/matrix-sdk-crypto/src/olm/signing/pk_signing.rs +++ b/crates/matrix-sdk-crypto/src/olm/signing/pk_signing.rs @@ -224,12 +224,13 @@ impl SelfSigning { let serialized = serde_json::to_value(&device_keys)?; let signature = self.inner.sign_json(serialized)?; - device_keys.signatures.entry(self.public_key.user_id().to_owned()).or_default().insert( + device_keys.signatures.add_signature( + self.public_key.user_id().to_owned(), DeviceKeyId::from_parts( DeviceKeyAlgorithm::Ed25519, self.inner.public_key.to_base64().as_str().into(), ), - signature.to_base64(), + signature, ); Ok(()) diff --git a/crates/matrix-sdk-crypto/src/types/device_keys.rs b/crates/matrix-sdk-crypto/src/types/device_keys.rs index 6a60be23c..50e23147e 100644 --- a/crates/matrix-sdk-crypto/src/types/device_keys.rs +++ b/crates/matrix-sdk-crypto/src/types/device_keys.rs @@ -28,6 +28,8 @@ use serde::{Deserialize, Serialize}; use serde_json::{value::to_raw_value, Value}; use vodozemac::{Curve25519PublicKey, Ed25519PublicKey}; +use super::Signatures; + /// Identity keys for a device. #[derive(Clone, Debug, Deserialize, Serialize)] #[serde(try_from = "DeviceKeyHelper", into = "DeviceKeyHelper")] @@ -49,7 +51,7 @@ pub struct DeviceKeys { pub keys: BTreeMap, /// Signatures for the device key object. - pub signatures: BTreeMap>, + pub signatures: Signatures, /// Additional data added to the device key information by intermediate /// servers, and not covered by the signatures. @@ -68,7 +70,7 @@ impl DeviceKeys { device_id: OwnedDeviceId, algorithms: Vec, keys: BTreeMap, - signatures: BTreeMap>, + signatures: Signatures, ) -> Self { Self { user_id, @@ -154,7 +156,7 @@ struct DeviceKeyHelper { pub device_id: OwnedDeviceId, pub algorithms: Vec, pub keys: BTreeMap, - pub signatures: BTreeMap>, + pub signatures: Signatures, #[serde(default, skip_serializing_if = "UnsignedDeviceInfo::is_empty")] pub unsigned: UnsignedDeviceInfo, #[serde(flatten)] diff --git a/crates/matrix-sdk-crypto/src/types/mod.rs b/crates/matrix-sdk-crypto/src/types/mod.rs index 402e2df9e..f9fea4bfa 100644 --- a/crates/matrix-sdk-crypto/src/types/mod.rs +++ b/crates/matrix-sdk-crypto/src/types/mod.rs @@ -101,6 +101,11 @@ impl Signatures { pub fn get(&self, signer: &UserId) -> Option<&BTreeMap> { self.0.get(signer) } + + /// Remove all the signatures we currently hold. + pub fn clear(&mut self) { + self.0.clear() + } } impl Default for Signatures { diff --git a/crates/matrix-sdk-crypto/src/verification/mod.rs b/crates/matrix-sdk-crypto/src/verification/mod.rs index 496ca7188..672129cf2 100644 --- a/crates/matrix-sdk-crypto/src/verification/mod.rs +++ b/crates/matrix-sdk-crypto/src/verification/mod.rs @@ -20,10 +20,7 @@ mod qrcode; mod requests; mod sas; -use std::{ - collections::{BTreeMap, HashMap}, - sync::Arc, -}; +use std::{collections::HashMap, sync::Arc}; use event_enums::OutgoingContent; pub use machine::VerificationMachine; @@ -47,8 +44,8 @@ use ruma::{ }, AnyMessageLikeEventContent, AnyToDeviceEventContent, }, - DeviceId, EventId, OwnedDeviceId, OwnedDeviceKeyId, OwnedEventId, OwnedRoomId, - OwnedTransactionId, OwnedUserId, RoomId, UserId, + DeviceId, EventId, OwnedDeviceId, OwnedEventId, OwnedRoomId, OwnedTransactionId, RoomId, + UserId, }; pub use sas::{AcceptSettings, Sas}; use tracing::{error, info, trace, warn}; @@ -58,6 +55,7 @@ use crate::{ gossiping::{GossipMachine, GossipRequest}, olm::{PrivateCrossSigningIdentity, ReadOnlyAccount, Session}, store::{Changes, CryptoStore}, + types::Signatures, CryptoStoreError, LocalTrust, ReadOnlyDevice, ReadOnlyOwnUserIdentity, ReadOnlyUserIdentities, }; @@ -140,10 +138,7 @@ impl VerificationStore { } /// Get the signatures that have signed our own device. - pub async fn device_signatures( - &self, - ) -> Result>>, CryptoStoreError> - { + pub async fn device_signatures(&self) -> Result, CryptoStoreError> { Ok(self .inner .get_device(self.account.user_id(), self.account.device_id()) From 9b5b4ab2b234190345c7e2d18ce155d11ad477bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 26 May 2022 13:17:13 +0200 Subject: [PATCH 3/5] refactor(crypto): Use the Signatures struct for the CrossSigningKey --- .../matrix-sdk-crypto/src/identities/user.rs | 6 +-- crates/matrix-sdk-crypto/src/olm/account.rs | 5 ++- .../matrix-sdk-crypto/src/olm/signing/mod.rs | 7 ++- .../src/olm/signing/pk_signing.rs | 45 +++++++++---------- .../src/types/cross_signing_key.rs | 12 +++-- crates/matrix-sdk-crypto/src/types/mod.rs | 10 +++++ 6 files changed, 46 insertions(+), 39 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/identities/user.rs b/crates/matrix-sdk-crypto/src/identities/user.rs index b64e79a67..1491e5cc1 100644 --- a/crates/matrix-sdk-crypto/src/identities/user.rs +++ b/crates/matrix-sdk-crypto/src/identities/user.rs @@ -28,7 +28,7 @@ use ruma::{ events::{ key::verification::VerificationMethod, room::message::KeyVerificationRequestEventContent, }, - DeviceKeyId, EventId, OwnedDeviceId, OwnedDeviceKeyId, OwnedUserId, RoomId, UserId, + DeviceKeyId, EventId, OwnedDeviceId, OwnedDeviceKeyId, RoomId, UserId, }; use serde::{Deserialize, Serialize}; use serde_json::to_value; @@ -40,7 +40,7 @@ use crate::{ error::SignatureError, olm::VerifyJson, store::{Changes, IdentityChanges}, - types::{CrossSigningKey, DeviceKeys, SigningKey}, + types::{CrossSigningKey, DeviceKeys, Signatures, SigningKey}, verification::VerificationMachine, CryptoStoreError, OutgoingVerificationRequest, ReadOnlyDevice, VerificationRequest, }; @@ -404,7 +404,7 @@ impl MasterPubkey { } /// Get the signatures map of this cross signing key. - pub fn signatures(&self) -> &BTreeMap> { + pub fn signatures(&self) -> &Signatures { &self.0.signatures } diff --git a/crates/matrix-sdk-crypto/src/olm/account.rs b/crates/matrix-sdk-crypto/src/olm/account.rs index e9c21ec73..7997e55e2 100644 --- a/crates/matrix-sdk-crypto/src/olm/account.rs +++ b/crates/matrix-sdk-crypto/src/olm/account.rs @@ -780,9 +780,10 @@ impl ReadOnlyAccount { ) -> Result<(), SignatureError> { let signature = self.sign_json(serde_json::to_value(&cross_signing_key)?).await?; - cross_signing_key.signatures.entry(self.user_id().to_owned()).or_default().insert( + cross_signing_key.signatures.add_signature( + self.user_id().to_owned(), DeviceKeyId::from_parts(DeviceKeyAlgorithm::Ed25519, self.device_id()), - signature.to_base64(), + signature, ); Ok(()) diff --git a/crates/matrix-sdk-crypto/src/olm/signing/mod.rs b/crates/matrix-sdk-crypto/src/olm/signing/mod.rs index 501b5f134..56c0da79c 100644 --- a/crates/matrix-sdk-crypto/src/olm/signing/mod.rs +++ b/crates/matrix-sdk-crypto/src/olm/signing/mod.rs @@ -780,8 +780,11 @@ mod tests { let master = user_signing.sign_user(&bob_public).unwrap(); - let num_signatures: usize = master.signatures.iter().map(|(_, u)| u.len()).sum(); - assert_eq!(num_signatures, 1, "We're only uploading our own signature"); + assert_eq!( + master.signatures.signature_count(), + 1, + "We're only uploading our own signature" + ); bob_public.master_key = master.into(); diff --git a/crates/matrix-sdk-crypto/src/olm/signing/pk_signing.rs b/crates/matrix-sdk-crypto/src/olm/signing/pk_signing.rs index fd437feb2..6c445642c 100644 --- a/crates/matrix-sdk-crypto/src/olm/signing/pk_signing.rs +++ b/crates/matrix-sdk-crypto/src/olm/signing/pk_signing.rs @@ -24,7 +24,7 @@ use crate::{ error::SignatureError, identities::{MasterPubkey, SelfSigningPubkey, UserSigningPubkey}, olm::utility::SignJson, - types::{CrossSigningKey, CrossSigningKeySignatures, DeviceKeys}, + types::{CrossSigningKey, DeviceKeys, Signatures}, utilities::{encode, DecodeError}, ReadOnlyUserIdentity, }; @@ -128,17 +128,14 @@ impl MasterSigning { let json_subkey = serde_json::to_value(&subkey).expect("Can't serialize cross signing key"); let signature = self.inner.sign_json(json_subkey).expect("Can't sign cross signing keys"); - subkey - .signatures - .entry(self.public_key.user_id().to_owned()) - .or_insert_with(BTreeMap::new) - .insert( - DeviceKeyId::from_parts( - DeviceKeyAlgorithm::Ed25519, - self.inner.public_key.to_base64().as_str().into(), - ), - signature.to_base64(), - ); + subkey.signatures.add_signature( + self.public_key.user_id().to_owned(), + DeviceKeyId::from_parts( + DeviceKeyAlgorithm::Ed25519, + self.inner.public_key.to_base64().as_str().into(), + ), + signature, + ); } } @@ -175,22 +172,20 @@ impl UserSigning { pub fn sign_user_helper( &self, user: &ReadOnlyUserIdentity, - ) -> Result { + ) -> Result { let user_master: &CrossSigningKey = user.master_key().as_ref(); let signature = self.inner.sign_json(serde_json::to_value(user_master)?)?; - let mut signatures = BTreeMap::new(); + let mut signatures = Signatures::new(); - signatures - .entry(self.public_key.user_id().to_owned()) - .or_insert_with(BTreeMap::new) - .insert( - DeviceKeyId::from_parts( - DeviceKeyAlgorithm::Ed25519, - self.inner.public_key.to_base64().as_str().into(), - ), - signature.to_base64(), - ); + signatures.add_signature( + self.public_key.user_id().to_owned(), + DeviceKeyId::from_parts( + DeviceKeyAlgorithm::Ed25519, + self.inner.public_key.to_base64().as_str().into(), + ), + signature, + ); Ok(signatures) } @@ -311,7 +306,7 @@ impl Signing { self.inner.public_key().into(), )]); - CrossSigningKey::new(user_id, vec![usage], keys, BTreeMap::new()) + CrossSigningKey::new(user_id, vec![usage], keys, Default::default()) } pub fn sign(&self, message: &str) -> Ed25519Signature { diff --git a/crates/matrix-sdk-crypto/src/types/cross_signing_key.rs b/crates/matrix-sdk-crypto/src/types/cross_signing_key.rs index d1f74820c..3ae3babff 100644 --- a/crates/matrix-sdk-crypto/src/types/cross_signing_key.rs +++ b/crates/matrix-sdk-crypto/src/types/cross_signing_key.rs @@ -26,8 +26,7 @@ use serde::{Deserialize, Serialize}; use serde_json::{value::to_raw_value, Value}; use vodozemac::Ed25519PublicKey; -/// Signatures for a `CrossSigningKey` object. -pub type CrossSigningKeySignatures = BTreeMap>; +use super::Signatures; /// A cross signing key. #[derive(Clone, Debug, Deserialize, Serialize)] @@ -47,8 +46,7 @@ pub struct CrossSigningKey { /// Signatures of the key. /// /// Only optional for master key. - #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] - pub signatures: CrossSigningKeySignatures, + pub signatures: Signatures, #[serde(flatten)] other: BTreeMap, @@ -61,7 +59,7 @@ impl CrossSigningKey { user_id: OwnedUserId, usage: Vec, keys: BTreeMap, - signatures: CrossSigningKeySignatures, + signatures: Signatures, ) -> Self { Self { user_id, usage, keys, signatures, other: BTreeMap::new() } } @@ -106,8 +104,8 @@ struct CrossSigningKeyHelper { pub user_id: OwnedUserId, pub usage: Vec, pub keys: BTreeMap, - #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] - pub signatures: CrossSigningKeySignatures, + #[serde(default, skip_serializing_if = "Signatures::is_empty")] + pub signatures: Signatures, #[serde(flatten)] other: BTreeMap, } diff --git a/crates/matrix-sdk-crypto/src/types/mod.rs b/crates/matrix-sdk-crypto/src/types/mod.rs index f9fea4bfa..aff5df542 100644 --- a/crates/matrix-sdk-crypto/src/types/mod.rs +++ b/crates/matrix-sdk-crypto/src/types/mod.rs @@ -106,6 +106,16 @@ impl Signatures { pub fn clear(&mut self) { self.0.clear() } + + /// Do we hold any signatures or is our collection completely empty. + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + /// How many signatures do we currently hold. + pub fn signature_count(&self) -> usize { + self.0.iter().map(|(_, u)| u.len()).sum() + } } impl Default for Signatures { From a14549d5cc2078450b104b358e6e2f77934aefe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 27 May 2022 11:17:45 +0200 Subject: [PATCH 4/5] fix(crypto): Don't unwrap when converting to canonical json --- crates/matrix-sdk-crypto/Cargo.toml | 4 ++-- crates/matrix-sdk-crypto/src/error.rs | 10 ++++++++-- crates/matrix-sdk-crypto/src/olm/utility.rs | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/crates/matrix-sdk-crypto/Cargo.toml b/crates/matrix-sdk-crypto/Cargo.toml index 66debbaed..df7b71915 100644 --- a/crates/matrix-sdk-crypto/Cargo.toml +++ b/crates/matrix-sdk-crypto/Cargo.toml @@ -50,7 +50,7 @@ zeroize = { version = "1.3.0", features = ["zeroize_derive"] } [target.'cfg(target_arch = "wasm32")'.dependencies.ruma] version = "0.6.1" -features = ["client-api-c", "js", "rand", "unstable-msc2676", "unstable-msc2677"] +features = ["client-api-c", "js", "rand", "signatures", "unstable-msc2676", "unstable-msc2677"] [target.'cfg(target_arch = "wasm32")'.dependencies.vodozemac] git = "https://github.com/matrix-org/vodozemac/" @@ -59,7 +59,7 @@ features = ["js"] [target.'cfg(not(target_arch = "wasm32"))'.dependencies.ruma] version = "0.6.1" -features = ["client-api-c", "rand", "unstable-msc2676", "unstable-msc2677"] +features = ["client-api-c", "rand", "signatures", "unstable-msc2676", "unstable-msc2677"] [target.'cfg(not(target_arch = "wasm32"))'.dependencies.vodozemac] git = "https://github.com/matrix-org/vodozemac/" diff --git a/crates/matrix-sdk-crypto/src/error.rs b/crates/matrix-sdk-crypto/src/error.rs index 6cf1037eb..f6069a727 100644 --- a/crates/matrix-sdk-crypto/src/error.rs +++ b/crates/matrix-sdk-crypto/src/error.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use ruma::{IdParseError, OwnedDeviceId, OwnedRoomId, OwnedUserId}; +use ruma::{signatures::CanonicalJsonError, IdParseError, OwnedDeviceId, OwnedRoomId, OwnedUserId}; use serde_json::Error as SerdeError; use thiserror::Error; @@ -181,7 +181,13 @@ pub enum SignatureError { /// The signed object couldn't be deserialized. #[error(transparent)] - JsonError(#[from] SerdeError), + JsonError(#[from] CanonicalJsonError), +} + +impl From for SignatureError { + fn from(e: SerdeError) -> Self { + CanonicalJsonError::SerDe(e).into() + } } #[derive(Error, Debug)] diff --git a/crates/matrix-sdk-crypto/src/olm/utility.rs b/crates/matrix-sdk-crypto/src/olm/utility.rs index 76c5ad6b6..60b3b54fc 100644 --- a/crates/matrix-sdk-crypto/src/olm/utility.rs +++ b/crates/matrix-sdk-crypto/src/olm/utility.rs @@ -28,7 +28,7 @@ pub trait SignJson { let _ = json_object.remove("signatures"); let _ = json_object.remove("unsigned"); - let canonical_json: CanonicalJsonValue = value.try_into().unwrap(); + let canonical_json: CanonicalJsonValue = value.try_into()?; Ok(canonical_json.to_string()) } } From 808c4c6f3c1ac8c8719ab4aff353570d71fca11f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 27 May 2022 20:25:18 +0200 Subject: [PATCH 5/5] chore(crypto): Remove some indentation levels in the types module Co-authored-by: Jonas Platte --- crates/matrix-sdk-crypto/src/types/mod.rs | 46 +++++++++++------------ 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/types/mod.rs b/crates/matrix-sdk-crypto/src/types/mod.rs index aff5df542..e891dbf1a 100644 --- a/crates/matrix-sdk-crypto/src/types/mod.rs +++ b/crates/matrix-sdk-crypto/src/types/mod.rs @@ -143,31 +143,29 @@ impl<'de> Deserialize<'de> for Signatures { let map: BTreeMap> = serde::Deserialize::deserialize(deserializer)?; - Ok(Signatures( - map.into_iter() - .map(|(user, signatures)| { - Ok(( - user, - signatures - .into_iter() - .map(|(key_id, s)| { - let algorithm = key_id.algorithm(); - let signature = match algorithm { - DeviceKeyAlgorithm::Ed25519 => { - Ed25519Signature::from_base64(&s) - .map_err(serde::de::Error::custom)? - .into() - } - _ => Signature::Other(s), - }; + let map = map + .into_iter() + .map(|(user, signatures)| { + let signatures = signatures + .into_iter() + .map(|(key_id, s)| { + let algorithm = key_id.algorithm(); + let signature = match algorithm { + DeviceKeyAlgorithm::Ed25519 => Ed25519Signature::from_base64(&s) + .map_err(serde::de::Error::custom)? + .into(), + _ => Signature::Other(s), + }; - Ok((key_id, signature)) - }) - .collect::, _>>()?, - )) - }) - .collect::>()?, - )) + Ok((key_id, signature)) + }) + .collect::, _>>()?; + + Ok((user, signatures)) + }) + .collect::>()?; + + Ok(Signatures(map)) } }