From e5390e18dedeacadeaadc88520169bcf3aa81e32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 26 May 2022 11:53:45 +0200 Subject: [PATCH] refactor(crypto): Introduce a SignJson trait This patch collects our json signing logic into a single place. --- crates/matrix-sdk-crypto/src/olm/account.rs | 35 ++++++------- .../matrix-sdk-crypto/src/olm/signing/mod.rs | 27 ++++++++-- .../src/olm/signing/pk_signing.rs | 51 ++++++++----------- crates/matrix-sdk-crypto/src/olm/utility.rs | 30 +++++++++++ 4 files changed, 89 insertions(+), 54 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/olm/account.rs b/crates/matrix-sdk-crypto/src/olm/account.rs index 3341f5020..bea1c9a85 100644 --- a/crates/matrix-sdk-crypto/src/olm/account.rs +++ b/crates/matrix-sdk-crypto/src/olm/account.rs @@ -35,7 +35,7 @@ use ruma::{ }, AnyToDeviceEvent, OlmV1Keys, }, - serde::{CanonicalJsonValue, Raw}, + serde::Raw, DeviceId, DeviceKeyAlgorithm, DeviceKeyId, EventEncryptionAlgorithm, OwnedDeviceId, OwnedDeviceKeyId, OwnedUserId, RoomId, SecondsSinceUnixEpoch, UInt, UserId, }; @@ -49,8 +49,8 @@ use vodozemac::{ }; use super::{ - EncryptionSettings, InboundGroupSession, OutboundGroupSession, PrivateCrossSigningIdentity, - Session, + utility::SignJson, EncryptionSettings, InboundGroupSession, OutboundGroupSession, + PrivateCrossSigningIdentity, Session, }; use crate::{ error::{EventError, OlmResult, SessionCreationError}, @@ -752,10 +752,14 @@ impl ReadOnlyAccount { // get signed. let json_device_keys = serde_json::to_value(&device_keys).expect("device key is always safe to serialize"); + let signature = self + .sign_json(json_device_keys) + .await + .expect("Newly created device keys can always be signed"); device_keys.signatures.entry(self.user_id().to_owned()).or_default().insert( DeviceKeyId::from_parts(DeviceKeyAlgorithm::Ed25519, &self.device_id), - self.sign_json(json_device_keys).await.to_base64(), + signature.to_base64(), ); device_keys @@ -773,7 +777,7 @@ impl ReadOnlyAccount { &self, cross_signing_key: &mut CrossSigningKey, ) -> Result<(), SignatureError> { - let signature = self.sign_json(serde_json::to_value(&cross_signing_key)?).await; + 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( DeviceKeyId::from_parts(DeviceKeyAlgorithm::Ed25519, self.device_id()), @@ -809,19 +813,8 @@ impl ReadOnlyAccount { /// /// * `json` - The value that should be converted into a canonical JSON /// string. - /// - /// # Panic - /// - /// Panics if the json value can't be serialized. - pub async fn sign_json(&self, mut json: Value) -> Ed25519Signature { - let object = json.as_object_mut().expect("Canonical json value isn't an object"); - object.remove("unsigned"); - object.remove("signatures"); - - let canonical_json: CanonicalJsonValue = - json.try_into().expect("Can't canonicalize the json value"); - - self.sign(&canonical_json.to_string()).await + pub async fn sign_json(&self, json: Value) -> Result { + self.inner.lock().await.sign_json(json) } /// Generate, sign and prepare one-time keys to be uploaded. @@ -885,8 +878,10 @@ impl ReadOnlyAccount { SignedKey::new(key.to_owned()) }; - let signature = - self.sign_json(serde_json::to_value(&key).expect("Can't serialize a signed key")).await; + let signature = self + .sign_json(serde_json::to_value(&key).expect("Can't serialize a signed key")) + .await + .expect("Newly created one-time keys can always be signed"); let signatures = BTreeMap::from([( self.user_id().to_owned(), diff --git a/crates/matrix-sdk-crypto/src/olm/signing/mod.rs b/crates/matrix-sdk-crypto/src/olm/signing/mod.rs index 9a538653b..501b5f134 100644 --- a/crates/matrix-sdk-crypto/src/olm/signing/mod.rs +++ b/crates/matrix-sdk-crypto/src/olm/signing/mod.rs @@ -651,13 +651,16 @@ impl PrivateCrossSigningIdentity { #[cfg(test)] mod tests { + use std::collections::BTreeMap; + use matrix_sdk_test::async_test; - use ruma::{device_id, user_id, UserId}; + use ruma::{device_id, user_id, DeviceKeyAlgorithm, DeviceKeyId, UserId}; + use serde_json::json; use super::{PrivateCrossSigningIdentity, Signing}; use crate::{ identities::{ReadOnlyDevice, ReadOnlyUserIdentity}, - olm::ReadOnlyAccount, + olm::{utility::SignJson, ReadOnlyAccount}, }; fn user_id() -> &'static UserId { @@ -667,11 +670,25 @@ mod tests { #[test] fn signature_verification() { let signing = Signing::new(); + let user_id = user_id(); + let key_id = DeviceKeyId::from_parts(DeviceKeyAlgorithm::Ed25519, "DEVICEID".into()); - let message = "Hello world"; + let json = json!({ + "hello": "world" + }); - let signature = signing.sign(message); - assert!(signing.verify(message, &signature).is_ok()); + let signature = + signing.sign_json(json).expect("We should be able to sign a simple json object"); + let signatures = + BTreeMap::from([(user_id, BTreeMap::from([(key_id.clone(), signature.to_base64())]))]); + + let mut json = json!({ + "hello": "world", + "signatures": signatures, + + }); + + assert!(signing.verify_json(user_id, &key_id, &mut json).is_ok()); } #[test] 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 1f7b137ba..5e02da1be 100644 --- a/crates/matrix-sdk-crypto/src/olm/signing/pk_signing.rs +++ b/crates/matrix-sdk-crypto/src/olm/signing/pk_signing.rs @@ -12,11 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::{collections::BTreeMap, convert::TryInto}; +use std::collections::BTreeMap; -use ruma::{ - encryption::KeyUsage, serde::CanonicalJsonValue, DeviceKeyAlgorithm, DeviceKeyId, OwnedUserId, -}; +use ruma::{encryption::KeyUsage, DeviceKeyAlgorithm, DeviceKeyId, OwnedUserId}; use serde::{Deserialize, Serialize}; use serde_json::{Error as JsonError, Value}; use thiserror::Error; @@ -25,6 +23,7 @@ use vodozemac::{Ed25519PublicKey, Ed25519SecretKey, Ed25519Signature, KeyError}; use crate::{ error::SignatureError, identities::{MasterPubkey, SelfSigningPubkey, UserSigningPubkey}, + olm::utility::SignJson, types::{CrossSigningKey, CrossSigningKeySignatures, DeviceKeys}, utilities::{encode, DecodeError}, ReadOnlyUserIdentity, @@ -64,6 +63,12 @@ impl PartialEq for Signing { } } +impl SignJson for Signing { + fn sign_json(&self, value: Value) -> Result { + self.inner.sign_json(value) + } +} + #[derive(PartialEq, Debug)] pub struct MasterSigning { pub inner: Signing, @@ -215,12 +220,9 @@ impl SelfSigning { Ok(Self { inner, public_key }) } - pub fn sign_device_helper(&self, value: Value) -> Result { - self.inner.sign_json(value) - } - pub fn sign_device(&self, device_keys: &mut DeviceKeys) -> Result<(), SignatureError> { - let signature = self.sign_device_helper(serde_json::to_value(&device_keys)?)?; + 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( DeviceKeyId::from_parts( @@ -311,27 +313,18 @@ impl Signing { CrossSigningKey::new(user_id, vec![usage], keys, BTreeMap::new()) } - #[cfg(test)] - pub fn verify( - &self, - message: &str, - signature: &Ed25519Signature, - ) -> Result<(), SignatureError> { - Ok(self.public_key.verify(message.as_bytes(), signature)?) - } - - pub fn sign_json(&self, mut json: Value) -> Result { - let json_object = json.as_object_mut().ok_or(SignatureError::NotAnObject)?; - let _ = json_object.remove("signatures"); - let _ = json_object.remove("unsigned"); - - let canonical_json: CanonicalJsonValue = - json.try_into().expect("Can't canonicalize the json value"); - - Ok(self.sign(&canonical_json.to_string())) - } - pub fn sign(&self, message: &str) -> Ed25519Signature { self.inner.sign(message.as_bytes()) } + + #[cfg(test)] + pub fn verify_json( + &self, + user_id: &ruma::UserId, + key_id: &DeviceKeyId, + message: &mut Value, + ) -> Result<(), SignatureError> { + use crate::olm::VerifyJson; + self.public_key.verify_json(user_id, key_id, message) + } } diff --git a/crates/matrix-sdk-crypto/src/olm/utility.rs b/crates/matrix-sdk-crypto/src/olm/utility.rs index a95ec607f..76c5ad6b6 100644 --- a/crates/matrix-sdk-crypto/src/olm/utility.rs +++ b/crates/matrix-sdk-crypto/src/olm/utility.rs @@ -16,9 +16,39 @@ use std::convert::TryInto; use ruma::{serde::CanonicalJsonValue, DeviceKeyAlgorithm, DeviceKeyId, UserId}; use serde_json::Value; +use vodozemac::{olm::Account, Ed25519SecretKey, Ed25519Signature}; use crate::error::SignatureError; +pub trait SignJson { + fn sign_json(&self, value: Value) -> Result; + + fn to_signable_json(mut value: Value) -> Result { + let json_object = value.as_object_mut().ok_or(SignatureError::NotAnObject)?; + let _ = json_object.remove("signatures"); + let _ = json_object.remove("unsigned"); + + let canonical_json: CanonicalJsonValue = value.try_into().unwrap(); + Ok(canonical_json.to_string()) + } +} + +impl SignJson for Account { + fn sign_json(&self, value: Value) -> Result { + let serialized = Self::to_signable_json(value)?; + + Ok(self.sign(serialized.as_ref())) + } +} + +impl SignJson for Ed25519SecretKey { + fn sign_json(&self, value: Value) -> Result { + let serialized = Self::to_signable_json(value)?; + + Ok(self.sign(serialized.as_ref())) + } +} + pub trait VerifyJson { /// Verify a signed JSON object. ///