From b68fc0604f1b1cbc4d58339fd18d79a83e8607e9 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Tue, 9 Apr 2024 12:06:27 +0200 Subject: [PATCH 1/2] chore: Fix clippy warnings about non-local impl definition Signed-off-by: Johannes Marbach --- src/megolm/group_session.rs | 73 ++++---- src/megolm/inbound_group_session.rs | 95 +++++----- src/olm/session/mod.rs | 271 +++++++++++++++------------- 3 files changed, 231 insertions(+), 208 deletions(-) diff --git a/src/megolm/group_session.rs b/src/megolm/group_session.rs index 62bc5f26..8d99ea70 100644 --- a/src/megolm/group_session.rs +++ b/src/megolm/group_session.rs @@ -140,46 +140,51 @@ impl GroupSession { pub fn from_pickle(pickle: GroupSessionPickle) -> Self { pickle.into() } +} - #[cfg(feature = "libolm-compat")] - pub fn from_libolm_pickle( - pickle: &str, - pickle_key: &[u8], - ) -> Result { - use matrix_pickle::Decode; - use zeroize::Zeroize; - - use crate::{ - megolm::libolm::LibolmRatchetPickle, - utilities::{unpickle_libolm, LibolmEd25519Keypair}, - }; - - #[derive(Zeroize, Decode)] - #[zeroize(drop)] - struct Pickle { - version: u32, - ratchet: LibolmRatchetPickle, - ed25519_keypair: LibolmEd25519Keypair, - } +#[cfg(feature = "libolm-compat")] +mod libolm_compat { + use matrix_pickle::Decode; + use zeroize::Zeroize; + + use super::GroupSession; + use crate::{ + megolm::{libolm::LibolmRatchetPickle, SessionConfig}, + utilities::{unpickle_libolm, LibolmEd25519Keypair}, + Ed25519Keypair, + }; + + #[derive(Zeroize, Decode)] + #[zeroize(drop)] + struct Pickle { + version: u32, + ratchet: LibolmRatchetPickle, + ed25519_keypair: LibolmEd25519Keypair, + } - impl TryFrom for GroupSession { - type Error = crate::LibolmPickleError; + impl TryFrom for GroupSession { + type Error = crate::LibolmPickleError; - fn try_from(pickle: Pickle) -> Result { - // Removing the borrow doesn't work and clippy complains about - // this on nightly. - #[allow(clippy::needless_borrow)] - let ratchet = (&pickle.ratchet).into(); - let signing_key = - Ed25519Keypair::from_expanded_key(&pickle.ed25519_keypair.private_key)?; + fn try_from(pickle: Pickle) -> Result { + // Removing the borrow doesn't work and clippy complains about + // this on nightly. + #[allow(clippy::needless_borrow)] + let ratchet = (&pickle.ratchet).into(); + let signing_key = + Ed25519Keypair::from_expanded_key(&pickle.ed25519_keypair.private_key)?; - Ok(Self { ratchet, signing_key, config: SessionConfig::version_1() }) - } + Ok(Self { ratchet, signing_key, config: SessionConfig::version_1() }) } + } - const PICKLE_VERSION: u32 = 1; - - unpickle_libolm::(pickle, pickle_key, PICKLE_VERSION) + impl GroupSession { + pub fn from_libolm_pickle( + pickle: &str, + pickle_key: &[u8], + ) -> Result { + const PICKLE_VERSION: u32 = 1; + unpickle_libolm::(pickle, pickle_key, PICKLE_VERSION) + } } } diff --git a/src/megolm/inbound_group_session.rs b/src/megolm/inbound_group_session.rs index 233d52c7..3664dc35 100644 --- a/src/megolm/inbound_group_session.rs +++ b/src/megolm/inbound_group_session.rs @@ -19,7 +19,6 @@ use hmac::digest::MacError; use serde::{Deserialize, Serialize}; use subtle::ConstantTimeEq; use thiserror::Error; -use zeroize::Zeroize; use super::{ default_config, @@ -365,53 +364,61 @@ impl InboundGroupSession { pub fn from_pickle(pickle: InboundGroupSessionPickle) -> Self { Self::from(pickle) } +} - #[cfg(feature = "libolm-compat")] - pub fn from_libolm_pickle( - pickle: &str, - pickle_key: &[u8], - ) -> Result { - use matrix_pickle::Decode; - - use super::libolm::LibolmRatchetPickle; - use crate::utilities::unpickle_libolm; - - #[derive(Zeroize, Decode)] - #[zeroize(drop)] - struct Pickle { - version: u32, - initial_ratchet: LibolmRatchetPickle, - latest_ratchet: LibolmRatchetPickle, - signing_key: [u8; 32], - signing_key_verified: bool, - } +#[cfg(feature = "libolm-compat")] +mod libolm_compat { + use matrix_pickle::Decode; + use zeroize::Zeroize; - impl TryFrom for InboundGroupSession { - type Error = crate::LibolmPickleError; - - fn try_from(pickle: Pickle) -> Result { - // Removing the borrow doesn't work and clippy complains about - // this on nightly. - #[allow(clippy::needless_borrow)] - let initial_ratchet = (&pickle.initial_ratchet).into(); - #[allow(clippy::needless_borrow)] - let latest_ratchet = (&pickle.latest_ratchet).into(); - let signing_key = Ed25519PublicKey::from_slice(&pickle.signing_key)?; - let signing_key_verified = pickle.signing_key_verified; - - Ok(Self { - initial_ratchet, - latest_ratchet, - signing_key, - signing_key_verified, - config: SessionConfig::version_1(), - }) - } - } + use super::InboundGroupSession; + use crate::{ + megolm::{libolm::LibolmRatchetPickle, SessionConfig}, + utilities::unpickle_libolm, + Ed25519PublicKey, + }; + + #[derive(Zeroize, Decode)] + #[zeroize(drop)] + struct Pickle { + version: u32, + initial_ratchet: LibolmRatchetPickle, + latest_ratchet: LibolmRatchetPickle, + signing_key: [u8; 32], + signing_key_verified: bool, + } - const PICKLE_VERSION: u32 = 2; + impl TryFrom for InboundGroupSession { + type Error = crate::LibolmPickleError; + + fn try_from(pickle: Pickle) -> Result { + // Removing the borrow doesn't work and clippy complains about + // this on nightly. + #[allow(clippy::needless_borrow)] + let initial_ratchet = (&pickle.initial_ratchet).into(); + #[allow(clippy::needless_borrow)] + let latest_ratchet = (&pickle.latest_ratchet).into(); + let signing_key = Ed25519PublicKey::from_slice(&pickle.signing_key)?; + let signing_key_verified = pickle.signing_key_verified; + + Ok(Self { + initial_ratchet, + latest_ratchet, + signing_key, + signing_key_verified, + config: SessionConfig::version_1(), + }) + } + } - unpickle_libolm::(pickle, pickle_key, PICKLE_VERSION) + impl InboundGroupSession { + pub fn from_libolm_pickle( + pickle: &str, + pickle_key: &[u8], + ) -> Result { + const PICKLE_VERSION: u32 = 2; + unpickle_libolm::(pickle, pickle_key, PICKLE_VERSION) + } } } diff --git a/src/olm/session/mod.rs b/src/olm/session/mod.rs index 8b9a9f16..3590b650 100644 --- a/src/olm/session/mod.rs +++ b/src/olm/session/mod.rs @@ -32,7 +32,6 @@ use receiver_chain::ReceiverChain; use root_key::RemoteRootKey; use serde::{Deserialize, Serialize}; use thiserror::Error; -use zeroize::Zeroize; use super::{ session_config::Version, @@ -318,155 +317,167 @@ impl Session { pub fn from_pickle(pickle: SessionPickle) -> Self { pickle.into() } +} - /// Create a [`Session`] object by unpickling a session pickle in libolm - /// legacy pickle format. - /// - /// Such pickles are encrypted and need to first be decrypted using - /// `pickle_key`. - #[cfg(feature = "libolm-compat")] - pub fn from_libolm_pickle( - pickle: &str, - pickle_key: &[u8], - ) -> Result { - use chain_key::ChainKey; - use matrix_pickle::Decode; - use message_key::RemoteMessageKey; - use ratchet::{Ratchet, RatchetKey}; - use root_key::RootKey; - - use crate::{types::Curve25519SecretKey, utilities::unpickle_libolm}; - - #[derive(Debug, Decode, Zeroize)] - #[zeroize(drop)] - struct SenderChain { - public_ratchet_key: [u8; 32], - #[secret] - secret_ratchet_key: Box<[u8; 32]>, - chain_key: Box<[u8; 32]>, - chain_key_index: u32, - } +#[cfg(feature = "libolm-compat")] +mod libolm_compat { + use matrix_pickle::Decode; + use zeroize::Zeroize; + + use super::{ + chain_key::{ChainKey, RemoteChainKey}, + double_ratchet::{DoubleRatchet, RatchetCount}, + message_key::RemoteMessageKey, + ratchet::{Ratchet, RatchetKey, RemoteRatchetKey}, + receiver_chain::ReceiverChain, + root_key::{RemoteRootKey, RootKey}, + ChainStore, Session, + }; + use crate::{ + olm::{SessionConfig, SessionKeys}, + types::Curve25519SecretKey, + utilities::unpickle_libolm, + Curve25519PublicKey, + }; - #[derive(Debug, Decode, Zeroize)] - #[zeroize(drop)] - struct ReceivingChain { - public_ratchet_key: [u8; 32], - #[secret] - chain_key: Box<[u8; 32]>, - chain_key_index: u32, + #[derive(Debug, Decode, Zeroize)] + #[zeroize(drop)] + struct SenderChain { + public_ratchet_key: [u8; 32], + #[secret] + secret_ratchet_key: Box<[u8; 32]>, + chain_key: Box<[u8; 32]>, + chain_key_index: u32, + } + + #[derive(Debug, Decode, Zeroize)] + #[zeroize(drop)] + struct ReceivingChain { + public_ratchet_key: [u8; 32], + #[secret] + chain_key: Box<[u8; 32]>, + chain_key_index: u32, + } + + impl From<&ReceivingChain> for ReceiverChain { + fn from(chain: &ReceivingChain) -> Self { + let ratchet_key = RemoteRatchetKey::from(chain.public_ratchet_key); + let chain_key = RemoteChainKey::from_bytes_and_index( + chain.chain_key.clone(), + chain.chain_key_index, + ); + + ReceiverChain::new(ratchet_key, chain_key, RatchetCount::unknown()) } + } - impl From<&ReceivingChain> for ReceiverChain { - fn from(chain: &ReceivingChain) -> Self { - let ratchet_key = RemoteRatchetKey::from(chain.public_ratchet_key); - let chain_key = RemoteChainKey::from_bytes_and_index( - chain.chain_key.clone(), - chain.chain_key_index, - ); + #[derive(Debug, Decode, Zeroize)] + #[zeroize(drop)] + struct MessageKey { + ratchet_key: [u8; 32], + #[secret] + message_key: Box<[u8; 32]>, + index: u32, + } - ReceiverChain::new(ratchet_key, chain_key, RatchetCount::unknown()) - } + impl From<&MessageKey> for RemoteMessageKey { + fn from(key: &MessageKey) -> Self { + RemoteMessageKey { key: key.message_key.clone(), index: key.index.into() } } + } - #[derive(Debug, Decode, Zeroize)] - #[zeroize(drop)] - struct MessageKey { - ratchet_key: [u8; 32], - #[secret] - message_key: Box<[u8; 32]>, - index: u32, + #[derive(Decode)] + struct Pickle { + #[allow(dead_code)] + version: u32, + #[allow(dead_code)] + received_message: bool, + session_keys: SessionKeys, + #[secret] + root_key: Box<[u8; 32]>, + sender_chains: Vec, + receiver_chains: Vec, + message_keys: Vec, + } + + impl Drop for Pickle { + fn drop(&mut self) { + self.root_key.zeroize(); + self.sender_chains.zeroize(); + self.receiver_chains.zeroize(); + self.message_keys.zeroize(); } + } - impl From<&MessageKey> for RemoteMessageKey { - fn from(key: &MessageKey) -> Self { - RemoteMessageKey { key: key.message_key.clone(), index: key.index.into() } - } - } + impl TryFrom for Session { + type Error = crate::LibolmPickleError; - #[derive(Decode)] - struct Pickle { - #[allow(dead_code)] - version: u32, - #[allow(dead_code)] - received_message: bool, - session_keys: SessionKeys, - #[secret] - root_key: Box<[u8; 32]>, - sender_chains: Vec, - receiver_chains: Vec, - message_keys: Vec, - } + fn try_from(pickle: Pickle) -> Result { + let mut receiving_chains = ChainStore::new(); - impl Drop for Pickle { - fn drop(&mut self) { - self.root_key.zeroize(); - self.sender_chains.zeroize(); - self.receiver_chains.zeroize(); - self.message_keys.zeroize(); + for chain in &pickle.receiver_chains { + receiving_chains.push(chain.into()) } - } - - impl TryFrom for Session { - type Error = crate::LibolmPickleError; - fn try_from(pickle: Pickle) -> Result { - let mut receiving_chains = ChainStore::new(); + for key in &pickle.message_keys { + let ratchet_key = + RemoteRatchetKey::from(Curve25519PublicKey::from(key.ratchet_key)); - for chain in &pickle.receiver_chains { - receiving_chains.push(chain.into()) + if let Some(receiving_chain) = receiving_chains.find_ratchet(&ratchet_key) { + receiving_chain.insert_message_key(key.into()) } + } - for key in &pickle.message_keys { - let ratchet_key = - RemoteRatchetKey::from(Curve25519PublicKey::from(key.ratchet_key)); - - if let Some(receiving_chain) = receiving_chains.find_ratchet(&ratchet_key) { - receiving_chain.insert_message_key(key.into()) - } - } + if let Some(chain) = pickle.sender_chains.first() { + // XXX: Passing in secret array as value. + let ratchet_key = RatchetKey::from(Curve25519SecretKey::from_slice( + chain.secret_ratchet_key.as_ref(), + )); + let chain_key = + ChainKey::from_bytes_and_index(chain.chain_key.clone(), chain.chain_key_index); + + let root_key = RootKey::new(pickle.root_key.clone()); + + let ratchet = Ratchet::new_with_ratchet_key(root_key, ratchet_key); + let sending_ratchet = DoubleRatchet::from_ratchet_and_chain_key(ratchet, chain_key); + + Ok(Self { + session_keys: pickle.session_keys, + sending_ratchet, + receiving_chains, + config: SessionConfig::version_1(), + }) + } else if let Some(chain) = receiving_chains.get(0) { + let sending_ratchet = DoubleRatchet::inactive_from_libolm_pickle( + RemoteRootKey::new(pickle.root_key.clone()), + chain.ratchet_key(), + ); - if let Some(chain) = pickle.sender_chains.first() { - // XXX: Passing in secret array as value. - let ratchet_key = RatchetKey::from(Curve25519SecretKey::from_slice( - chain.secret_ratchet_key.as_ref(), - )); - let chain_key = ChainKey::from_bytes_and_index( - chain.chain_key.clone(), - chain.chain_key_index, - ); - - let root_key = RootKey::new(pickle.root_key.clone()); - - let ratchet = Ratchet::new_with_ratchet_key(root_key, ratchet_key); - let sending_ratchet = - DoubleRatchet::from_ratchet_and_chain_key(ratchet, chain_key); - - Ok(Self { - session_keys: pickle.session_keys, - sending_ratchet, - receiving_chains, - config: SessionConfig::version_1(), - }) - } else if let Some(chain) = receiving_chains.get(0) { - let sending_ratchet = DoubleRatchet::inactive_from_libolm_pickle( - RemoteRootKey::new(pickle.root_key.clone()), - chain.ratchet_key(), - ); - - Ok(Self { - session_keys: pickle.session_keys, - sending_ratchet, - receiving_chains, - config: SessionConfig::version_1(), - }) - } else { - Err(crate::LibolmPickleError::InvalidSession) - } + Ok(Self { + session_keys: pickle.session_keys, + sending_ratchet, + receiving_chains, + config: SessionConfig::version_1(), + }) + } else { + Err(crate::LibolmPickleError::InvalidSession) } } + } - const PICKLE_VERSION: u32 = 1; - unpickle_libolm::(pickle, pickle_key, PICKLE_VERSION) + impl Session { + /// Create a [`Session`] object by unpickling a session pickle in libolm + /// legacy pickle format. + /// + /// Such pickles are encrypted and need to first be decrypted using + /// `pickle_key`. + pub fn from_libolm_pickle( + pickle: &str, + pickle_key: &[u8], + ) -> Result { + const PICKLE_VERSION: u32 = 1; + unpickle_libolm::(pickle, pickle_key, PICKLE_VERSION) + } } } From e6722e423ee4ec7b72e64aa6978b78f6cd3cca1a Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 19 Apr 2024 13:31:14 +0200 Subject: [PATCH 2/2] Move public methods back into their original place --- src/megolm/group_session.rs | 25 +++++++++++---------- src/megolm/inbound_group_session.rs | 26 ++++++++++++---------- src/olm/session/mod.rs | 34 ++++++++++++++--------------- 3 files changed, 44 insertions(+), 41 deletions(-) diff --git a/src/megolm/group_session.rs b/src/megolm/group_session.rs index 8d99ea70..194a2438 100644 --- a/src/megolm/group_session.rs +++ b/src/megolm/group_session.rs @@ -140,6 +140,17 @@ impl GroupSession { pub fn from_pickle(pickle: GroupSessionPickle) -> Self { pickle.into() } + + #[cfg(feature = "libolm-compat")] + pub fn from_libolm_pickle( + pickle: &str, + pickle_key: &[u8], + ) -> Result { + use crate::{megolm::group_session::libolm_compat::Pickle, utilities::unpickle_libolm}; + + const PICKLE_VERSION: u32 = 1; + unpickle_libolm::(pickle, pickle_key, PICKLE_VERSION) + } } #[cfg(feature = "libolm-compat")] @@ -150,13 +161,13 @@ mod libolm_compat { use super::GroupSession; use crate::{ megolm::{libolm::LibolmRatchetPickle, SessionConfig}, - utilities::{unpickle_libolm, LibolmEd25519Keypair}, + utilities::LibolmEd25519Keypair, Ed25519Keypair, }; #[derive(Zeroize, Decode)] #[zeroize(drop)] - struct Pickle { + pub(super) struct Pickle { version: u32, ratchet: LibolmRatchetPickle, ed25519_keypair: LibolmEd25519Keypair, @@ -176,16 +187,6 @@ mod libolm_compat { Ok(Self { ratchet, signing_key, config: SessionConfig::version_1() }) } } - - impl GroupSession { - pub fn from_libolm_pickle( - pickle: &str, - pickle_key: &[u8], - ) -> Result { - const PICKLE_VERSION: u32 = 1; - unpickle_libolm::(pickle, pickle_key, PICKLE_VERSION) - } - } } /// A format suitable for serialization which implements [`serde::Serialize`] diff --git a/src/megolm/inbound_group_session.rs b/src/megolm/inbound_group_session.rs index 3664dc35..aeab6763 100644 --- a/src/megolm/inbound_group_session.rs +++ b/src/megolm/inbound_group_session.rs @@ -364,6 +364,19 @@ impl InboundGroupSession { pub fn from_pickle(pickle: InboundGroupSessionPickle) -> Self { Self::from(pickle) } + + #[cfg(feature = "libolm-compat")] + pub fn from_libolm_pickle( + pickle: &str, + pickle_key: &[u8], + ) -> Result { + use crate::{ + megolm::inbound_group_session::libolm_compat::Pickle, utilities::unpickle_libolm, + }; + + const PICKLE_VERSION: u32 = 2; + unpickle_libolm::(pickle, pickle_key, PICKLE_VERSION) + } } #[cfg(feature = "libolm-compat")] @@ -374,13 +387,12 @@ mod libolm_compat { use super::InboundGroupSession; use crate::{ megolm::{libolm::LibolmRatchetPickle, SessionConfig}, - utilities::unpickle_libolm, Ed25519PublicKey, }; #[derive(Zeroize, Decode)] #[zeroize(drop)] - struct Pickle { + pub(super) struct Pickle { version: u32, initial_ratchet: LibolmRatchetPickle, latest_ratchet: LibolmRatchetPickle, @@ -410,16 +422,6 @@ mod libolm_compat { }) } } - - impl InboundGroupSession { - pub fn from_libolm_pickle( - pickle: &str, - pickle_key: &[u8], - ) -> Result { - const PICKLE_VERSION: u32 = 2; - unpickle_libolm::(pickle, pickle_key, PICKLE_VERSION) - } - } } /// A format suitable for serialization which implements [`serde::Serialize`] diff --git a/src/olm/session/mod.rs b/src/olm/session/mod.rs index 3590b650..59c72a20 100644 --- a/src/olm/session/mod.rs +++ b/src/olm/session/mod.rs @@ -317,6 +317,22 @@ impl Session { pub fn from_pickle(pickle: SessionPickle) -> Self { pickle.into() } + + /// Create a [`Session`] object by unpickling a session pickle in libolm + /// legacy pickle format. + /// + /// Such pickles are encrypted and need to first be decrypted using + /// `pickle_key`. + #[cfg(feature = "libolm-compat")] + pub fn from_libolm_pickle( + pickle: &str, + pickle_key: &[u8], + ) -> Result { + use crate::{olm::session::libolm_compat::Pickle, utilities::unpickle_libolm}; + + const PICKLE_VERSION: u32 = 1; + unpickle_libolm::(pickle, pickle_key, PICKLE_VERSION) + } } #[cfg(feature = "libolm-compat")] @@ -336,7 +352,6 @@ mod libolm_compat { use crate::{ olm::{SessionConfig, SessionKeys}, types::Curve25519SecretKey, - utilities::unpickle_libolm, Curve25519PublicKey, }; @@ -387,7 +402,7 @@ mod libolm_compat { } #[derive(Decode)] - struct Pickle { + pub(super) struct Pickle { #[allow(dead_code)] version: u32, #[allow(dead_code)] @@ -464,21 +479,6 @@ mod libolm_compat { } } } - - impl Session { - /// Create a [`Session`] object by unpickling a session pickle in libolm - /// legacy pickle format. - /// - /// Such pickles are encrypted and need to first be decrypted using - /// `pickle_key`. - pub fn from_libolm_pickle( - pickle: &str, - pickle_key: &[u8], - ) -> Result { - const PICKLE_VERSION: u32 = 1; - unpickle_libolm::(pickle, pickle_key, PICKLE_VERSION) - } - } } /// A format suitable for serialization which implements [`serde::Serialize`]