From 04c96f9664137facb9904beadf3cbab8566ae569 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 17 Nov 2022 13:18:00 +0100 Subject: [PATCH] refactor: Use the matrix-pickle crate for libolm unpickling support --- Cargo.toml | 3 +- src/lib.rs | 2 +- src/megolm/group_session.rs | 17 +-- src/megolm/inbound_group_session.rs | 20 +--- src/megolm/mod.rs | 16 +-- src/olm/account/mod.rs | 51 ++------- src/olm/session/mod.rs | 72 ++----------- src/olm/session/ratchet.rs | 14 +-- src/olm/session_keys.rs | 16 +-- src/types/curve25519.rs | 9 +- src/utilities/libolm_compat.rs | 154 +--------------------------- src/utilities/mod.rs | 4 +- 12 files changed, 43 insertions(+), 335 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 8fa9e58d..3cbff241 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,7 @@ version = "0.3.0" edition = "2021" keywords = ["matrix", "chat", "messaging", "olm"] license = "Apache-2.0" -rust-version = "1.56" +rust-version = "1.65" [package.metadata.docs.rs] all-features = true @@ -40,6 +40,7 @@ ed25519-dalek = { version = "1.0.1", default-features = false, features = [ ] } hkdf = "0.12.3" hmac = "0.12.1" +matrix-pickle = { version = "0.1.0" } pkcs7 = "0.3.0" prost = "0.11.0" rand = "0.7.3" diff --git a/src/lib.rs b/src/lib.rs index 5df8ed64..f7611df3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -265,7 +265,7 @@ pub enum LibolmPickleError { InvalidSession, /// The payload of the pickle could not be decoded. #[error(transparent)] - Decode(#[from] crate::utilities::LibolmDecodeError), + Decode(#[from] matrix_pickle::DecodeError), } /// Error type describing the different ways message decoding can fail. diff --git a/src/megolm/group_session.rs b/src/megolm/group_session.rs index 6bf37c5d..62bc5f26 100644 --- a/src/megolm/group_session.rs +++ b/src/megolm/group_session.rs @@ -146,16 +146,15 @@ impl GroupSession { pickle: &str, pickle_key: &[u8], ) -> Result { - use std::io::Read; - + use matrix_pickle::Decode; use zeroize::Zeroize; use crate::{ megolm::libolm::LibolmRatchetPickle, - utilities::{unpickle_libolm, Decode, LibolmEd25519Keypair}, + utilities::{unpickle_libolm, LibolmEd25519Keypair}, }; - #[derive(Zeroize)] + #[derive(Zeroize, Decode)] #[zeroize(drop)] struct Pickle { version: u32, @@ -163,16 +162,6 @@ impl GroupSession { ed25519_keypair: LibolmEd25519Keypair, } - impl Decode for Pickle { - fn decode(reader: &mut impl Read) -> Result { - Ok(Pickle { - version: u32::decode(reader)?, - ratchet: LibolmRatchetPickle::decode(reader)?, - ed25519_keypair: LibolmEd25519Keypair::decode(reader)?, - }) - } - } - impl TryFrom for GroupSession { type Error = crate::LibolmPickleError; diff --git a/src/megolm/inbound_group_session.rs b/src/megolm/inbound_group_session.rs index 6870caf6..3486bea1 100644 --- a/src/megolm/inbound_group_session.rs +++ b/src/megolm/inbound_group_session.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::{cmp::Ordering, io::Read}; +use std::cmp::Ordering; use aes::cipher::block_padding::UnpadError; use hmac::digest::MacError; @@ -371,10 +371,12 @@ impl InboundGroupSession { pickle: &str, pickle_key: &[u8], ) -> Result { + use matrix_pickle::Decode; + use super::libolm::LibolmRatchetPickle; - use crate::utilities::{unpickle_libolm, Decode}; + use crate::utilities::unpickle_libolm; - #[derive(Zeroize)] + #[derive(Zeroize, Decode)] #[zeroize(drop)] struct Pickle { version: u32, @@ -384,18 +386,6 @@ impl InboundGroupSession { signing_key_verified: bool, } - impl Decode for Pickle { - fn decode(reader: &mut impl Read) -> Result { - Ok(Pickle { - version: u32::decode(reader)?, - initial_ratchet: LibolmRatchetPickle::decode(reader)?, - latest_ratchet: LibolmRatchetPickle::decode(reader)?, - signing_key: <[u8; 32]>::decode(reader)?, - signing_key_verified: bool::decode(reader)?, - }) - } - } - impl TryFrom for InboundGroupSession { type Error = crate::LibolmPickleError; diff --git a/src/megolm/mod.rs b/src/megolm/mod.rs index 78fc5066..cda61c97 100644 --- a/src/megolm/mod.rs +++ b/src/megolm/mod.rs @@ -36,16 +36,15 @@ fn default_config() -> SessionConfig { #[cfg(feature = "libolm-compat")] mod libolm { - use std::io::Read; - + use matrix_pickle::Decode; use zeroize::Zeroize; use super::ratchet::Ratchet; - use crate::utilities::{Decode, DecodeSecret}; - #[derive(Zeroize)] + #[derive(Zeroize, Decode)] #[zeroize(drop)] pub(crate) struct LibolmRatchetPickle { + #[secret] ratchet: Box<[u8; 128]>, index: u32, } @@ -55,15 +54,6 @@ mod libolm { Ratchet::from_bytes(pickle.ratchet.clone(), pickle.index) } } - - impl Decode for LibolmRatchetPickle { - fn decode(reader: &mut impl Read) -> Result { - Ok(LibolmRatchetPickle { - ratchet: <[u8; 128]>::decode_secret(reader)?, - index: u32::decode(reader)?, - }) - } - } } #[cfg(test)] diff --git a/src/olm/account/mod.rs b/src/olm/account/mod.rs index 5b9a1f02..0f83825b 100644 --- a/src/olm/account/mod.rs +++ b/src/olm/account/mod.rs @@ -406,6 +406,7 @@ impl From for Account { #[cfg(feature = "libolm-compat")] mod libolm { + use matrix_pickle::{Decode, DecodeError}; use zeroize::Zeroize; use super::{ @@ -415,11 +416,11 @@ mod libolm { }; use crate::{ types::{Curve25519Keypair, Curve25519SecretKey}, - utilities::{Decode, DecodeSecret}, + utilities::LibolmEd25519Keypair, Ed25519Keypair, KeyId, }; - #[derive(Debug, Zeroize)] + #[derive(Debug, Zeroize, Decode)] #[zeroize(drop)] struct OneTimeKey { key_id: u32, @@ -428,19 +429,6 @@ mod libolm { private_key: Box<[u8; 32]>, } - impl Decode for OneTimeKey { - fn decode( - reader: &mut impl std::io::Read, - ) -> Result { - let key_id = u32::decode(reader)?; - let published = bool::decode(reader)?; - let public_key = <[u8; 32]>::decode(reader)?; - let private_key = <[u8; 32]>::decode_secret(reader)?; - - Ok(Self { key_id, published, public_key, private_key }) - } - } - impl From<&OneTimeKey> for FallbackKey { fn from(key: &OneTimeKey) -> Self { FallbackKey { @@ -459,9 +447,7 @@ mod libolm { } impl Decode for FallbackKeysArray { - fn decode( - reader: &mut impl std::io::Read, - ) -> Result { + fn decode(reader: &mut impl std::io::Read) -> Result { let count = u8::decode(reader)?; let (fallback_key, previous_fallback_key) = if count >= 1 { @@ -479,11 +465,11 @@ mod libolm { } } - #[derive(Zeroize)] + #[derive(Zeroize, Decode)] #[zeroize(drop)] pub(super) struct Pickle { version: u32, - ed25519_keypair: crate::utilities::LibolmEd25519Keypair, + ed25519_keypair: LibolmEd25519Keypair, public_curve25519_key: [u8; 32], private_curve25519_key: Box<[u8; 32]>, one_time_keys: Vec, @@ -491,31 +477,6 @@ mod libolm { next_key_id: u32, } - impl Decode for Pickle { - fn decode( - reader: &mut impl std::io::Read, - ) -> Result { - let version = u32::decode(reader)?; - - let ed25519_keypair = crate::utilities::LibolmEd25519Keypair::decode(reader)?; - let public_curve25519_key = <[u8; 32]>::decode(reader)?; - let private_curve25519_key = <[u8; 32]>::decode_secret(reader)?; - let one_time_keys = Vec::decode(reader)?; - let fallback_keys = FallbackKeysArray::decode(reader)?; - let next_key_id = u32::decode(reader)?; - - Ok(Self { - version, - ed25519_keypair, - public_curve25519_key, - private_curve25519_key, - one_time_keys, - fallback_keys, - next_key_id, - }) - } - } - impl TryFrom for Account { type Error = crate::LibolmPickleError; diff --git a/src/olm/session/mod.rs b/src/olm/session/mod.rs index 5c66cc04..cd02b5df 100644 --- a/src/olm/session/mod.rs +++ b/src/olm/session/mod.rs @@ -43,7 +43,7 @@ use super::{ use crate::hazmat::olm::MessageKey; use crate::{ olm::messages::{Message, OlmMessage, PreKeyMessage}, - utilities::{base64_encode, pickle, unpickle, DecodeSecret}, + utilities::{base64_encode, pickle, unpickle}, Curve25519PublicKey, PickleError, }; @@ -329,57 +329,32 @@ impl Session { 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, Decode}, - }; + use crate::{types::Curve25519SecretKey, utilities::unpickle_libolm}; - #[derive(Debug, Zeroize)] + #[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, } - impl Decode for SenderChain { - fn decode( - reader: &mut impl std::io::Read, - ) -> Result { - Ok(Self { - public_ratchet_key: <[u8; 32]>::decode(reader)?, - secret_ratchet_key: <[u8; 32]>::decode_secret(reader)?, - chain_key: <[u8; 32]>::decode_secret(reader)?, - chain_key_index: u32::decode(reader)?, - }) - } - } - - #[derive(Debug, Zeroize)] + #[derive(Debug, Decode, Zeroize)] #[zeroize(drop)] struct ReceivingChain { public_ratchet_key: [u8; 32], + #[secret] chain_key: Box<[u8; 32]>, chain_key_index: u32, } - impl Decode for ReceivingChain { - fn decode( - reader: &mut impl std::io::Read, - ) -> Result { - Ok(Self { - public_ratchet_key: <[u8; 32]>::decode(reader)?, - chain_key: <[u8; 32]>::decode_secret(reader)?, - chain_key_index: u32::decode(reader)?, - }) - } - } - impl From<&ReceivingChain> for ReceiverChain { fn from(chain: &ReceivingChain) -> Self { let ratchet_key = RemoteRatchetKey::from(chain.public_ratchet_key); @@ -392,60 +367,35 @@ impl Session { } } - #[derive(Debug, Zeroize)] + #[derive(Debug, Decode, Zeroize)] #[zeroize(drop)] struct MessageKey { ratchet_key: [u8; 32], + #[secret] message_key: Box<[u8; 32]>, index: u32, } - impl Decode for MessageKey { - fn decode( - reader: &mut impl std::io::Read, - ) -> Result { - Ok(Self { - ratchet_key: <[u8; 32]>::decode(reader)?, - message_key: <[u8; 32]>::decode_secret(reader)?, - index: u32::decode(reader)?, - }) - } - } - impl From<&MessageKey> for RemoteMessageKey { fn from(key: &MessageKey) -> Self { RemoteMessageKey { key: key.message_key.clone(), index: key.index.into() } } } + #[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 Decode for Pickle { - fn decode( - reader: &mut impl std::io::Read, - ) -> Result { - Ok(Self { - version: u32::decode(reader)?, - received_message: bool::decode(reader)?, - session_keys: SessionKeys::decode(reader)?, - root_key: <[u8; 32]>::decode_secret(reader)?, - sender_chains: Vec::decode(reader)?, - receiver_chains: Vec::decode(reader)?, - message_keys: Vec::decode(reader)?, - }) - } - } - impl Drop for Pickle { fn drop(&mut self) { self.root_key.zeroize(); diff --git a/src/olm/session/ratchet.rs b/src/olm/session/ratchet.rs index baaa4de3..d119fa91 100644 --- a/src/olm/session/ratchet.rs +++ b/src/olm/session/ratchet.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use matrix_pickle::Decode; use serde::{Deserialize, Serialize}; use x25519_dalek::SharedSecret; @@ -28,21 +29,10 @@ pub(super) struct RatchetKey(Curve25519SecretKey); #[derive(Debug, PartialEq, Eq, Clone, Copy)] pub struct RatchetPublicKey(Curve25519PublicKey); -#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize, Decode)] #[serde(transparent)] pub struct RemoteRatchetKey(Curve25519PublicKey); -#[cfg(feature = "libolm-compat")] -impl crate::utilities::Decode for RemoteRatchetKey { - fn decode( - reader: &mut impl std::io::Read, - ) -> Result { - let key = Curve25519PublicKey::decode(reader)?; - - Ok(RemoteRatchetKey(key)) - } -} - impl RatchetKey { pub fn new() -> Self { Self(Curve25519SecretKey::new()) diff --git a/src/olm/session_keys.rs b/src/olm/session_keys.rs index f2fb2d81..6b93aa95 100644 --- a/src/olm/session_keys.rs +++ b/src/olm/session_keys.rs @@ -13,12 +13,13 @@ // See the License for the specific language governing permissions and // limitations under the License. +use matrix_pickle::Decode; use serde::{Deserialize, Serialize}; use crate::Curve25519PublicKey; /// The set of keys that were used to establish the Olm Session, -#[derive(Clone, Copy, Deserialize, Serialize, PartialEq, Eq)] +#[derive(Clone, Copy, Deserialize, Serialize, PartialEq, Eq, Decode)] pub struct SessionKeys { pub identity_key: Curve25519PublicKey, pub base_key: Curve25519PublicKey, @@ -34,16 +35,3 @@ impl std::fmt::Debug for SessionKeys { .finish() } } - -#[cfg(feature = "libolm-compat")] -impl crate::utilities::Decode for SessionKeys { - fn decode( - reader: &mut impl std::io::Read, - ) -> Result { - Ok(Self { - identity_key: Curve25519PublicKey::decode(reader)?, - base_key: Curve25519PublicKey::decode(reader)?, - one_time_key: Curve25519PublicKey::decode(reader)?, - }) - } -} diff --git a/src/types/curve25519.rs b/src/types/curve25519.rs index 71a8a1b5..a83cead1 100644 --- a/src/types/curve25519.rs +++ b/src/types/curve25519.rs @@ -14,6 +14,7 @@ use std::fmt::Display; +use matrix_pickle::{Decode, DecodeError}; use rand::thread_rng; use serde::{Deserialize, Serialize}; use x25519_dalek::{EphemeralSecret, PublicKey, ReusableSecret, SharedSecret, StaticSecret}; @@ -51,7 +52,6 @@ impl Curve25519SecretKey { /// /// **Note**: This creates a copy of the key which won't be zeroized, the /// caller of the method needs to make sure to zeroize the returned array. - #[cfg(test)] pub fn to_bytes(&self) -> [u8; 32] { self.0.to_bytes() } @@ -103,11 +103,8 @@ pub struct Curve25519PublicKey { pub(crate) inner: PublicKey, } -#[cfg(feature = "libolm-compat")] -impl crate::utilities::Decode for Curve25519PublicKey { - fn decode( - reader: &mut impl std::io::Read, - ) -> Result { +impl Decode for Curve25519PublicKey { + fn decode(reader: &mut impl std::io::Read) -> Result { let key = <[u8; 32]>::decode(reader)?; Ok(Curve25519PublicKey::from(key)) diff --git a/src/utilities/libolm_compat.rs b/src/utilities/libolm_compat.rs index 5dd294b7..5f91ca6f 100644 --- a/src/utilities/libolm_compat.rs +++ b/src/utilities/libolm_compat.rs @@ -12,33 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::io::{Cursor, Read}; +use std::io::Cursor; -use thiserror::Error; +use matrix_pickle::Decode; use zeroize::Zeroize; use super::base64_decode; use crate::{cipher::Cipher, LibolmPickleError}; -/// Error type describing failure modes for libolm pickle decoding. -#[derive(Debug, Error)] -pub enum LibolmDecodeError { - /// There was an error while reading from the source of the libolm, usually - /// not enough data was provided. - #[error(transparent)] - IO(#[from] std::io::Error), - /// The encoded usize doesn't fit into the usize of the architecture that is - /// decoding. - #[error( - "The decoded value {0} does not fit into the usize type of this \ - architecture" - )] - OutsideUsizeRange(u64), - /// An array in the pickle has too many elements. - #[error("An array has too many elements: {0}")] - ArrayTooBig(usize), -} - /// Decrypt and decode the given pickle with the given pickle key. /// /// # Arguments @@ -84,137 +65,10 @@ pub(crate) fn unpickle_libolm Result - where - Self: Sized; -} - -/// Like `Decode`, but for decoding secret values. -/// -/// Unlike `Decode`, this trait allocates the buffer for the target value on the -/// heap and returns it in a `Box`. This reduces the number of inadvertent -/// copies made when the value is moved, allowing the value to be properly -/// zeroized. -pub(crate) trait DecodeSecret { - /// Try to read and decode a secret value from the given reader which is - /// reading from a libolm-compatible pickle. - fn decode_secret(reader: &mut impl Read) -> Result, LibolmDecodeError> - where - Self: Sized; -} - -impl Decode for u8 { - fn decode(reader: &mut impl Read) -> Result { - let mut buffer = [0u8; 1]; - - reader.read_exact(&mut buffer)?; - - Ok(buffer[0]) - } -} - -impl Decode for bool { - fn decode(reader: &mut impl Read) -> Result { - let value = u8::decode(reader)?; - - Ok(value != 0) - } -} - -impl Decode for u32 { - fn decode(reader: &mut impl Read) -> Result { - let mut buffer = [0u8; 4]; - reader.read_exact(&mut buffer)?; - - Ok(u32::from_be_bytes(buffer)) - } -} - -impl Decode for usize { - fn decode(reader: &mut impl Read) -> Result { - let size = u32::decode(reader)?; - - size.try_into().map_err(|_| LibolmDecodeError::OutsideUsizeRange(size as u64)) - } -} - -impl Decode for [u8; N] { - fn decode(reader: &mut impl Read) -> Result { - let mut buffer = [0u8; N]; - reader.read_exact(&mut buffer)?; - - Ok(buffer) - } -} - -impl DecodeSecret for [u8; N] { - fn decode_secret(reader: &mut impl Read) -> Result, LibolmDecodeError> { - let mut buffer = Box::new([0u8; N]); - reader.read_exact(buffer.as_mut_slice())?; - - Ok(buffer) - } -} - -impl Decode for Vec { - fn decode(reader: &mut impl Read) -> Result { - let length = usize::decode(reader)?; - - if length > u16::MAX.into() { - Err(LibolmDecodeError::ArrayTooBig(length)) - } else { - let mut buffer = Vec::with_capacity(length); - - for _ in 0..length { - let element = T::decode(reader)?; - buffer.push(element); - } - - Ok(buffer) - } - } -} - -#[derive(Zeroize)] +#[derive(Zeroize, Decode)] #[zeroize(drop)] pub(crate) struct LibolmEd25519Keypair { pub public_key: [u8; 32], + #[secret] pub private_key: Box<[u8; 64]>, } - -impl Decode for LibolmEd25519Keypair { - fn decode(reader: &mut impl Read) -> Result - where - Self: Sized, - { - let public_key = <[u8; 32]>::decode(reader)?; - let private_key = <[u8; 64]>::decode_secret(reader)?; - - Ok(Self { public_key, private_key }) - } -} diff --git a/src/utilities/mod.rs b/src/utilities/mod.rs index 55115156..9566b281 100644 --- a/src/utilities/mod.rs +++ b/src/utilities/mod.rs @@ -18,9 +18,7 @@ mod libolm_compat; pub use base64::DecodeError; #[cfg(feature = "libolm-compat")] -pub(crate) use libolm_compat::{ - unpickle_libolm, Decode, DecodeSecret, LibolmDecodeError, LibolmEd25519Keypair, -}; +pub(crate) use libolm_compat::{unpickle_libolm, LibolmEd25519Keypair}; /// Decode the input as base64 with no padding. pub fn base64_decode(input: impl AsRef<[u8]>) -> Result, DecodeError> {